From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:46477) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1V9FUC-0000zY-Hc for qemu-devel@nongnu.org; Tue, 13 Aug 2013 10:22:17 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1V9FU7-0000Cm-KO for qemu-devel@nongnu.org; Tue, 13 Aug 2013 10:22:12 -0400 Received: from mail-ee0-x232.google.com ([2a00:1450:4013:c00::232]:48730) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1V9FU7-0000Cb-E0 for qemu-devel@nongnu.org; Tue, 13 Aug 2013 10:22:07 -0400 Received: by mail-ee0-f50.google.com with SMTP id d51so4283491eek.9 for ; Tue, 13 Aug 2013 07:22:06 -0700 (PDT) Date: Tue, 13 Aug 2013 16:22:04 +0200 From: Stefan Hajnoczi Message-ID: <20130813142204.GA3008@stefanha-thinkpad.redhat.com> References: <1376239405-4084-1-git-send-email-alex@alex.org.uk> <520A2511.4000709@siemens.com> <307AE3B5-FAFE-4E9C-A336-092245809528@alex.org.uk> <520A33B4.9030207@siemens.com> <14A27B81-C9FD-4EE5-BC4A-7106CD70527A@alex.org.uk> <520A3888.9020307@siemens.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <520A3888.9020307@siemens.com> Subject: Re: [Qemu-devel] [RFC] [PATCHv10 00/31] aio / timers: Add AioContext timers and use ppoll List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jan Kiszka Cc: Kevin Wolf , Anthony Liguori , Alex Bligh , qemu-devel@nongnu.org, liu ping fan , Stefan Hajnoczi , Paolo Bonzini , MORITA Kazutaka , rth@twiddle.net On Tue, Aug 13, 2013 at 03:45:44PM +0200, Jan Kiszka wrote: > On 2013-08-13 15:39, Alex Bligh wrote: > > Jan, > > > > On 13 Aug 2013, at 14:25, Jan Kiszka wrote: > > > >> To my understanding, the use case behind the current behavior is > >> qemu_aio_wait() which is only supposed to block when there are pending > >> requests for the main aio context. We should be able to address this > >> scenarios also in a different way. I would definitely prefer to not > >> depend on that hack above. > > > > I don't *think* so. If I'm right the problem is line 233 of > > aio-posix.c (and similar in the windows variant): > > > > /* No AIO operations? Get us out of here */ > > if (!busy) { > > return progress; > > } > > > > ... do qemu_poll_ns ... > > > > busy is set to true if there are any FDs for which ->flush > > is true and ->io_flush() returns non-zero. > > Right. > > > > > I think this should instead be looking the equivalent of > > FD_ISSET across all FDs (read and write) and the blocking flag. > > IE if blocking is set to true, then it should ALWAYS do > > qemu_poll_ns, lest it busyloop rather than wait for the > > next timer expiry. > > Yes, that would be needed. > > > > > More here: > > https://lists.gnu.org/archive/html/qemu-devel/2013-07/msg03950.html > > > > I'm not very happy with this logic (but it's the same as before), > > and I note Stefan removes the horrible busy flag in this > > series: > > http://lists.nongnu.org/archive/html/qemu-devel/2013-07/msg00092.html > > Yeah: > > - /* No AIO operations? Get us out of here */ > - if (!busy) { > + /* early return if we only have the aio_notify() fd */ > + if (ctx->pollfds->len == 1) { > return progress; > } > > So this is even worse for my use case. We can change the semantics of aio_poll() so long as we don't break existing callers and tests. It would make sense to do that after merging the io_flush and AioContext timers series. Stefan