Series comparison

-[PULL 0/3] Block patches
+[PULL 0/2] Block patches
-The following changes since commit d0ed6a69d399ae193959225cdeaa9382746c91cc:
+The following changes since commit ca61fa4b803e5d0abaf6f1ceb690f23bb78a4def:
-  Update version for v5.1.0 release (2020-08-11 17:07:03 +0100)
+  Merge remote-tracking branch 'remotes/quic/tags/pull-hex-20211006' into staging (2021-10-06 12:11:14 -0700)
 are available in the Git repository at:
-  https://github.com/stefanha/qemu.git tags/block-pull-request
+  https://gitlab.com/stefanha/qemu.git tags/block-pull-request
-for you to fetch changes up to 44277bf914471962c9e88e09c859aae65ae109c4:
+for you to fetch changes up to 1cc7eada97914f090125e588497986f6f7900514:
-  aio-posix: keep aio_notify_me disabled during polling (2020-08-13 13:34:14 =
+  iothread: use IOThreadParamInfo in iothread_[set|get]_param() (2021-10-07 15:29:50 +0100)
 +0100)
 ----------------------------------------------------------------
 Pull request
 ----------------------------------------------------------------
-Stefan Hajnoczi (3):
+Stefano Garzarella (2):
-  async: rename event_notifier_dummy_cb/poll()
+  iothread: rename PollParamInfo to IOThreadParamInfo
-  async: always set ctx->notified in aio_notify()
+  iothread: use IOThreadParamInfo in iothread_[set|get]_param()
   aio-posix: keep aio_notify_me disabled during polling
- util/aio-posix.c | 47 +++++++++++++++++++++++++----------------------
+ iothread.c | 28 +++++++++++++++-------------
- util/async.c     | 36 +++++++++++++++++++++++-------------
+file changed, 15 insertions(+), 13 deletions(-)
 files changed, 48 insertions(+), 35 deletions(-)
---=20
+--
-.26.2
+.31.1

-[PULL 3/3] aio-posix: keep aio_notify_me disabled during polling
+[PULL 1/2] iothread: rename PollParamInfo to IOThreadParamInfo
-Polling only monitors the ctx->notified field and does not need the
+From: Stefano Garzarella <sgarzare@redhat.com>
 ctx->notifier EventNotifier to be signalled. Keep ctx->aio_notify_me
 disabled while polling to avoid unnecessary EventNotifier syscalls.
-This optimization improves virtio-blk 4KB random read performance by
+Commit 1793ad0247 ("iothread: add aio-max-batch parameter") added
-%. The following results are with an IOThread and the null-co block
+a new parameter (aio-max-batch) to IOThread and used PollParamInfo
-driver:
+structure to handle it.
-Test         IOPS   Error
+Since it is not a parameter of the polling mechanism, we rename the
-Before  244518.62 ± 1.20%
+structure to a more generic IOThreadParamInfo.
 After   290706.11 ± 0.44%
-Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
+Suggested-by: Kevin Wolf <kwolf@redhat.com>
-Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
+Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
 Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
-Message-id: 20200806131802.569478-4-stefanha@redhat.com
+Message-id: 20210727145936.147032-2-sgarzare@redhat.com
 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 ---
- util/aio-posix.c | 47 +++++++++++++++++++++++++----------------------
+ iothread.c | 14 +++++++-------
-file changed, 25 insertions(+), 22 deletions(-)
+file changed, 7 insertions(+), 7 deletions(-)
-diff --git a/util/aio-posix.c b/util/aio-posix.c
+diff --git a/iothread.c b/iothread.c
 index XXXXXXX..XXXXXXX 100644
