| Message ID | 20210923120236.3692135-3-wei.chen@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add device tree based NUMA support to Arm | expand |
On Thu, 23 Sep 2021, 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. This is OK but I think you should also mention in the commit message that you are taking the opportunity to remove NODES_SHIFT because it is currently unused. With that: Reviewed-by: Stefano Stabellini <sstabellini@kernel.org> > Signed-off-by: Wei Chen <wei.chen@arm.com> > --- > xen/arch/Kconfig | 11 +++++++++++ > xen/include/asm-x86/numa.h | 2 -- > xen/include/xen/numa.h | 10 +++++----- > 3 files changed, 16 insertions(+), 7 deletions(-) > > diff --git a/xen/arch/Kconfig b/xen/arch/Kconfig > index f16eb0df43..8a20da67ed 100644 > --- 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 > + default "64" > + depends on NUMA > + help > + Controls the build-time size of various arrays and bitmaps > + associated with multiple-nodes management. It is the upper bound of > + the number of NUMA nodes the scheduler, memory allocation and other > + NUMA-aware components can handle. > diff --git a/xen/include/asm-x86/numa.h b/xen/include/asm-x86/numa.h > index bada2c0bb9..3cf26c2def 100644 > --- 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; > > extern int srat_rev; > diff --git a/xen/include/xen/numa.h b/xen/include/xen/numa.h > index 7aef1a88dc..52950a3150 100644 > --- a/xen/include/xen/numa.h > +++ b/xen/include/xen/numa.h > @@ -3,14 +3,14 @@ > > #include <asm/numa.h> > > -#ifndef NODES_SHIFT > -#define NODES_SHIFT 0 > -#endif > - > #define NUMA_NO_NODE 0xFF > #define NUMA_NO_DISTANCE 0xFF > > -#define MAX_NUMNODES (1 << NODES_SHIFT) > +#ifdef CONFIG_NR_NUMA_NODES > +#define MAX_NUMNODES CONFIG_NR_NUMA_NODES > +#else > +#define MAX_NUMNODES 1 > +#endif > > #define vcpu_to_node(v) (cpu_to_node((v)->processor)) > > -- > 2.25.1 >
Hi Stefano, > -----Original Message----- > From: Stefano Stabellini <sstabellini@kernel.org> > Sent: 2021年9月24日 7:45 > To: Wei Chen <Wei.Chen@arm.com> > Cc: xen-devel@lists.xenproject.org; sstabellini@kernel.org; julien@xen.org; > Bertrand Marquis <Bertrand.Marquis@arm.com> > Subject: Re: [PATCH 02/37] xen: introduce a Kconfig option to configure > NUMA nodes number > > On Thu, 23 Sep 2021, 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. > > This is OK but I think you should also mention in the commit message > that you are taking the opportunity to remove NODES_SHIFT because it is > currently unused. > > With that: > > Reviewed-by: Stefano Stabellini <sstabellini@kernel.org> > > Thanks, I will update it in next version. > > Signed-off-by: Wei Chen <wei.chen@arm.com> > > --- > > xen/arch/Kconfig | 11 +++++++++++ > > xen/include/asm-x86/numa.h | 2 -- > > xen/include/xen/numa.h | 10 +++++----- > > 3 files changed, 16 insertions(+), 7 deletions(-) > > > > diff --git a/xen/arch/Kconfig b/xen/arch/Kconfig > > index f16eb0df43..8a20da67ed 100644 > > --- 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 > > + default "64" > > + depends on NUMA > > + help > > + Controls the build-time size of various arrays and bitmaps > > + associated with multiple-nodes management. It is the upper bound > of > > + the number of NUMA nodes the scheduler, memory allocation and > other > > + NUMA-aware components can handle. > > diff --git a/xen/include/asm-x86/numa.h b/xen/include/asm-x86/numa.h > > index bada2c0bb9..3cf26c2def 100644 > > --- 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; > > > > extern int srat_rev; > > diff --git a/xen/include/xen/numa.h b/xen/include/xen/numa.h > > index 7aef1a88dc..52950a3150 100644 > > --- a/xen/include/xen/numa.h > > +++ b/xen/include/xen/numa.h > > @@ -3,14 +3,14 @@ > > > > #include <asm/numa.h> > > > > -#ifndef NODES_SHIFT > > -#define NODES_SHIFT 0 > > -#endif > > - > > #define NUMA_NO_NODE 0xFF > > #define NUMA_NO_DISTANCE 0xFF > > > > -#define MAX_NUMNODES (1 << NODES_SHIFT) > > +#ifdef CONFIG_NR_NUMA_NODES > > +#define MAX_NUMNODES CONFIG_NR_NUMA_NODES > > +#else > > +#define MAX_NUMNODES 1 > > +#endif > > > > #define vcpu_to_node(v) (cpu_to_node((v)->processor)) > > > > -- > > 2.25.1 > >
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. Jan
Hi Jan, > -----Original Message----- > From: Jan Beulich <jbeulich@suse.com> > Sent: 2021年9月24日 16:56 > To: Wei Chen <Wei.Chen@arm.com> > Cc: Bertrand Marquis <Bertrand.Marquis@arm.com>; xen- > devel@lists.xenproject.org; sstabellini@kernel.org; julien@xen.org > 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
On 24.09.2021 12:33, Wei Chen wrote: >> From: Jan Beulich <jbeulich@suse.com> >> Sent: 2021年9月24日 16:56 >> >> On 23.09.2021 14:02, Wei Chen wrote: >>> --- 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? I think it is, yes, but you will want to properly check. Jan
diff --git a/xen/arch/Kconfig b/xen/arch/Kconfig index f16eb0df43..8a20da67ed 100644 --- 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 + default "64" + depends on NUMA + help + Controls the build-time size of various arrays and bitmaps + associated with multiple-nodes management. It is the upper bound of + the number of NUMA nodes the scheduler, memory allocation and other + NUMA-aware components can handle. diff --git a/xen/include/asm-x86/numa.h b/xen/include/asm-x86/numa.h index bada2c0bb9..3cf26c2def 100644 --- 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; extern int srat_rev; diff --git a/xen/include/xen/numa.h b/xen/include/xen/numa.h index 7aef1a88dc..52950a3150 100644 --- a/xen/include/xen/numa.h +++ b/xen/include/xen/numa.h @@ -3,14 +3,14 @@ #include <asm/numa.h> -#ifndef NODES_SHIFT -#define NODES_SHIFT 0 -#endif - #define NUMA_NO_NODE 0xFF #define NUMA_NO_DISTANCE 0xFF -#define MAX_NUMNODES (1 << NODES_SHIFT) +#ifdef CONFIG_NR_NUMA_NODES +#define MAX_NUMNODES CONFIG_NR_NUMA_NODES +#else +#define MAX_NUMNODES 1 +#endif #define vcpu_to_node(v) (cpu_to_node((v)->processor))
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. Signed-off-by: Wei Chen <wei.chen@arm.com> --- xen/arch/Kconfig | 11 +++++++++++ xen/include/asm-x86/numa.h | 2 -- xen/include/xen/numa.h | 10 +++++----- 3 files changed, 16 insertions(+), 7 deletions(-)