[PATCH] cpu-throttle: Remove timer_mod() from cpu_throttle_set()

[PATCH] cpu-throttle: Remove timer_mod() from cpu_throttle_set()

[Utkarsh Tripathi]

During migrations, after each iteration, cpu_throttle_set() is called,
which irrespective of input, re-arms the timer according to value of
new_throttle_pct. This causes cpu_throttle_thread() to be delayed in
getting scheduled and consqeuntly lets guest run for more time than what
the throttle value should allow. This leads to spikes in guest throughput
at high cpu-throttle percentage whenever cpu_throttle_set() is called.

A solution would be not to modify the timer immediately in
cpu_throttle_set(), instead, only modify throttle_percentage so that the
throttle would automatically adjust to the required percentage when
cpu_throttle_timer_tick() is invoked.

Manually tested the patch using following configuration:

Guest:
Centos7 (3.10.0-123.el7.x86_64)
Total Memory - 64GB , CPUs - 16
Tool used - stress (1.0.4)
Workload - stress --vm 32 --vm-bytes 1G --vm-keep

Migration Parameters:
Network Bandwidth - 500MBPS
cpu-throttle-initial - 99

Results:
With timer_mod(): fails to converge, continues indefinitely
Without timer_mod(): converges in 249 sec

Signed-off-by: Utkarsh Tripathi <utkarsh.tripathi@nutanix.com>
---
 softmmu/cpu-throttle.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/softmmu/cpu-throttle.c b/softmmu/cpu-throttle.c
index 2ec4b8e..8c2144a 100644
--- a/softmmu/cpu-throttle.c
+++ b/softmmu/cpu-throttle.c
@@ -90,14 +90,21 @@ static void cpu_throttle_timer_tick(void *opaque)
 
 void cpu_throttle_set(int new_throttle_pct)
 {
+    /*
+     * boolean to store whether throttle is already active or not,
+     * before modifying throttle_percentage
+     */
+    bool throttle_active = cpu_throttle_active();
+
     /* Ensure throttle percentage is within valid range */
     new_throttle_pct = MIN(new_throttle_pct, CPU_THROTTLE_PCT_MAX);
     new_throttle_pct = MAX(new_throttle_pct, CPU_THROTTLE_PCT_MIN);
 
     qatomic_set(&throttle_percentage, new_throttle_pct);
 
-    timer_mod(throttle_timer, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL_RT) +
-                                       CPU_THROTTLE_TIMESLICE_NS);
+    if (!throttle_active) {
+        cpu_throttle_timer_tick(NULL);
+    }
 }
 
 void cpu_throttle_stop(void)
-- 
1.8.3.1

Re: [PATCH] cpu-throttle: Remove timer_mod() from cpu_throttle_set()

[Utkarsh Tripathi]

[-- Attachment #1: Type: text/plain, Size: 2580 bytes --]

PING

From: Utkarsh Tripathi <utkarsh.tripathi@nutanix.com>
Date: Thursday, 31 December 2020 at 6:43 PM
To: qemu-devel@nongnu.org <qemu-devel@nongnu.org>
Cc: Utkarsh Tripathi <utkarsh.tripathi@nutanix.com>, Paolo Bonzini <pbonzini@redhat.com>
Subject: [PATCH] cpu-throttle: Remove timer_mod() from cpu_throttle_set()
During migrations, after each iteration, cpu_throttle_set() is called,
which irrespective of input, re-arms the timer according to value of
new_throttle_pct. This causes cpu_throttle_thread() to be delayed in
getting scheduled and consqeuntly lets guest run for more time than what
the throttle value should allow. This leads to spikes in guest throughput
at high cpu-throttle percentage whenever cpu_throttle_set() is called.

A solution would be not to modify the timer immediately in
cpu_throttle_set(), instead, only modify throttle_percentage so that the
throttle would automatically adjust to the required percentage when
cpu_throttle_timer_tick() is invoked.

Manually tested the patch using following configuration:

Guest:
Centos7 (3.10.0-123.el7.x86_64)
Total Memory - 64GB , CPUs - 16
Tool used - stress (1.0.4)
Workload - stress --vm 32 --vm-bytes 1G --vm-keep

Migration Parameters:
Network Bandwidth - 500MBPS
cpu-throttle-initial - 99

Results:
With timer_mod(): fails to converge, continues indefinitely
Without timer_mod(): converges in 249 sec

Signed-off-by: Utkarsh Tripathi <utkarsh.tripathi@nutanix.com>
---
 softmmu/cpu-throttle.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/softmmu/cpu-throttle.c b/softmmu/cpu-throttle.c
index 2ec4b8e..8c2144a 100644
--- a/softmmu/cpu-throttle.c
+++ b/softmmu/cpu-throttle.c
@@ -90,14 +90,21 @@ static void cpu_throttle_timer_tick(void *opaque)

 void cpu_throttle_set(int new_throttle_pct)
 {
+    /*
+     * boolean to store whether throttle is already active or not,
+     * before modifying throttle_percentage
+     */
+    bool throttle_active = cpu_throttle_active();
+
     /* Ensure throttle percentage is within valid range */
     new_throttle_pct = MIN(new_throttle_pct, CPU_THROTTLE_PCT_MAX);
     new_throttle_pct = MAX(new_throttle_pct, CPU_THROTTLE_PCT_MIN);

     qatomic_set(&throttle_percentage, new_throttle_pct);

-    timer_mod(throttle_timer, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL_RT) +
-                                       CPU_THROTTLE_TIMESLICE_NS);
+    if (!throttle_active) {
+        cpu_throttle_timer_tick(NULL);
+    }
 }

 void cpu_throttle_stop(void)
