diff mbox series

[v7,5/9] blk-throttle: improve handling of re-entered bio for bps limit

Message ID 20220802140415.2960284-6-yukuai1@huaweicloud.com (mailing list archive)
State New, archived
Headers show
Series bugfix and cleanup for blk-throttle | expand

Commit Message

Yu Kuai Aug. 2, 2022, 2:04 p.m. UTC 
From: Yu Kuai <yukuai3@huawei.com>

Currently, if new slice is started while bio is re-entered, such bio
will count multiple times for bps limit. Since 'bytes_skipped'
represents how many bytes will be skipped while dispatching bios, which
can handle that case, increasing it instead of decreasing 'bytes_disp'.

Signed-off-by: Yu Kuai <yukuai3@huawei.com>
---
 block/blk-throttle.c | 8 ++++----
 block/blk-throttle.h | 4 +++-
 2 files changed, 7 insertions(+), 5 deletions(-)

Comments

Tejun Heo Aug. 16, 2022, 8:02 p.m. UTC | #1 
On Tue, Aug 02, 2022 at 10:04:11PM +0800, Yu Kuai wrote:
> From: Yu Kuai <yukuai3@huawei.com>
> 
> Currently, if new slice is started while bio is re-entered, such bio
> will count multiple times for bps limit. Since 'bytes_skipped'
> represents how many bytes will be skipped while dispatching bios, which
> can handle that case, increasing it instead of decreasing 'bytes_disp'.

The same thing. I think we're working around the problem in the wrong place.

Thanks.
diff mbox series

Patch

diff --git a/block/blk-throttle.c b/block/blk-throttle.c
index 621402cf2576..237668328e98 100644
--- a/block/blk-throttle.c
+++ b/block/blk-throttle.c
@@ -2183,14 +2183,14 @@  bool __blk_throtl_bio(struct bio *bio)
 		 * Splited bios can be re-entered because iops limit should be
 		 * counted again, however, bps limit should not. Since bps limit
 		 * will be counted again while dispatching it, compensate the
-		 * over-accounting here. Noted that compensation can fail if
-		 * new slice is started.
+		 * over-accounting here. Since decrement of 'bytes_disp' can't
+		 * handle the case that new slice is started, increase
+		 * 'bytes_skipped' instead.
 		 */
 		if (bio_flagged(bio, BIO_THROTTLED)) {
 			unsigned int bio_size = throtl_bio_data_size(bio);
 
-			if (tg->bytes_disp[rw] >= bio_size)
-				tg->bytes_disp[rw] -= bio_size;
+			tg->bytes_skipped[rw] += bio_size;
 			if (tg->last_bytes_disp[rw] >= bio_size)
 				tg->last_bytes_disp[rw] -= bio_size;
 		}
diff --git a/block/blk-throttle.h b/block/blk-throttle.h
index 0163aa9104c3..c9545616ba12 100644
--- a/block/blk-throttle.h
+++ b/block/blk-throttle.h
@@ -121,7 +121,9 @@  struct throtl_grp {
 	 * bytes/io are waited already in previous configuration, and they will
 	 * be used to calculate wait time under new configuration.
 	 *
-	 * Number of bytes will be skipped in current slice
+	 * Number of bytes will be skipped in current slice. In addition, this
+	 * field will help to handle re-entered bio for bps limit, see details
+	 * in __blk_throtl_bio().
 	 */
 	uint64_t bytes_skipped[2];
 	/* Number of bio will be skipped in current slice */