Hi,
在 2025/04/17 9:50, Zizhi Wo 写道:
Now the tg->[bytes/io]_disp type is signed, and calculate_bytes/io_allowed
return type is unsigned. Even if the bps/iops limit is not set to max, the
return value of the function may still exceed INT_MAX or LLONG_MAX, which
can cause overflow in outer variables. In such cases, we can add additional
checks accordingly.
And in throtl_trim_slice(), if the BPS/IOPS limit is set to max, there's
no need to call calculate_bytes/io_allowed(). Introduces the helper
functions throtl_trim_bps/iops to simplifies the process. For cases when
the calculated trim value exceeds INT_MAX (causing an overflow), we reset
tg->[bytes/io]_disp to zero, so return original tg->[bytes/io]_disp because
it is the size that is actually trimmed.
Signed-off-by: Zizhi Wo <wozizhi@xxxxxxxxxx>
---
block/blk-throttle.c | 94 ++++++++++++++++++++++++++++++++++----------
1 file changed, 73 insertions(+), 21 deletions(-)
diff --git a/block/blk-throttle.c b/block/blk-throttle.c
index 4e4609dce85d..cfb9c47d1af7 100644
--- a/block/blk-throttle.c
+++ b/block/blk-throttle.c
@@ -571,6 +571,56 @@ static u64 calculate_bytes_allowed(u64 bps_limit, unsigned long jiffy_elapsed)
return mul_u64_u64_div_u64(bps_limit, (u64)jiffy_elapsed, (u64)HZ);
}
+static long long throtl_trim_bps(struct throtl_grp *tg, bool rw,
+ unsigned long time_elapsed)
+{
+ u64 bps_limit = tg_bps_limit(tg, rw);
+ long long bytes_trim;
+
+ if (bps_limit == U64_MAX)
+ return 0;
+
+ /* Need to consider the case of bytes_allowed overflow. */
+ bytes_trim = calculate_bytes_allowed(bps_limit, time_elapsed);
+ if (bytes_trim <= 0) {
+ bytes_trim = tg->bytes_disp[rw];
+ tg->bytes_disp[rw] = 0;
+ goto out;
+ }
+
+ if (tg->bytes_disp[rw] >= bytes_trim)
+ tg->bytes_disp[rw] -= bytes_trim;
+ else
+ tg->bytes_disp[rw] = 0;
Perhaps
if (bytes_trim <=0 || tg->bytes_disp[rw] < bytes_trim) {
bytes_trim = tg->bytes_disp[rw];
tg->bytes_disp[rw] = 0;
} else {
tg->bytes_disp[rw] -= bytes_trim;
}
And you don't need the out tag.
+out:
+ return bytes_trim;
+}
+
+static int throtl_trim_iops(struct throtl_grp *tg, bool rw,
+ unsigned long time_elapsed)
+{
+ u32 iops_limit = tg_iops_limit(tg, rw);
+ int io_trim;
+
+ if (iops_limit == UINT_MAX)
+ return 0;
+
+ /* Need to consider the case of io_allowed overflow. */
+ io_trim = calculate_io_allowed(iops_limit, time_elapsed);
+ if (io_trim <= 0) {
+ io_trim = tg->io_disp[rw];
+ tg->io_disp[rw] = 0;
+ goto out;
+ }
+
+ if (tg->io_disp[rw] >= io_trim)
+ tg->io_disp[rw] -= io_trim;
+ else
+ tg->io_disp[rw] = 0;
+out:
+ return io_trim;
Same as above.
Thanks,
Kuai
+}
+
/* Trim the used slices and adjust slice start accordingly */
static inline void throtl_trim_slice(struct throtl_grp *tg, bool rw)
{
@@ -612,22 +662,11 @@ static inline void throtl_trim_slice(struct throtl_grp *tg, bool rw)
* one extra slice is preserved for deviation.
*/
time_elapsed -= tg->td->throtl_slice;
- bytes_trim = calculate_bytes_allowed(tg_bps_limit(tg, rw),
- time_elapsed);
- io_trim = calculate_io_allowed(tg_iops_limit(tg, rw), time_elapsed);
- if (bytes_trim <= 0 && io_trim <= 0)
+ bytes_trim = throtl_trim_bps(tg, rw, time_elapsed);
+ io_trim = throtl_trim_iops(tg, rw, time_elapsed);
+ if (!bytes_trim && !io_trim)
return;
- if ((long long)tg->bytes_disp[rw] >= bytes_trim)
- tg->bytes_disp[rw] -= bytes_trim;
- else
- tg->bytes_disp[rw] = 0;
-
- if ((int)tg->io_disp[rw] >= io_trim)
- tg->io_disp[rw] -= io_trim;
- else
- tg->io_disp[rw] = 0;
-
tg->slice_start[rw] += time_elapsed;
throtl_log(&tg->service_queue,
@@ -643,6 +682,8 @@ static void __tg_update_carryover(struct throtl_grp *tg, bool rw,
unsigned long jiffy_elapsed = jiffies - tg->slice_start[rw];
u64 bps_limit = tg_bps_limit(tg, rw);
u32 iops_limit = tg_iops_limit(tg, rw);
+ long long bytes_allowed;
+ int io_allowed;
/*
* If the queue is empty, carryover handling is not needed. In such cases,
@@ -661,19 +702,28 @@ static void __tg_update_carryover(struct throtl_grp *tg, bool rw,
* accumulate how many bytes/ios are waited across changes. And use the
* calculated carryover (@bytes/@ios) to update [bytes/io]_disp, which
* will be used to calculate new wait time under new configuration.
+ * And we need to consider the case of bytes/io_allowed overflow.
*/
if (bps_limit != U64_MAX) {
- *bytes = calculate_bytes_allowed(bps_limit, jiffy_elapsed) -
- tg->bytes_disp[rw];
- tg->bytes_disp[rw] = -*bytes;
+ bytes_allowed = calculate_bytes_allowed(bps_limit, jiffy_elapsed);
+ if (bytes_allowed < 0) {
+ tg->bytes_disp[rw] = 0;
+ } else {
+ *bytes = bytes_allowed - tg->bytes_disp[rw];
+ 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->io_disp[rw] = -*ios;
+ io_allowed = calculate_io_allowed(iops_limit, jiffy_elapsed);
+ if (io_allowed < 0) {
+ tg->io_disp[rw] = 0;
+ } else {
+ *ios = io_allowed - tg->io_disp[rw];
+ tg->io_disp[rw] = -*ios;
+ }
} else {
tg->io_disp[rw] = 0;
}
@@ -741,7 +791,9 @@ static unsigned long tg_within_bps_limit(struct throtl_grp *tg, struct bio *bio,
jiffy_elapsed_rnd = roundup(jiffy_elapsed_rnd, tg->td->throtl_slice);
bytes_allowed = calculate_bytes_allowed(bps_limit, jiffy_elapsed_rnd);
- if (bytes_allowed > 0 && tg->bytes_disp[rw] + bio_size <= bytes_allowed)
+ /* Need to consider the case of bytes_allowed overflow. */
+ if ((bytes_allowed > 0 && tg->bytes_disp[rw] + bio_size <= bytes_allowed)
+ || bytes_allowed < 0)
return 0;
/* Calc approx time to dispatch */
