Re: [PATCH 2/2][4.15?] x86: fix build when NR_CPUS == 1

[Ian Jackson <iwj@xxxxxxxxxxxxxx>]

Jan Beulich writes ("Re: [PATCH 2/2][4.15?] x86: fix build when NR_CPUS == 1"):
> On 01.03.2021 17:03, Ian Jackson wrote:
> > Jan Beulich writes ("[PATCH 2/2][4.15?] x86: fix build when NR_CPUS == 1"):
> >> In this case the compiler is recognizing that no valid array indexes
> >> remain (in x2apic_cluster()'s access to per_cpu(cpu_2_logical_apicid,
> >> ...)), but oddly enough isn't really consistent about the checking it
> >> does (see the code comment).
> > ...
> >> -        if (this_cpu == cpu || x2apic_cluster(this_cpu) != 
> >> x2apic_cluster(cpu))
> >> +        if ( this_cpu == cpu )
> >> +            continue;
> >> +        /*
> >> +         * Guard in particular against the compiler suspecting 
> >> out-of-bounds
> >> +         * array accesses below when NR_CPUS=1 (oddly enough with gcc 10 
> >> it
> >> +         * is the 1st of these alone which actually helps, not the 2nd, 
> >> nor
> >> +         * are both required together there).
> >> +         */
> >> +        BUG_ON(this_cpu >= NR_CPUS);
> >> +        BUG_ON(cpu >= NR_CPUS);
> >> +        if ( x2apic_cluster(this_cpu) != x2apic_cluster(cpu) )
> >>              continue;
> > 
> > Is there some particular reason for not putting the BUG_ON before the
> > if test ?  That would avoid the refactoring.
> 
> I want it to be as close as possible to the place where the issue
> is. I also view the refactoring as a good thing, since it allows
> a style correction as a side effect.

I'm afraid that at this stage of the release I would prefer changes to
be as small as reasonably sensible.  So unless there is some
reason, other than taste, style or formatting, could we please just
introduce the new BUG_ON and not also do other refactoring.

Ian.