On Mon, Mar 27, 2017 at 03:21:28PM +0800, Peter Xu wrote:
> IIUC the throttle idea is that: we split a CPU_THROTTLE_TIMESLICE_NS
> time slice into two parts - one for vcpu, one for throttle thread (which
> will suspend the thread by a sleep). However current algorithm on
> calculating the working piece and sleeping piece is strange.
>
> Assume a 99% throttle, what we want is to merely stop vcpu from running,
> but the old logic will just first let the vcpu run for a very long
> time (which is "CPU_THROTTLE_TIMESLICE_NS / (1-pct)" = 1 second) before
> doing anything else.
>
> Fixing it up to the simplest but imho accurate logic.
Oh, looks like I need to switch the two pct below... :)
>
> CC: Paolo Bonzini <pbonz...@redhat.com>
> CC: Richard Henderson <r...@twiddle.net>
> CC: Jason J. Herne <jjhe...@linux.vnet.ibm.com>
> Signed-off-by: Peter Xu <pet...@redhat.com>
> ---
> cpus.c | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/cpus.c b/cpus.c
> index 167d961..7976ce4 100644
> --- a/cpus.c
> +++ b/cpus.c
> @@ -633,7 +633,6 @@ static const VMStateDescription vmstate_timers = {
> static void cpu_throttle_thread(CPUState *cpu, run_on_cpu_data opaque)
> {
> double pct;
> - double throttle_ratio;
> long sleeptime_ns;
>
> if (!cpu_throttle_get_percentage()) {
> @@ -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);
> + sleeptime_ns = (long)((1 - pct) * CPU_THROTTLE_TIMESLICE_NS);
^^^^^^^^^
here it should be "pct", while...
>
> 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));
> + CPU_THROTTLE_TIMESLICE_NS * pct);
^^^
here it should be (1 - pct)
I'll wait for review comment on the raw idea, to see whether I will
need a repost. Sorry for the misunderstanding.
-- peterx
