From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:35055) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VLxqf-0007Qw-D5 for qemu-devel@nongnu.org; Tue, 17 Sep 2013 12:10:02 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VLxqZ-0005Zu-Nr for qemu-devel@nongnu.org; Tue, 17 Sep 2013 12:09:57 -0400 Received: from goliath.siemens.de ([192.35.17.28]:31600) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VLxqZ-0005ZP-Ec for qemu-devel@nongnu.org; Tue, 17 Sep 2013 12:09:51 -0400 Message-ID: <52387EC2.6000502@siemens.com> Date: Tue, 17 Sep 2013 18:09:38 +0200 From: Jan Kiszka MIME-Version: 1.0 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> <20130813142204.GA3008@stefanha-thinkpad.redhat.com> In-Reply-To: <20130813142204.GA3008@stefanha-thinkpad.redhat.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit 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: Stefan Hajnoczi Cc: Kevin Wolf , Anthony Liguori , Alex Bligh , qemu-devel@nongnu.org, liu ping fan , Stefan Hajnoczi , Paolo Bonzini , MORITA Kazutaka , rth@twiddle.net On 2013-08-13 16:22, Stefan Hajnoczi wrote: > 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. Need to pick up this topic again because above change is now mainline and breaks aio_poll-based timer threads: How can we make progress with overcoming that check, at least for the timer thread use case? Additional argument "truly_block" for aio_poll? Jan -- Siemens AG, Corporate Technology, CT RTC ITP SES-DE Corporate Competence Center Embedded Linux