Hi, Tejun
在 2022/08/22 15:25, Tejun Heo 写道:
Hello,
On Mon, Aug 22, 2022 at 11:06:44AM +0800, Yu Kuai wrote:
While implementing the new method, I found that there seems to be a
misunderstanding here, the code seems to try to add flag to split bio
so that it won't be throttled again for bps limit, however:
1) for blk throttle, split bio is issued directly and will never be
throttled again, while orignal bio will go through throttle path again.
2) if cloned bio is directed to a new disk, the flag is cleared anyway.
Doesn't that make the current code correct then? But you were seeing
incorrect behaviors, right?
According to the commit message in commit 111be8839817 ("block-throttle:
avoid double charge"):
If the bio is cloned/split, we copy the flag to new bio too to avoid a
double charge.
Which make me think the split bio will be resubmitted, and after
implementing the new solution, I found that test result is not as
expected. After spending sometime figuring out what is wrong, I found
that split bio will be dispatched directly from caller, while orignal
bio will be resubmitted.
I guess commit 111be8839817 made a mistake, however, there should be
no problem because orignal bio is flagged already, and it's handled
correctly.
Anyway, I removed the code in __bio_clone() and check flag in
__bio_split_to_limits in my patch:
--- a/block/bio.c
+++ b/block/bio.c
@@ -760,8 +760,6 @@ EXPORT_SYMBOL(bio_put);
static int __bio_clone(struct bio *bio, struct bio *bio_src, gfp_t gfp)
{
bio_set_flag(bio, BIO_CLONED);
- if (bio_flagged(bio_src, BIO_THROTTLED))
- bio_set_flag(bio, BIO_THROTTLED);
bio->bi_ioprio = bio_src->bi_ioprio;
bio->bi_iter = bio_src->bi_iter;
diff --git a/block/blk-merge.c b/block/blk-merge.c
index ff04e9290715..10330bb038ca 100644
--- a/block/blk-merge.c
+++ b/block/blk-merge.c
@@ -358,6 +358,13 @@ struct bio *__bio_split_to_limits(struct bio *bio,
struct queue_limits *lim,
blkcg_bio_issue_init(split);
bio_chain(split, bio);
trace_block_split(split, bio->bi_iter.bi_sector);
+
+ /*
+ * original bio will be resubmited and throttled again,
clear
+ * the iops flag so that it can be count again for iops
limit.
+ */
+ if (bio_flagged(bio, BIO_IOPS_THROTTLED))
+ bio_clear_flag(bio, BIO_IOPS_THROTTLED);
submit_bio_noacct(bio);
return split;
}
Thanks.
