Re: [Qemu-devel] [PATCH v3 1/5] cpu: Provide vcpu throttling interface

["Jason J. Herne" <jjherne@linux.vnet.ibm.com>]

On 06/26/2015 03:02 PM, Jason J. Herne wrote:
> On 06/26/2015 02:07 PM, Dr. David Alan Gilbert wrote:
>> * Jason J. Herne (jjherne@linux.vnet.ibm.com) wrote:
>>> Provide a method to throttle guest cpu execution. CPUState is
>>> augmented with
>>> timeout controls and throttle start/stop functions. To throttle the
>>> guest cpu
>>> the caller simply has to call the throttle set function and provide a
>>> percentage
>>> of throttle time.
>>
>> I'm worried about atomicity and threads and all those fun things.
>>
>> I think all the starting/stopping/setting the throttling level is done
>> in the
>> migration thread; I think the timers run in the main/io thread?
>> So you really need to be careful with at least:
>>      throttle_timer_stop - which may have a minor effect
>>      throttle_timer  - I worry about the way cpu_timer_active checks
>> the pointer
>>                        yet it's freed when the timer goes off.   It's
>> probably
>>                        not too bad because it never dereferences it.
>>
>> So, probably need some atomic's in there (cc'ing paolo)
>>
>> Dave
>>
>
> I think we're ok with respect to throttle_timer. As you mentioned, we
> rely on the
> value only to know if throttling is active or not.
>
> I'm not seeing any other race conditions or serialization issues. But
> that doesn't
> mean the code is perfect either :)
>

Is there any more discussion on this before I send a v4? Paolo?


