2022-08-27 10:08:50

by Yu Kuai

[permalink] [raw]
Subject: [PATCH v2 0/3] blk-throttle cleanups

From: Yu Kuai <[email protected]>

Changes in v2:
- add tag
- remove patch 4

There are no functional changes.

Yu Kuai (3):
blk-throttle: use 'READ/WRITE' instead of '0/1'
blk-throttle: calling throtl_dequeue/enqueue_tg in pairs
blk-throttle: cleanup tg_update_disptime()

block/blk-throttle.c | 21 ++++++++++++---------
1 file changed, 12 insertions(+), 9 deletions(-)

--
2.31.1


2022-08-27 10:09:15

by Yu Kuai

[permalink] [raw]
Subject: [PATCH v2 2/3] blk-throttle: calling throtl_dequeue/enqueue_tg in pairs

From: Yu Kuai <[email protected]>

It's a little weird to call throtl_dequeue_tg() unconditionally in
throtl_select_dispatch(), since it will be called in tg_update_disptime()
again if some bio is still throttled. Thus call it later if there are no
throttled bio. There are no functional changes.

Signed-off-by: Yu Kuai <[email protected]>
Acked-by: Tejun Heo <[email protected]>
---
block/blk-throttle.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/block/blk-throttle.c b/block/blk-throttle.c
index fe1fa6441105..64002435fa43 100644
--- a/block/blk-throttle.c
+++ b/block/blk-throttle.c
@@ -1101,13 +1101,13 @@ static int throtl_select_dispatch(struct throtl_service_queue *parent_sq)
if (time_before(jiffies, tg->disptime))
break;

- throtl_dequeue_tg(tg);
-
nr_disp += throtl_dispatch_tg(tg);

sq = &tg->service_queue;
if (sq->nr_queued[READ] || sq->nr_queued[WRITE])
tg_update_disptime(tg);
+ else
+ throtl_dequeue_tg(tg);

if (nr_disp >= THROTL_QUANTUM)
break;
--
2.31.1

2022-08-27 10:09:27

by Yu Kuai

[permalink] [raw]
Subject: [PATCH v2 1/3] blk-throttle: use 'READ/WRITE' instead of '0/1'

From: Yu Kuai <[email protected]>

Make the code easier to read, like everywhere else.

