diff mbox series

[3/5] throttle: support read-only and write-only

Message ID 20230625085631.372238-4-pizhenwei@bytedance.com (mailing list archive)
State New, archived
Headers show
Series Misc fixes for throttle | expand

Commit Message

zhenwei pi June 25, 2023, 8:56 a.m. UTC 
Only one direction is necessary in several scenarios:
- a read-only disk
- operations on a device are considered as *write* only. For example,
  encrypt/decrypt/sign/verify operations on a cryptodev use a single
  *write* timer(read timer callback is defined, but never invoked).

Allow a single direction in throttle, this reduces memory, and uplayer
does not need a dummy callback any more.

Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
---
 util/throttle.c | 24 ++++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)

Comments

Alberto Garcia June 26, 2023, 9:38 p.m. UTC | #1 
On Sun 25 Jun 2023 04:56:29 PM +08, zhenwei pi wrote:
>  void throttle_timers_attach_aio_context(ThrottleTimers *tt,
>                                          AioContext *new_context)
>  {
> -    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);
> +    if (tt->timer_cb[THROTTLE_TIMER_READ]) {
> +        tt->timers[THROTTLE_TIMER_READ] =
> +            aio_timer_new(new_context, tt->clock_type, SCALE_NS,
> +                          tt->timer_cb[THROTTLE_TIMER_READ], tt->timer_opaque);
> +    }
> +
> +    if (tt->timer_cb[THROTTLE_TIMER_WRITE]) {
> +        tt->timers[THROTTLE_TIMER_WRITE] =
> +            aio_timer_new(new_context, tt->clock_type, SCALE_NS,
> +                          tt->timer_cb[THROTTLE_TIMER_WRITE], tt->timer_opaque);
> +    }
>  }

If now any of the timers can be NULL, don't you want to add additional
checks / assertions to (at least) throttle_schedule_timer() ?

Berto
zhenwei pi June 27, 2023, 5:25 a.m. UTC | #2 
On 6/27/23 05:38, Alberto Garcia wrote:
> On Sun 25 Jun 2023 04:56:29 PM +08, zhenwei pi wrote:
>>   void throttle_timers_attach_aio_context(ThrottleTimers *tt,
>>                                           AioContext *new_context)
>>   {
>> -    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);
>> +    if (tt->timer_cb[THROTTLE_TIMER_READ]) {
>> +        tt->timers[THROTTLE_TIMER_READ] =
>> +            aio_timer_new(new_context, tt->clock_type, SCALE_NS,
>> +                          tt->timer_cb[THROTTLE_TIMER_READ], tt->timer_opaque);
>> +    }
>> +
>> +    if (tt->timer_cb[THROTTLE_TIMER_WRITE]) {
>> +        tt->timers[THROTTLE_TIMER_WRITE] =
>> +            aio_timer_new(new_context, tt->clock_type, SCALE_NS,
>> +                          tt->timer_cb[THROTTLE_TIMER_WRITE], tt->timer_opaque);
>> +    }
>>   }
> 
> If now any of the timers can be NULL, don't you want to add additional
> checks / assertions to (at least) throttle_schedule_timer() ?
> 
> Berto

OK.
diff mbox series

Patch

diff --git a/util/throttle.c b/util/throttle.c
index c1cc24831c..01faee783c 100644
--- a/util/throttle.c
+++ b/util/throttle.c
@@ -199,12 +199,17 @@  static bool throttle_compute_timer(ThrottleState *ts,
 void throttle_timers_attach_aio_context(ThrottleTimers *tt,
                                         AioContext *new_context)
 {
-    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);
+    if (tt->timer_cb[THROTTLE_TIMER_READ]) {
+        tt->timers[THROTTLE_TIMER_READ] =
+            aio_timer_new(new_context, tt->clock_type, SCALE_NS,
+                          tt->timer_cb[THROTTLE_TIMER_READ], tt->timer_opaque);
+    }
+
+    if (tt->timer_cb[THROTTLE_TIMER_WRITE]) {
+        tt->timers[THROTTLE_TIMER_WRITE] =
+            aio_timer_new(new_context, tt->clock_type, SCALE_NS,
+                          tt->timer_cb[THROTTLE_TIMER_WRITE], tt->timer_opaque);
+    }
 }
 
 /*
@@ -235,6 +240,7 @@  void throttle_timers_init(ThrottleTimers *tt,
                           QEMUTimerCB *write_timer_cb,
                           void *timer_opaque)
 {
+    assert(read_timer_cb || write_timer_cb);
     memset(tt, 0, sizeof(ThrottleTimers));
 
     tt->clock_type = clock_type;
@@ -247,7 +253,9 @@  void throttle_timers_init(ThrottleTimers *tt,
 /* destroy a timer */
 static void throttle_timer_destroy(QEMUTimer **timer)
 {
-    assert(*timer != NULL);
+    if (*timer == NULL) {
+        return;
+    }
 
     timer_free(*timer);
     *timer = NULL;
@@ -272,7 +280,7 @@  void throttle_timers_destroy(ThrottleTimers *tt)
 /* is any throttling timer configured */
 bool throttle_timers_are_initialized(ThrottleTimers *tt)
 {
-    if (tt->timers[0]) {
+    if (tt->timers[THROTTLE_TIMER_READ] || tt->timers[THROTTLE_TIMER_WRITE]) {
         return true;
     }