RE: [PATCH 02/37] xen: introduce a Kconfig option to configure NUMA nodes number

[Wei Chen <Wei.Chen@xxxxxxx>]

Hi Jan,

> -----Original Message-----
> From: Jan Beulich <jbeulich@xxxxxxxx>
> Sent: 2021年9月24日 16:56
> To: Wei Chen <Wei.Chen@xxxxxxx>
> Cc: Bertrand Marquis <Bertrand.Marquis@xxxxxxx>; xen-
> devel@xxxxxxxxxxxxxxxxxxxx; sstabellini@xxxxxxxxxx; julien@xxxxxxx
> Subject: Re: [PATCH 02/37] xen: introduce a Kconfig option to configure
> NUMA nodes number
> 
> On 23.09.2021 14:02, Wei Chen wrote:
> > Current NUMA nodes number is a hardcode configuration. This
> > configuration is difficult for an administrator to change
> > unless changing the code.
> >
> > So in this patch, we introduce this new Kconfig option for
> > administrators to change NUMA nodes number conveniently.
> > Also considering that not all architectures support NUMA,
> > this Kconfig option only can be visible on NUMA enabled
> > architectures. Non-NUMA supported architectures can still
> > use 1 as MAX_NUMNODES.
> 
> Do you really mean administrators here? To me command line options
> are for administrators, but build decisions are usually taken by
> build managers of distros.
> 
> > --- a/xen/arch/Kconfig
> > +++ b/xen/arch/Kconfig
> > @@ -17,3 +17,14 @@ config NR_CPUS
> >       For CPU cores which support Simultaneous Multi-Threading or
> similar
> >       technologies, this the number of logical threads which Xen will
> >       support.
> > +
> > +config NR_NUMA_NODES
> > +   int "Maximum number of NUMA nodes supported"
> > +   range 1 4095
> 
> How was this upper bound established? Seeing 4095 is the limit of the
> number of CPUs, do we really expect a CPU per node on such huge
> systems? And did you check that whichever involved data types and
> structures are actually suitable? I'm thinking e.g. of things like ...
> 
> > --- a/xen/include/asm-x86/numa.h
> > +++ b/xen/include/asm-x86/numa.h
> > @@ -3,8 +3,6 @@
> >
> >  #include <xen/cpumask.h>
> >
> > -#define NODES_SHIFT 6
> > -
> >  typedef u8 nodeid_t;
> 
> ... this.
> 

you're right, we use u8 as nodeid_t. 4095 for node number in this option
is not reasonable. Maybe a 255 upper bound is good?

> Jan