Commit f66cf69e authored by Zizhi Wo's avatar Zizhi Wo Committed by Jens Axboe
Browse files

blk-throttle: Fix wrong tg->[bytes/io]_disp update in __tg_update_carryover() 



In commit 6cc477c3 ("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: 6cc477c3 ("blk-throttle: carry over directly")
Signed-off-by: default avatarZizhi Wo <wozizhi@huawei.com>
Reviewed-by: default avatarYu Kuai <yukuai3@huawei.com>
Reviewed-by: default avatarMing Lei <ming.lei@redhat.com>
Link: https://lore.kernel.org/r/20250417132054.2866409-2-wozizhi@huaweicloud.com


Signed-off-by: default avatarJens Axboe <axboe@kernel.dk>
parent e371b9d3

block/blk-throttle.c

+16 −6
Original line number Diff line number Diff line
@@ -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,9 +677,7 @@ 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]);



	/* see comments in struct throtl_grp for meaning of these fields. */