Re: [Qemu-devel] [RFC] [PATCHv10 19/31] aio / timers: Use all timerlists in icount warp calculations

[Stefan Hajnoczi]

On Sun, Aug 11, 2013 at 05:43:13PM +0100, Alex Bligh wrote:
> @@ -314,7 +314,18 @@ void qemu_clock_warp(QEMUClock *clock)
>      }
>  
>      vm_clock_warp_start = qemu_get_clock_ns(rt_clock);
> -    deadline = qemu_clock_deadline(vm_clock);
> +    /* We want to use the earliest deadline from ALL vm_clocks */
> +    deadline = qemu_clock_deadline_ns_all(vm_clock);
> +
> +    /* Maintain prior (possibly buggy) behaviour where if no deadline
> +     * was set (as there is no vm_clock timer) or it is more than
> +     * INT32_MAX nanoseconds ahead, we still use INT32_MAX
> +     * nanoseconds.
> +     */
> +    if ((deadline < 0) || (deadline > INT32_MAX)) {
> +        deadline = INT32_MAX;
> +    }
> +

I see no value in a spurious wakeup if no deadline was set, but it's
harmless.

As for overflow, I don't really understand how INT32_MAX prevents
overflow.  If the base timer value we're adding to is already huge then
INT32_MAX could still overflow it.

>      if (deadline > 0) {
>          /*
>           * Ensure the vm_clock proceeds even when the virtual CPU goes to
> @@ -333,8 +344,8 @@ void qemu_clock_warp(QEMUClock *clock)
>           * packets continuously instead of every 100ms.
>           */
>          qemu_mod_timer(icount_warp_timer, vm_clock_warp_start + deadline);
> -    } else {
> -        qemu_notify_event();
> +    } else if (deadline == 0) {
> +        qemu_clock_notify(vm_clock);

It seems this change is just for clarity: deadline can either be >0 or
0?  I think a plain else statement is clearer and doesn't leave you
wondering in what case this qemu_clock_notify() isn't taken.