[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 5/5] cpu: throttle: fix throttle time slice
From: |
Dr. David Alan Gilbert |
Subject: |
Re: [Qemu-devel] [PATCH 5/5] cpu: throttle: fix throttle time slice |
Date: |
Fri, 31 Mar 2017 20:13:22 +0100 |
User-agent: |
Mutt/1.8.0 (2017-02-23) |
Ignoring the details below for a minute, this patch belongs in a separate
series; all the rest of the patches in this set are nice simple ones.
* Paolo Bonzini (address@hidden) wrote:
>
>
> 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.
Yes, and I'm worried that with the 10ms timeslice it is a lot of overhead,
especially if your timer that wakes you have that much higher resolution than
that.
> 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 seems odd to have a threshold like that on something that's supposedly
a linear scale.
> 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.
Can you really run a CPU for 975us ?
Dave
> Paolo
>
> > + CPU_THROTTLE_TIMESLICE_NS * pct);
> > }
--
Dr. David Alan Gilbert / address@hidden / Manchester, UK
- Re: [Qemu-devel] [PATCH 1/5] migration: set current_active_state once, (continued)