On 27.09.2022 17:53, Roger Pau Monné wrote:
> On Tue, Sep 27, 2022 at 04:15:19PM +0200, Jan Beulich wrote:
>> SRAT may describe even a single node system (including such with
>> multiple nodes, but only one having any memory) using multiple ranges.
>> Hence simply counting the number of ranges (note that function
>> parameters are mis-named) is not an indication of the number of nodes in
>> use. Since we only care about knowing whether we're on a single node
>> system, accounting for this is easy: Increment the local variable only
>> when adjacent ranges are for different nodes. That way the count may
>> still end up larger than the number of nodes in use, but it won't be
>> larger than 1 when only a single node has any memory.
>>
>> To compensate populate_memnodemap() now needs to be prepared to find
>> the correct node ID already in place for a range. (This could of course
>> also happen when there's more than one node with memory, while at least
>> one node has multiple adjacent ranges, provided extract_lsb_from_nodes()
>> would also know to recognize this case.)
>>
>> Signed-off-by: Jan Beulich <[email protected]>
>
> Acked-by: Roger Pau Monné <[email protected]>
Thanks.
>> --- a/xen/arch/x86/numa.c
>> +++ b/xen/arch/x86/numa.c
>> @@ -78,7 +78,8 @@ static int __init populate_memnodemap(co
>> if ( (epdx >> shift) >= memnodemapsize )
>> return 0;
>> do {
>> - if ( memnodemap[spdx >> shift] != NUMA_NO_NODE )
>> + if ( memnodemap[spdx >> shift] != NUMA_NO_NODE &&
>> + (!nodeids || memnodemap[spdx >> shift] != nodeids[i]) )
>> return -1;
>>
>> if ( !nodeids )
>> @@ -114,7 +115,7 @@ static int __init allocate_cachealigned_
>> * maximum possible shift.
>> */
>> static int __init extract_lsb_from_nodes(const struct node *nodes,
>> - int numnodes)
>> + int numnodes, const nodeid_t
>> *nodeids)
>> {
>> int i, nodes_used = 0;
>> unsigned long spdx, epdx;
>> @@ -127,7 +128,7 @@ static int __init extract_lsb_from_nodes
>> if ( spdx >= epdx )
>> continue;
>> bitfield |= spdx;
>> - nodes_used++;
>> + nodes_used += i == 0 || !nodeids || nodeids[i - 1] != nodeids[i];
>
> I think I would also prefer the `if ( ... ) nodes_used++;` form, as
> it's clearer.
Okay, will switch then. This isn't for 4.17 anyway (I think), so
there's no rush.
Jan
