On 8/26/19 5:37 AM, Yury Kotov wrote:
> Throttling thread sleeps in VCPU thread. For high throttle percentage
> this sleep is more than 10ms. E.g. for 60% - 15ms, for 99% - 990ms.
> vm_stop() kicks all VCPUs and waits for them. It's called at the end of
> migration and because of the long sleep the migration downtime might be
> more than 100ms even for downtime-limit 1ms.
> Use qemu_cond_timedwait for high percentage to wake up during vm_stop.
>
> Signed-off-by: Yury Kotov <yury-ko...@yandex-team.ru>
> ---
> cpus.c | 25 +++++++++++++++++--------
> 1 file changed, 17 insertions(+), 8 deletions(-)
>
> @@ -790,11 +792,20 @@ static void cpu_throttle_thread(CPUState *cpu,
> run_on_cpu_data opaque)
>
> pct = (double)cpu_throttle_get_percentage()/100;
> throttle_ratio = pct / (1 - pct);
> - sleeptime_ns = (long)(throttle_ratio * CPU_THROTTLE_TIMESLICE_NS);
> -
> - qemu_mutex_unlock_iothread();
> - g_usleep(sleeptime_ns / 1000); /* Convert ns to us for usleep call */
> - qemu_mutex_lock_iothread();
> + /* Add 1ns to fix double's rounding error (like 0.9999999...) */
> + sleeptime_ns = (int64_t)(throttle_ratio * CPU_THROTTLE_TIMESLICE_NS + 1);
The cast to int64_t is not strictly necessary here, but doesn't hurt
(since it shows you DO know you are going from double to 64-bit int).
> + endtime_ns = qemu_clock_get_ns(QEMU_CLOCK_REALTIME) + sleeptime_ns;
> + while (sleeptime_ns > 0 && !cpu->stop) {
> + if (sleeptime_ns > SCALE_MS) {
> + qemu_cond_timedwait(cpu->halt_cond, &qemu_global_mutex,
> + sleeptime_ns / SCALE_MS);
> + } else {
> + qemu_mutex_unlock_iothread();
> + g_usleep(sleeptime_ns / SCALE_US);
> + qemu_mutex_lock_iothread();
> + }
> + sleeptime_ns = endtime_ns - qemu_clock_get_ns(QEMU_CLOCK_REALTIME);
> + }
Looks reasonable.
(I wonder if an alternative approach, of doing a poll() or similar
instead of g_usleep, and using a pipe-to-self where we write to the pipe
in the same scenarios where cpu->halt_cond would be broadcast, in order
to wake up the sleeping poll in a responsive manner, would be any easier
or more efficient - but don't rewrite the patch just because of my question)
Reviewed-by: Eric Blake <ebl...@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
