Re: [Qemu-devel] [PATCH 5/5] cpu: throttle: fix throttle time slice

[Paolo Bonzini]

On 27/03/2017 09:21, Peter Xu wrote:
> @@ -641,8 +640,7 @@ 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);

Say pct = 0.25, then throttle_ratio = 0.25/0.75 = 1/3.

> +    sleeptime_ns = (long)((1 - pct) * CPU_THROTTLE_TIMESLICE_NS);
>  
>      qemu_mutex_unlock_iothread();
>      atomic_set(&cpu->throttle_thread_scheduled, 0);
> @@ -668,7 +666,7 @@ static void cpu_throttle_timer_tick(void *opaque)
>  
>      pct = (double)cpu_throttle_get_percentage()/100;
>      timer_mod(throttle_timer, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL_RT) +
> -                                   CPU_THROTTLE_TIMESLICE_NS / (1-pct));

And the timer is running every 1/0.75 = 4/3 * CPU_THROTTLE_TIMESLICE_NS.

Of these, 1/3 is spent sleeping (3.33 ms), while 1 (10 ms) is spent not
sleeping.

When pct = 0.75, throttle_ratio = 3 and the timer is running every 4 *
CPU_THROTTLE_TIMESLICE_NS (40 ms).  Of these, 3 slices (30 ms) are spent
sleeping, while 10 ms are spent not sleeping.

The rationale _could_ be (I don't remember) that a CPU with a very high
throttling frequency leaves little time for the migration thread to do
any work.  So QEMU keeps the "on" phase always the same and lengthens
the "off" phase, which as you found out can be unsatisfactory.

However, I think your patch has the opposite problem: the frequency is
constant, but with high throttling all time reserved for the CPU will be
lost in overhead.  For example, at 99% throttling you only have 100
microseconds to wake up, do work and go back to sleep.

So I'm inclined _not_ to take your patch.  One possibility could be to
do the following:

- for throttling between 0% and 80%, use the current algorithm.  At 66%,
the CPU will work for 10 ms and sleep for 40 ms.

- for throttling above 80% adapt your algorithm to have a variable
timeslice, going from 50 ms at 66% to 100 ms at 100%.  This way, the CPU
time will shrink below 10 ms and the sleep time will grow.

It looks like this: http://i.imgur.com/lyFie04.png

So at 99% the timeslice will be 97.5 ms; the CPU will work for 975 us
and sleep for the rest (10x more than with just your patch).  But I'm
not sure it's really worth it.

Paolo

> +                                   CPU_THROTTLE_TIMESLICE_NS * pct);
>  }