Message ID | 20230517165116.475123-19-kwolf@redhat.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | [PULL,01/18] blockdev: refactor transaction to use Transaction API | expand |
On 5/17/23 09:51, Kevin Wolf wrote: > From: Stefan Hajnoczi <stefanha@redhat.com> > > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> > Message-Id: <20230502184134.534703-3-stefanha@redhat.com> > Tested-by: Kevin Wolf <kwolf@redhat.com> > Signed-off-by: Kevin Wolf <kwolf@redhat.com> > --- > tests/unit/test-nested-aio-poll.c | 130 ++++++++++++++++++++++++++++++ > tests/unit/meson.build | 1 + > 2 files changed, 131 insertions(+) > create mode 100644 tests/unit/test-nested-aio-poll.c This new test fails on windows: https://gitlab.com/qemu-project/qemu/-/jobs/4304413315#L3375 https://gitlab.com/qemu-project/qemu/-/jobs/4304413313#L3357 r~
Am 17.05.2023 um 21:10 hat Richard Henderson geschrieben: > On 5/17/23 09:51, Kevin Wolf wrote: > > From: Stefan Hajnoczi <stefanha@redhat.com> > > > > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> > > Message-Id: <20230502184134.534703-3-stefanha@redhat.com> > > Tested-by: Kevin Wolf <kwolf@redhat.com> > > Signed-off-by: Kevin Wolf <kwolf@redhat.com> > > --- > > tests/unit/test-nested-aio-poll.c | 130 ++++++++++++++++++++++++++++++ > > tests/unit/meson.build | 1 + > > 2 files changed, 131 insertions(+) > > create mode 100644 tests/unit/test-nested-aio-poll.c > > This new test fails on windows: > > https://gitlab.com/qemu-project/qemu/-/jobs/4304413315#L3375 > https://gitlab.com/qemu-project/qemu/-/jobs/4304413313#L3357 What the CI output doesn't show is that the problem seems to be that the test doesn't even make sense on Windows. When I run it manually: Unexpected error in aio_context_set_poll_params() at ../../home/kwolf/source/qemu/util/aio-win32.c:443: Z:\tmp\build-win32\tests\unit\test-nested-aio-poll.exe: AioContext polling is not implemented on Windows Stefan, I'll squash in the following, so you don't have to resubmit the series. Kevin diff --git a/tests/unit/meson.build b/tests/unit/meson.build index a314f82baa..8ed81786ee 100644 --- a/tests/unit/meson.build +++ b/tests/unit/meson.build @@ -67,7 +67,6 @@ if have_block 'test-coroutine': [testblock], 'test-aio': [testblock], 'test-aio-multithread': [testblock], - 'test-nested-aio-poll': [testblock], 'test-throttle': [testblock], 'test-thread-pool': [testblock], 'test-hbitmap': [testblock], @@ -115,7 +114,10 @@ if have_block tests += {'test-crypto-xts': [crypto, io]} endif if 'CONFIG_POSIX' in config_host - tests += {'test-image-locking': [testblock]} + tests += { + 'test-image-locking': [testblock], + 'test-nested-aio-poll': [testblock], + } endif if config_host_data.get('CONFIG_REPLICATION') tests += {'test-replication': [testblock]}
On Fri, May 19, 2023 at 11:23:30AM +0200, Kevin Wolf wrote: > Am 17.05.2023 um 21:10 hat Richard Henderson geschrieben: > > On 5/17/23 09:51, Kevin Wolf wrote: > > > From: Stefan Hajnoczi <stefanha@redhat.com> > > > > > > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> > > > Message-Id: <20230502184134.534703-3-stefanha@redhat.com> > > > Tested-by: Kevin Wolf <kwolf@redhat.com> > > > Signed-off-by: Kevin Wolf <kwolf@redhat.com> > > > --- > > > tests/unit/test-nested-aio-poll.c | 130 ++++++++++++++++++++++++++++++ > > > tests/unit/meson.build | 1 + > > > 2 files changed, 131 insertions(+) > > > create mode 100644 tests/unit/test-nested-aio-poll.c > > > > This new test fails on windows: > > > > https://gitlab.com/qemu-project/qemu/-/jobs/4304413315#L3375 > > https://gitlab.com/qemu-project/qemu/-/jobs/4304413313#L3357 > > What the CI output doesn't show is that the problem seems to be that the > test doesn't even make sense on Windows. When I run it manually: > > Unexpected error in aio_context_set_poll_params() at ../../home/kwolf/source/qemu/util/aio-win32.c:443: > Z:\tmp\build-win32\tests\unit\test-nested-aio-poll.exe: AioContext polling is not implemented on Windows > > Stefan, I'll squash in the following, so you don't have to resubmit the > series. Thank you, Kevin! Stefan > diff --git a/tests/unit/meson.build b/tests/unit/meson.build > index a314f82baa..8ed81786ee 100644 > --- a/tests/unit/meson.build > +++ b/tests/unit/meson.build > @@ -67,7 +67,6 @@ if have_block > 'test-coroutine': [testblock], > 'test-aio': [testblock], > 'test-aio-multithread': [testblock], > - 'test-nested-aio-poll': [testblock], > 'test-throttle': [testblock], > 'test-thread-pool': [testblock], > 'test-hbitmap': [testblock], > @@ -115,7 +114,10 @@ if have_block > tests += {'test-crypto-xts': [crypto, io]} > endif > if 'CONFIG_POSIX' in config_host > - tests += {'test-image-locking': [testblock]} > + tests += { > + 'test-image-locking': [testblock], > + 'test-nested-aio-poll': [testblock], > + } > endif > if config_host_data.get('CONFIG_REPLICATION') > tests += {'test-replication': [testblock]} >
diff --git a/tests/unit/test-nested-aio-poll.c b/tests/unit/test-nested-aio-poll.c new file mode 100644 index 0000000000..9bbe18b839 --- /dev/null +++ b/tests/unit/test-nested-aio-poll.c @@ -0,0 +1,130 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ +/* + * Test that poll handlers are not re-entrant in nested aio_poll() + * + * Copyright Red Hat + * + * Poll handlers are usually level-triggered. That means they continue firing + * until the condition is reset (e.g. a virtqueue becomes empty). If a poll + * handler calls nested aio_poll() before the condition is reset, then infinite + * recursion occurs. + * + * aio_poll() is supposed to prevent this by disabling poll handlers in nested + * aio_poll() calls. This test case checks that this is indeed what happens. + */ +#include "qemu/osdep.h" +#include "block/aio.h" +#include "qapi/error.h" + +typedef struct { + AioContext *ctx; + + /* This is the EventNotifier that drives the test */ + EventNotifier poll_notifier; + + /* This EventNotifier is only used to wake aio_poll() */ + EventNotifier dummy_notifier; + + bool nested; +} TestData; + +static void io_read(EventNotifier *notifier) +{ + fprintf(stderr, "%s %p\n", __func__, notifier); + event_notifier_test_and_clear(notifier); +} + +static bool io_poll_true(void *opaque) +{ + fprintf(stderr, "%s %p\n", __func__, opaque); + return true; +} + +static bool io_poll_false(void *opaque) +{ + fprintf(stderr, "%s %p\n", __func__, opaque); + return false; +} + +static void io_poll_ready(EventNotifier *notifier) +{ + TestData *td = container_of(notifier, TestData, poll_notifier); + + fprintf(stderr, "> %s\n", __func__); + + g_assert(!td->nested); + td->nested = true; + + /* Wake the following nested aio_poll() call */ + event_notifier_set(&td->dummy_notifier); + + /* This nested event loop must not call io_poll()/io_poll_ready() */ + g_assert(aio_poll(td->ctx, true)); + + td->nested = false; + + fprintf(stderr, "< %s\n", __func__); +} + +/* dummy_notifier never triggers */ +static void io_poll_never_ready(EventNotifier *notifier) +{ + g_assert_not_reached(); +} + +static void test(void) +{ + TestData td = { + .ctx = aio_context_new(&error_abort), + }; + + qemu_set_current_aio_context(td.ctx); + + /* Enable polling */ + aio_context_set_poll_params(td.ctx, 1000000, 2, 2, &error_abort); + + /* + * The GSource is unused but this has the side-effect of changing the fdmon + * that AioContext uses. + */ + aio_get_g_source(td.ctx); + + /* Make the event notifier active (set) right away */ + event_notifier_init(&td.poll_notifier, 1); + aio_set_event_notifier(td.ctx, &td.poll_notifier, false, + io_read, io_poll_true, io_poll_ready); + + /* This event notifier will be used later */ + event_notifier_init(&td.dummy_notifier, 0); + aio_set_event_notifier(td.ctx, &td.dummy_notifier, false, + io_read, io_poll_false, io_poll_never_ready); + + /* Consume aio_notify() */ + g_assert(!aio_poll(td.ctx, false)); + + /* + * Run the io_read() handler. This has the side-effect of activating + * polling in future aio_poll() calls. + */ + g_assert(aio_poll(td.ctx, true)); + + /* The second time around the io_poll()/io_poll_ready() handler runs */ + g_assert(aio_poll(td.ctx, true)); + + /* Run io_poll()/io_poll_ready() one more time to show it keeps working */ + g_assert(aio_poll(td.ctx, true)); + + aio_set_event_notifier(td.ctx, &td.dummy_notifier, false, + NULL, NULL, NULL); + aio_set_event_notifier(td.ctx, &td.poll_notifier, false, NULL, NULL, NULL); + event_notifier_cleanup(&td.dummy_notifier); + event_notifier_cleanup(&td.poll_notifier); + aio_context_unref(td.ctx); +} + +int main(int argc, char **argv) +{ + g_test_init(&argc, &argv, NULL); + g_test_add_func("/nested-aio-poll", test); + return g_test_run(); +} diff --git a/tests/unit/meson.build b/tests/unit/meson.build index 3bc78d8660..a314f82baa 100644 --- a/tests/unit/meson.build +++ b/tests/unit/meson.build @@ -67,6 +67,7 @@ if have_block 'test-coroutine': [testblock], 'test-aio': [testblock], 'test-aio-multithread': [testblock], + 'test-nested-aio-poll': [testblock], 'test-throttle': [testblock], 'test-thread-pool': [testblock], 'test-hbitmap': [testblock],