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.
This patch fixes the issue.
And if the bps/iops limit was previously set to max and later changed to a
smaller value, we may not update tg->[bytes/io]_disp to 0 in
tg_update_carryover(). Relying solely on throtl_trim_slice() is not
sufficient, which can lead to subsequent bio dispatches not behaving as
expected. We should set tg->[bytes/io]_disp to 0 in non_carryover case.
The same handling applies when nr_queued is 0.
Fixes: 6cc477c36875 ("blk-throttle: carry over directly")
Signed-off-by: Zizhi Wo <wozizhi@xxxxxxxxxx>
---
block/blk-throttle.c | 33 +++++++++++++++++++++++++--------
1 file changed, 25 insertions(+), 8 deletions(-)
diff --git a/block/blk-throttle.c b/block/blk-throttle.c
index 91dab43c65ab..df9825eb83be 100644
--- a/block/blk-throttle.c
+++ b/block/blk-throttle.c
@@ -644,20 +644,39 @@ 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
* carryover_bytes/ios will be used to calculate new wait time under new
* configuration.
*/
- if (bps_limit != U64_MAX)
+ if (bps_limit != U64_MAX) {
*bytes = calculate_bytes_allowed(bps_limit, jiffy_elapsed) -
tg->bytes_disp[rw];
- if (iops_limit != UINT_MAX)
+ tg->bytes_disp[rw] = -*bytes;
+ } else {
+ tg->bytes_disp[rw] = 0;
+ }
+
+ 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->io_disp[rw] = -*ios;
+ } else {
+ tg->io_disp[rw] = 0;
+ }
}
static void tg_update_carryover(struct throtl_grp *tg)
@@ -665,10 +684,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__,
--
2.46.1
