qemu-block
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Re: [PATCH v4 9/9] block/throttle-groups: Use ThrottleDirection inst


From: zhenwei pi
Subject: Re: Re: [PATCH v4 9/9] block/throttle-groups: Use ThrottleDirection instread of bool is_write
Date: Fri, 28 Jul 2023 10:19:32 +0800
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.13.0



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 <pizhenwei@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



reply via email to

[Prev in Thread] Current Thread [Next in Thread]