[PATCH RFC v2 1/3] aio-poll: avoid unnecessary polling time computation

Jaehoon Kim posted 3 patches 1 week, 4 days ago
Maintainers: Stefan Hajnoczi <stefanha@redhat.com>, Fam Zheng <fam@euphon.net>, "Dr. David Alan Gilbert" <dave@treblig.org>, Eric Blake <eblake@redhat.com>, Markus Armbruster <armbru@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, "Daniel P. Berrangé" <berrange@redhat.com>, Stefan Weil <sw@weilnetz.de>
[PATCH RFC v2 1/3] aio-poll: avoid unnecessary polling time computation
Posted by Jaehoon Kim 1 week, 4 days ago
Nodes are no longer added to poll_aio_handlers when adaptive polling is
disabled, preventing unnecessary try_poll_mode() calls. Additionally,
aio_poll() skips try_poll_mode() when timeout is 0.

This avoids iterating over all nodes to compute max_ns unnecessarily
when polling is disabled or timeout is 0.

Signed-off-by: Jaehoon Kim <jhkim@linux.ibm.com>
---
 util/aio-posix.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/util/aio-posix.c b/util/aio-posix.c
index 488d964611..b02beb0505 100644
--- a/util/aio-posix.c
+++ b/util/aio-posix.c
@@ -307,9 +307,8 @@ static bool aio_dispatch_handler(AioContext *ctx, AioHandler *node)
      * fdmon_supports_polling(), but only until the fd fires for the first
      * time.
      */
