On 26/06/2015 20:07, Dr. David Alan Gilbert wrote: > * Jason J. Herne (jjhe...@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.
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 <jjhe...@linux.vnet.ibm.com> >> Reviewed-by: Matthew Rosato <mjros...@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? > >> +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 / dgilb...@redhat.com / Manchester, UK > >