[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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.

> Of course putting it in next to the array indexing would address that
> too.

See my earlier reply to Roger's similar remark (which still was
along the lines of what I've said above).

Jan



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.