Re: [PATCH 3/3] aio-posix: keep aio_notify_me disabled during polling

[Stefan Hajnoczi]

On Tue, Aug 04, 2020 at 06:53:09PM +0200, Paolo Bonzini wrote:
> On 04/08/20 12:29, Stefan Hajnoczi wrote:
> > On Tue, Aug 04, 2020 at 06:28:04AM +0100, Stefan Hajnoczi wrote:
> >> @@ -597,15 +574,38 @@ bool aio_poll(AioContext *ctx, bool blocking)
> >>       * system call---a single round of run_poll_handlers_once suffices.
> >>       */
> >>      if (timeout || ctx->fdmon_ops->need_wait(ctx)) {
> >> +        /*
> >> +         * 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 (timeout) {
> >> +            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
> >> +             * smp_mb in aio_notify().
> >> +             */
> >> +            smp_mb();
> >> +
> >> +            /* Check again in case a shorter timer was added */
> >> +            timeout = qemu_soonest_timeout(timeout, 
> >> aio_compute_timeout(ctx));
> >> +        }
> >> +
> >>          ret = ctx->fdmon_ops->wait(ctx, &ready_list, timeout);
> >> -    }
> >>  
> >> -    if (blocking) {
> >> -        /* Finish the poll before clearing the flag.  */
> >> -        atomic_store_release(&ctx->notify_me, 
> >> atomic_read(&ctx->notify_me) - 2);
> >> -        aio_notify_accept(ctx);
> >> +        if (timeout) {
> >> +            /* Finish the poll before clearing the flag.  */
> >> +            atomic_store_release(&ctx->notify_me,
> >> +                                 atomic_read(&ctx->notify_me) - 2);
> >> +        }
> >>      }
> > 
> > Hi Paolo,
> > We can avoid calling aio_compute_timeout() like this, what do you think?
> 
> I don't understand :) except I guess you mean we can avoid the second
> call.  Can you post either a complete patch with this squashed, or a 4th
> patch (whatever you think is best)?

Sure, I'll post a new revision of this series.

Stefan

signature.asc
Description: PGP signature