From: Naman Jain <[email protected]> Sent: Tuesday, September 1, 2026
10:03 PM
>
> On 9/1/2026 5:32 AM, Michael Kelley wrote:
> > From: Naman Jain <[email protected]> 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/[email protected]/
>
> >> + (v + spread_offset) % nv->ngroups < extra_grps)
> >> cpus_per_grp++;
> >> - --extra_grps;
> >> - }
