On 26.09.2023 09:32, Jan Beulich wrote:
> On 26.09.2023 00:42, Shawn Anastasio wrote:
>> When building for Power with CONFIG_DEBUG unset,
Hmm, depending on what gcc versions are used in CI, the above may be the
reason why ...
>> a compiler error gets
>> raised inside page_alloc.c's node_to_scrub function, likely due to the
>> increased optimization level:
>>
>> common/page_alloc.c: In function 'node_to_scrub.part.0':
>> common/page_alloc.c:1217:29: error: array subscript 1 is above array
>> bounds of 'long unsigned int[1]' [-Werror=array-bounds]
>> 1217 | if ( node_need_scrub[node] )
>
> That's gcc13?
>
>> It appears that this is a false positive, given that in practice
>> cycle_node should never return a node ID >= MAX_NUMNODES as long as the
>> architecture's node_online_map is properly defined and initialized, so
>> this additional bounds check is only to satisfy GCC.
>
> Looks very similar to the situation that c890499871cc ("timer: fix
> NR_CPUS=1 build with gcc13") was dealing with, just that here it's
> MAX_NUMNODES. I'd therefore prefer a solution similar to the one
> taken there, i.e. make code conditional rather than add yet more
> code.
>
> Otherwise, ...
>
>> --- a/xen/common/page_alloc.c
>> +++ b/xen/common/page_alloc.c
>> @@ -1211,6 +1211,9 @@ static unsigned int node_to_scrub(bool get_node)
>> } while ( !cpumask_empty(&node_to_cpumask(node)) &&
>> (node != local_node) );
>>
>> + if ( node >= MAX_NUMNODES )
>> + break;
>
> ... this clearly redundant check would need to gain a comment.
>
> What I'm puzzled by is that on Arm we had no reports of a similar
> problem, despite NUMA also not getting selected there (yet).
... this wasn't observed, yet. As far as I'm concerned, all my Arm builds
are debug ones (which I may need to change).
Jan