-    if (!QLIST_IS_INSERTED(node, node_deleted) &&
-        !QLIST_IS_INSERTED(node, node_poll) &&
-        node->io_poll) {
+    if (ctx->poll_max_ns && !QLIST_IS_INSERTED(node, node_deleted) &&
+        !QLIST_IS_INSERTED(node, node_poll) && node->io_poll) {
         trace_poll_add(ctx, node, node->pfd.fd, revents);
         if (ctx->poll_started && node->io_poll_begin) {
             node->io_poll_begin(node->opaque);
@@ -631,7 +630,7 @@ static void adjust_polling_time(AioContext *ctx, AioPolledEvent *poll,
 bool aio_poll(AioContext *ctx, bool blocking)
 {
     AioHandlerList ready_list = QLIST_HEAD_INITIALIZER(ready_list);
-    bool progress;
+    bool progress = false;
     bool use_notify_me;
     int64_t timeout;
     int64_t start = 0;
@@ -656,7 +655,9 @@ bool aio_poll(AioContext *ctx, bool blocking)
     }
 
     timeout = blocking ? aio_compute_timeout(ctx) : 0;
-    progress = try_poll_mode(ctx, &ready_list, &timeout);
+    if ((ctx->poll_max_ns != 0) && (timeout != 0)) {
+        progress = try_poll_mode(ctx, &ready_list, &timeout);
+    }
     assert(!(timeout && progress));
 
     /*
-- 
2.50.1
Re: [PATCH RFC v2 1/3] aio-poll: avoid unnecessary polling time computation
Posted by Stefan Hajnoczi 1 week, 2 days ago
On Mon, Mar 23, 2026 at 08:54:49AM -0500, Jaehoon Kim wrote:
> Nodes are no longer added to poll_aio_handlers when adaptive polling is
> disabled, preventing unnecessary try_poll_mode() calls. Additionally,
> aio_poll() skips try_poll_mode() when timeout is 0.

Skipping when timeout is 0 seems risky to me. VIRTIO devices disable
guest kicks when polling mode is started. When aio_poll(ctx,
blocking=false) is called, we will skip polling and
ctx->fdmon_ops->need_wait(ctx) won't detect an event either. aio_poll()
will return without noticing that the VIRTIO device's AioHandler is
ready.

Is skipping when timeout 0 necessary for performance or can it be
dropped from the patch?

Aside from this:

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Re: [PATCH RFC v2 1/3] aio-poll: avoid unnecessary polling time computation
Posted by JAEHOON KIM 1 week, 1 day ago
On 3/25/2026 12:22 PM, Stefan Hajnoczi wrote:
> On Mon, Mar 23, 2026 at 08:54:49AM -0500, Jaehoon Kim wrote:
>> Nodes are no longer added to poll_aio_handlers when adaptive polling is
>> disabled, preventing unnecessary try_poll_mode() calls. Additionally,
>> aio_poll() skips try_poll_mode() when timeout is 0.
> Skipping when timeout is 0 seems risky to me. VIRTIO devices disable
> guest kicks when polling mode is started. When aio_poll(ctx,
> blocking=false) is called, we will skip polling and
> ctx->fdmon_ops->need_wait(ctx) won't detect an event either. aio_poll()
> will return without noticing that the VIRTIO device's AioHandler is
> ready.
>
> Is skipping when timeout 0 necessary for performance or can it be
> dropped from the patch?
>
> Aside from this:
>
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

Thank you for the review!
Removing the (timeout != 0) check does not cause any issue on my side,
so I plan to remove it in the next version.

However, I'd like to clarify: when timeout is 0, is there any concern
in the try_poll_mode function below that I might be missing?
I just want to make sure I understand correctly.

static bool try_poll_mode(AioContext *ctx, AioHandlerList *ready_list,
                           int64_t *timeout)
{
     AioHandler *node;
     int64_t max_ns;

     if (QLIST_EMPTY_RCU(&ctx->poll_aio_handlers)) {
         return false;
     }

     max_ns = 0;
     QLIST_FOREACH(node, &ctx->poll_aio_handlers, node_poll) {
         max_ns = MAX(max_ns, node->poll.ns);
     }
     max_ns = qemu_soonest_timeout(*timeout, max_ns);

     if (max_ns && !ctx->fdmon_ops->need_wait(ctx)) {
         /*
          * Enable poll mode. It pairs with the poll_set_started() in
          * aio_poll() which disables poll mode.
          */
         poll_set_started(ctx, ready_list, true);


     .....


Regards,
Jaehoon Kim.



Re: [PATCH RFC v2 1/3] aio-poll: avoid unnecessary polling time computation
Posted by Stefan Hajnoczi 1 week, 1 day ago
On Thu, Mar 26, 2026 at 01:17:44PM -0500, JAEHOON KIM wrote:
> On 3/25/2026 12:22 PM, Stefan Hajnoczi wrote:
> > On Mon, Mar 23, 2026 at 08:54:49AM -0500, Jaehoon Kim wrote:
> > > Nodes are no longer added to poll_aio_handlers when adaptive polling is
> > > disabled, preventing unnecessary try_poll_mode() calls. Additionally,
> > > aio_poll() skips try_poll_mode() when timeout is 0.
> > Skipping when timeout is 0 seems risky to me. VIRTIO devices disable
> > guest kicks when polling mode is started. When aio_poll(ctx,
> > blocking=false) is called, we will skip polling and
> > ctx->fdmon_ops->need_wait(ctx) won't detect an event either. aio_poll()
> > will return without noticing that the VIRTIO device's AioHandler is
> > ready.
> > 
> > Is skipping when timeout 0 necessary for performance or can it be
> > dropped from the patch?
> > 
> > Aside from this:
> > 
> > Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> 
> Thank you for the review!
> Removing the (timeout != 0) check does not cause any issue on my side,
> so I plan to remove it in the next version.
> 
> However, I'd like to clarify: when timeout is 0, is there any concern
> in the try_poll_mode function below that I might be missing?
> I just want to make sure I understand correctly.
> 
> static bool try_poll_mode(AioContext *ctx, AioHandlerList *ready_list,
>                           int64_t *timeout)
> {
>     AioHandler *node;
>     int64_t max_ns;
> 
>     if (QLIST_EMPTY_RCU(&ctx->poll_aio_handlers)) {
>         return false;
>     }
> 
>     max_ns = 0;
>     QLIST_FOREACH(node, &ctx->poll_aio_handlers, node_poll) {
>         max_ns = MAX(max_ns, node->poll.ns);
>     }
>     max_ns = qemu_soonest_timeout(*timeout, max_ns);
> 
>     if (max_ns && !ctx->fdmon_ops->need_wait(ctx)) {

I think you are pointing out that max_ns will be 0 and therefore polling
will be skipped? At least this is what came to mind when I read this
code again.

That is a problem because the exact scenario I described in my reply to
you can already happen in the existing code before your patch :(.

Avoiding the timeout != 0 check in your patch would help contain the bug
in try_poll_mode() rather than extending it to aio_poll() as well.

Thanks for pointing this out!

Stefan