* Jason J. Herne (jjhe...@linux.vnet.ibm.com) wrote:
> On 06/26/2015 02:07 PM, 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.
> >
> >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 <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?
> >
>
> 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?
My preference would be _tick.
Dave
>
> >>+
> >>+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
> >
>
> --
> -- Jason J. Herne (jjhe...@linux.vnet.ibm.com)
>
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
