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

[Michael Kelley]

From: Naman Jain <namjain@xxxxxxxxxxxxxxxxxxx> Sent: Tuesday, September 1, 2026 10:03 PM
> 
> On 9/1/2026 5:32 AM, Michael Kelley wrote:
> > From: Naman Jain <namjain@xxxxxxxxxxxxxxxxxxx> Sent: Sunday, August 9, 2026 11:22 PM

[snip]

> >> @@ -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 &&
> >
> > Explicitly testing extra_grps for zero isn't necessary. If it is
> > zero, the "less than" test below will always be false anyway.
> >
> 
> That is true, but the intention was to improve readability of this
> block. Unless you really feel that this should be removed, I would
> prefer to retain this.
> 

Fair enough. It's your call unless Thomas Gleixner as the maintainer
says otherwise. I flag these cases because I spend time looking for
some subtle reason why the test is actually needed. For example,
if the left side of the "less than" test below has side-effects, then
checking extra_gaps for zero prevents the side-effects from occurring.
To me the redundant test introduces ambiguity -- is there a reason
it is needed, or is it superfluous?

Interestingly, I had an identical conversation in another patch
review [1] three days ago.

Michael

[1] https://lore.kernel.org/linux-hyperv/8d38f7c0-f15d-4568-bc2f-559179ccb45c@xxxxxxxxxx/

> 
> >> +		    (v + spread_offset) % nv->ngroups < extra_grps)
> >>   			cpus_per_grp++;
> >> -			--extra_grps;
> >> -		}