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

2015-07-02 Thread Paolo Bonzini


On 02/07/2015 18:33, Jason J. Herne wrote:
> I've made all of the changes you have suggested except adding atomics.  I'm 
> having
> a bit of trouble figuring out what is needed here. Perhaps I should be using
> atomic_read() to read throttle_percentage? If so, I don't undertand why. 
> Rather
> than trying to explain everything here I'm going to submit my V4 patches.
> If any atomic operations are still necessary with V4 please let me know.

Fair enough!

Paolo



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

2015-07-02 Thread Jason J. Herne

On 07/01/2015 10:03 AM, Paolo Bonzini wrote:



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.



I've made all of the changes you have suggested except adding atomics. 
I'm having

a bit of trouble figuring out what is needed here. Perhaps I should be using
atomic_read() to read throttle_percentage? If so, I don't undertand why. 
Rather

than trying to explain everything here I'm going to submit my V4 patches.
If any atomic operations are still necessary with V4 please let me know.

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




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

2015-07-02 Thread Paolo Bonzini


On 02/07/2015 16:25, Jason J. Herne wrote:
> Hi Paolo,
> I like your suggestions. I'll switch to a static timer and create it in
> cpu_ticks_init.
> So no need for a timer_free anywhere then?

Nope. :)

Paolo



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

2015-07-02 Thread Jason J. Herne

On 07/01/2015 10:03 AM, Paolo Bonzini wrote:



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



Hi Paolo,
I like your suggestions. I'll switch to a static timer and create it in 
cpu_ticks_init.

So no need for a timer_free anywhere then?

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




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

2015-07-01 Thread Paolo Bonzini


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 
>> Reviewed-by: Matthew Rosato 
>> ---
>>  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 

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

2015-07-01 Thread Paolo Bonzini


On 25/06/2015 19:46, Jason J. Herne wrote:
> +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);

The timer need not run while the VM is stopped.  Please use
QEMU_CLOCK_VIRTUAL_RT.

> +}
> +

Are you sure you want each CPU to set the timer?  I think this should be
done in cpu_throttle_timer_pop, or it could use timer_mod_anticipate.

> +static void cpu_throttle_timer_pop(void *opaque)
> +{
> +CPUState *cpu;
> +
> +/* Stop the timer if needed */
> +if (throttle_timer_stop) {
> +timer_del(throttle_timer);

timer_del is not needed in the timer callback.

Paolo

> +timer_free(throttle_timer);
> +throttle_timer = NULL;
> +return;
> +}
> +
> +CPU_FOREACH(cpu) {
> +async_run_on_cpu(cpu, cpu_throttle_thread, NULL);
> +}
> +}
> +



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

2015-06-29 Thread Jason J. Herne

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 (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 :)



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



Signed-off-by: Jason J. Herne 
Reviewed-by: Matthew Rosato 
---
  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 adju

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

2015-06-29 Thread Dr. David Alan Gilbert
* 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 
> >>Reviewed-by: Matthew Rosato 
> >>---
> >>  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/

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

2015-06-26 Thread Jason J. Herne

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 
Reviewed-by: Matthew Rosato 
---
  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 ca

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

2015-06-26 Thread Dr. David Alan Gilbert
* 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

> Signed-off-by: Jason J. Herne 
> Reviewed-by: Matthew Rosato 
> ---
>  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 throttle

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

2015-06-25 Thread Jason J. Herne
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.

Signed-off-by: Jason J. Herne 
Reviewed-by: Matthew Rosato 
---
 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;
+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);
+}
+}
+
+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