Hi Jan,
> -----Original Message-----
> From: Jan Beulich <jbeul...@suse.com>
> Sent: 2022年8月25日 18:18
> 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>; xen-
> de...@lists.xenproject.org
> Subject: Re: [PATCH v3 1/6] xen/x86: Provide helpers for common code to
> access acpi_numa
>
> On 22.08.2022 04:58, Wei Chen wrote:
> > --- a/xen/arch/x86/include/asm/numa.h
> > +++ b/xen/arch/x86/include/asm/numa.h
> > @@ -32,8 +32,9 @@ extern void numa_add_cpu(int cpu);
> > extern void numa_init_array(void);
> > extern bool numa_off;
> >
> > -
> > -extern int srat_disabled(void);
> > +extern int arch_numa_setup(const char *opt);
> > +extern bool arch_numa_disabled(bool init_as_disable);
>
> What is the parameter name intended to mean? Since the only caller
> passes "false", this also isn't really possible to guess from the
> use(s) in this patch. In any event perhaps best for the parameter
> to be introduced only once it's actually needed.
>
This parameter will be used in patch#5 and set to true, I will introduce
this parameter in that patch.
> > --- a/xen/arch/x86/numa.c
> > +++ b/xen/arch/x86/numa.c
> > @@ -50,9 +50,31 @@ nodemask_t __read_mostly node_online_map = { { [0] =
> 1UL } };
> > bool numa_off;
> > s8 acpi_numa = 0;
> >
> > -int srat_disabled(void)
> > +int __init arch_numa_setup(const char *opt)
> > {
> > - return numa_off || acpi_numa < 0;
> > +#ifdef CONFIG_ACPI_NUMA
> > + if ( !strncmp(opt, "noacpi", 6) )
> > + {
> > + numa_off = false;
> > + acpi_numa = -1;
> > + return 0;
>
> With this "return" ...
>
> > + }
> > + else
>
> ... this "else" is unnecessary and hence would better be dropped,
> not the least to ...
>
> > +#endif
> > + return -EINVAL;
>
> ... avoid the otherwise ambiguous indentation of this line.
>
This is a good suggestion, current indentation looks weird, I will fix above 3
in next version.
Thanks,
Wei Che
> Jan
