From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58444) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1V2HaQ-0008LP-Dj for qemu-devel@nongnu.org; Thu, 25 Jul 2013 05:11:52 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1V2HaN-0004Vq-9I for qemu-devel@nongnu.org; Thu, 25 Jul 2013 05:11:50 -0400 Received: from mx1.redhat.com ([209.132.183.28]:51819) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1V2HaN-0004VS-1a for qemu-devel@nongnu.org; Thu, 25 Jul 2013 05:11:47 -0400 Message-ID: <51F0EBBA.7060802@redhat.com> Date: Thu, 25 Jul 2013 11:11:22 +0200 From: Paolo Bonzini MIME-Version: 1.0 References: <1E8E204.8000201@redhat.com> <1374343603-29183-1-git-send-email-alex@alex.org.uk> <1374343603-29183-2-git-send-email-alex@alex.org.uk> <20130725091015.GC21033@stefanha-thinkpad.redhat.com> In-Reply-To: <20130725091015.GC21033@stefanha-thinkpad.redhat.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCHv2] [RFC 1/7] aio / timers: Remove alarm timers 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, Stefan Hajnoczi , rth@twiddle.net Il 25/07/2013 11:10, Stefan Hajnoczi ha scritto: > On Sat, Jul 20, 2013 at 07:06:37PM +0100, Alex Bligh wrote: >> Remove alarm timers from qemu-timers.c in anticipation of using >> timeouts for g_poll / p_poll instead. >> >> Signed-off-by: Alex Bligh >> --- >> include/qemu/timer.h | 2 - >> main-loop.c | 4 - >> qemu-timer.c | 501 +------------------------------------------------- >> vl.c | 5 +- >> 4 files changed, 7 insertions(+), 505 deletions(-) > > This should be one of the last patches so qemu.git remains bisectable. > Only remove the alarm timer once the event loops are already using the > timeout argument. > >> @@ -245,11 +82,7 @@ static QEMUClock *qemu_new_clock(int type) >> >> void qemu_clock_enable(QEMUClock *clock, bool enabled) >> { >> - bool old = clock->enabled; >> clock->enabled = enabled; >> - if (enabled && !old) { >> - qemu_rearm_alarm_timer(alarm_timer); >> - } > > If this function is supposed to work when called from another thread > (e.g. vcpu thread), then you need to call qemu_notify_event(). For > AioContext clocks that should be aio_notify() with the relevant > AioContext, but we don't need that yet. In general QEMUClock should have an AioContext pointer so that it can call aio_notify. Paolo >> } >> >> int64_t qemu_clock_has_timers(QEMUClock *clock) >> @@ -340,10 +173,9 @@ void qemu_mod_timer_ns(QEMUTimer *ts, int64_t expire_time) >> >> /* Rearm if necessary */ >> if (pt == &ts->clock->active_timers) { >> - if (!alarm_timer->pending) { >> - qemu_rearm_alarm_timer(alarm_timer); >> - } >> - /* Interrupt execution to force deadline recalculation. */ >> + /* Interrupt execution to force deadline recalculation. >> + * FIXME: Do we need to do this now? >> + */ >> qemu_clock_warp(ts->clock); >> if (use_icount) { >> qemu_notify_event(); > > Same here. > >> diff --git a/vl.c b/vl.c >> index 25b8f2f..612c609 100644 >> --- a/vl.c >> +++ b/vl.c >> @@ -3714,7 +3714,10 @@ int main(int argc, char **argv, char **envp) >> old_param = 1; >> break; >> case QEMU_OPTION_clock: >> - configure_alarms(optarg); >> + /* Once upon a time we did: >> + * configure_alarms(optarg); >> + * here. This is stubbed out for compatibility. >> + */ >> break; > > This could be made clearer to say outright that the options don't exist > anymore: > > /* Clock options no longer exist. Keep this option for backward > * compatibility. > */ >