[PATCH 0/2] blk-throttle: minor cleanups

[PATCH 0/2] blk-throttle: minor cleanups

[Tao Cui]

From: Tao Cui <cuitao@kylinos.cn>

This short series collects two independent, behavior-preserving cleanups
to the blk-throttle policy.

  Patch 1 reworks calculate_bytes_allowed() to short-circuit a zero
  jiffy_elapsed explicitly, rather than relying on ilog2(0) == -1
  (fls64(0) - 1) to skip the overflow guard. __tg_update_carryover()
  reaches it with a zero elapsed time right after a slice starts.

  Patch 2 factors the four identical rbps/wbps/riops/wiops print blocks
  in tg_prfill_limit() into a small helper and drops two alias locals.

Neither changes behavior; the io.max seq_file output is unchanged. Built
and boot-tested in QEMU (cgroup v2 io.max read-back matches, throttled IO
completes without warning/oops).

Tao Cui (2):
  blk-throttle: avoid ilog2(0) in calculate_bytes_allowed()
  blk-throttle: factor out limit field printing in tg_prfill_limit()

 block/blk-throttle.c | 52 ++++++++++++++++++++------------------------
 1 file changed, 23 insertions(+), 29 deletions(-)

-- 
2.43.0

[PATCH 1/2] blk-throttle: avoid ilog2(0) in calculate_bytes_allowed()

[Tao Cui]

From: Tao Cui <cuitao@kylinos.cn>

__tg_update_carryover() can call calculate_bytes_allowed() with a zero
jiffy_elapsed right after a slice starts. The overflow guard

    if (ilog2(bps_limit) + ilog2(jiffy_elapsed) - ilog2(HZ) > 62)

relies on ilog2(0) == -1 (fls64(0) - 1) to stay below the threshold so
that the subsequent mul_u64_u64_div_u64(bps, 0, HZ) == 0 is reached.
That works, but the ilog2(0) dependency is non-obvious.

Add an explicit early return for jiffy_elapsed == 0, which is equivalent
(mul_u64_u64_div_u64(bps_limit, 0, HZ) == 0) and removes the reliance on
ilog2(0).

No behavior change.

Signed-off-by: Tao Cui <cuitao@kylinos.cn>
---
 block/blk-throttle.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/block/blk-throttle.c b/block/blk-throttle.c
