Hi Jan, > -----Original Message----- > From: Jan Beulich <jbeul...@suse.com> > Sent: 2022年9月9日 0:02 > To: Wei Chen <wei.c...@arm.com> > Cc: nd <n...@arm.com>; Andrew Cooper <andrew.coop...@citrix.com>; Roger Pau > Monné <roger....@citrix.com>; Wei Liu <w...@xen.org>; George Dunlap > <george.dun...@citrix.com>; Julien Grall <jul...@xen.org>; Stefano > Stabellini <sstabell...@kernel.org>; xen-devel@lists.xenproject.org > Subject: Re: [PATCH v4 5/6] xen/x86: move NUMA scan nodes codes from x86 > to common > > On 08.09.2022 17:26, Wei Chen wrote: > >> -----Original Message----- > >> From: Jan Beulich <jbeul...@suse.com> > >> Sent: 2022年9月8日 21:03 > >> > >> On 02.09.2022 05:31, Wei Chen wrote: > >>> --- a/xen/arch/x86/numa.c > >>> +++ b/xen/arch/x86/numa.c > >>> @@ -41,9 +41,12 @@ int __init arch_numa_setup(const char *opt) > >>> return -EINVAL; > >>> } > >>> > >>> -bool arch_numa_disabled(void) > >>> +bool arch_numa_disabled(bool init_as_disable) > >> > >> I'm afraid my question as to the meaning of the name of the parameter > has > >> remained unanswered. > >> > > > > Sorry, I might missed some contents of your reply in v3. The name of > this > > parameter has been bothering me for a long time, and now this is > actually > > quite awkward. The origin of this parameter is because the current NUMA > > implementation will make different judgments under different usage > > conditions when using acpi_numa. In acpi_scan_nodes, it uses acpi_numa > <= 0 > > as the condition for judging that ACPI NUMA is turned off. But only use > > acpi_numa < 0 as condition in srat_disabled and elsewhere. I use this > > parameter in the hope that we can keep the same semantics as the > original > > code without changing the code of the caller. > > The difference is "bad only" vs "bad or no data". Maybe that's easier > to express via two functions - arch_numa_disabled() (checking <= 0) > and arch_numa_broken() (checking < 0)? With a single function I guess > the name of the parameter would always be clumsy at best. Unless > someone has a good idea for a suitable name ... >
Yes, I can't find a good name for the parameter, so break into two functions would be better, I will do it in next version. > >>> --- a/xen/drivers/acpi/Kconfig > >>> +++ b/xen/drivers/acpi/Kconfig > >>> @@ -7,4 +7,5 @@ config ACPI_LEGACY_TABLES_LOOKUP > >>> > >>> config ACPI_NUMA > >>> bool > >>> + select HAS_NUMA_NODE_FWID > >> > >> Are you selecting an option here which doesn't exist anywhere? Or > >> am I overlooking where this new option is being added? > >> > > > > Yes, this is a new Kconfig option. Should I need to introduce in a > > separate patch? > > I don't think that'll need to be in a separate patch; it can simply > be another hunk in the one here, adding the needed 2 lines (plus a > blank one) to, presumably, common/Kconfig. Ok. Thanks, Wei Chen > > Jan
