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]> > --- > On my Skylake system this changes memnodemapsize from 17 to 1 (and the > shift from 20 to 63). > > --- 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. Thanks, Roger.