Signed-off-by: Yu Kuai <[email protected]>
Acked-by: Tejun Heo <[email protected]>
---
block/blk-throttle.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/block/blk-throttle.c b/block/blk-throttle.c
index 9f5fe62afff9..fe1fa6441105 100644
--- a/block/blk-throttle.c
+++ b/block/blk-throttle.c
@@ -329,8 +329,8 @@ static struct bio *throtl_pop_queued(struct list_head *queued,
/* init a service_queue, assumes the caller zeroed it */
static void throtl_service_queue_init(struct throtl_service_queue *sq)
{
- INIT_LIST_HEAD(&sq->queued[0]);
- INIT_LIST_HEAD(&sq->queued[1]);
+ INIT_LIST_HEAD(&sq->queued[READ]);
+ INIT_LIST_HEAD(&sq->queued[WRITE]);
sq->pending_tree = RB_ROOT_CACHED;
timer_setup(&sq->pending_timer, throtl_pending_timer_fn, 0);
}
@@ -1106,7 +1106,7 @@ static int throtl_select_dispatch(struct throtl_service_queue *parent_sq)
nr_disp += throtl_dispatch_tg(tg);

sq = &tg->service_queue;
- if (sq->nr_queued[0] || sq->nr_queued[1])
+ if (sq->nr_queued[READ] || sq->nr_queued[WRITE])
tg_update_disptime(tg);

if (nr_disp >= THROTL_QUANTUM)
--
2.31.1

2022-08-27 10:09:36

by Yu Kuai

[permalink] [raw]
Subject: [PATCH v2 3/3] blk-throttle: cleanup tg_update_disptime()

From: Yu Kuai <[email protected]>

tg_update_disptime() only need to adjust postion for 'tg' in
'parent_sq', there is no need to call throtl_enqueue/dequeue_tg(),
since they will set/clear flag THROTL_TG_PENDING and increase/decrease
nr_pending, which is useless. By the way, clear the flag/decrease
nr_pending while there are still throttled bios is not good for debugging.

There are no functional changes.

Signed-off-by: Yu Kuai <[email protected]>
Acked-by: Tejun Heo <[email protected]>
---
block/blk-throttle.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/block/blk-throttle.c b/block/blk-throttle.c
index 64002435fa43..47142a1dd102 100644
--- a/block/blk-throttle.c
+++ b/block/blk-throttle.c
@@ -520,7 +520,6 @@ static void throtl_rb_erase(struct rb_node *n,
{
rb_erase_cached(n, &parent_sq->pending_tree);
RB_CLEAR_NODE(n);
- --parent_sq->nr_pending;
}

static void update_min_dispatch_time(struct throtl_service_queue *parent_sq)
@@ -572,7 +571,11 @@ static void throtl_enqueue_tg(struct throtl_grp *tg)
static void throtl_dequeue_tg(struct throtl_grp *tg)
{
if (tg->flags & THROTL_TG_PENDING) {
- throtl_rb_erase(&tg->rb_node, tg->service_queue.parent_sq);
+ struct throtl_service_queue *parent_sq =
+ tg->service_queue.parent_sq;
+
+ throtl_rb_erase(&tg->rb_node, parent_sq);
+ --parent_sq->nr_pending;
tg->flags &= ~THROTL_TG_PENDING;
}
}
@@ -990,9 +993,9 @@ static void tg_update_disptime(struct throtl_grp *tg)
disptime = jiffies + min_wait;

/* Update dispatch time */
- throtl_dequeue_tg(tg);
+ throtl_rb_erase(&tg->rb_node, tg->service_queue.parent_sq);
tg->disptime = disptime;
- throtl_enqueue_tg(tg);
+ tg_service_queue_add(tg);

/* see throtl_add_bio_tg() */
tg->flags &= ~THROTL_TG_WAS_EMPTY;
--
2.31.1

2022-09-12 06:43:40

by Yu Kuai

[permalink] [raw]
Subject: Re: [PATCH v2 0/3] blk-throttle cleanups

Hi, jens

Can you apply this patchset?

Thanks,
Kuai

?? 2022/08/27 18:16, Yu Kuai ะด??:
> From: Yu Kuai <[email protected]>
>
> Changes in v2:
> - add tag
> - remove patch 4
>
> There are no functional changes.
>
> Yu Kuai (3):
> blk-throttle: use 'READ/WRITE' instead of '0/1'
> blk-throttle: calling throtl_dequeue/enqueue_tg in pairs
> blk-throttle: cleanup tg_update_disptime()
>
> block/blk-throttle.c | 21 ++++++++++++---------
> 1 file changed, 12 insertions(+), 9 deletions(-)
>

2022-09-12 07:04:19

by Jens Axboe

[permalink] [raw]
Subject: Re: [PATCH v2 0/3] blk-throttle cleanups

On Sat, 27 Aug 2022 18:16:34 +0800, Yu Kuai wrote:
> From: Yu Kuai <[email protected]>
>
> Changes in v2:
> - add tag
> - remove patch 4
>
> There are no functional changes.
>
> [...]

Applied, thanks!

[1/3] blk-throttle: use 'READ/WRITE' instead of '0/1'
commit: 7e9c5c54d440bd6402ffdba4dc4f3df5bfe64ea4
[2/3] blk-throttle: calling throtl_dequeue/enqueue_tg in pairs
commit: 8c25ed0cb9d2e349ebebfeacf7ce1ae015afe54d
[3/3] blk-throttle: cleanup tg_update_disptime()
commit: c013710e1a7eba8e33da9380a068fe1cec017226

Best regards,
--
Jens Axboe