Re: [Qemu-devel] [PATCH 1.1?] qemu_rearm_alarm_timer: do not call rearm if the next deadline is INT64_MAX

[Stefan Weil]

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>

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.

Regards,
Stefan W.