From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:38529)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <jan.kiszka@siemens.com>) id 1V9EvB-0001lO-Q1
	for qemu-devel@nongnu.org; Tue, 13 Aug 2013 09:46:07 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <jan.kiszka@siemens.com>) id 1V9Ev4-0003v6-6f
	for qemu-devel@nongnu.org; Tue, 13 Aug 2013 09:46:01 -0400
Received: from david.siemens.de ([192.35.17.14]:16406)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <jan.kiszka@siemens.com>) id 1V9Ev3-0003uM-TK
	for qemu-devel@nongnu.org; Tue, 13 Aug 2013 09:45:54 -0400
Message-ID: <520A3888.9020307@siemens.com>
Date: Tue, 13 Aug 2013 15:45:44 +0200
From: Jan Kiszka <jan.kiszka@siemens.com>
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>
In-Reply-To: <14A27B81-C9FD-4EE5-BC4A-7106CD70527A@alex.org.uk>
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: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Alex Bligh <alex@alex.org.uk>
Cc: Kevin Wolf <kwolf@redhat.com>, Anthony Liguori <aliguori@us.ibm.com>, qemu-devel@nongnu.org, liu ping fan <qemulist@gmail.com>, Stefan Hajnoczi <stefanha@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, MORITA Kazutaka <morita.kazutaka@lab.ntt.co.jp>, rth@twiddle.net

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.

Jan

-- 
Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux