From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34460) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Uyplx-0004f8-EZ for qemu-devel@nongnu.org; Mon, 15 Jul 2013 16:53:30 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Uyplw-0001fv-7T for qemu-devel@nongnu.org; Mon, 15 Jul 2013 16:53:29 -0400 Received: from mail-ea0-x233.google.com ([2a00:1450:4013:c01::233]:56859) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Uyplv-0001fr-U1 for qemu-devel@nongnu.org; Mon, 15 Jul 2013 16:53:28 -0400 Received: by mail-ea0-f179.google.com with SMTP id b15so8116632eae.10 for ; Mon, 15 Jul 2013 13:53:27 -0700 (PDT) Sender: Paolo Bonzini Message-ID: <51E4613D.9000106@redhat.com> Date: Mon, 15 Jul 2013 22:53:17 +0200 From: Paolo Bonzini MIME-Version: 1.0 References: <1373127897-3445-1-git-send-email-alex@alex.org.uk> <51E4063D.6010308@redhat.com> <75638CBA408455BF52192DA7@Ximines.local> In-Reply-To: <75638CBA408455BF52192DA7@Ximines.local> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] [RFC] aio/async: Add timed bottom-halves 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 , rth@twiddle.net Il 15/07/2013 22:15, Alex Bligh ha scritto: > Paolo, > > --On 15 July 2013 16:25:01 +0200 Paolo Bonzini wrote: > > Thanks for the review. > >> Il 06/07/2013 18:24, Alex Bligh ha scritto: >>> Add timed bottom halves. A timed bottom half is a bottom half that >>> will not execute until a given time has passed (qemu_bh_schedule_at) >>> or a given interval has passed (qemu_bh_schedule_in). >> >> ... and may be delayed arbitrarily past that given interval if you are >> running in qemu-img or in other synchronous I/O APIs. > > That's true. However, the problem with timers is worse, in that we > poll for timers even less frequently as far as I can tell. Right, we poll for bottom halves during qemu_aio_wait(). We don't poll for timers. >> I'm especially >> worried that this will not have any effect if bdrv_aio_cancel is calling >> qemu_aio_wait. bdrv_aio_cancel is presumably one place where you want >> timeout/reconnect functionality to trigger. > > Well, I'm a newbie here, so may well be wrong but I thought qemu_aio_wait > /did/ call bottom halves (but didn't call QemuTimers). Provided time > does actually advance (which inspection suggests it does), then > these bh's should be called just like any other bh's. I may have missed > the point here entirely. So far you are right. But this only happens if qemu_aio_wait() actually returns, so that on the next call we poll for timers. If QEMU is stuck in qemu_aio_wait()'s infinite-timeout poll(), it will never advance and process the timed bottom halves. This goes to the question of having aio_notify() or not. If you have it, you will immediately process timed BHs that are "born expired". For other bottom halves, there will be no difference if you add it or not. >> I would really prefer to have a TimeEventNotifier or something like >> that, which is API-compatibile with EventNotifier (so you can use the >> regular aio-*.c APIs) but triggers when a given time has passed. >> Basically an "heavyweight" QEMUTimer; that would be a timerfd on Linux, >> and a queue timer on Windows. No idea on other POSIX systems, >> unfortunately. > > I was trying to use the bh API because that's what the existing block > drivers use, and what I really wanted was a bh that wouldn't execute > for a while. Do EventNotifiers run whilst AIO is polling for completion? Yes, and they can actually interrupt qemu_aio_wait(). See aio_set_event_notifier. >> Even better would be to remove the whole timer stuff (POSIX timers, >> setitimer, and the Win32 equivalents), and just make the timers use a >> shorter timeout for the main loop. If you do this, I suspect adding >> timer support to AioContext would be much simpler. > > In discussion with Stefan H on IRC, I originally suggested moving the > QemuTimer poll to the AIO loop (or adding another), which is a half > arsed way to do what you are suggesting. He suggested this would be > hairy because the existing users might not be safe to be called there. > This was an attempt at a minimal change to fix that use. > >> BTW, note that qemu-nbd (and qemu-io too) does call timers. > > I'd thought I tested qemu-io. qemu-convert definitely does not. qemu-io is the tool that is used by the unit tests. Conversion is in qemu-img. Paolo > Alex > >> Paolo >> >>> Any qemu >>> clock can be used, and times are specified in nanoseconds. >>> >>> Timed bottom halves can be used where timers cannot. For instance, >>> in block drivers where there is no mainloop that calls timers >>> (qemu-nbd, qemu-img), or where (per stefanha@redhat.com) the >>> aio code loops internally and thus timers never get called. >>> >>> Signed-off-by: Alex Bligh >> >> > > >