--
1.8.3.1

[-- Attachment #2: Type: text/html, Size: 5058 bytes --]

Re: [PATCH] cpu-throttle: Remove timer_mod() from cpu_throttle_set()

[Paolo Bonzini]

On 31/12/20 14:13, Utkarsh Tripathi wrote:
> During migrations, after each iteration, cpu_throttle_set() is called,
> which irrespective of input, re-arms the timer according to value of
> new_throttle_pct. This causes cpu_throttle_thread() to be delayed in
> getting scheduled and consqeuntly lets guest run for more time than what
> the throttle value should allow. This leads to spikes in guest throughput
> at high cpu-throttle percentage whenever cpu_throttle_set() is called.
> 
> A solution would be not to modify the timer immediately in
> cpu_throttle_set(), instead, only modify throttle_percentage so that the
> throttle would automatically adjust to the required percentage when
> cpu_throttle_timer_tick() is invoked.
> 
> Manually tested the patch using following configuration:
> 
> Guest:
> Centos7 (3.10.0-123.el7.x86_64)
> Total Memory - 64GB , CPUs - 16
> Tool used - stress (1.0.4)
> Workload - stress --vm 32 --vm-bytes 1G --vm-keep
> 
> Migration Parameters:
> Network Bandwidth - 500MBPS
> cpu-throttle-initial - 99
> 
> Results:
> With timer_mod(): fails to converge, continues indefinitely
> Without timer_mod(): converges in 249 sec
> 
> Signed-off-by: Utkarsh Tripathi <utkarsh.tripathi@nutanix.com>
> ---
>   softmmu/cpu-throttle.c | 11 +++++++++--
>   1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/softmmu/cpu-throttle.c b/softmmu/cpu-throttle.c
> index 2ec4b8e..8c2144a 100644
> --- a/softmmu/cpu-throttle.c
> +++ b/softmmu/cpu-throttle.c
> @@ -90,14 +90,21 @@ static void cpu_throttle_timer_tick(void *opaque)
>   
>   void cpu_throttle_set(int new_throttle_pct)
>   {
> +    /*
> +     * boolean to store whether throttle is already active or not,
> +     * before modifying throttle_percentage
> +     */
> +    bool throttle_active = cpu_throttle_active();
> +
>       /* Ensure throttle percentage is within valid range */
>       new_throttle_pct = MIN(new_throttle_pct, CPU_THROTTLE_PCT_MAX);
>       new_throttle_pct = MAX(new_throttle_pct, CPU_THROTTLE_PCT_MIN);
>   
>       qatomic_set(&throttle_percentage, new_throttle_pct);
>   
> -    timer_mod(throttle_timer, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL_RT) +
> -                                       CPU_THROTTLE_TIMESLICE_NS);
> +    if (!throttle_active) {
> +        cpu_throttle_timer_tick(NULL);
> +    }
>   }
>   
>   void cpu_throttle_stop(void)
> 

Queued, thanks.

Paolo