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

[Jason J. Herne]

On 06/26/2015 02:07 PM, 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.
>
> 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 :)

> > 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?

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 / address@hidden / Manchester, UK

--
-- Jason J. Herne (address@hidden)