[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 1.1?] qemu_rearm_alarm_timer: do not call rearm
From: |
Stefano Stabellini |
Subject: |
Re: [Qemu-devel] [PATCH 1.1?] qemu_rearm_alarm_timer: do not call rearm if the next deadline is INT64_MAX |
Date: |
Tue, 29 May 2012 18:23:42 +0100 |
User-agent: |
Alpine 2.00 (DEB 1167 2008-08-23) |
On Tue, 29 May 2012, Stefan Weil wrote:
> Am 29.05.2012 15:35, schrieb Stefano Stabellini:
> > qemu_rearm_alarm_timer partially duplicates the code in
> > qemu_next_alarm_deadline to figure out if it needs to rearm the timer.
> > If it calls qemu_next_alarm_deadline, it always rearms the timer even if
> > the next deadline is INT64_MAX.
> >
> > This patch simplifies the behavior of qemu_rearm_alarm_timer and removes
> > the duplicated code, always calling qemu_next_alarm_deadline and only
> > rearming the timer if the deadline is less than INT64_MAX.
> >
> > Signed-off-by: Stefano Stabellini<address@hidden>
> >
> > diff --git a/qemu-timer.c b/qemu-timer.c
> > index de98977..81ff824 100644
> > --- a/qemu-timer.c
> > +++ b/qemu-timer.c
> > @@ -112,14 +112,10 @@ static int64_t qemu_next_alarm_deadline(void)
> >
> > static void qemu_rearm_alarm_timer(struct qemu_alarm_timer *t)
> > {
> > - int64_t nearest_delta_ns;
> > - if (!rt_clock->active_timers&&
> > - !vm_clock->active_timers&&
> > - !host_clock->active_timers) {
> > - return;
> > + int64_t nearest_delta_ns = qemu_next_alarm_deadline();
> > + if (nearest_delta_ns< INT64_MAX) {
> > + t->rearm(t, nearest_delta_ns);
> > }
> > - nearest_delta_ns = qemu_next_alarm_deadline();
> > - t->rearm(t, nearest_delta_ns);
> > }
> >
> > /* TODO: MIN_TIMER_REARM_NS should be optimized */
>
> Reviewed-by: Stefan Weil <address@hidden>
thanks
> This patch clearly improves the current code and fixes
> an abort on Darwin (reported by Andreas Färber) and maybe
> other hosts. Therefore I changed the subject and suggest
> to consider this patch for QEMU 1.1.
>
> There remain issues which can be fixed after 1.1:
>
> nearest_delta_ns also gets negative values (rtdelta < 0,
> maybe because the expiration time already expired).
> I did not check whether all different timers handle
> a negative time gracefully.
>
> nearest_delta_ns should also be limited to INT32_MAX
> seconds, because some timers assign the seconds
> to a long (see setitimer) or UINT value. On 32 bit
> Linux and on all variants of Windows, long is less
> or equal INT32_MAX. If we limit nearest_delta_ns
> to 1000000 seconds (or some other limit which allows
> ULONG milliseconds), we could further simplify the code
> because most timers would no longer have to test the
> upper limit.
If that's the issue we could limit nearest_delta_ns to LONG_MAX.
However I got the feeling that Darwin has an undocumented limit
for tv_sec, lower than INT32_MAX.