* [PATCH 0/2] fdmon-poll: reset npfd when upgrading to fdmon-epoll
@ 2020-08-21 10:12 Stefan Hajnoczi
2020-08-21 10:12 ` [PATCH 1/2] " Stefan Hajnoczi
2020-08-21 10:12 ` [PATCH 2/2] tests: add test-fdmon-epoll Stefan Hajnoczi
0 siblings, 2 replies; 4+ messages in thread
From: Stefan Hajnoczi @ 2020-08-21 10:12 UTC (permalink / raw)
To: qemu-devel; +Cc: Fam Zheng, qemu-block, qemu-stable, Stefan Hajnoczi
Fix an assertion failure when aio_poll() is called in a aio_disable_external()
region. The failure happens when fdmon-poll is first upgraded to fdmon-epoll
and then downgraded again due to aio_disable_external().
Stefan Hajnoczi (2):
fdmon-poll: reset npfd when upgrading to fdmon-epoll
tests: add test-fdmon-epoll
MAINTAINERS | 1 +
tests/Makefile.include | 4 +++
tests/test-fdmon-epoll.c | 73 ++++++++++++++++++++++++++++++++++++++++
util/fdmon-poll.c | 1 +
4 files changed, 79 insertions(+)
create mode 100644 tests/test-fdmon-epoll.c
--
2.26.2
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH 1/2] fdmon-poll: reset npfd when upgrading to fdmon-epoll
2020-08-21 10:12 [PATCH 0/2] fdmon-poll: reset npfd when upgrading to fdmon-epoll Stefan Hajnoczi
@ 2020-08-21 10:12 ` Stefan Hajnoczi
2020-08-21 13:54 ` Philippe Mathieu-Daudé
2020-08-21 10:12 ` [PATCH 2/2] tests: add test-fdmon-epoll Stefan Hajnoczi
1 sibling, 1 reply; 4+ messages in thread
From: Stefan Hajnoczi @ 2020-08-21 10:12 UTC (permalink / raw)
To: qemu-devel; +Cc: Fam Zheng, qemu-block, qemu-stable, Stefan Hajnoczi
npfd keeps track of how many pollfds are currently being monitored. It
must be reset to 0 when fdmon_poll_wait() returns.
When npfd reaches a treshold we switch to fdmon-epoll because it scales
better.
This patch resets npfd in the case where we switch to fdmon-epoll.
Forgetting to do so results in the following assertion failure:
util/fdmon-poll.c:65: fdmon_poll_wait: Assertion `npfd == 0' failed.
Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1869952
Fixes: 1f050a4690f62a1e7dabc4f44141e9f762c3769f ("aio-posix: extract ppoll(2) and epoll(7) fd monitoring")
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
util/fdmon-poll.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/util/fdmon-poll.c b/util/fdmon-poll.c
index 488067b679..5fe3b47865 100644
--- a/util/fdmon-poll.c
+++ b/util/fdmon-poll.c
@@ -73,6 +73,7 @@ static int fdmon_poll_wait(AioContext *ctx, AioHandlerList *ready_list,
/* epoll(7) is faster above a certain number of fds */
if (fdmon_epoll_try_upgrade(ctx, npfd)) {
+ npfd = 0; /* we won't need pollfds[], reset npfd */
return ctx->fdmon_ops->wait(ctx, ready_list, timeout);
}
--
2.26.2
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH 2/2] tests: add test-fdmon-epoll
2020-08-21 10:12 [PATCH 0/2] fdmon-poll: reset npfd when upgrading to fdmon-epoll Stefan Hajnoczi
2020-08-21 10:12 ` [PATCH 1/2] " Stefan Hajnoczi
@ 2020-08-21 10:12 ` Stefan Hajnoczi
1 sibling, 0 replies; 4+ messages in thread
From: Stefan Hajnoczi @ 2020-08-21 10:12 UTC (permalink / raw)
To: qemu-devel; +Cc: Fam Zheng, qemu-block, qemu-stable, Stefan Hajnoczi
Test aio_disable_external(), which switches from fdmon-epoll back to
fdmon-poll. This resulted in an assertion failure that was fixed in the
previous patch.
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
MAINTAINERS | 1 +
tests/Makefile.include | 4 +++
tests/test-fdmon-epoll.c | 73 ++++++++++++++++++++++++++++++++++++++++
3 files changed, 78 insertions(+)
create mode 100644 tests/test-fdmon-epoll.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 0886eb3d2b..6d2e99b94f 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2109,6 +2109,7 @@ F: migration/block*
F: include/block/aio.h
F: include/block/aio-wait.h
F: scripts/qemugdb/aio.py
+F: tests/test-fdmon-epoll.c
T: git https://github.com/stefanha/qemu.git block
Block SCSI subsystem
diff --git a/tests/Makefile.include b/tests/Makefile.include
index c7e4646ded..b785e487b7 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -78,6 +78,9 @@ check-unit-$(CONFIG_SOFTMMU) += tests/test-iov$(EXESUF)
check-unit-y += tests/test-bitmap$(EXESUF)
check-unit-$(CONFIG_BLOCK) += tests/test-aio$(EXESUF)
check-unit-$(CONFIG_BLOCK) += tests/test-aio-multithread$(EXESUF)
+ifeq ($(CONFIG_EPOLL_CREATE1),y)
+check-unit-$(CONFIG_BLOCK) += tests/test-fdmon-epoll$(EXESUF)
+endif
check-unit-$(CONFIG_BLOCK) += tests/test-throttle$(EXESUF)
check-unit-$(CONFIG_BLOCK) += tests/test-thread-pool$(EXESUF)
check-unit-$(CONFIG_BLOCK) += tests/test-hbitmap$(EXESUF)
@@ -408,6 +411,7 @@ tests/test-char$(EXESUF): tests/test-char.o $(test-util-obj-y) $(test-io-obj-y)
tests/test-coroutine$(EXESUF): tests/test-coroutine.o $(test-block-obj-y)
tests/test-aio$(EXESUF): tests/test-aio.o $(test-block-obj-y)
tests/test-aio-multithread$(EXESUF): tests/test-aio-multithread.o $(test-block-obj-y)
+tests/test-fdmon-epoll$(EXESUF): tests/test-fdmon-epoll.o $(test-block-obj-y)
tests/test-throttle$(EXESUF): tests/test-throttle.o $(test-block-obj-y)
tests/test-bdrv-drain$(EXESUF): tests/test-bdrv-drain.o $(test-block-obj-y) $(test-util-obj-y)
tests/test-bdrv-graph-mod$(EXESUF): tests/test-bdrv-graph-mod.o $(test-block-obj-y) $(test-util-obj-y)
diff --git a/tests/test-fdmon-epoll.c b/tests/test-fdmon-epoll.c
new file mode 100644
index 0000000000..11fd8a2fa9
--- /dev/null
+++ b/tests/test-fdmon-epoll.c
@@ -0,0 +1,73 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * fdmon-epoll tests
+ *
+ * Copyright (c) 2020 Red Hat, Inc.
+ */
+
+#include "qemu/osdep.h"
+#include "block/aio.h"
+#include "qapi/error.h"
+#include "qemu/main-loop.h"
+
+static AioContext *ctx;
+
+static void dummy_fd_handler(EventNotifier *notifier)
+{
+ event_notifier_test_and_clear(notifier);
+}
+
+static void add_event_notifiers(EventNotifier *notifiers, size_t n)
+{
+ for (size_t i = 0; i < n; i++) {
+ event_notifier_init(¬ifiers[i], false);
+ aio_set_event_notifier(ctx, ¬ifiers[i], false,
+ dummy_fd_handler, NULL);
+ }
+}
+
+static void remove_event_notifiers(EventNotifier *notifiers, size_t n)
+{
+ for (size_t i = 0; i < n; i++) {
+ aio_set_event_notifier(ctx, ¬ifiers[i], false, NULL, NULL);
+ event_notifier_cleanup(¬ifiers[i]);
+ }
+}
+
+/* Check that fd handlers work when external clients are disabled */
+static void test_external_disabled(void)
+{
+ EventNotifier notifiers[100];
+
+ /* fdmon-epoll is only enabled when many fd handlers are registered */
+ add_event_notifiers(notifiers, G_N_ELEMENTS(notifiers));
+
+ event_notifier_set(¬ifiers[0]);
+ assert(aio_poll(ctx, true));
+
+ aio_disable_external(ctx);
+ event_notifier_set(¬ifiers[0]);
+ assert(aio_poll(ctx, true));
+ aio_enable_external(ctx);
+
+ remove_event_notifiers(notifiers, G_N_ELEMENTS(notifiers));
+}
+
+int main(int argc, char **argv)
+{
+ /*
+ * This code relies on the fact that fdmon-io_uring disables itself when
+ * the glib main loop is in use. The main loop uses fdmon-poll and upgrades
+ * to fdmon-epoll when the number of fds exceeds a threshold.
+ */
+ qemu_init_main_loop(&error_fatal);
+ ctx = qemu_get_aio_context();
+
+ while (g_main_context_iteration(NULL, false)) {
+ /* Do nothing */
+ }
+
+ g_test_init(&argc, &argv, NULL);
+ g_test_add_func("/fdmon-epoll/external-disabled", test_external_disabled);
+ return g_test_run();
+}
--
2.26.2
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 1/2] fdmon-poll: reset npfd when upgrading to fdmon-epoll
2020-08-21 10:12 ` [PATCH 1/2] " Stefan Hajnoczi
@ 2020-08-21 13:54 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 4+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-08-21 13:54 UTC (permalink / raw)
To: Stefan Hajnoczi, qemu-devel; +Cc: Fam Zheng, qemu-stable, qemu-block
On 8/21/20 12:12 PM, Stefan Hajnoczi wrote:
> npfd keeps track of how many pollfds are currently being monitored. It
> must be reset to 0 when fdmon_poll_wait() returns.
>
> When npfd reaches a treshold we switch to fdmon-epoll because it scales
> better.
>
> This patch resets npfd in the case where we switch to fdmon-epoll.
> Forgetting to do so results in the following assertion failure:
>
> util/fdmon-poll.c:65: fdmon_poll_wait: Assertion `npfd == 0' failed.
>
> Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1869952
> Fixes: 1f050a4690f62a1e7dabc4f44141e9f762c3769f ("aio-posix: extract ppoll(2) and epoll(7) fd monitoring")
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
> util/fdmon-poll.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/util/fdmon-poll.c b/util/fdmon-poll.c
> index 488067b679..5fe3b47865 100644
> --- a/util/fdmon-poll.c
> +++ b/util/fdmon-poll.c
> @@ -73,6 +73,7 @@ static int fdmon_poll_wait(AioContext *ctx, AioHandlerList *ready_list,
>
> /* epoll(7) is faster above a certain number of fds */
> if (fdmon_epoll_try_upgrade(ctx, npfd)) {
> + npfd = 0; /* we won't need pollfds[], reset npfd */
> return ctx->fdmon_ops->wait(ctx, ready_list, timeout);
> }
>
>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-08-21 13:56 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-21 10:12 [PATCH 0/2] fdmon-poll: reset npfd when upgrading to fdmon-epoll Stefan Hajnoczi
2020-08-21 10:12 ` [PATCH 1/2] " Stefan Hajnoczi
2020-08-21 13:54 ` Philippe Mathieu-Daudé
2020-08-21 10:12 ` [PATCH 2/2] tests: add test-fdmon-epoll Stefan Hajnoczi
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.