On 13.07.23 08:41, zhenwei pi wrote:
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.
Reviewed-by: Alberto Garcia<be...@igalia.com>
Signed-off-by: zhenwei pi<pizhen...@bytedance.com>
---
util/throttle.c | 32 ++++++++++++++++++++++----------
1 file changed, 22 insertions(+), 10 deletions(-)
diff --git a/util/throttle.c b/util/throttle.c
index 5642e61763..c0bd0c26c3 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_READ] =
- aio_timer_new(new_context, tt->clock_type, SCALE_NS,
- tt->timer_cb[THROTTLE_READ], tt->timer_opaque);
- tt->timers[THROTTLE_WRITE] =
- aio_timer_new(new_context, tt->clock_type, SCALE_NS,
- tt->timer_cb[THROTTLE_WRITE], tt->timer_opaque);
+ if (tt->timer_cb[THROTTLE_READ]) {
+ tt->timers[THROTTLE_READ] =
+ aio_timer_new(new_context, tt->clock_type, SCALE_NS,
+ tt->timer_cb[THROTTLE_READ], tt->timer_opaque);
+ }
+
+ if (tt->timer_cb[THROTTLE_WRITE]) {
+ tt->timers[THROTTLE_WRITE] =
+ aio_timer_new(new_context, tt->clock_type, SCALE_NS,
+ tt->timer_cb[THROTTLE_WRITE], tt->timer_opaque);
+ }
I think a `for (int i = 0; i < THROTTLE_MAX; i++)` loop would make
this nicer. (Again: Optional.)
}
/*
[...]
@@ -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_READ] || tt->timers[THROTTLE_WRITE]) {
Not wrong, but I’d prefer the more general
```
for (int i = 0; i < THROTTLE_MAX; i++) {
if (tt->timers[i]) {
return true;
}
}
return false;
```
return true;
}
@@ -424,8 +432,12 @@ bool throttle_schedule_timer(ThrottleState *ts,
{
int64_t now = qemu_clock_get_ns(tt->clock_type);
int64_t next_timestamp;
+ QEMUTimer *timer;
bool must_wait;
+ timer = is_write ? tt->timers[THROTTLE_WRITE] : tt->timers[THROTTLE_READ];
Could be shorter as `timer = tt->timers[is_write ? THROTTLE_WRITE :
THROTTLE_READ];`.
Anyway, I only have style suggestion to offer, so either way:
Reviewed-by: Hanna Czenczek <hre...@redhat.com>
+ assert(timer);
+
must_wait = throttle_compute_timer(ts,
is_write,
now,
