| Message ID | 20210811102423.28908-8-wei.chen@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add device tree based NUMA support to Arm64 | expand |
Hi Wei, On 11/08/2021 11:23, Wei Chen wrote: > Only Arm64 supports NUMA, the CONFIG_NUMA could not be > enabled for Arm32. What do you mean by "could not be enabled"? > Even in Arm64, users still can disable > the CONFIG_NUMA through Kconfig option. In this case, keep > current fake NUMA API, will make Arm code still can work > with NUMA aware memory allocation and scheduler. > > Signed-off-by: Wei Chen <wei.chen@arm.com> > --- > xen/include/asm-arm/numa.h | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/xen/include/asm-arm/numa.h b/xen/include/asm-arm/numa.h > index 31a6de4e23..ab9c4a2448 100644 > --- a/xen/include/asm-arm/numa.h > +++ b/xen/include/asm-arm/numa.h > @@ -5,6 +5,8 @@ > > typedef u8 nodeid_t; > > +#if !defined(CONFIG_NUMA) NIT: We tend to use #ifndef rather than #if !defined(...) > + > /* Fake one node for now. See also node_online_map. */ > #define cpu_to_node(cpu) 0 > #define node_to_cpumask(node) (cpu_online_map) > @@ -25,6 +27,8 @@ extern mfn_t first_valid_mfn; > #define node_start_pfn(nid) (mfn_x(first_valid_mfn)) > #define __node_distance(a, b) (20) > > +#endif > + > #endif /* __ARCH_ARM_NUMA_H */ > /* > * Local variables: > Cheers,
Hi Julien, > -----Original Message----- > From: Julien Grall <julien@xen.org> > Sent: 2021年8月19日 21:34 > To: Wei Chen <Wei.Chen@arm.com>; xen-devel@lists.xenproject.org; > sstabellini@kernel.org; jbeulich@suse.com > Cc: Bertrand Marquis <Bertrand.Marquis@arm.com> > Subject: Re: [XEN RFC PATCH 07/40] xen/arm: use !CONFIG_NUMA to keep fake > NUMA API > > Hi Wei, > > On 11/08/2021 11:23, Wei Chen wrote: > > Only Arm64 supports NUMA, the CONFIG_NUMA could not be > > enabled for Arm32. > > What do you mean by "could not be enabled"? I have not seen any Arm32 hardware support NUMA, so I think we don't need to support Arm32 NUMA. In this case, this Kconfig option could not be enabled on Arm32. > > > Even in Arm64, users still can disable > > the CONFIG_NUMA through Kconfig option. In this case, keep > > current fake NUMA API, will make Arm code still can work > > with NUMA aware memory allocation and scheduler. > > > > Signed-off-by: Wei Chen <wei.chen@arm.com> > > --- > > xen/include/asm-arm/numa.h | 4 ++++ > > 1 file changed, 4 insertions(+) > > > > diff --git a/xen/include/asm-arm/numa.h b/xen/include/asm-arm/numa.h > > index 31a6de4e23..ab9c4a2448 100644 > > --- a/xen/include/asm-arm/numa.h > > +++ b/xen/include/asm-arm/numa.h > > @@ -5,6 +5,8 @@ > > > > typedef u8 nodeid_t; > > > > +#if !defined(CONFIG_NUMA) > > NIT: We tend to use #ifndef rather than #if !defined(...) > OK, I will change related changes in this series. > > + > > /* Fake one node for now. See also node_online_map. */ > > #define cpu_to_node(cpu) 0 > > #define node_to_cpumask(node) (cpu_online_map) > > @@ -25,6 +27,8 @@ extern mfn_t first_valid_mfn; > > #define node_start_pfn(nid) (mfn_x(first_valid_mfn)) > > #define __node_distance(a, b) (20) > > > > +#endif > > + > > #endif /* __ARCH_ARM_NUMA_H */ > > /* > > * Local variables: > > > > Cheers, > > -- > Julien Grall
On 20/08/2021 03:08, Wei Chen wrote: > Hi Julien, Hi Wei, > >> -----Original Message----- >> From: Julien Grall <julien@xen.org> >> Sent: 2021年8月19日 21:34 >> To: Wei Chen <Wei.Chen@arm.com>; xen-devel@lists.xenproject.org; >> sstabellini@kernel.org; jbeulich@suse.com >> Cc: Bertrand Marquis <Bertrand.Marquis@arm.com> >> Subject: Re: [XEN RFC PATCH 07/40] xen/arm: use !CONFIG_NUMA to keep fake >> NUMA API >> >> Hi Wei, >> >> On 11/08/2021 11:23, Wei Chen wrote: >>> Only Arm64 supports NUMA, the CONFIG_NUMA could not be >>> enabled for Arm32. >> >> What do you mean by "could not be enabled"? > > I have not seen any Arm32 hardware support NUMA, so I think > we don't need to support Arm32 NUMA. I understand that there may not be 32-bit platform with NUMA. And that's fine stating that in the commit message. However... > In this case, this Kconfig > option could not be enabled on Arm32. ... you continue to say "couldn't be enabled" without clarifying whether this mean that the build will break or this was just not tested because you don't have any platform. To put it differently, the code for NUMA looks bitness neutral. So I cannot really what what prevent us to potentially use it on Arm 32-bit. > >> >>> Even in Arm64, users still can disable >>> the CONFIG_NUMA through Kconfig option. In this case, keep >>> current fake NUMA API, will make Arm code still can work >>> with NUMA aware memory allocation and scheduler. >>> >>> Signed-off-by: Wei Chen <wei.chen@arm.com> >>> --- >>> xen/include/asm-arm/numa.h | 4 ++++ >>> 1 file changed, 4 insertions(+) >>> >>> diff --git a/xen/include/asm-arm/numa.h b/xen/include/asm-arm/numa.h >>> index 31a6de4e23..ab9c4a2448 100644 >>> --- a/xen/include/asm-arm/numa.h >>> +++ b/xen/include/asm-arm/numa.h >>> @@ -5,6 +5,8 @@ >>> >>> typedef u8 nodeid_t; >>> >>> +#if !defined(CONFIG_NUMA) >> >> NIT: We tend to use #ifndef rather than #if !defined(...) >> > > OK, I will change related changes in this series. > >>> + >>> /* Fake one node for now. See also node_online_map. */ >>> #define cpu_to_node(cpu) 0 >>> #define node_to_cpumask(node) (cpu_online_map) >>> @@ -25,6 +27,8 @@ extern mfn_t first_valid_mfn; >>> #define node_start_pfn(nid) (mfn_x(first_valid_mfn)) >>> #define __node_distance(a, b) (20) >>> >>> +#endif >>> + >>> #endif /* __ARCH_ARM_NUMA_H */ >>> /* >>> * Local variables: >>> >> >> Cheers, >> >> -- >> Julien Grall Cheers,
Hi Julien, > -----Original Message----- > From: Julien Grall <julien@xen.org> > Sent: 2021年8月20日 16:24 > To: Wei Chen <Wei.Chen@arm.com>; xen-devel@lists.xenproject.org; > sstabellini@kernel.org; jbeulich@suse.com > Cc: Bertrand Marquis <Bertrand.Marquis@arm.com> > Subject: Re: [XEN RFC PATCH 07/40] xen/arm: use !CONFIG_NUMA to keep fake > NUMA API > > > > On 20/08/2021 03:08, Wei Chen wrote: > > Hi Julien, > > Hi Wei, > > > > >> -----Original Message----- > >> From: Julien Grall <julien@xen.org> > >> Sent: 2021年8月19日 21:34 > >> To: Wei Chen <Wei.Chen@arm.com>; xen-devel@lists.xenproject.org; > >> sstabellini@kernel.org; jbeulich@suse.com > >> Cc: Bertrand Marquis <Bertrand.Marquis@arm.com> > >> Subject: Re: [XEN RFC PATCH 07/40] xen/arm: use !CONFIG_NUMA to keep > fake > >> NUMA API > >> > >> Hi Wei, > >> > >> On 11/08/2021 11:23, Wei Chen wrote: > >>> Only Arm64 supports NUMA, the CONFIG_NUMA could not be > >>> enabled for Arm32. > >> > >> What do you mean by "could not be enabled"? > > > > I have not seen any Arm32 hardware support NUMA, so I think > > we don't need to support Arm32 NUMA. > > I understand that there may not be 32-bit platform with NUMA. And that's > fine stating that in the commit message. However... > > > In this case, this Kconfig > > option could not be enabled on Arm32. > > ... you continue to say "couldn't be enabled" without clarifying whether > this mean that the build will break or this was just not tested because > you don't have any platform. Ok, I understand your concern. Yes, my words would lead to mis-understanding. If we make CONFIG_NUMA enabled in Arm32, it need Arm32 to implement some code to support NUMA common code. Otherwise the Arm32 build will failed. I have not tried to implement those code for Arm32. And I found there is no Arm32 machine support NUMA, so I wanted Arm32 to use fake NUMA API as before. > > To put it differently, the code for NUMA looks bitness neutral. So I > cannot really what what prevent us to potentially use it on Arm 32-bit. > Yes, you're right, it's neutral. But do we really need to add code to an ARCH that it may never use? And how can we test this code? Before this patch, I had checked Linux, and found that OF_NUMA only selected by Arm64 not Arm32. But if you feel the need to add to arm32, I have no problem with that. > > > >> > >>> Even in Arm64, users still can disable > >>> the CONFIG_NUMA through Kconfig option. In this case, keep > >>> current fake NUMA API, will make Arm code still can work > >>> with NUMA aware memory allocation and scheduler. > >>> > >>> Signed-off-by: Wei Chen <wei.chen@arm.com> > >>> --- > >>> xen/include/asm-arm/numa.h | 4 ++++ > >>> 1 file changed, 4 insertions(+) > >>> > >>> diff --git a/xen/include/asm-arm/numa.h b/xen/include/asm-arm/numa.h > >>> index 31a6de4e23..ab9c4a2448 100644 > >>> --- a/xen/include/asm-arm/numa.h > >>> +++ b/xen/include/asm-arm/numa.h > >>> @@ -5,6 +5,8 @@ > >>> > >>> typedef u8 nodeid_t; > >>> > >>> +#if !defined(CONFIG_NUMA) > >> > >> NIT: We tend to use #ifndef rather than #if !defined(...) > >> > > > > OK, I will change related changes in this series. > > > >>> + > >>> /* Fake one node for now. See also node_online_map. */ > >>> #define cpu_to_node(cpu) 0 > >>> #define node_to_cpumask(node) (cpu_online_map) > >>> @@ -25,6 +27,8 @@ extern mfn_t first_valid_mfn; > >>> #define node_start_pfn(nid) (mfn_x(first_valid_mfn)) > >>> #define __node_distance(a, b) (20) > >>> > >>> +#endif > >>> + > >>> #endif /* __ARCH_ARM_NUMA_H */ > >>> /* > >>> * Local variables: > >>> > >> > >> Cheers, > >> > >> -- > >> Julien Grall > > Cheers, > > -- > Julien Grall
On 20/08/2021 11:24, Wei Chen wrote: > Hi Julien, Hi Wei, > >> -----Original Message----- >> From: Julien Grall <julien@xen.org> >> Sent: 2021年8月20日 16:24 >> To: Wei Chen <Wei.Chen@arm.com>; xen-devel@lists.xenproject.org; >> sstabellini@kernel.org; jbeulich@suse.com >> Cc: Bertrand Marquis <Bertrand.Marquis@arm.com> >> Subject: Re: [XEN RFC PATCH 07/40] xen/arm: use !CONFIG_NUMA to keep fake >> NUMA API >> >> >> >> On 20/08/2021 03:08, Wei Chen wrote: >>> Hi Julien, >> >> Hi Wei, >> >>> >>>> -----Original Message----- >>>> From: Julien Grall <julien@xen.org> >>>> Sent: 2021年8月19日 21:34 >>>> To: Wei Chen <Wei.Chen@arm.com>; xen-devel@lists.xenproject.org; >>>> sstabellini@kernel.org; jbeulich@suse.com >>>> Cc: Bertrand Marquis <Bertrand.Marquis@arm.com> >>>> Subject: Re: [XEN RFC PATCH 07/40] xen/arm: use !CONFIG_NUMA to keep >> fake >>>> NUMA API >>>> >>>> Hi Wei, >>>> >>>> On 11/08/2021 11:23, Wei Chen wrote: >>>>> Only Arm64 supports NUMA, the CONFIG_NUMA could not be >>>>> enabled for Arm32. >>>> >>>> What do you mean by "could not be enabled"? >>> >>> I have not seen any Arm32 hardware support NUMA, so I think >>> we don't need to support Arm32 NUMA. >> >> I understand that there may not be 32-bit platform with NUMA. And that's >> fine stating that in the commit message. However... >> >>> In this case, this Kconfig >>> option could not be enabled on Arm32. >> >> ... you continue to say "couldn't be enabled" without clarifying whether >> this mean that the build will break or this was just not tested because >> you don't have any platform. > > Ok, I understand your concern. Yes, my words would lead to mis-understanding. > If we make CONFIG_NUMA enabled in Arm32, it need Arm32 to implement some > code to support NUMA common code. Otherwise the Arm32 build will failed. When I skimmed through the series, most of the code seems to be either in common, arm (bitness neutral). So I am not quite too sure why it would not build. Do you have more details? > I have not tried to implement those code for Arm32. And I found there is > no Arm32 machine support NUMA, so I wanted Arm32 to use fake NUMA API > as before. > >> >> To put it differently, the code for NUMA looks bitness neutral. So I >> cannot really what what prevent us to potentially use it on Arm 32-bit. >> > > Yes, you're right, it's neutral. But do we really need to add code to an > ARCH that it may never use? Technically you already added the code because arch/arm/ is common between arm32 and arm64. My only ask is to not make the new config depends on arm64. If you only build test it that fine because... And how can we test this code? I don't expect any of the code to be an issue on arm32 as the code should mostly be arch neutral. Cheers,
Hi Julien, > -----Original Message----- > From: Julien Grall <julien@xen.org> > Sent: 2021年8月20日 19:24 > To: Wei Chen <Wei.Chen@arm.com>; xen-devel@lists.xenproject.org; > sstabellini@kernel.org > Cc: Bertrand Marquis <Bertrand.Marquis@arm.com> > Subject: Re: [XEN RFC PATCH 07/40] xen/arm: use !CONFIG_NUMA to keep fake > NUMA API > > > > On 20/08/2021 11:24, Wei Chen wrote: > > Hi Julien, > > Hi Wei, > > > > >> -----Original Message----- > >> From: Julien Grall <julien@xen.org> > >> Sent: 2021年8月20日 16:24 > >> To: Wei Chen <Wei.Chen@arm.com>; xen-devel@lists.xenproject.org; > >> sstabellini@kernel.org; jbeulich@suse.com > >> Cc: Bertrand Marquis <Bertrand.Marquis@arm.com> > >> Subject: Re: [XEN RFC PATCH 07/40] xen/arm: use !CONFIG_NUMA to keep > fake > >> NUMA API > >> > >> > >> > >> On 20/08/2021 03:08, Wei Chen wrote: > >>> Hi Julien, > >> > >> Hi Wei, > >> > >>> > >>>> -----Original Message----- > >>>> From: Julien Grall <julien@xen.org> > >>>> Sent: 2021年8月19日 21:34 > >>>> To: Wei Chen <Wei.Chen@arm.com>; xen-devel@lists.xenproject.org; > >>>> sstabellini@kernel.org; jbeulich@suse.com > >>>> Cc: Bertrand Marquis <Bertrand.Marquis@arm.com> > >>>> Subject: Re: [XEN RFC PATCH 07/40] xen/arm: use !CONFIG_NUMA to keep > >> fake > >>>> NUMA API > >>>> > >>>> Hi Wei, > >>>> > >>>> On 11/08/2021 11:23, Wei Chen wrote: > >>>>> Only Arm64 supports NUMA, the CONFIG_NUMA could not be > >>>>> enabled for Arm32. > >>>> > >>>> What do you mean by "could not be enabled"? > >>> > >>> I have not seen any Arm32 hardware support NUMA, so I think > >>> we don't need to support Arm32 NUMA. > >> > >> I understand that there may not be 32-bit platform with NUMA. And > that's > >> fine stating that in the commit message. However... > >> > >>> In this case, this Kconfig > >>> option could not be enabled on Arm32. > >> > >> ... you continue to say "couldn't be enabled" without clarifying > whether > >> this mean that the build will break or this was just not tested because > >> you don't have any platform. > > > > Ok, I understand your concern. Yes, my words would lead to mis- > understanding. > > If we make CONFIG_NUMA enabled in Arm32, it need Arm32 to implement some > > code to support NUMA common code. Otherwise the Arm32 build will failed. > > When I skimmed through the series, most of the code seems to be either > in common, arm (bitness neutral). So I am not quite too sure why it > would not build. Do you have more details? > It could not build because I have not tried to enable device_tree_numa option for Arm32 but enabled NUMA for arm32. I have tested it again, yes, simple enable device_tree_numa and NUMA for arm32 can build a image successfully. So, I think it's OK to enable this on Arm32, and I will do it in next version. But, can we still keep these FAKE APIs? If user don't want to enable NUMA they still can make Xen work? And I will remove "could not enable for Arm32" from commit log. > > I have not tried to implement those code for Arm32. And I found there is > > no Arm32 machine support NUMA, so I wanted Arm32 to use fake NUMA API > > as before. > > > >> > >> To put it differently, the code for NUMA looks bitness neutral. So I > >> cannot really what what prevent us to potentially use it on Arm 32-bit. > >> > > > > Yes, you're right, it's neutral. But do we really need to add code to an > > ARCH that it may never use? > > Technically you already added the code because arch/arm/ is common > between arm32 and arm64. My only ask is to not make the new config > depends on arm64. If you only build test it that fine because... > > And how can we test this code? > > I don't expect any of the code to be an issue on arm32 as the code > should mostly be arch neutral. I mean, we don't have Arm32 NUMA machine to test, I don't know the code works well on Arm32 NUMA or not. I only can verify them on non-NUMA arm32, and make sure this code will not break existed machines. > > Cheers, > > -- > Julien Grall
On 20/08/2021 13:23, Wei Chen wrote: > Hi Julien, Hi Wei, >> -----Original Message----- >> From: Julien Grall <julien@xen.org> >> Sent: 2021年8月20日 19:24 >> To: Wei Chen <Wei.Chen@arm.com>; xen-devel@lists.xenproject.org; >> sstabellini@kernel.org >> Cc: Bertrand Marquis <Bertrand.Marquis@arm.com> >> Subject: Re: [XEN RFC PATCH 07/40] xen/arm: use !CONFIG_NUMA to keep fake >> NUMA API >> >> >> >> On 20/08/2021 11:24, Wei Chen wrote: >>> Hi Julien, >> >> Hi Wei, >> >>> >>>> -----Original Message----- >>>> From: Julien Grall <julien@xen.org> >>>> Sent: 2021年8月20日 16:24 >>>> To: Wei Chen <Wei.Chen@arm.com>; xen-devel@lists.xenproject.org; >>>> sstabellini@kernel.org; jbeulich@suse.com >>>> Cc: Bertrand Marquis <Bertrand.Marquis@arm.com> >>>> Subject: Re: [XEN RFC PATCH 07/40] xen/arm: use !CONFIG_NUMA to keep >> fake >>>> NUMA API >>>> >>>> >>>> >>>> On 20/08/2021 03:08, Wei Chen wrote: >>>>> Hi Julien, >>>> >>>> Hi Wei, >>>> >>>>> >>>>>> -----Original Message----- >>>>>> From: Julien Grall <julien@xen.org> >>>>>> Sent: 2021年8月19日 21:34 >>>>>> To: Wei Chen <Wei.Chen@arm.com>; xen-devel@lists.xenproject.org; >>>>>> sstabellini@kernel.org; jbeulich@suse.com >>>>>> Cc: Bertrand Marquis <Bertrand.Marquis@arm.com> >>>>>> Subject: Re: [XEN RFC PATCH 07/40] xen/arm: use !CONFIG_NUMA to keep >>>> fake >>>>>> NUMA API >>>>>> >>>>>> Hi Wei, >>>>>> >>>>>> On 11/08/2021 11:23, Wei Chen wrote: >>>>>>> Only Arm64 supports NUMA, the CONFIG_NUMA could not be >>>>>>> enabled for Arm32. >>>>>> >>>>>> What do you mean by "could not be enabled"? >>>>> >>>>> I have not seen any Arm32 hardware support NUMA, so I think >>>>> we don't need to support Arm32 NUMA. >>>> >>>> I understand that there may not be 32-bit platform with NUMA. And >> that's >>>> fine stating that in the commit message. However... >>>> >>>>> In this case, this Kconfig >>>>> option could not be enabled on Arm32. >>>> >>>> ... you continue to say "couldn't be enabled" without clarifying >> whether >>>> this mean that the build will break or this was just not tested because >>>> you don't have any platform. >>> >>> Ok, I understand your concern. Yes, my words would lead to mis- >> understanding. >>> If we make CONFIG_NUMA enabled in Arm32, it need Arm32 to implement some >>> code to support NUMA common code. Otherwise the Arm32 build will failed. >> >> When I skimmed through the series, most of the code seems to be either >> in common, arm (bitness neutral). So I am not quite too sure why it >> would not build. Do you have more details? >> > > It could not build because I have not tried to enable device_tree_numa > option for Arm32 but enabled NUMA for arm32. > > I have tested it again, yes, simple enable device_tree_numa and NUMA > for arm32 can build a image successfully. > > So, I think it's OK to enable this on Arm32, and I will do it in next > version. But, can we still keep these FAKE APIs? If user don't want to > enable NUMA they still can make Xen work? Yes, we still need to keep the FAKE APIs. I was only commenting about the wording in the commit message. > And I will remove "could not > enable for Arm32" from commit log. > >>> I have not tried to implement those code for Arm32. And I found there is >>> no Arm32 machine support NUMA, so I wanted Arm32 to use fake NUMA API >>> as before. >>> >>>> >>>> To put it differently, the code for NUMA looks bitness neutral. So I >>>> cannot really what what prevent us to potentially use it on Arm 32-bit. >>>> >>> >>> Yes, you're right, it's neutral. But do we really need to add code to an >>> ARCH that it may never use? >> >> Technically you already added the code because arch/arm/ is common >> between arm32 and arm64. My only ask is to not make the new config >> depends on arm64. If you only build test it that fine because... >> >> And how can we test this code? >> >> I don't expect any of the code to be an issue on arm32 as the code >> should mostly be arch neutral. > > I mean, we don't have Arm32 NUMA machine to test, I don't know > the code works well on Arm32 NUMA or not. I only can verify them > on non-NUMA arm32, and make sure this code will not break existed > machines. I understood you don't have any Arm32 NUMA machine. But I don't see the lack of testing as an issue because the code doesn't look to contain bits that may rely on arm64. So there are very limited reasons for the code to break on arm32. If we really want to test it, then it should be feasible to fake the NUMA node in the DT. However, I don't expect you to do it. Cheers,
diff --git a/xen/include/asm-arm/numa.h b/xen/include/asm-arm/numa.h index 31a6de4e23..ab9c4a2448 100644 --- a/xen/include/asm-arm/numa.h +++ b/xen/include/asm-arm/numa.h @@ -5,6 +5,8 @@ typedef u8 nodeid_t; +#if !defined(CONFIG_NUMA) + /* Fake one node for now. See also node_online_map. */ #define cpu_to_node(cpu) 0 #define node_to_cpumask(node) (cpu_online_map) @@ -25,6 +27,8 @@ extern mfn_t first_valid_mfn; #define node_start_pfn(nid) (mfn_x(first_valid_mfn)) #define __node_distance(a, b) (20) +#endif + #endif /* __ARCH_ARM_NUMA_H */ /* * Local variables:
Only Arm64 supports NUMA, the CONFIG_NUMA could not be enabled for Arm32. Even in Arm64, users still can disable the CONFIG_NUMA through Kconfig option. In this case, keep current fake NUMA API, will make Arm code still can work with NUMA aware memory allocation and scheduler. Signed-off-by: Wei Chen <wei.chen@arm.com> --- xen/include/asm-arm/numa.h | 4 ++++ 1 file changed, 4 insertions(+)