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

[Paolo Bonzini]

On 26/06/2015 20:07, Dr. David Alan Gilbert wrote:
> * Jason J. Herne (address@hidden) 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.

Agreed.  I think the only atomic should be throttle_percentage; if zero,
throttling is inactive.

In particular, throttle_ratio can be computed in cpu_throttle_thread.

If you have exactly one variable that is shared between the threads,
everything is much simpler.

There is no need to allocate and free the timer; it's very cheap and in
fact we probably should convert to statically allocated timers sooner or
later.  So you can just create it once, for example in cpu_ticks_init.

Paolo

> So, probably need some atomic's in there (cc'ing paolo)
> 
> Dave
> 
>> Signed-off-by: Jason J. Herne <address@hidden>
>> Reviewed-by: Matthew Rosato <address@hidden>
>> ---
>>  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?
> 
>> +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.
> 
>> +
>> +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 / address@hidden / Manchester, UK
> 
>