On 7/28/23 00:12, Hanna Czenczek wrote:
On 24.07.23 12:09, zhenwei pi wrote:
'bool is_write' style is obsolete from throttle framework, adapt
block throttle groups to the new style.
Use a simple python script to test the new style:
#!/usr/bin/python3
import subprocess
import random
import time
commands = ['virsh blkdeviotune jammy vda --write-bytes-sec ', \
'virsh blkdeviotune jammy vda --write-iops-sec ', \
'virsh blkdeviotune jammy vda --read-bytes-sec ', \
'virsh blkdeviotune jammy vda --read-iops-sec ']
for loop in range(1, 1000):
time.sleep(random.randrange(3, 5))
command = commands[random.randrange(0, 3)] +
str(random.randrange(0, 1000000))
subprocess.run(command, shell=True, check=True)
This works fine.
Signed-off-by: zhenwei pi <pizhen...@bytedance.com>
---
block/throttle-groups.c | 105 ++++++++++++++++----------------
block/throttle.c | 8 +--
include/block/throttle-groups.h | 6 +-
3 files changed, 60 insertions(+), 59 deletions(-)
diff --git a/block/throttle-groups.c b/block/throttle-groups.c
index 3847d27801..c7c700fdb6 100644
--- a/block/throttle-groups.c
+++ b/block/throttle-groups.c
Thanks a lot! The changes done look good, and I think they’re nice.
There are some functions left untouched, though, which I think should
use ThrottleDirection, too:
throttle_group_register_tgm() should use THROTTLE_READ/THROTTLE_WRITE to
index tgm->throttled_reqs[] (instead of 0/1). It also has a `for (i =
0; i < 2; i++)` loop over tg->tokens[], which probably should use
THROTTLE_MAX as the upper limit, or iterate over a ThrottleDirection
variable altogether (as you’ve done it in patch 3 e.g. for
throttle_timers_attach_aio_context()).
throttle_group_unregister_tgm() has such a loop, too (over
tgm->pending_reqs[], tgm->throttled_reqs[],
tgm->throttle_timers.timers[], and tg->tokens[], all of which are arrays
over ThrottleDirection).
throttle_group_detach_aio_context() also has both the indexing “problem”
(integers instead of THROTTLE_* for tgm->pending_reqs[] and
tgm->throttled_reqs[]) and such a loop. There, the loop probably really
should be over a ThrottleDirection variable, because it passes that
variable to schedule_next_request().
throttle_group_restart_tgm() also has such a loop, it uses that variable
to index tgm->throttle_timers.timers[], and passes it to both timer_cb()
and throttle_group_restart_queue().
read_timer_cb() and write_timer_cb() should call timer_cb() with
THROTTLE_READ/THROTTLE_WRITE instead of false/true.
diff --git a/include/block/throttle-groups.h
b/include/block/throttle-groups.h
index ff282fc0f8..2355e8d9de 100644
--- a/include/block/throttle-groups.h
+++ b/include/block/throttle-groups.h
[...]
@@ -78,7 +78,7 @@ void throttle_group_restart_tgm(ThrottleGroupMember
*tgm);
void coroutine_fn
throttle_group_co_io_limits_intercept(ThrottleGroupMember *tgm,
int64_t bytes,
- bool is_write);
+
ThrottleDirection direction);
block/block-backend.c calls this function in
blk_co_do_p{read,write}v_part(), those calls need to be adjusted, too.
void throttle_group_attach_aio_context(ThrottleGroupMember *tgm,
AioContext *new_context);
void throttle_group_detach_aio_context(ThrottleGroupMember *tgm);
Sorry, I missed these. Please see the v5 version. Thanks!
--
zhenwei pi