From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:39006) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1V2Hvi-0003O6-JN for qemu-devel@nongnu.org; Thu, 25 Jul 2013 05:33:54 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1V2Hvf-0003j7-Ds for qemu-devel@nongnu.org; Thu, 25 Jul 2013 05:33:50 -0400 Received: from mail-ee0-x232.google.com ([2a00:1450:4013:c00::232]:39217) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1V2Hvf-0003ir-6i for qemu-devel@nongnu.org; Thu, 25 Jul 2013 05:33:47 -0400 Received: by mail-ee0-f50.google.com with SMTP id d49so776336eek.23 for ; Thu, 25 Jul 2013 02:33:45 -0700 (PDT) Date: Thu, 25 Jul 2013 11:33:43 +0200 From: Stefan Hajnoczi Message-ID: <20130725093343.GF21033@stefanha-thinkpad.redhat.com> References: <1E8E204.8000201@redhat.com> <1374343603-29183-1-git-send-email-alex@alex.org.uk> <1374343603-29183-7-git-send-email-alex@alex.org.uk> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1374343603-29183-7-git-send-email-alex@alex.org.uk> Subject: Re: [Qemu-devel] [PATCHv2] [RFC 6/7] aio / timers: Switch to ppoll, run AioContext timers in aio_poll/aio_dispatch List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alex Bligh Cc: Kevin Wolf , Anthony Liguori , qemu-devel@nongnu.org, Stefan Hajnoczi , Paolo Bonzini , rth@twiddle.net 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.