---- a/util/aio-posix.c
+--- a/iothread.c
-+++ b/util/aio-posix.c
++++ b/iothread.c
-@@ -XXX,XX +XXX,XX @@ static bool remove_idle_poll_handlers(AioContext *ctx, int64_t now)
+@@ -XXX,XX +XXX,XX @@ static void iothread_complete(UserCreatable *obj, Error **errp)
-  *
+ typedef struct {
-  * Polls for a given time.
+     const char *name;
-  *
+     ptrdiff_t offset; /* field's byte offset in IOThread struct */
-- * Note that ctx->notify_me must be non-zero so this function can detect
+-} PollParamInfo;
-- * aio_notify().
++} IOThreadParamInfo;
-- *
-  * Note that the caller must have incremented ctx->list_lock.
+-static PollParamInfo poll_max_ns_info = {
-  *
++static IOThreadParamInfo poll_max_ns_info = {
-  * Returns: true if progress was made, false otherwise
+     "poll-max-ns", offsetof(IOThread, poll_max_ns),
-@@ -XXX,XX +XXX,XX @@ static bool run_poll_handlers(AioContext *ctx, int64_t max_ns, int64_t *timeout)
+ };
-     bool progress;
+-static PollParamInfo poll_grow_info = {
-     int64_t start_time, elapsed_time;
++static IOThreadParamInfo poll_grow_info = {
+     "poll-grow", offsetof(IOThread, poll_grow),
--    assert(ctx->notify_me);
+ };
-     assert(qemu_lockcnt_count(&ctx->list_lock) > 0);
+-static PollParamInfo poll_shrink_info = {
++static IOThreadParamInfo poll_shrink_info = {
-     trace_run_poll_handlers_begin(ctx, max_ns, *timeout);
+     "poll-shrink", offsetof(IOThread, poll_shrink),
-@@ -XXX,XX +XXX,XX @@ static bool run_poll_handlers(AioContext *ctx, int64_t max_ns, int64_t *timeout)
+ };
-  * @timeout: timeout for blocking wait, computed by the caller and updated if
+-static PollParamInfo aio_max_batch_info = {
-  *    polling succeeds.
++static IOThreadParamInfo aio_max_batch_info = {
-  *
+     "aio-max-batch", offsetof(IOThread, aio_max_batch),
-- * ctx->notify_me must be non-zero so this function can detect aio_notify().
+ };
-- *
-  * Note that the caller must have incremented ctx->list_lock.
+@@ -XXX,XX +XXX,XX @@ static void iothread_get_param(Object *obj, Visitor *v,
-  *
+         const char *name, void *opaque, Error **errp)
-  * Returns: true if progress was made, false otherwise
+ {
-@@ -XXX,XX +XXX,XX @@ bool aio_poll(AioContext *ctx, bool blocking)
+     IOThread *iothread = IOTHREAD(obj);
-     AioHandlerList ready_list = QLIST_HEAD_INITIALIZER(ready_list);
+-    PollParamInfo *info = opaque;
-     int ret = 0;
++    IOThreadParamInfo *info = opaque;
-     bool progress;
+     int64_t *field = (void *)iothread + info->offset;
-+    bool use_notify_me;
-     int64_t timeout;
+     visit_type_int64(v, name, field, errp);
-     int64_t start = 0;
+@@ -XXX,XX +XXX,XX @@ static bool iothread_set_param(Object *obj, Visitor *v,
+         const char *name, void *opaque, Error **errp)
-@@ -XXX,XX +XXX,XX @@ bool aio_poll(AioContext *ctx, bool blocking)
+ {
-      */
+     IOThread *iothread = IOTHREAD(obj);
-     assert(in_aio_context_home_thread(ctx));
+-    PollParamInfo *info = opaque;
++    IOThreadParamInfo *info = opaque;
--    /* aio_notify can avoid the expensive event_notifier_set if
+     int64_t *field = (void *)iothread + info->offset;
-+    qemu_lockcnt_inc(&ctx->list_lock);
+     int64_t value;
-+
 +    if (ctx->poll_max_ns) {
 +        start = qemu_clock_get_ns(QEMU_CLOCK_REALTIME);
 +    }
 +
 +    timeout = blocking ? aio_compute_timeout(ctx) : 0;
 +    progress = try_poll_mode(ctx, &timeout);
 +    assert(!(timeout && progress));
 +
 +    /*
 +     * aio_notify can avoid the expensive event_notifier_set if
       * everything (file descriptors, bottom halves, timers) will
       * be re-evaluated before the next blocking poll().  This is
       * already true when aio_poll is called with blocking == false;
       * if blocking == true, it is only true after poll() returns,
       * so disable the optimization now.
       */
 -    if (blocking) {
 +    use_notify_me = timeout != 0;
 +    if (use_notify_me) {
          atomic_set(&ctx->notify_me, atomic_read(&ctx->notify_me) + 2);
          /*
 -         * Write ctx->notify_me before computing the timeout
 -         * (reading bottom half flags, etc.).  Pairs with
 +         * Write ctx->notify_me before reading ctx->notified.  Pairs with
           * smp_mb in aio_notify().
           */
          smp_mb();
 -    }
 -
 -    qemu_lockcnt_inc(&ctx->list_lock);
 -    if (ctx->poll_max_ns) {
 -        start = qemu_clock_get_ns(QEMU_CLOCK_REALTIME);
 +        /* Don't block if aio_notify() was called */
 +        if (atomic_read(&ctx->notified)) {
 +            timeout = 0;
 +        }
      }
 -    timeout = blocking ? aio_compute_timeout(ctx) : 0;
 -    progress = try_poll_mode(ctx, &timeout);
 -    assert(!(timeout && progress));
 -
      /* If polling is allowed, non-blocking aio_poll does not need the
       * system call---a single round of run_poll_handlers_once suffices.
       */
@@ -XXX,XX +XXX,XX @@ bool aio_poll(AioContext *ctx, bool blocking)
          ret = ctx->fdmon_ops->wait(ctx, &ready_list, timeout);
      }
 -    if (blocking) {
 +    if (use_notify_me) {
          /* Finish the poll before clearing the flag.  */
 -        atomic_store_release(&ctx->notify_me, atomic_read(&ctx->notify_me) - 2);
 -        aio_notify_accept(ctx);
 +        atomic_store_release(&ctx->notify_me,
 +                             atomic_read(&ctx->notify_me) - 2);
      }
 +    aio_notify_accept(ctx);
 +
      /* Adjust polling time */
      if (ctx->poll_max_ns) {
          int64_t block_ns = qemu_clock_get_ns(QEMU_CLOCK_REALTIME) - start;
 --
-.26.2
+.31.1

-[PULL 1/3] async: rename event_notifier_dummy_cb/poll()
+[PULL 2/2] iothread: use IOThreadParamInfo in iothread_[set|get]_param()
-The event_notifier_*() prefix can be confused with the EventNotifier
+From: Stefano Garzarella <sgarzare@redhat.com>
 APIs that are also called event_notifier_*().
-Rename the functions to aio_context_notifier_*() to make it clear that
+Commit 0445409d74 ("iothread: generalize
-they relate to the AioContext::notifier field.
+iothread_set_param/iothread_get_param") moved common code to set and
 get IOThread parameters in two new functions.
-Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
+These functions are called inside callbacks, so we don't need to use an
 opaque pointer. Let's replace `void *opaque` parameter with
 `IOThreadParamInfo *info`.
 Suggested-by: Kevin Wolf <kwolf@redhat.com>
 Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
 Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
-Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
+Message-id: 20210727145936.147032-3-sgarzare@redhat.com
 Message-id: 20200806131802.569478-2-stefanha@redhat.com
 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 ---
- util/async.c | 8 ++++----
+ iothread.c | 18 ++++++++++--------
-file changed, 4 insertions(+), 4 deletions(-)
+file changed, 10 insertions(+), 8 deletions(-)
-diff --git a/util/async.c b/util/async.c
+diff --git a/iothread.c b/iothread.c
 index XXXXXXX..XXXXXXX 100644
---- a/util/async.c
+--- a/iothread.c
-+++ b/util/async.c
++++ b/iothread.c
-@@ -XXX,XX +XXX,XX @@ static void aio_timerlist_notify(void *opaque, QEMUClockType type)
+@@ -XXX,XX +XXX,XX @@ static IOThreadParamInfo aio_max_batch_info = {
-     aio_notify(opaque);
+ };
  static void iothread_get_param(Object *obj, Visitor *v,
 -        const char *name, void *opaque, Error **errp)
 +        const char *name, IOThreadParamInfo *info, Error **errp)
  {
      IOThread *iothread = IOTHREAD(obj);
 -    IOThreadParamInfo *info = opaque;
      int64_t *field = (void *)iothread + info->offset;
      visit_type_int64(v, name, field, errp);
  }
--static void event_notifier_dummy_cb(EventNotifier *e)
+ static bool iothread_set_param(Object *obj, Visitor *v,
-+static void aio_context_notifier_dummy_cb(EventNotifier *e)
+-        const char *name, void *opaque, Error **errp)
 +        const char *name, IOThreadParamInfo *info, Error **errp)
  {
+     IOThread *iothread = IOTHREAD(obj);
+-    IOThreadParamInfo *info = opaque;
+     int64_t *field = (void *)iothread + info->offset;
+     int64_t value;
+@@ -XXX,XX +XXX,XX @@ static bool iothread_set_param(Object *obj, Visitor *v,
+ static void iothread_get_poll_param(Object *obj, Visitor *v,
+         const char *name, void *opaque, Error **errp)
+ {
++    IOThreadParamInfo *info = opaque;
+-    iothread_get_param(obj, v, name, opaque, errp);
++    iothread_get_param(obj, v, name, info, errp);
  }
- /* Returns true if aio_notify() was called (e.g. a BH was scheduled) */
+ static void iothread_set_poll_param(Object *obj, Visitor *v,
--static bool event_notifier_poll(void *opaque)
+         const char *name, void *opaque, Error **errp)
 +static bool aio_context_notifier_poll(void *opaque)
  {
-     EventNotifier *e = opaque;
+     IOThread *iothread = IOTHREAD(obj);
-     AioContext *ctx = container_of(e, AioContext, notifier);
++    IOThreadParamInfo *info = opaque;
-@@ -XXX,XX +XXX,XX @@ AioContext *aio_context_new(Error **errp)
+-    if (!iothread_set_param(obj, v, name, opaque, errp)) {
-     aio_set_event_notifier(ctx, &ctx->notifier,
++    if (!iothread_set_param(obj, v, name, info, errp)) {
-                            false,
+         return;
--                           event_notifier_dummy_cb,
+     }
--                           event_notifier_poll);
-+                           aio_context_notifier_dummy_cb,
+@@ -XXX,XX +XXX,XX @@ static void iothread_set_poll_param(Object *obj, Visitor *v,
-+                           aio_context_notifier_poll);
+ static void iothread_get_aio_param(Object *obj, Visitor *v,
- #ifdef CONFIG_LINUX_AIO
+         const char *name, void *opaque, Error **errp)
-     ctx->linux_aio = NULL;
+ {
- #endif
++    IOThreadParamInfo *info = opaque;
 -    iothread_get_param(obj, v, name, opaque, errp);
 +    iothread_get_param(obj, v, name, info, errp);
  }
  static void iothread_set_aio_param(Object *obj, Visitor *v,
          const char *name, void *opaque, Error **errp)
  {
      IOThread *iothread = IOTHREAD(obj);
 +    IOThreadParamInfo *info = opaque;
 -    if (!iothread_set_param(obj, v, name, opaque, errp)) {
 +    if (!iothread_set_param(obj, v, name, info, errp)) {
          return;
      }
 --
-.26.2
+.31.1

-[PULL 2/3] async: always set ctx->notified in aio_notify()
+Deleted patch
-aio_notify() does not set ctx->notified when called with
-ctx->aio_notify_me disabled. Therefore aio_notify_me needs to be enabled
-during polling.
-This is suboptimal since expensive event_notifier_set(&ctx->notifier)
-and event_notifier_test_and_clear(&ctx->notifier) calls are required
-when ctx->aio_notify_me is enabled.
-Change aio_notify() so that aio->notified is always set, regardless of
-ctx->aio_notify_me. This will make polling cheaper since
-ctx->aio_notify_me can remain disabled. Move the
-event_notifier_test_and_clear() to the fd handler function (which is now
-no longer an empty function so "dummy" has been dropped from its name).
-The next patch takes advantage of this by optimizing polling in
-util/aio-posix.c.
-Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
-Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
-Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
-Message-id: 20200806131802.569478-3-stefanha@redhat.com
-[Paolo Bonzini pointed out that the smp_wmb() in aio_notify_accept()
-should be smp_wb() but the comment should be smp_wmb() instead of
-smp_wb(). Fixed.
---Stefan]
-Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
----
- util/async.c | 32 +++++++++++++++++++++-----------
-file changed, 21 insertions(+), 11 deletions(-)
-diff --git a/util/async.c b/util/async.c
-index XXXXXXX..XXXXXXX 100644
---- a/util/async.c
-+++ b/util/async.c
-@@ -XXX,XX +XXX,XX @@ LuringState *aio_get_linux_io_uring(AioContext *ctx)
- void aio_notify(AioContext *ctx)
- {
--    /* Write e.g. bh->scheduled before reading ctx->notify_me.  Pairs
-+    /*
-+     * Write e.g. bh->flags before writing ctx->notified.  Pairs with smp_mb in
-+     * aio_notify_accept.
-+     */
-+    smp_wmb();
-+    atomic_set(&ctx->notified, true);
-+
-+    /*
-+     * Write ctx->notified before reading ctx->notify_me.  Pairs
-      * with smp_mb in aio_ctx_prepare or aio_poll.
-      */
-     smp_mb();
-     if (atomic_read(&ctx->notify_me)) {
-         event_notifier_set(&ctx->notifier);
--        atomic_mb_set(&ctx->notified, true);
-     }
- }
- void aio_notify_accept(AioContext *ctx)
- {
--    if (atomic_xchg(&ctx->notified, false)
--#ifdef WIN32
--        || true
--#endif
--    ) {
--        event_notifier_test_and_clear(&ctx->notifier);
--    }
-+    atomic_set(&ctx->notified, false);
-+
-+    /*
-+     * Write ctx->notified before reading e.g. bh->flags.  Pairs with smp_wmb
-+     * in aio_notify.
-+     */
-+    smp_mb();
- }
- static void aio_timerlist_notify(void *opaque, QEMUClockType type)
-@@ -XXX,XX +XXX,XX @@ static void aio_timerlist_notify(void *opaque, QEMUClockType type)
-     aio_notify(opaque);
- }
--static void aio_context_notifier_dummy_cb(EventNotifier *e)
-+static void aio_context_notifier_cb(EventNotifier *e)
- {
-+    AioContext *ctx = container_of(e, AioContext, notifier);
-+
-+    event_notifier_test_and_clear(&ctx->notifier);
- }
- /* Returns true if aio_notify() was called (e.g. a BH was scheduled) */
-@@ -XXX,XX +XXX,XX @@ AioContext *aio_context_new(Error **errp)
-     aio_set_event_notifier(ctx, &ctx->notifier,
-                            false,
--                           aio_context_notifier_dummy_cb,
-+                           aio_context_notifier_cb,
-                            aio_context_notifier_poll);
- #ifdef CONFIG_LINUX_AIO
-     ctx->linux_aio = NULL;
---
-.26.2

The event_notifier_*() prefix can be confused with the EventNotifier
APIs that are also called event_notifier_*().

Rename the functions to aio_context_notifier_*() to make it clear that
they relate to the AioContext::notifier field.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Message-id: 20200806131802.569478-2-stefanha@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 util/async.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/util/async.c b/util/async.c
index XXXXXXX..XXXXXXX 100644
--- a/util/async.c
+++ b/util/async.c
@@ -XXX,XX +XXX,XX @@ static void aio_timerlist_notify(void *opaque, QEMUClockType type)
     aio_notify(opaque);
 }
 
-static void event_notifier_dummy_cb(EventNotifier *e)
+static void aio_context_notifier_dummy_cb(EventNotifier *e)
 {
 }
 
 /* Returns true if aio_notify() was called (e.g. a BH was scheduled) */
-static bool event_notifier_poll(void *opaque)
+static bool aio_context_notifier_poll(void *opaque)
 {
     EventNotifier *e = opaque;
     AioContext *ctx = container_of(e, AioContext, notifier);
@@ -XXX,XX +XXX,XX @@ AioContext *aio_context_new(Error **errp)
 
     aio_set_event_notifier(ctx, &ctx->notifier,
                            false,
-                           event_notifier_dummy_cb,
-                           event_notifier_poll);
+                           aio_context_notifier_dummy_cb,
+                           aio_context_notifier_poll);
 #ifdef CONFIG_LINUX_AIO
     ctx->linux_aio = NULL;
 #endif
-- 
2.26.2

aio_notify() does not set ctx->notified when called with
ctx->aio_notify_me disabled. Therefore aio_notify_me needs to be enabled
during polling.

This is suboptimal since expensive event_notifier_set(&ctx->notifier)
and event_notifier_test_and_clear(&ctx->notifier) calls are required
when ctx->aio_notify_me is enabled.

Change aio_notify() so that aio->notified is always set, regardless of
ctx->aio_notify_me. This will make polling cheaper since
ctx->aio_notify_me can remain disabled. Move the
event_notifier_test_and_clear() to the fd handler function (which is now
no longer an empty function so "dummy" has been dropped from its name).

The next patch takes advantage of this by optimizing polling in
util/aio-posix.c.

[Paolo Bonzini pointed out that the smp_wmb() in aio_notify_accept()
should be smp_wb() but the comment should be smp_wmb() instead of
smp_wb(). Fixed.
--Stefan]

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 util/async.c | 32 +++++++++++++++++++++-----------
 1 file changed, 21 insertions(+), 11 deletions(-)

diff --git a/util/async.c b/util/async.c
index XXXXXXX..XXXXXXX 100644
--- a/util/async.c
+++ b/util/async.c
@@ -XXX,XX +XXX,XX @@ LuringState *aio_get_linux_io_uring(AioContext *ctx)
 
 void aio_notify(AioContext *ctx)
 {
-    /* Write e.g. bh->scheduled before reading ctx->notify_me.  Pairs
+    /*
+     * Write e.g. bh->flags before writing ctx->notified.  Pairs with smp_mb in
+     * aio_notify_accept.
+     */
+    smp_wmb();
+    atomic_set(&ctx->notified, true);
+
+    /*
+     * Write ctx->notified before reading ctx->notify_me.  Pairs
      * with smp_mb in aio_ctx_prepare or aio_poll.
      */
     smp_mb();
     if (atomic_read(&ctx->notify_me)) {
         event_notifier_set(&ctx->notifier);
-        atomic_mb_set(&ctx->notified, true);
     }
 }
 
 void aio_notify_accept(AioContext *ctx)
 {
-    if (atomic_xchg(&ctx->notified, false)
-#ifdef WIN32
-        || true
-#endif
-    ) {
-        event_notifier_test_and_clear(&ctx->notifier);
-    }
+    atomic_set(&ctx->notified, false);
+
+    /*
+     * Write ctx->notified before reading e.g. bh->flags.  Pairs with smp_wmb
+     * in aio_notify.
+     */
+    smp_mb();
 }
 
 static void aio_timerlist_notify(void *opaque, QEMUClockType type)
@@ -XXX,XX +XXX,XX @@ static void aio_timerlist_notify(void *opaque, QEMUClockType type)
     aio_notify(opaque);
 }
 
-static void aio_context_notifier_dummy_cb(EventNotifier *e)
+static void aio_context_notifier_cb(EventNotifier *e)
 {
+    AioContext *ctx = container_of(e, AioContext, notifier);
+
+    event_notifier_test_and_clear(&ctx->notifier);
 }
 
 /* Returns true if aio_notify() was called (e.g. a BH was scheduled) */
@@ -XXX,XX +XXX,XX @@ AioContext *aio_context_new(Error **errp)
 
     aio_set_event_notifier(ctx, &ctx->notifier,
                            false,
-                           aio_context_notifier_dummy_cb,
+                           aio_context_notifier_cb,
                            aio_context_notifier_poll);
 #ifdef CONFIG_LINUX_AIO
     ctx->linux_aio = NULL;
-- 
2.26.2

Polling only monitors the ctx->notified field and does not need the
ctx->notifier EventNotifier to be signalled. Keep ctx->aio_notify_me
disabled while polling to avoid unnecessary EventNotifier syscalls.

This optimization improves virtio-blk 4KB random read performance by
18%. The following results are with an IOThread and the null-co block
driver:

Test         IOPS   Error
Before  244518.62 ± 1.20%
After   290706.11 ± 0.44%

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-id: 20200806131802.569478-4-stefanha@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 util/aio-posix.c | 47 +++++++++++++++++++++++++----------------------
 1 file changed, 25 insertions(+), 22 deletions(-)

diff --git a/util/aio-posix.c b/util/aio-posix.c
index XXXXXXX..XXXXXXX 100644
--- a/util/aio-posix.c
+++ b/util/aio-posix.c
@@ -XXX,XX +XXX,XX @@ static bool remove_idle_poll_handlers(AioContext *ctx, int64_t now)
  *
  * Polls for a given time.
  *
- * Note that ctx->notify_me must be non-zero so this function can detect
- * aio_notify().
- *
  * Note that the caller must have incremented ctx->list_lock.
  *
  * Returns: true if progress was made, false otherwise
@@ -XXX,XX +XXX,XX @@ static bool run_poll_handlers(AioContext *ctx, int64_t max_ns, int64_t *timeout)
     bool progress;
     int64_t start_time, elapsed_time;
 
-    assert(ctx->notify_me);
     assert(qemu_lockcnt_count(&ctx->list_lock) > 0);
 
     trace_run_poll_handlers_begin(ctx, max_ns, *timeout);
@@ -XXX,XX +XXX,XX @@ static bool run_poll_handlers(AioContext *ctx, int64_t max_ns, int64_t *timeout)
  * @timeout: timeout for blocking wait, computed by the caller and updated if
  *    polling succeeds.
  *
- * ctx->notify_me must be non-zero so this function can detect aio_notify().
- *
  * Note that the caller must have incremented ctx->list_lock.
  *
  * Returns: true if progress was made, false otherwise
@@ -XXX,XX +XXX,XX @@ bool aio_poll(AioContext *ctx, bool blocking)
     AioHandlerList ready_list = QLIST_HEAD_INITIALIZER(ready_list);
     int ret = 0;
     bool progress;
+    bool use_notify_me;
     int64_t timeout;
     int64_t start = 0;
 
@@ -XXX,XX +XXX,XX @@ bool aio_poll(AioContext *ctx, bool blocking)
      */
     assert(in_aio_context_home_thread(ctx));
 
-    /* aio_notify can avoid the expensive event_notifier_set if
+    qemu_lockcnt_inc(&ctx->list_lock);
+
+    if (ctx->poll_max_ns) {
+        start = qemu_clock_get_ns(QEMU_CLOCK_REALTIME);
+    }
+
+    timeout = blocking ? aio_compute_timeout(ctx) : 0;
+    progress = try_poll_mode(ctx, &timeout);
+    assert(!(timeout && progress));
+
+    /*
+     * aio_notify can avoid the expensive event_notifier_set if
      * everything (file descriptors, bottom halves, timers) will
      * be re-evaluated before the next blocking poll().  This is
      * already true when aio_poll is called with blocking == false;
      * if blocking == true, it is only true after poll() returns,
      * so disable the optimization now.
      */
-    if (blocking) {
+    use_notify_me = timeout != 0;
+    if (use_notify_me) {
         atomic_set(&ctx->notify_me, atomic_read(&ctx->notify_me) + 2);
         /*
-         * Write ctx->notify_me before computing the timeout
-         * (reading bottom half flags, etc.).  Pairs with
+         * Write ctx->notify_me before reading ctx->notified.  Pairs with
          * smp_mb in aio_notify().
          */
         smp_mb();
-    }
-
-    qemu_lockcnt_inc(&ctx->list_lock);
 
-    if (ctx->poll_max_ns) {
-        start = qemu_clock_get_ns(QEMU_CLOCK_REALTIME);
+        /* Don't block if aio_notify() was called */
+        if (atomic_read(&ctx->notified)) {
+            timeout = 0;
+        }
     }
 
-    timeout = blocking ? aio_compute_timeout(ctx) : 0;
-    progress = try_poll_mode(ctx, &timeout);
-    assert(!(timeout && progress));
-
     /* If polling is allowed, non-blocking aio_poll does not need the
      * system call---a single round of run_poll_handlers_once suffices.
      */
@@ -XXX,XX +XXX,XX @@ bool aio_poll(AioContext *ctx, bool blocking)
         ret = ctx->fdmon_ops->wait(ctx, &ready_list, timeout);
     }
 
-    if (blocking) {
+    if (use_notify_me) {
         /* Finish the poll before clearing the flag.  */
-        atomic_store_release(&ctx->notify_me, atomic_read(&ctx->notify_me) - 2);
-        aio_notify_accept(ctx);
+        atomic_store_release(&ctx->notify_me,
+                             atomic_read(&ctx->notify_me) - 2);
     }
 
+    aio_notify_accept(ctx);
+
     /* Adjust polling time */
     if (ctx->poll_max_ns) {
         int64_t block_ns = qemu_clock_get_ns(QEMU_CLOCK_REALTIME) - start;
-- 
2.26.2

From: Stefano Garzarella <sgarzare@redhat.com>

Commit 1793ad0247 ("iothread: add aio-max-batch parameter") added
a new parameter (aio-max-batch) to IOThread and used PollParamInfo
structure to handle it.

Since it is not a parameter of the polling mechanism, we rename the
structure to a more generic IOThreadParamInfo.

Suggested-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-id: 20210727145936.147032-2-sgarzare@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 iothread.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/iothread.c b/iothread.c
index XXXXXXX..XXXXXXX 100644
--- a/iothread.c
+++ b/iothread.c
@@ -XXX,XX +XXX,XX @@ static void iothread_complete(UserCreatable *obj, Error **errp)
 typedef struct {
     const char *name;
     ptrdiff_t offset; /* field's byte offset in IOThread struct */
-} PollParamInfo;
+} IOThreadParamInfo;
 
-static PollParamInfo poll_max_ns_info = {
+static IOThreadParamInfo poll_max_ns_info = {
     "poll-max-ns", offsetof(IOThread, poll_max_ns),
 };
-static PollParamInfo poll_grow_info = {
+static IOThreadParamInfo poll_grow_info = {
     "poll-grow", offsetof(IOThread, poll_grow),
 };
-static PollParamInfo poll_shrink_info = {
+static IOThreadParamInfo poll_shrink_info = {
     "poll-shrink", offsetof(IOThread, poll_shrink),
 };
-static PollParamInfo aio_max_batch_info = {
+static IOThreadParamInfo aio_max_batch_info = {
     "aio-max-batch", offsetof(IOThread, aio_max_batch),
 };
 
@@ -XXX,XX +XXX,XX @@ static void iothread_get_param(Object *obj, Visitor *v,
         const char *name, void *opaque, Error **errp)
 {
     IOThread *iothread = IOTHREAD(obj);
-    PollParamInfo *info = opaque;
+    IOThreadParamInfo *info = opaque;
     int64_t *field = (void *)iothread + info->offset;
 
     visit_type_int64(v, name, field, errp);
@@ -XXX,XX +XXX,XX @@ static bool iothread_set_param(Object *obj, Visitor *v,
         const char *name, void *opaque, Error **errp)
 {
     IOThread *iothread = IOTHREAD(obj);
-    PollParamInfo *info = opaque;
+    IOThreadParamInfo *info = opaque;
     int64_t *field = (void *)iothread + info->offset;
     int64_t value;
 
-- 
2.31.1

From: Stefano Garzarella <sgarzare@redhat.com>

Commit 0445409d74 ("iothread: generalize
iothread_set_param/iothread_get_param") moved common code to set and
get IOThread parameters in two new functions.

These functions are called inside callbacks, so we don't need to use an
opaque pointer. Let's replace `void *opaque` parameter with
`IOThreadParamInfo *info`.

Suggested-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-id: 20210727145936.147032-3-sgarzare@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 iothread.c | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/iothread.c b/iothread.c
index XXXXXXX..XXXXXXX 100644
--- a/iothread.c
+++ b/iothread.c
@@ -XXX,XX +XXX,XX @@ static IOThreadParamInfo aio_max_batch_info = {
 };
 
 static void iothread_get_param(Object *obj, Visitor *v,
-        const char *name, void *opaque, Error **errp)
+        const char *name, IOThreadParamInfo *info, Error **errp)
 {
     IOThread *iothread = IOTHREAD(obj);
-    IOThreadParamInfo *info = opaque;
     int64_t *field = (void *)iothread + info->offset;
 
     visit_type_int64(v, name, field, errp);
 }
 
 static bool iothread_set_param(Object *obj, Visitor *v,
-        const char *name, void *opaque, Error **errp)
+        const char *name, IOThreadParamInfo *info, Error **errp)
 {
     IOThread *iothread = IOTHREAD(obj);
-    IOThreadParamInfo *info = opaque;
     int64_t *field = (void *)iothread + info->offset;
     int64_t value;
 
@@ -XXX,XX +XXX,XX @@ static bool iothread_set_param(Object *obj, Visitor *v,
 static void iothread_get_poll_param(Object *obj, Visitor *v,
         const char *name, void *opaque, Error **errp)
 {
+    IOThreadParamInfo *info = opaque;
 
-    iothread_get_param(obj, v, name, opaque, errp);
+    iothread_get_param(obj, v, name, info, errp);
 }
 
 static void iothread_set_poll_param(Object *obj, Visitor *v,
         const char *name, void *opaque, Error **errp)
 {
     IOThread *iothread = IOTHREAD(obj);
+    IOThreadParamInfo *info = opaque;
 
-    if (!iothread_set_param(obj, v, name, opaque, errp)) {
+    if (!iothread_set_param(obj, v, name, info, errp)) {
         return;
     }
 
@@ -XXX,XX +XXX,XX @@ static void iothread_set_poll_param(Object *obj, Visitor *v,
 static void iothread_get_aio_param(Object *obj, Visitor *v,
         const char *name, void *opaque, Error **errp)
 {
+    IOThreadParamInfo *info = opaque;
 
-    iothread_get_param(obj, v, name, opaque, errp);
+    iothread_get_param(obj, v, name, info, errp);
 }
 
 static void iothread_set_aio_param(Object *obj, Visitor *v,
         const char *name, void *opaque, Error **errp)
 {
     IOThread *iothread = IOTHREAD(obj);
+    IOThreadParamInfo *info = opaque;
 
-    if (!iothread_set_param(obj, v, name, opaque, errp)) {
+    if (!iothread_set_param(obj, v, name, info, errp)) {
         return;
     }
 
-- 
2.31.1