[1/5] throttle: introduce enum ThrottleTimerType

Commit Message

zhenwei pi June 25, 2023, 8:56 a.m. UTC

Use enum ThrottleTimerType instead of number index.

Signed-off-by: zhenwei pi <[e-mail]>
---
 include/qemu/throttle.h | 12 +++++++++---
 util/throttle.c         | 16 +++++++++-------
 2 files changed, 18 insertions(+), 10 deletions(-)

Comments

Alberto Garcia June 26, 2023, 9:24 p.m. UTC | #1

On Sun 25 Jun 2023 04:56:27 PM +08, zhenwei pi wrote:
> Use enum ThrottleTimerType instead of number index.

> +typedef enum {
> +    THROTTLE_TIMER_READ = 0,
> +    THROTTLE_TIMER_WRITE,
> +    THROTTLE_TIMER_MAX
> +} ThrottleTimerType;

If you're doing this I suppose you could also change 'bool is_write'
with something like 'ThrottleTimerType timer', i.e

static bool throttle_compute_timer(ThrottleState *ts,
                                   ThrottleTimerType timer,
                                   int64_t now,
                                   int64_t *next_timestamp)

Berto

zhenwei pi June 27, 2023, 5:06 a.m. UTC | #2

On 6/27/23 05:24, Alberto Garcia wrote:
> On Sun 25 Jun 2023 04:56:27 PM +08, zhenwei pi wrote:
>> Use enum ThrottleTimerType instead of number index.
> 
>> +typedef enum {
>> +    THROTTLE_TIMER_READ = 0,
>> +    THROTTLE_TIMER_WRITE,
>> +    THROTTLE_TIMER_MAX
>> +} ThrottleTimerType;
> 
> If you're doing this I suppose you could also change 'bool is_write'
> with something like 'ThrottleTimerType timer', i.e
> 
> static bool throttle_compute_timer(ThrottleState *ts,
>                                     ThrottleTimerType timer,
>                                     int64_t now,
>                                     int64_t *next_timestamp)
> 
> Berto

Hi,

Right, it's in my plan. But I prefer to do this in a followup patch 
after this series applies. Because this API leads changes from other 
subsystems.

By the way, I prepare to rename 'THROTTLE_TIMER_READ' to 'THROTTLE_READ' 
in next version.

Do you have any suggestion?

Patch

diff --git a/include/qemu/throttle.h b/include/qemu/throttle.h
index 05f6346137..0956094115 100644
--- a/include/qemu/throttle.h
+++ b/include/qemu/throttle.h
@@ -99,13 +99,19 @@  typedef struct ThrottleState {
     int64_t previous_leak;    /* timestamp of the last leak done */
 } ThrottleState;
 
+typedef enum {
+    THROTTLE_TIMER_READ = 0,
+    THROTTLE_TIMER_WRITE,
+    THROTTLE_TIMER_MAX
+} ThrottleTimerType;
+
 typedef struct ThrottleTimers {
-    QEMUTimer *timers[2];     /* timers used to do the throttling */
+    /* timers used to do the throttling */
+    QEMUTimer *timers[THROTTLE_TIMER_MAX];
     QEMUClockType clock_type; /* the clock used */
 
     /* Callbacks */
-    QEMUTimerCB *read_timer_cb;
-    QEMUTimerCB *write_timer_cb;
+    QEMUTimerCB *timer_cb[THROTTLE_TIMER_MAX];
     void *timer_opaque;
 } ThrottleTimers;
 
diff --git a/util/throttle.c b/util/throttle.c
index 81f247a8d1..c1cc24831c 100644
--- a/util/throttle.c
+++ b/util/throttle.c
@@ -199,10 +199,12 @@  static bool throttle_compute_timer(ThrottleState *ts,
 void throttle_timers_attach_aio_context(ThrottleTimers *tt,
                                         AioContext *new_context)
 {
-    tt->timers[0] = aio_timer_new(new_context, tt->clock_type, SCALE_NS,
-                                  tt->read_timer_cb, tt->timer_opaque);
-    tt->timers[1] = aio_timer_new(new_context, tt->clock_type, SCALE_NS,
-                                  tt->write_timer_cb, tt->timer_opaque);
+    tt->timers[THROTTLE_TIMER_READ] =
+        aio_timer_new(new_context, tt->clock_type, SCALE_NS,
+                      tt->timer_cb[THROTTLE_TIMER_READ], tt->timer_opaque);
+    tt->timers[THROTTLE_TIMER_WRITE] =
+        aio_timer_new(new_context, tt->clock_type, SCALE_NS,
+                      tt->timer_cb[THROTTLE_TIMER_WRITE], tt->timer_opaque);
 }
 
 /*
@@ -236,8 +238,8 @@  void throttle_timers_init(ThrottleTimers *tt,
     memset(tt, 0, sizeof(ThrottleTimers));
 
     tt->clock_type = clock_type;
-    tt->read_timer_cb = read_timer_cb;
-    tt->write_timer_cb = write_timer_cb;
+    tt->timer_cb[THROTTLE_TIMER_READ] = read_timer_cb;
+    tt->timer_cb[THROTTLE_TIMER_WRITE] = write_timer_cb;
     tt->timer_opaque = timer_opaque;
     throttle_timers_attach_aio_context(tt, aio_context);
 }
@@ -256,7 +258,7 @@  void throttle_timers_detach_aio_context(ThrottleTimers *tt)
 {
     int i;
 
-    for (i = 0; i < 2; i++) {
+    for (i = 0; i < THROTTLE_TIMER_MAX; i++) {
         throttle_timer_destroy(&tt->timers[i]);
     }
 }