2022-11-29 03:07:27

by Kemeng Shi

[permalink] [raw]
Subject: [PATCH v2 02/10] blk-throttle: Fix that bps of child could exceed bps limited in parent

Consider situation as following (on the default hierarchy):
HDD
|
root (bps limit: 4k)
|
child (bps limit :8k)
|
fio bs=8k
Rate of fio is supposed to be 4k, but result is 8k. Reason is as
following:
Size of single IO from fio is larger than bytes allowed in one
throtl_slice in child, so IOs are always queued in child group first.
When queued IOs in child are dispatched to parent group, BIO_BPS_THROTTLED
is set and these IOs will not be limited by tg_within_bps_limit anymore.
Fix this by only set BIO_BPS_THROTTLED when the bio traversed the entire
tree.

There patch has no influence on situation which is not on the default
hierarchy as each group is a single root group without parent.

Signed-off-by: Kemeng Shi <[email protected]>
---
block/blk-throttle.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/blk-throttle.c b/block/blk-throttle.c
index 59acfac87764..d516a37e82fb 100644
--- a/block/blk-throttle.c
+++ b/block/blk-throttle.c
@@ -1067,7 +1067,6 @@ static void tg_dispatch_one_bio(struct throtl_grp *tg, bool rw)
sq->nr_queued[rw]--;

throtl_charge_bio(tg, bio);
- bio_set_flag(bio, BIO_BPS_THROTTLED);

/*
* If our parent is another tg, we just need to transfer @bio to
@@ -1080,6 +1079,7 @@ static void tg_dispatch_one_bio(struct throtl_grp *tg, bool rw)
throtl_add_bio_tg(bio, &tg->qnode_on_parent[rw], parent_tg);
start_parent_slice_with_credit(tg, parent_tg, rw);
} else {
+ bio_set_flag(bio, BIO_BPS_THROTTLED);
throtl_qnode_add_bio(bio, &tg->qnode_on_parent[rw],
&parent_sq->queued[rw]);
BUG_ON(tg->td->nr_queued[rw] <= 0);
--
2.30.0


2022-11-30 21:25:34

by Tejun Heo

[permalink] [raw]
Subject: Re: [PATCH v2 02/10] blk-throttle: Fix that bps of child could exceed bps limited in parent

On Tue, Nov 29, 2022 at 11:01:39AM +0800, Kemeng Shi wrote:
> Consider situation as following (on the default hierarchy):
> HDD
> |
> root (bps limit: 4k)
> |
> child (bps limit :8k)
> |
> fio bs=8k
> Rate of fio is supposed to be 4k, but result is 8k. Reason is as
> following:
> Size of single IO from fio is larger than bytes allowed in one
> throtl_slice in child, so IOs are always queued in child group first.
> When queued IOs in child are dispatched to parent group, BIO_BPS_THROTTLED
> is set and these IOs will not be limited by tg_within_bps_limit anymore.
> Fix this by only set BIO_BPS_THROTTLED when the bio traversed the entire
> tree.
>
> There patch has no influence on situation which is not on the default
> hierarchy as each group is a single root group without parent.
>
> Signed-off-by: Kemeng Shi <[email protected]>

Acked-by: Tejun Heo <[email protected]>

--
tejun