| Message ID | 20220708145424.1848572-10-wei.chen@arm.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Device tree based NUMA support for Arm - Part#2 | expand |
On 08.07.2022 16:54, 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 255 > + default "64" > + depends on NUMA Does 1 make sense? That's not going to be NUMA then, I would say. Does the value being (perhaps far) larger than NR_CPUS make sense? Why does the range end at a not-power-of-2 value? (I was actually going to suggest to have a shift value specified here, until spotting that NODES_SHIFT isn't used anywhere else, and hence you're rightfully deleting it.) > + 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 that the scheduler, memory allocation and > + other NUMA-aware components can handle. Nit: indentation. Jan
Hi Jan, > -----Original Message----- > From: Jan Beulich <jbeulich@suse.com> > Sent: 2022年7月12日 22:34 > To: Wei Chen <Wei.Chen@arm.com> > Cc: nd <nd@arm.com>; Andrew Cooper <andrew.cooper3@citrix.com>; George > Dunlap <george.dunlap@citrix.com>; Julien Grall <julien@xen.org>; Stefano > Stabellini <sstabellini@kernel.org>; Wei Liu <wl@xen.org>; Roger Pau Monné > <roger.pau@citrix.com>; xen-devel@lists.xenproject.org > Subject: Re: [PATCH v2 9/9] xen: introduce a Kconfig option to configure > NUMA nodes number > > On 08.07.2022 16:54, 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 255 > > + default "64" > > + depends on NUMA > > Does 1 make sense? That's not going to be NUMA then, I would say. > Ok, we need at least 2 nodes to be a real NUMA. > Does the value being (perhaps far) larger than NR_CPUS make sense? > Arm has 128 default NR_CPUS (except some platforms) and x86 has 256. So I am not very clear about your comments about far larger? As my Understanding, one node has 2 or 4 cores are very common in a NUMA System. > Why does the range end at a not-power-of-2 value? (I was actually > going to suggest to have a shift value specified here, until > spotting that NODES_SHIFT isn't used anywhere else, and hence > you're rightfully deleting it.) > I think we have discussed about the 255 in v1. Because Xen is using u8 as nodeid_t, so 255 may be a upper bound. And if use a shift value, from a user perspective, I don't like it. It needs to be converted, not intuitive enough. It also limits my input range, even though my numerical values are reasonable. Yes, if a machine has 15 node, we can ask them to input 16, but why not let the users decide? instead of being forced to enter 16 by the program? > > + 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 that the scheduler, memory allocation and > > + other NUMA-aware components can handle. > > Nit: indentation. > Ok. Cheers, Wei Chen > Jan
On 14.07.2022 12:14, Wei Chen wrote: > Hi Jan, > >> -----Original Message----- >> From: Jan Beulich <jbeulich@suse.com> >> Sent: 2022年7月12日 22:34 >> To: Wei Chen <Wei.Chen@arm.com> >> Cc: nd <nd@arm.com>; Andrew Cooper <andrew.cooper3@citrix.com>; George >> Dunlap <george.dunlap@citrix.com>; Julien Grall <julien@xen.org>; Stefano >> Stabellini <sstabellini@kernel.org>; Wei Liu <wl@xen.org>; Roger Pau Monné >> <roger.pau@citrix.com>; xen-devel@lists.xenproject.org >> Subject: Re: [PATCH v2 9/9] xen: introduce a Kconfig option to configure >> NUMA nodes number >> >> On 08.07.2022 16:54, 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 255 >>> + default "64" >>> + depends on NUMA >> >> Does 1 make sense? That's not going to be NUMA then, I would say. >> > > Ok, we need at least 2 nodes to be a real NUMA. > >> Does the value being (perhaps far) larger than NR_CPUS make sense? >> > > Arm has 128 default NR_CPUS (except some platforms) and x86 has 256. > So I am not very clear about your comments about far larger? As my > Understanding, one node has 2 or 4 cores are very common in a NUMA > System. The defaults are fine. But does it make sense to have 255 nodes when just 32 CPUs were selected? I'm afraid kconfig is going to get in the way, but I think I'd like the upper bound to be min(NR_CPUS, 255). >> Why does the range end at a not-power-of-2 value? (I was actually >> going to suggest to have a shift value specified here, until >> spotting that NODES_SHIFT isn't used anywhere else, and hence >> you're rightfully deleting it.) >> > > I think we have discussed about the 255 in v1. Because Xen is using > u8 as nodeid_t, so 255 may be a upper bound. Indeed, but that's something you could have mentioned in the commit message as justification. But you could also have opted to make the upper bound 128. Let me ask you: Are you aware of systems with more than a dozen or so nodes, that Xen can in principle run on? > And if use a shift value, from a user perspective, I don't like it. > It needs to be converted, not intuitive enough. It also limits my > input range, even though my numerical values are reasonable. Yes, > if a machine has 15 node, we can ask them to input 16, but why not > let the users decide? instead of being forced to enter 16 by the program? At least x86 Linux also wants this specified as a shift value, so people may actually be (more) used to that. Plus non-pwer-of-2 values may yield more complex calculations when the compiler generates code. Think of a two-dimensional distance table, for example. Jan
Hi Jan, On 14/07/2022 12:10, Jan Beulich wrote: > On 14.07.2022 12:14, Wei Chen wrote: >> Hi Jan, >> >>> -----Original Message----- >>> From: Jan Beulich <jbeulich@suse.com> >>> Sent: 2022年7月12日 22:34 >>> To: Wei Chen <Wei.Chen@arm.com> >>> Cc: nd <nd@arm.com>; Andrew Cooper <andrew.cooper3@citrix.com>; George >>> Dunlap <george.dunlap@citrix.com>; Julien Grall <julien@xen.org>; Stefano >>> Stabellini <sstabellini@kernel.org>; Wei Liu <wl@xen.org>; Roger Pau Monné >>> <roger.pau@citrix.com>; xen-devel@lists.xenproject.org >>> Subject: Re: [PATCH v2 9/9] xen: introduce a Kconfig option to configure >>> NUMA nodes number >>> >>> On 08.07.2022 16:54, 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 255 >>>> + default "64" >>>> + depends on NUMA >>> >>> Does 1 make sense? That's not going to be NUMA then, I would say. >>> >> >> Ok, we need at least 2 nodes to be a real NUMA. >> >>> Does the value being (perhaps far) larger than NR_CPUS make sense? >>> >> >> Arm has 128 default NR_CPUS (except some platforms) and x86 has 256. >> So I am not very clear about your comments about far larger? As my >> Understanding, one node has 2 or 4 cores are very common in a NUMA >> System. > > The defaults are fine. But does it make sense to have 255 nodes when > just 32 CPUs were selected? I'm afraid kconfig is going to get in the > way, but I think I'd like the upper bound to be min(NR_CPUS, 255). Looking around NUMA nodes with 0 CPUs exists. So I don't think we should tie the two. Cheers,
Hi Julien, Jan, > -----Original Message----- > From: Julien Grall <julien@xen.org> > Sent: 2022年7月14日 19:27 > To: Jan Beulich <jbeulich@suse.com>; Wei Chen <Wei.Chen@arm.com> > Cc: nd <nd@arm.com>; Andrew Cooper <andrew.cooper3@citrix.com>; George > Dunlap <george.dunlap@citrix.com>; Stefano Stabellini > <sstabellini@kernel.org>; Wei Liu <wl@xen.org>; Roger Pau Monné > <roger.pau@citrix.com>; xen-devel@lists.xenproject.org > Subject: Re: [PATCH v2 9/9] xen: introduce a Kconfig option to configure > NUMA nodes number > > Hi Jan, > > On 14/07/2022 12:10, Jan Beulich wrote: > > On 14.07.2022 12:14, Wei Chen wrote: > >> Hi Jan, > >> > >>> -----Original Message----- > >>> From: Jan Beulich <jbeulich@suse.com> > >>> Sent: 2022年7月12日 22:34 > >>> To: Wei Chen <Wei.Chen@arm.com> > >>> Cc: nd <nd@arm.com>; Andrew Cooper <andrew.cooper3@citrix.com>; George > >>> Dunlap <george.dunlap@citrix.com>; Julien Grall <julien@xen.org>; > Stefano > >>> Stabellini <sstabellini@kernel.org>; Wei Liu <wl@xen.org>; Roger Pau > Monné > >>> <roger.pau@citrix.com>; xen-devel@lists.xenproject.org > >>> Subject: Re: [PATCH v2 9/9] xen: introduce a Kconfig option to > configure > >>> NUMA nodes number > >>> > >>> On 08.07.2022 16:54, 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 255 > >>>> + default "64" > >>>> + depends on NUMA > >>> > >>> Does 1 make sense? That's not going to be NUMA then, I would say. > >>> > >> > >> Ok, we need at least 2 nodes to be a real NUMA. > >> > >>> Does the value being (perhaps far) larger than NR_CPUS make sense? > >>> > >> > >> Arm has 128 default NR_CPUS (except some platforms) and x86 has 256. > >> So I am not very clear about your comments about far larger? As my > >> Understanding, one node has 2 or 4 cores are very common in a NUMA > >> System. > > > > The defaults are fine. But does it make sense to have 255 nodes when > > just 32 CPUs were selected? I'm afraid kconfig is going to get in the > > way, but I think I'd like the upper bound to be min(NR_CPUS, 255). > > Looking around NUMA nodes with 0 CPUs exists. So I don't think we should > tie the two. > Yes, some nodes can only have RAM and some nodes can only have CPUs. So is it ok to use 2-255 for node range? > Cheers, > > -- > Julien Grall
On 18.07.2022 09:51, Wei Chen wrote: >> From: Julien Grall <julien@xen.org> >> Sent: 2022年7月14日 19:27 >> >> On 14/07/2022 12:10, Jan Beulich wrote: >>> On 14.07.2022 12:14, Wei Chen wrote: >>>>> From: Jan Beulich <jbeulich@suse.com> >>>>> Sent: 2022年7月12日 22:34 >>>>> >>>>> On 08.07.2022 16:54, 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 255 >>>>>> + default "64" >>>>>> + depends on NUMA >>>>> >>>>> Does 1 make sense? That's not going to be NUMA then, I would say. >>>>> >>>> >>>> Ok, we need at least 2 nodes to be a real NUMA. >>>> >>>>> Does the value being (perhaps far) larger than NR_CPUS make sense? >>>>> >>>> >>>> Arm has 128 default NR_CPUS (except some platforms) and x86 has 256. >>>> So I am not very clear about your comments about far larger? As my >>>> Understanding, one node has 2 or 4 cores are very common in a NUMA >>>> System. >>> >>> The defaults are fine. But does it make sense to have 255 nodes when >>> just 32 CPUs were selected? I'm afraid kconfig is going to get in the >>> way, but I think I'd like the upper bound to be min(NR_CPUS, 255). >> >> Looking around NUMA nodes with 0 CPUs exists. So I don't think we should >> tie the two. >> > > Yes, some nodes can only have RAM and some nodes can only have CPUs. > So is it ok to use 2-255 for node range? Personally I think we shouldn't allow unreasonably high node counts, unless proven by real hardware existing. Which goes hand in hand with me wanting the upper bound to be a power of 2 value, if for nothing else than a hint that using power-of-2 values here is preferable. Hence I'd like to propose 64 or 128 as upper bound, in this order of (my personal) preference. Jan
Hi Jan, > -----Original Message----- > From: Jan Beulich <jbeulich@suse.com> > Sent: 2022年7月18日 16:10 > To: Wei Chen <Wei.Chen@arm.com> > Cc: nd <nd@arm.com>; Andrew Cooper <andrew.cooper3@citrix.com>; George > Dunlap <george.dunlap@citrix.com>; Stefano Stabellini > <sstabellini@kernel.org>; Wei Liu <wl@xen.org>; Roger Pau Monné > <roger.pau@citrix.com>; xen-devel@lists.xenproject.org; Julien Grall > <julien@xen.org> > Subject: Re: [PATCH v2 9/9] xen: introduce a Kconfig option to configure > NUMA nodes number > > >>>>> Sent: 2022年7月12日 22:34 > >>>>> > >>>>> On 08.07.2022 16:54, 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 255 > >>>>>> + default "64" > >>>>>> + depends on NUMA > >>>>> > >>>>> Does 1 make sense? That's not going to be NUMA then, I would say. > >>>>> > >>>> > >>>> Ok, we need at least 2 nodes to be a real NUMA. > >>>> > >>>>> Does the value being (perhaps far) larger than NR_CPUS make sense? > >>>>> > >>>> > >>>> Arm has 128 default NR_CPUS (except some platforms) and x86 has 256. > >>>> So I am not very clear about your comments about far larger? As my > >>>> Understanding, one node has 2 or 4 cores are very common in a NUMA > >>>> System. > >>> > >>> The defaults are fine. But does it make sense to have 255 nodes when > >>> just 32 CPUs were selected? I'm afraid kconfig is going to get in the > >>> way, but I think I'd like the upper bound to be min(NR_CPUS, 255). > >> > >> Looking around NUMA nodes with 0 CPUs exists. So I don't think we > should > >> tie the two. > >> > > > > Yes, some nodes can only have RAM and some nodes can only have CPUs. > > So is it ok to use 2-255 for node range? > > Personally I think we shouldn't allow unreasonably high node counts, > unless proven by real hardware existing. Which goes hand in hand with > me wanting the upper bound to be a power of 2 value, if for nothing > else than a hint that using power-of-2 values here is preferable. > Hence I'd like to propose 64 or 128 as upper bound, in this order of > (my personal) preference. > Thanks, I will use 64 in next version. Wei Chen > Jan
diff --git a/xen/arch/Kconfig b/xen/arch/Kconfig index f16eb0df43..4fc16f83ac 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 255 + 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 that the scheduler, memory allocation and + other NUMA-aware components can handle. diff --git a/xen/arch/x86/include/asm/numa.h b/xen/arch/x86/include/asm/numa.h index eeb431cdb7..db76281c08 100644 --- a/xen/arch/x86/include/asm/numa.h +++ b/xen/arch/x86/include/asm/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 4c4632ec27..cac92d2688 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. As NODES_SHIFT is currently unused, we're taking this opportunity to remove it. Signed-off-by: Wei Chen <wei.chen@arm.com> --- v1 -> v2: 1. Add NODES_SHIFT remove message in commit log 2. Change NR_NUMA_NODES upper bound from 4095 to 255. --- xen/arch/Kconfig | 11 +++++++++++ xen/arch/x86/include/asm/numa.h | 2 -- xen/include/xen/numa.h | 10 +++++----- 3 files changed, 16 insertions(+), 7 deletions(-)