RE: [XEN RFC PATCH 07/40] xen/arm: use !CONFIG_NUMA to keep fake NUMA API

[Wei Chen <Wei.Chen@xxxxxxx>]

Hi Julien,

> -----Original Message-----
> From: Julien Grall <julien@xxxxxxx>
> Sent: 2021年8月20日 16:24
> To: Wei Chen <Wei.Chen@xxxxxxx>; xen-devel@xxxxxxxxxxxxxxxxxxxx;
> sstabellini@xxxxxxxxxx; jbeulich@xxxxxxxx
> Cc: Bertrand Marquis <Bertrand.Marquis@xxxxxxx>
> 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@xxxxxxx>
> >> Sent: 2021年8月19日 21:34
> >> To: Wei Chen <Wei.Chen@xxxxxxx>; xen-devel@xxxxxxxxxxxxxxxxxxxx;
> >> sstabellini@xxxxxxxxxx; jbeulich@xxxxxxxx
> >> Cc: Bertrand Marquis <Bertrand.Marquis@xxxxxxx>
> >> 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@xxxxxxx>
> >>> ---
> >>>    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