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

[Stefan Hajnoczi]

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?

  bool 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
       * smp_mb in aio_notify().
       */
      smp_mb();

      /* Don't block if aio_notify() was called */
      if (atomic_read(ctx->notified)) {
          timeout = 0;
      }
  }

  ret = ctx->fdmon_ops->wait(ctx, &ready_list, timeout);

  if (use_notify_me) {
      /* Finish the poll before clearing the flag.  */
      atomic_store_release(&ctx->notify_me,
                           atomic_read(&ctx->notify_me) - 2);
  }

signature.asc
Description: PGP signature