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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 02.03.2021 13:28, Ian Jackson wrote:
> 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.

FAOD: That's fine - I'll keep this queued for 4.16 then. I did put
a question mark behind the version anyway.

Jan