From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=34154 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Pk25U-0003AY-Qt for qemu-devel@nongnu.org; Mon, 31 Jan 2011 17:19:19 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Pk25L-0001jZ-BM for qemu-devel@nongnu.org; Mon, 31 Jan 2011 17:19:00 -0500 Received: from mail-qw0-f45.google.com ([209.85.216.45]:53256) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Pk25L-0001jU-7g for qemu-devel@nongnu.org; Mon, 31 Jan 2011 17:18:59 -0500 Received: by qwk4 with SMTP id 4so6067982qwk.4 for ; Mon, 31 Jan 2011 14:18:58 -0800 (PST) Message-ID: <4D473510.7020209@codemonkey.ws> Date: Mon, 31 Jan 2011 16:17:52 -0600 From: Anthony Liguori MIME-Version: 1.0 Subject: Re: [Qemu-devel] [PATCH 1/3] use nanoseconds everywhere for timeout computation References: <1296510679-12268-1-git-send-email-pbonzini@redhat.com> <1296510679-12268-2-git-send-email-pbonzini@redhat.com> In-Reply-To: <1296510679-12268-2-git-send-email-pbonzini@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: qemu-devel@nongnu.org On 01/31/2011 03:51 PM, Paolo Bonzini wrote: > Suggested by Aurelien Jarno. > > Signed-off-by: Paolo Bonzini > Something I've found is that we have a lot of bugs that are the result of unit conversions when the unit can't be mapped directly to base 10. This happens in both the PIT and RTC and probably every other fixed bus speed based clock we support. I think it would be better to have a Unit argument to qemu_get_clock to specify the desired units. That way, we could request NS as the time unit or something like PC_FREQ0 which would map to the bus speed of the PIT. That eliminates all the unit conversion code throughout QEMU and thereby a big class of bugs. Regards, Anthony Liguori > --- > qemu-timer.c | 30 +++++++++++++++--------------- > 1 files changed, 15 insertions(+), 15 deletions(-) > > diff --git a/qemu-timer.c b/qemu-timer.c > index db1ec49..60283a8 100644 > --- a/qemu-timer.c > +++ b/qemu-timer.c > @@ -197,8 +197,8 @@ static void qemu_rearm_alarm_timer(struct qemu_alarm_timer *t) > t->rearm(t); > } > > -/* TODO: MIN_TIMER_REARM_US should be optimized */ > -#define MIN_TIMER_REARM_US 250 > +/* TODO: MIN_TIMER_REARM_NS should be optimized */ > +#define MIN_TIMER_REARM_NS 250000 > > #ifdef _WIN32 > > @@ -698,11 +698,11 @@ int64_t qemu_next_deadline(void) > > if (active_timers[QEMU_CLOCK_VIRTUAL]) { > delta = active_timers[QEMU_CLOCK_VIRTUAL]->expire_time - > - qemu_get_clock(vm_clock); > + qemu_get_clock_ns(vm_clock); > } > if (active_timers[QEMU_CLOCK_HOST]) { > int64_t hdelta = active_timers[QEMU_CLOCK_HOST]->expire_time - > - qemu_get_clock(host_clock); > + qemu_get_clock_ns(host_clock); > if (hdelta< delta) > delta = hdelta; > } > @@ -727,17 +727,17 @@ static uint64_t qemu_next_deadline_dyntick(void) > if (use_icount) > delta = INT32_MAX; > else > - delta = (qemu_next_deadline() + 999) / 1000; > + delta = qemu_next_deadline(); > > if (active_timers[QEMU_CLOCK_REALTIME]) { > rtdelta = (active_timers[QEMU_CLOCK_REALTIME]->expire_time - > - qemu_get_clock(rt_clock))*1000; > + qemu_get_clock_ns(rt_clock)); > if (rtdelta< delta) > delta = rtdelta; > } > > - if (delta< MIN_TIMER_REARM_US) > - delta = MIN_TIMER_REARM_US; > + if (delta< MIN_TIMER_REARM_NS) > + delta = MIN_TIMER_REARM_NS; > > return delta; > } > @@ -887,8 +887,8 @@ static void dynticks_rearm_timer(struct qemu_alarm_timer *t) > { > timer_t host_timer = (timer_t)(long)t->priv; > struct itimerspec timeout; > - int64_t nearest_delta_us = INT64_MAX; > - int64_t current_us; > + int64_t nearest_delta_ns = INT64_MAX; > + int64_t current_ns; > > assert(alarm_has_dynticks(t)); > if (!active_timers[QEMU_CLOCK_REALTIME]&& > @@ -896,7 +896,7 @@ static void dynticks_rearm_timer(struct qemu_alarm_timer *t) > !active_timers[QEMU_CLOCK_HOST]) > return; > > - nearest_delta_us = qemu_next_deadline_dyntick(); > + nearest_delta_ns = qemu_next_deadline_dyntick(); > > /* check whether a timer is already running */ > if (timer_gettime(host_timer,&timeout)) { > @@ -904,14 +904,14 @@ static void dynticks_rearm_timer(struct qemu_alarm_timer *t) > fprintf(stderr, "Internal timer error: aborting\n"); > exit(1); > } > - current_us = timeout.it_value.tv_sec * 1000000 + timeout.it_value.tv_nsec/1000; > - if (current_us&& current_us<= nearest_delta_us) > + current_ns = timeout.it_value.tv_sec * 1000000000LL + timeout.it_value.tv_nsec; > + if (current_ns&& current_ns<= nearest_delta_ns) > return; > > timeout.it_interval.tv_sec = 0; > timeout.it_interval.tv_nsec = 0; /* 0 for one-shot timer */ > - timeout.it_value.tv_sec = nearest_delta_us / 1000000; > - timeout.it_value.tv_nsec = (nearest_delta_us % 1000000) * 1000; > + timeout.it_value.tv_sec = nearest_delta_ns / 1000000000; > + timeout.it_value.tv_nsec = nearest_delta_ns % 1000000000; > if (timer_settime(host_timer, 0 /* RELATIVE */,&timeout, NULL)) { > perror("settime"); > fprintf(stderr, "Internal timer error: aborting\n"); >