Re: [PATCH v7 1/9] blk-throttle: fix that io throttle can only work for single bio

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

 



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.





[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]     [Monitors]

  Powered by Linux