>>> Signed-off-by: Jason J. Herne <jjherne@linux.vnet.ibm.com>
>>> Reviewed-by: Matthew Rosato <mjrosato@linux.vnet.ibm.com>
>>> ---
>>>   cpus.c            | 76
>>> +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>   include/qom/cpu.h | 38 ++++++++++++++++++++++++++++
>>>   2 files changed, 114 insertions(+)
>>>
>>> diff --git a/cpus.c b/cpus.c
>>> index de6469f..f57cf4f 100644
>>> --- a/cpus.c
>>> +++ b/cpus.c
>>> @@ -68,6 +68,16 @@ static CPUState *next_cpu;
>>>   int64_t max_delay;
>>>   int64_t max_advance;
>>>
>>> +/* vcpu throttling controls */
>>> +static QEMUTimer *throttle_timer;
>>> +static bool throttle_timer_stop;
>>> +static int throttle_percentage;
>>
>> Unsigned?
>>
>
> Yep. Can do.
>
>>> +static float throttle_ratio;
>>> +
>>> +#define CPU_THROTTLE_PCT_MIN 1
>>> +#define CPU_THROTTLE_PCT_MAX 99
>>> +#define CPU_THROTTLE_TIMESLICE 10
>>> +
>>>   bool cpu_is_stopped(CPUState *cpu)
>>>   {
>>>       return cpu->stopped || !runstate_is_running();
>>> @@ -919,6 +929,72 @@ static void qemu_kvm_wait_io_event(CPUState *cpu)
>>>       qemu_wait_io_event_common(cpu);
>>>   }
>>>
>>> +static void cpu_throttle_thread(void *opaque)
>>> +{
>>> +    long sleeptime_ms = (long)(throttle_ratio *
>>> CPU_THROTTLE_TIMESLICE);
>>> +
>>> +    qemu_mutex_unlock_iothread();
>>> +    g_usleep(sleeptime_ms * 1000); /* Convert ms to us for usleep
>>> call */
>>> +    qemu_mutex_lock_iothread();
>>> +
>>> +    timer_mod(throttle_timer, qemu_clock_get_ms(QEMU_CLOCK_REALTIME) +
>>> +                                   CPU_THROTTLE_TIMESLICE);
>>> +}
>>> +
>>> +static void cpu_throttle_timer_pop(void *opaque)
>>> +{
>>> +    CPUState *cpu;
>>> +
>>> +    /* Stop the timer if needed */
>>> +    if (throttle_timer_stop) {
>>> +        timer_del(throttle_timer);
>>> +        timer_free(throttle_timer);
>>> +        throttle_timer = NULL;
>>> +        return;
>>> +    }
>>> +
>>> +    CPU_FOREACH(cpu) {
>>> +        async_run_on_cpu(cpu, cpu_throttle_thread, NULL);
>>> +    }
>>> +}
>>
>> Why pop? I pop stacks, balloons and bubbles.
>>
>
> Hmmm... timer pops are a very common term in System Z land :). But then
> again we do have a ton of odd terminology around here. Do you have a
> better suggestion?  cpu_throttle_timer_expire? cpu_throttle_timer_tick?
>
>>> +
>>> +void cpu_throttle_set(int new_throttle_pct)
>>> +{
>>> +    double pct;
>>> +
>>> +    /* Ensure throttle percentage is within valid range */
>>> +    new_throttle_pct = MIN(new_throttle_pct, CPU_THROTTLE_PCT_MAX);
>>> +    throttle_percentage = MAX(new_throttle_pct, CPU_THROTTLE_PCT_MIN);
>>> +
>>> +    pct = (double)throttle_percentage/100;
>>> +    throttle_ratio = pct / (1 - pct);
>>> +
>>> +    if (!cpu_throttle_active()) {
>>> +        throttle_timer_stop = false;
>>> +        throttle_timer = timer_new_ms(QEMU_CLOCK_REALTIME,
>>> +                                           cpu_throttle_timer_pop,
>>> NULL);
>>> +        timer_mod(throttle_timer,
>>> qemu_clock_get_ms(QEMU_CLOCK_REALTIME) +
>>> +                                       CPU_THROTTLE_TIMESLICE);
>>> +    }
>>> +}
>>> +
>>> +void cpu_throttle_stop(void)
>>> +{
>>> +    if (cpu_throttle_active()) {
>>> +        throttle_timer_stop = true;
>>> +    }
>>> +}
>>> +
>>> +bool cpu_throttle_active(void)
>>> +{
>>> +    return (throttle_timer != NULL);
>>> +}
>>> +
>>> +int cpu_throttle_get_percentage(void)
>>> +{
>>> +    return throttle_percentage;
>>> +}
>>> +
>>>   static void *qemu_kvm_cpu_thread_fn(void *arg)
>>>   {
>>>       CPUState *cpu = arg;
>>> diff --git a/include/qom/cpu.h b/include/qom/cpu.h
>>> index 39f0f19..56eb964 100644
>>> --- a/include/qom/cpu.h
>>> +++ b/include/qom/cpu.h
>>> @@ -553,6 +553,44 @@ CPUState *qemu_get_cpu(int index);
>>>    */
>>>   bool cpu_exists(int64_t id);
>>>
>>> +/**
>>> + * cpu_throttle_set:
>>> + * @new_throttle_pct: Percent of sleep time to running time.
>>> + *                    Valid range is 1 to 99.
>>> + *
>>> + * Throttles all vcpus by forcing them to sleep for the given
>>> percentage of
>>> + * time. A throttle_percentage of 50 corresponds to a 50% duty cycle
>>> roughly.
>>> + * (example: 10ms sleep for every 10ms awake).
>>> + *
>>> + * cpu_throttle_set can be called as needed to adjust new_throttle_pct.
>>> + * Once the throttling starts, it will remain in effect until
>>> cpu_throttle_stop
>>> + * is called.
>>> + */
>>> +void cpu_throttle_set(int new_throttle_pct);
>>> +
>>> +/**
>>> + * cpu_throttle_stop:
>>> + *
>>> + * Stops the vcpu throttling started by cpu_throttle_set.
>>> + */
>>> +void cpu_throttle_stop(void);
>>> +
>>> +/**
>>> + * cpu_throttle_active:
>>> + *
>>> + * Returns %true if the vcpus are currently being throttled, %false
>>> otherwise.
>>> + */
>>> +bool cpu_throttle_active(void);
>>> +
>>> +/**
>>> + * cpu_throttle_get_percentage:
>>> + *
>>> + * Returns the vcpu throttle percentage. See cpu_throttle_set for
>>> details.
>>> + *
>>> + * Returns The throttle percentage in range 1 to 99.
>>> + */
>>> +int cpu_throttle_get_percentage(void);
>>> +
>>>   #ifndef CONFIG_USER_ONLY
>>>
>>>   typedef void (*CPUInterruptHandler)(CPUState *, int);
>>> --
>>> 1.9.1
>>>
>> --
>> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
>>
>

-- 
-- Jason J. Herne (jjherne@linux.vnet.ibm.com)