]> git.ipfire.org Git - thirdparty/kernel/linux.git/commitdiff
blk-throttle: Fix wrong tg->[bytes/io]_disp update in __tg_update_carryover()
authorZizhi Wo <wozizhi@huawei.com>
Thu, 17 Apr 2025 13:20:52 +0000 (21:20 +0800)
committerJens Axboe <axboe@kernel.dk>
Tue, 6 May 2025 01:08:34 +0000 (19:08 -0600)
In commit 6cc477c36875 ("blk-throttle: carry over directly"), the carryover
bytes/ios was be carried to [bytes/io]_disp. However, its update mechanism
has some issues.

In __tg_update_carryover(), we calculate "bytes" and "ios" to represent the
carryover, but the computation when updating [bytes/io]_disp is incorrect.
And if the sq->nr_queued is empty, we may not update tg->[bytes/io]_disp to
0 in tg_update_carryover(). We should set it to 0 in non carryover case.
This patch fixes the issue.

Fixes: 6cc477c36875 ("blk-throttle: carry over directly")
Signed-off-by: Zizhi Wo <wozizhi@huawei.com>
Reviewed-by: Yu Kuai <yukuai3@huawei.com>
Reviewed-by: Ming Lei <ming.lei@redhat.com>
Link: https://lore.kernel.org/r/20250417132054.2866409-2-wozizhi@huaweicloud.com
Signed-off-by: Jens Axboe <axboe@kernel.dk>
block/blk-throttle.c

index d6dd2e04787491fd2dc3fb2a2b06cf9d0ab3a3b8..7437de947120ed1f2e26d0362fd1b5a961f3ea3a 100644 (file)
@@ -644,6 +644,18 @@ static void __tg_update_carryover(struct throtl_grp *tg, bool rw,
        u64 bps_limit = tg_bps_limit(tg, rw);
        u32 iops_limit = tg_iops_limit(tg, rw);
 
+       /*
+        * If the queue is empty, carryover handling is not needed. In such cases,
+        * tg->[bytes/io]_disp should be reset to 0 to avoid impacting the dispatch
+        * of subsequent bios. The same handling applies when the previous BPS/IOPS
+        * limit was set to max.
+        */
+       if (tg->service_queue.nr_queued[rw] == 0) {
+               tg->bytes_disp[rw] = 0;
+               tg->io_disp[rw] = 0;
+               return;
+       }
+
        /*
         * If config is updated while bios are still throttled, calculate and
         * accumulate how many bytes/ios are waited across changes. And
@@ -656,8 +668,8 @@ static void __tg_update_carryover(struct throtl_grp *tg, bool rw,
        if (iops_limit != UINT_MAX)
                *ios = calculate_io_allowed(iops_limit, jiffy_elapsed) -
                        tg->io_disp[rw];
-       tg->bytes_disp[rw] -= *bytes;
-       tg->io_disp[rw] -= *ios;
+       tg->bytes_disp[rw] = -*bytes;
+       tg->io_disp[rw] = -*ios;
 }
 
 static void tg_update_carryover(struct throtl_grp *tg)
@@ -665,10 +677,8 @@ static void tg_update_carryover(struct throtl_grp *tg)
        long long bytes[2] = {0};
        int ios[2] = {0};
 
-       if (tg->service_queue.nr_queued[READ])
-               __tg_update_carryover(tg, READ, &bytes[READ], &ios[READ]);
-       if (tg->service_queue.nr_queued[WRITE])
-               __tg_update_carryover(tg, WRITE, &bytes[WRITE], &ios[WRITE]);
+       __tg_update_carryover(tg, READ, &bytes[READ], &ios[READ]);
+       __tg_update_carryover(tg, WRITE, &bytes[WRITE], &ios[WRITE]);
 
        /* see comments in struct throtl_grp for meaning of these fields. */
        throtl_log(&tg->service_queue, "%s: %lld %lld %d %d\n", __func__,