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.
