From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:41619) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1V2RAu-0006t0-A6 for qemu-devel@nongnu.org; Thu, 25 Jul 2013 15:26:10 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1V2RAt-0005Pb-2p for qemu-devel@nongnu.org; Thu, 25 Jul 2013 15:26:08 -0400 Received: from mail.avalus.com ([2001:41c8:10:1dd::10]:56764) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1V2QdV-00006q-2B for qemu-devel@nongnu.org; Thu, 25 Jul 2013 14:51:37 -0400 Date: Thu, 25 Jul 2013 19:51:20 +0100 From: Alex Bligh Message-ID: <18FA9A6B1FACABA153218D97@nimrod.local> In-Reply-To: <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> <20130725093343.GF21033@stefanha-thinkpad.redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii; format=flowed Content-Transfer-Encoding: 7bit Content-Disposition: inline Subject: Re: [Qemu-devel] [PATCHv2] [RFC 6/7] aio / timers: Switch to ppoll, run AioContext timers in aio_poll/aio_dispatch Reply-To: Alex Bligh List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Hajnoczi Cc: Kevin Wolf , Anthony Liguori , Alex Bligh , qemu-devel@nongnu.org, Stefan Hajnoczi , Paolo Bonzini , rth@twiddle.net Stefan, I missed a couple of comments. --On 25 July 2013 11:33:43 +0200 Stefan Hajnoczi wrote: >> @@ -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. Will fix >> } >> 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? The function I introduced takes an int64_t not an int. I think that's OK though. > >> } 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. Ditto >> @@ -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. Will fix -- Alex Bligh