| Message ID | 20210923120236.3692135-17-wei.chen@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add device tree based NUMA support to Arm | expand |
+x86 maintainers On Thu, 23 Sep 2021, Wei Chen wrote: > srat_bad is used when NUMA initialization code scan SRAT failed. > It will turn fw_numa to disabled status. Its implementation depends > on NUMA implementation. We want every NUMA implementation to provide > this function for common initialization code. > > In this patch, we export srat_bad to external. This will allow to > have the code mostly common. > > Signed-off-by: Wei Chen <wei.chen@arm.com> > --- > xen/arch/x86/srat.c | 2 +- > xen/include/asm-x86/numa.h | 1 + > 2 files changed, 2 insertions(+), 1 deletion(-) > > diff --git a/xen/arch/x86/srat.c b/xen/arch/x86/srat.c > index 0b8b0b0c95..94bd5b34da 100644 > --- a/xen/arch/x86/srat.c > +++ b/xen/arch/x86/srat.c > @@ -163,7 +163,7 @@ static __init void cutoff_node(int i, paddr_t start, paddr_t end) > } > } > > -static __init void bad_srat(void) > +__init void bad_srat(void) > { > int i; > printk(KERN_ERR "SRAT: SRAT not used.\n"); > diff --git a/xen/include/asm-x86/numa.h b/xen/include/asm-x86/numa.h > index 295f875a51..a5690a7098 100644 > --- a/xen/include/asm-x86/numa.h > +++ b/xen/include/asm-x86/numa.h > @@ -32,6 +32,7 @@ extern bool numa_off; > > > extern int srat_disabled(void); > +extern void bad_srat(void); > extern void numa_set_node(int cpu, nodeid_t node); > extern nodeid_t setup_node(unsigned int pxm); > extern void srat_detect_node(int cpu); > -- > 2.25.1 >
On 23.09.2021 14:02, Wei Chen wrote: > srat_bad is used when NUMA initialization code scan SRAT failed. > It will turn fw_numa to disabled status. Its implementation depends > on NUMA implementation. We want every NUMA implementation to provide > this function for common initialization code. > > In this patch, we export srat_bad to external. This will allow to > have the code mostly common. Here as well as in the subject it would help if the function name wasn't the wrong way round. I also don't see how you mean to use a function containing "srat" in its name from non-ACPI code. Perhaps alongside numa_mode() (see the reply to the earlier patch) you want to have a set_numa_off() helper (name subject to improvement)? > --- a/xen/arch/x86/srat.c > +++ b/xen/arch/x86/srat.c > @@ -163,7 +163,7 @@ static __init void cutoff_node(int i, paddr_t start, paddr_t end) > } > } > > -static __init void bad_srat(void) > +__init void bad_srat(void) Nit: Once again, when touching code, please take the opportunity and adjust style issues (here: __init and void want to change places). Jan
Hi Jan, > -----Original Message----- > From: Jan Beulich <jbeulich@suse.com> > Sent: 2022年1月25日 18:22 > 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 16/37] xen/x86: export srat_bad to external > > On 23.09.2021 14:02, Wei Chen wrote: > > srat_bad is used when NUMA initialization code scan SRAT failed. > > It will turn fw_numa to disabled status. Its implementation depends > > on NUMA implementation. We want every NUMA implementation to provide > > this function for common initialization code. > > > > In this patch, we export srat_bad to external. This will allow to > > have the code mostly common. > > Here as well as in the subject it would help if the function name > wasn't the wrong way round. I also don't see how you mean to use a > function containing "srat" in its name from non-ACPI code. > > Perhaps alongside numa_mode() (see the reply to the earlier patch) > you want to have a set_numa_off() helper (name subject to > improvement)? > Yes, actually, I want a function to disable the numa when we encounter any error in numa initialization. And we also so want to indicate the numa off is caused by configuration error. So we had thought to use numa_fw_fault to replace srat_bad. > > --- a/xen/arch/x86/srat.c > > +++ b/xen/arch/x86/srat.c > > @@ -163,7 +163,7 @@ static __init void cutoff_node(int i, paddr_t start, > paddr_t end) > > } > > } > > > > -static __init void bad_srat(void) > > +__init void bad_srat(void) > > Nit: Once again, when touching code, please take the opportunity and > adjust style issues (here: __init and void want to change places). > Thanks again! > Jan
On 27.01.2022 09:35, Wei Chen wrote: > Hi Jan, > >> -----Original Message----- >> From: Jan Beulich <jbeulich@suse.com> >> Sent: 2022年1月25日 18:22 >> 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 16/37] xen/x86: export srat_bad to external >> >> On 23.09.2021 14:02, Wei Chen wrote: >>> srat_bad is used when NUMA initialization code scan SRAT failed. >>> It will turn fw_numa to disabled status. Its implementation depends >>> on NUMA implementation. We want every NUMA implementation to provide >>> this function for common initialization code. >>> >>> In this patch, we export srat_bad to external. This will allow to >>> have the code mostly common. >> >> Here as well as in the subject it would help if the function name >> wasn't the wrong way round. I also don't see how you mean to use a >> function containing "srat" in its name from non-ACPI code. >> >> Perhaps alongside numa_mode() (see the reply to the earlier patch) >> you want to have a set_numa_off() helper (name subject to >> improvement)? >> > > Yes, actually, I want a function to disable the numa when we encounter > any error in numa initialization. And we also so want to indicate the > numa off is caused by configuration error. So we had thought to use > numa_fw_fault to replace srat_bad. Why not simply numa_bad() or, say, numa_disable()? Jan
Hi Jan, > -----Original Message----- > From: Jan Beulich <jbeulich@suse.com> > Sent: 2022年1月27日 16:37 > 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 16/37] xen/x86: export srat_bad to external > > On 27.01.2022 09:35, Wei Chen wrote: > > Hi Jan, > > > >> -----Original Message----- > >> From: Jan Beulich <jbeulich@suse.com> > >> Sent: 2022年1月25日 18:22 > >> 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 16/37] xen/x86: export srat_bad to external > >> > >> On 23.09.2021 14:02, Wei Chen wrote: > >>> srat_bad is used when NUMA initialization code scan SRAT failed. > >>> It will turn fw_numa to disabled status. Its implementation depends > >>> on NUMA implementation. We want every NUMA implementation to provide > >>> this function for common initialization code. > >>> > >>> In this patch, we export srat_bad to external. This will allow to > >>> have the code mostly common. > >> > >> Here as well as in the subject it would help if the function name > >> wasn't the wrong way round. I also don't see how you mean to use a > >> function containing "srat" in its name from non-ACPI code. > >> > >> Perhaps alongside numa_mode() (see the reply to the earlier patch) > >> you want to have a set_numa_off() helper (name subject to > >> improvement)? > >> > > > > Yes, actually, I want a function to disable the numa when we encounter > > any error in numa initialization. And we also so want to indicate the > > numa off is caused by configuration error. So we had thought to use > > numa_fw_fault to replace srat_bad. > > Why not simply numa_bad() or, say, numa_disable()? > numa_bad would be OK. > Jan
diff --git a/xen/arch/x86/srat.c b/xen/arch/x86/srat.c index 0b8b0b0c95..94bd5b34da 100644 --- a/xen/arch/x86/srat.c +++ b/xen/arch/x86/srat.c @@ -163,7 +163,7 @@ static __init void cutoff_node(int i, paddr_t start, paddr_t end) } } -static __init void bad_srat(void) +__init void bad_srat(void) { int i; printk(KERN_ERR "SRAT: SRAT not used.\n"); diff --git a/xen/include/asm-x86/numa.h b/xen/include/asm-x86/numa.h index 295f875a51..a5690a7098 100644 --- a/xen/include/asm-x86/numa.h +++ b/xen/include/asm-x86/numa.h @@ -32,6 +32,7 @@ extern bool numa_off; extern int srat_disabled(void); +extern void bad_srat(void); extern void numa_set_node(int cpu, nodeid_t node); extern nodeid_t setup_node(unsigned int pxm); extern void srat_detect_node(int cpu);
srat_bad is used when NUMA initialization code scan SRAT failed. It will turn fw_numa to disabled status. Its implementation depends on NUMA implementation. We want every NUMA implementation to provide this function for common initialization code. In this patch, we export srat_bad to external. This will allow to have the code mostly common. Signed-off-by: Wei Chen <wei.chen@arm.com> --- xen/arch/x86/srat.c | 2 +- xen/include/asm-x86/numa.h | 1 + 2 files changed, 2 insertions(+), 1 deletion(-)