Re: [PATCH v2] lib/group_cpus: rotate extra groups to avoid IRQ stacking

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Integer overflow in intra-cluster group rotation breaks CPU allocation permutation, causing empty CPU masks.
--

--- Patch [1]: [PATCH v2] lib/group_cpus: rotate extra groups to avoid IRQ stacking ---
commit 3ea99a586f68e414d616ef9d2a0f13f390a05e7d
Author: Naman Jain <[email protected]>

lib/group_cpus: rotate extra groups to avoid IRQ stacking

group_cpus_evenly() computes how a device's queue interrupts are spread
across CPUs. This commit introduces a per-caller rotation counter to
shift which groups get extra CPUs. The rotation ensures that single-CPU
groups from multiple identical devices do not stack on the same CPUs,
improving throughput and reducing tail latency.

> diff --git a/lib/group_cpus.c b/lib/group_cpus.c
> index e6e18d7a49bba..8bed0f9d2110b 100644
> --- a/lib/group_cpus.c
> +++ b/lib/group_cpus.c
[ ... ]
> @@ -270,11 +279,15 @@ static void assign_cpus_to_groups(unsigned int ncpus,
>  	for (v = 0; v < nv->ngroups; v++, *curgrp += 1) {
>  		cpus_per_grp = ncpus / nv->ngroups;
>  
> -		/* Account for extra groups to compensate rounding errors */
> -		if (extra_grps) {
> +		/*
> +		 * Rotate which groups get the extra CPU so that
> +		 * successive callers produce different mappings,
> +		 * avoiding IRQ stacking when multiple devices
> +		 * share the same CPU topology.
> +		 */
> +		if (extra_grps &&
> +		    (v + spread_offset) % nv->ngroups < extra_grps)
>  			cpus_per_grp++;

[Severity: Medium]
Can this addition overflow and break the CPU allocation?

The commit message and comments state that wrapping of the unsigned
counter is harmless. However, if spread_offset is very close to UINT_MAX,
v + spread_offset can wrap around mid-loop.

Since 2^32 is generally not a multiple of nv->ngroups, the overflow causes
the modulo result to skip some values and duplicate others.

This means the condition (v + spread_offset) % nv->ngroups < extra_grps
might evaluate to true more times than there are extra groups.

If cpus_per_grp is incremented too many times, the loop will consume more
CPUs than available, prematurely exhausting nmsk and leaving subsequent
masks empty.

Would it be safer to apply the modulo before the addition, such as:
(v + (spread_offset % nv->ngroups)) % nv->ngroups ?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1