Re: [Qemu-devel] [PATCHv2] [RFC 6/7] aio / timers: Switch to ppoll, run AioContext timers in aio_poll/aio_dispatch

[Stefan Hajnoczi <stefanha@gmail.com>]

On Sat, Jul 20, 2013 at 07:06:42PM +0100, Alex Bligh wrote:
> @@ -245,11 +249,13 @@ bool aio_poll(AioContext *ctx, bool blocking)
>                  node->pfd.revents = pfd->revents;
>              }
>          }
> -        if (aio_dispatch(ctx)) {
> -            progress = true;
> -        }
> +    }
> +
> +    /* Run dispatch even if there were no readable fds to run timers */
> +    if (aio_dispatch(ctx)) {
> +        progress = true;
>      }
>  
>      assert(progress || busy);
> -    return true;
> +    return progress;

Now aio_poll() can return false when it used to return true?

> @@ -214,6 +221,15 @@ bool aio_poll(AioContext *ctx, bool blocking)
>          events[ret - WAIT_OBJECT_0] = events[--count];
>      }
>  
> +    if (blocking) {
> +        /* Run the timers a second time. We do this because otherwise aio_wait
> +         * will not note progress - and will stop a drain early - if we have
> +         * a timer that was not ready to run entering g_poll but is ready
> +         * after g_poll. This will only do anything if a timer has expired.
> +         */
> +        progress |= qemu_run_timers(ctx->clock);
> +    }
> +
>      assert(progress || busy);
>      return true;

You didn't update this to return just progress.

>  }
> diff --git a/async.c b/async.c
> index 0d41431..cb6b1d4 100644
> --- a/async.c
> +++ b/async.c
> @@ -123,13 +123,16 @@ aio_ctx_prepare(GSource *source, gint    *timeout)
>  {
>      AioContext *ctx = (AioContext *) source;
>      QEMUBH *bh;
> +    int deadline;
>  
>      for (bh = ctx->first_bh; bh; bh = bh->next) {
>          if (!bh->deleted && bh->scheduled) {
>              if (bh->idle) {
>                  /* idle bottom halves will be polled at least
>                   * every 10ms */
> -                *timeout = 10;
> +                if ((*timeout < 0) || (*timeout > 10)) {
> +                    *timeout = 10;
> +                }

Use the function you introduced earlier to return the nearest timeout?

>              } else {
>                  /* non-idle bottom halves will be executed
>                   * immediately */
> @@ -139,6 +142,15 @@ aio_ctx_prepare(GSource *source, gint    *timeout)
>          }
>      }
>  
> +    deadline = qemu_timeout_ns_to_ms(qemu_clock_deadline_ns(ctx->clock));
> +    if (deadline == 0) {
> +        *timeout = 0;
> +        return true;
> +    } else if ((deadline > 0) &&
> +               ((*timeout < 0) || (deadline < *timeout))) {
> +        *timeout = deadline;

Same here.

> @@ -170,9 +171,13 @@ static void glib_pollfds_fill(uint32_t *cur_timeout)
>                                   glib_n_poll_fds);
>      } while (n != glib_n_poll_fds);
>  
> -    if (timeout >= 0 && timeout < *cur_timeout) {
> -        *cur_timeout = timeout;
> +    if (timeout < 0) {
> +        timeout_ns = -1;
> +    } else {
> +      timeout_ns = (int64_t)timeout * (int64_t)SCALE_MS;

Indentation.