Re: [PATCH v4] blk-throttle: fix divide-by-zero on legacy iops limit of 0
"yu kuai" <[email protected]> Thu, 23 Jul 2026 14:46:01 +0800
| Newsgroups | org.kernel.vger.cgroups,org.kernel.vger.linux-block,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <[email protected]> |
在 2026/7/22 18:24, Tao Cui 写道: > From: Tao Cui<[email protected]> > > Writing a multiple of 2^32 (e.g. 4294967296) to a legacy cgroup v1 > throttle iops file (blkio.throttle.{read,write}_iops_device) silently > truncates to 0: tg_set_conf() stores the sscanf-parsed u64 value into > an unsigned int field with no clamping. The cgroup v2 path, > tg_set_limit(), already clamps the same kind of value with > min_t(u64, val, UINT_MAX), but the legacy path never did. Note that > the "!v -> U64_MAX" mapping only catches an explicit zero and does not > catch a value that truncates to zero. > > With iops stored as 0, tg_update_has_rules() sets has_rules_iops[] and > the next IO reaches tg_within_iops_limit(), which computes > > jiffy_wait = max(jiffy_wait, HZ / iops_limit + 1); > > triggering a divide-by-zero oops. > > Fix it in tg_set_conf() by clamping the value to UINT_MAX, consistent > with tg_set_limit(). This closes the truncation root cause: with 0 no > longer reachable as a stored limit, the HZ / iops_limit divide is never > hit. > > Signed-off-by: Tao Cui<[email protected]> > > --- > Changes in v4: > - Drop the defensive "iops_limit == 0" check in tg_dispatch_iops_time(): > with the tg_set_conf() clamp in place, 0 can never be stored as a limit, > so the runtime check only guards an unreachable state. (Yu Kuai) > - Drop the Fixes: tag: the unclamped write -- and the iops=0 behavior it > can produce (calculate_io_allowed() returns 0, so no IO is issued) -- > long predates the commit that added the HZ / iops_limit divide, so > attributing it there was incorrect. (Yu Kuai) > > Changes in v3: > - Drop the (u64) cast on UINT_MAX: the kernel's type-checked min() accepts > two unsigned types of different width (both >= 4 bytes), so > min(v, UINT_MAX) compiles clean. (David Laight) > > Changes in v2: > - Use a "void *field" local for the config write so the assignment reads > *(u64 *)field / *(unsigned int *)field instead of the > (type *)((void *)tg + of_cft(of)->private) casts. > - Use min(v, UINT_MAX) instead of min_t(u64, v, UINT_MAX). > --- > block/blk-throttle.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) Reviewed-by: Yu Kuai <[email protected]> -- Thanks, Kuai