index ffc3b70065d4..f37911abefdd 100644
--- a/block/blk-throttle.c
+++ b/block/blk-throttle.c
@@ -603,6 +603,10 @@ static unsigned int calculate_io_allowed(u32 iops_limit,
 
 static u64 calculate_bytes_allowed(u64 bps_limit, unsigned long jiffy_elapsed)
 {
+	/* 0 elapsed => 0 bytes allowed; also avoids ilog2(0) below. */
+	if (!jiffy_elapsed)
+		return 0;
+
 	/*
 	 * Can result be wider than 64 bits?
 	 * We check against 62, not 64, due to ilog2 truncation.
-- 
2.43.0

[PATCH 2/2] blk-throttle: factor out limit field printing in tg_prfill_limit()

[Tao Cui]

From: Tao Cui <cuitao@kylinos.cn>

The four rbps/wbps/riops/wiops blocks in tg_prfill_limit() are identical
apart from the key string and whether the limit is 64- or 32-bit. Factor
them into tg_prfill_limit_field() and drop the bps_dft / iops_dft alias
locals in favor of U64_MAX / UINT_MAX directly.

The io.max seq_file output is byte-for-byte unchanged.

Signed-off-by: Tao Cui <cuitao@kylinos.cn>
---
 block/blk-throttle.c | 48 ++++++++++++++++++--------------------------
 1 file changed, 19 insertions(+), 29 deletions(-)

diff --git a/block/blk-throttle.c b/block/blk-throttle.c
index f37911abefdd..8f4563acbc16 100644
--- a/block/blk-throttle.c
+++ b/block/blk-throttle.c
@@ -1489,47 +1489,37 @@ static struct cftype throtl_legacy_files[] = {
 	{ }	/* terminate */
 };
 
+static void tg_prfill_limit_field(struct seq_file *sf, const char *key,
+				  u64 val, bool is_uint)
+{
+	u64 dflt = is_uint ? UINT_MAX : U64_MAX;
+
+	if (val == dflt)
+		seq_printf(sf, " %s=max", key);
+	else if (is_uint)
+		seq_printf(sf, " %s=%u", key, (unsigned int)val);
+	else
+		seq_printf(sf, " %s=%llu", key, val);
+}
+
 static u64 tg_prfill_limit(struct seq_file *sf, struct blkg_policy_data *pd,
 			 int off)
 {
 	struct throtl_grp *tg = pd_to_tg(pd);
 	const char *dname = blkg_dev_name(pd->blkg);
-	u64 bps_dft;
-	unsigned int iops_dft;
 
 	if (!dname)
 		return 0;
 
-	bps_dft = U64_MAX;
-	iops_dft = UINT_MAX;
-
-	if (tg->bps[READ] == bps_dft &&
-	    tg->bps[WRITE] == bps_dft &&
-	    tg->iops[READ] == iops_dft &&
-	    tg->iops[WRITE] == iops_dft)
+	if (tg->bps[READ] == U64_MAX && tg->bps[WRITE] == U64_MAX &&
+	    tg->iops[READ] == UINT_MAX && tg->iops[WRITE] == UINT_MAX)
 		return 0;
 
 	seq_printf(sf, "%s", dname);
-	if (tg->bps[READ] == U64_MAX)
-		seq_printf(sf, " rbps=max");
-	else
-		seq_printf(sf, " rbps=%llu", tg->bps[READ]);
-
-	if (tg->bps[WRITE] == U64_MAX)
-		seq_printf(sf, " wbps=max");
-	else
-		seq_printf(sf, " wbps=%llu", tg->bps[WRITE]);
-
-	if (tg->iops[READ] == UINT_MAX)
-		seq_printf(sf, " riops=max");
-	else
-		seq_printf(sf, " riops=%u", tg->iops[READ]);
-
-	if (tg->iops[WRITE] == UINT_MAX)
-		seq_printf(sf, " wiops=max");
-	else
-		seq_printf(sf, " wiops=%u", tg->iops[WRITE]);
-
+	tg_prfill_limit_field(sf, "rbps",  tg->bps[READ],  false);
+	tg_prfill_limit_field(sf, "wbps",  tg->bps[WRITE], false);
+	tg_prfill_limit_field(sf, "riops", tg->iops[READ], true);
+	tg_prfill_limit_field(sf, "wiops", tg->iops[WRITE], true);
 	seq_printf(sf, "\n");
 	return 0;
 }
-- 
2.43.0

Re: [PATCH 1/2] blk-throttle: avoid ilog2(0) in calculate_bytes_allowed()

[David Laight]

On Tue, 14 Jul 2026 18:30:27 +0800
Tao Cui <cui.tao@linux.dev> wrote:

> From: Tao Cui <cuitao@kylinos.cn>
> 
> __tg_update_carryover() can call calculate_bytes_allowed() with a zero
> jiffy_elapsed right after a slice starts. The overflow guard
> 
>     if (ilog2(bps_limit) + ilog2(jiffy_elapsed) - ilog2(HZ) > 62)
> 
> relies on ilog2(0) == -1 (fls64(0) - 1) to stay below the threshold so
> that the subsequent mul_u64_u64_div_u64(bps, 0, HZ) == 0 is reached.
> That works, but the ilog2(0) dependency is non-obvious.
> 
> Add an explicit early return for jiffy_elapsed == 0, which is equivalent
> (mul_u64_u64_div_u64(bps_limit, 0, HZ) == 0) and removes the reliance on
> ilog2(0).
> 
> No behavior change.

There is a pending patch to make the x86-64 mul_u64_u64_div_u64()
return ~0ULL on overflow to match the generic code.
That would completely remove the requirement for the overflow guard.

I can't remember why it got lost (again).
There have been arguments about what should happen for divide by zero.
Personally I'd return ~0ULL and let the caller decide what that means,
but the generic mul_u64_u64_div_u64() goes to lengths to generate any
trap that a normal divide by zero would generate.

	David

> 
> Signed-off-by: Tao Cui <cuitao@kylinos.cn>
> ---
>  block/blk-throttle.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/block/blk-throttle.c b/block/blk-throttle.c
> index ffc3b70065d4..f37911abefdd 100644
> --- a/block/blk-throttle.c
> +++ b/block/blk-throttle.c
> @@ -603,6 +603,10 @@ static unsigned int calculate_io_allowed(u32 iops_limit,
>  
>  static u64 calculate_bytes_allowed(u64 bps_limit, unsigned long jiffy_elapsed)
>  {
> +	/* 0 elapsed => 0 bytes allowed; also avoids ilog2(0) below. */
> +	if (!jiffy_elapsed)
> +		return 0;
> +
>  	/*
>  	 * Can result be wider than 64 bits?
>  	 * We check against 62, not 64, due to ilog2 truncation.