From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:55186) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UzzyO-0000sy-84 for qemu-devel@nongnu.org; Thu, 18 Jul 2013 21:59:11 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UzzyL-0007Sv-1H for qemu-devel@nongnu.org; Thu, 18 Jul 2013 21:59:08 -0400 Received: from mx1.redhat.com ([209.132.183.28]:27263) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UzzyK-0007Sh-QP for qemu-devel@nongnu.org; Thu, 18 Jul 2013 21:59:04 -0400 Date: Fri, 19 Jul 2013 09:58:50 +0800 From: Stefan Hajnoczi Message-ID: <20130719015850.GA29671@stefanha-thinkpad.redhat.com> References: <51E4E54A.10908@redhat.com> <51E4F77C.2090509@redhat.com> <794E19D97CCC267CCBFA8397@Ximines.local> <51E56A1A.50502@redhat.com> <19631228D7B62545DC6A2928@Ximines.local> <51E57AF3.1050409@redhat.com> <20130717030230.GA27807@stefanha-thinkpad.redhat.com> <51A6D731C894ECC429EA2171@nimrod.local> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <51A6D731C894ECC429EA2171@nimrod.local> 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 , Stefan Hajnoczi , qemu-devel@nongnu.org, Paolo Bonzini , rth@twiddle.net On Thu, Jul 18, 2013 at 07:48:28PM +0100, Alex Bligh wrote: > Stefan, > > --On 17 July 2013 11:02:30 +0800 Stefan Hajnoczi wrote: > > >The steps to achieving this: > > > >1. Drop alarm timers from qemu-timer.c and calculate g_poll() timeout > > instead for the main loop. > > > >2. Introduce a per-AioContext aio_ctx_clock that can be used with > > qemu_new_timer() to create a QEMUTimer that expires during > > aio_poll(). > > > >3. Calculate g_poll() timeout for aio_ctx_clock in aio_poll(). > > I've pretty much written this. > > Two issues: > > 1. I very much enjoyed deleting all the alarm timers code. However, it was > doing something vaguely useful, which was calling qemu_notify_event > when the timer expired. Under the new regime above, if the AioContext > clock's timers expires within aio_poll, life is good as the timeout > to g_poll will expire. However, this won't apply if any of the other > three static clock's timers expire. Also, it won't work within the > mainloop poll. Also, we lose the ability to respond to timers in a sub > millisecond way. > > Options: > > a) restore alarm timers (at least for the time being). Make all > alarm timers do qemu_notify_event. However, only run the AioContext > clock's timers within aio_poll. This is the least intrusive change. > > b) calculate the timeout in aio_poll with respect to the minimum > deadline across all clocks, not just AioContext's clock. Use the > same logic in mainloop. > > I'd go for (b), except for the millisecond accuracy thing. So my > temptation (sadly) is (a). I think this is a non-issue because host_alarm_handler() can only be called from the main loop: main-loop.c:qemu_signal_init() sets up signalfd to monitor SIGALRM. Therefore we do not asynchronously invoke the SIGALRM signals handler. It is only invoked from main-loop.c:sigfd_handler() when the main loop runs. That's how I read the code. I haven't checked a running process to be sure. > 2. If we do delete alarm timers, I'll need to delete the -clock option. I noticed this too because I think we should stub it out for compatibility. Accept existing options but ignore them, update documentation to state that they are kept for compatibility. Stefan