Hi Stefano,
> -----Original Message-----
> From: Stefano Stabellini <[email protected]>
> Sent: 2021年8月27日 7:52
> To: Wei Chen <[email protected]>
> Cc: [email protected]; [email protected]; [email protected];
> [email protected]; Bertrand Marquis <[email protected]>
> Subject: Re: [XEN RFC PATCH 20/40] xen/arm: implement node distance
> helpers for Arm64
>
> On Wed, 11 Aug 2021, Wei Chen wrote:
> > In current Xen code, __node_distance is a fake API, it always
> > returns NUMA_REMOTE_DISTANCE(20). Now we use a matrix to record
> > the distance between any two nodes. Accordingly, we provide a
> > set_node_distance API to set the distance for any two nodes in
> > this patch.
> >
> > Signed-off-by: Wei Chen <[email protected]>
> > ---
> > xen/arch/arm/numa.c | 44 ++++++++++++++++++++++++++++++++++++++
> > xen/include/asm-arm/numa.h | 12 ++++++++++-
> > xen/include/asm-x86/numa.h | 1 -
> > xen/include/xen/numa.h | 2 +-
> > 4 files changed, 56 insertions(+), 3 deletions(-)
> >
> > diff --git a/xen/arch/arm/numa.c b/xen/arch/arm/numa.c
> > index 566ad1e52b..f61a8df645 100644
> > --- a/xen/arch/arm/numa.c
> > +++ b/xen/arch/arm/numa.c
> > @@ -23,6 +23,11 @@
> > #include <xen/pfn.h>
> > #include <asm/setup.h>
> >
> > +static uint8_t __read_mostly
> > +node_distance_map[MAX_NUMNODES][MAX_NUMNODES] = {
> > + { NUMA_REMOTE_DISTANCE }
> > +};
> > +
> > void numa_set_node(int cpu, nodeid_t nid)
> > {
> > if ( nid >= MAX_NUMNODES ||
> > @@ -32,6 +37,45 @@ void numa_set_node(int cpu, nodeid_t nid)
> > cpu_to_node[cpu] = nid;
> > }
> >
> > +void __init numa_set_distance(nodeid_t from, nodeid_t to, uint32_t
> distance)
> > +{
> > + if ( from >= MAX_NUMNODES || to >= MAX_NUMNODES )
> > + {
> > + printk(KERN_WARNING
> > + "NUMA nodes are out of matrix, from=%u to=%u distance=%u\n",
> > + from, to, distance);
>
> NIT: please align. Example:
>
> printk(KERN_WARNING
> "NUMA nodes are out of matrix, from=%u to=%u distance=%u\n",
>
> Also please use PRIu32 for uint32_t. Probably should use PRIu8 for
> nodeids.
>
OK
>
> > + return;
> > + }
> > +
> > + /* NUMA defines 0xff as an unreachable node and 0-9 are undefined
> */
> > + if ( distance >= NUMA_NO_DISTANCE ||
> > + (distance >= NUMA_DISTANCE_UDF_MIN &&
> > + distance <= NUMA_DISTANCE_UDF_MAX) ||
> > + (from == to && distance != NUMA_LOCAL_DISTANCE) )
> > + {
> > + printk(KERN_WARNING
> > + "Invalid NUMA node distance, from:%d to:%d distance=%d\n",
> > + from, to, distance);
>
> NIT: please align
>
> Also you used %u before for nodeids, which is better because from and to
> are unsigned. Distance should be uint32_t.
>
OK
>
> > + return;
> > + }
> > +
> > + node_distance_map[from][to] = distance;
>
> Shouldn't we also be setting:
>
> node_distance_map[to][from] = distance;
>
> ?
>
No, we want numa_set_distance behavior is single.
"node_distance_map[to][from] = distance" is handled in caller.
>
> > +}
> > +
> > +uint8_t __node_distance(nodeid_t from, nodeid_t to)
> > +{
> > + /*
> > + * Check whether the nodes are in the matrix range.
> > + * When any node is out of range, except from and to nodes are the
> > + * same, we treat them as unreachable (return 0xFF)
> > + */
> > + if ( from >= MAX_NUMNODES || to >= MAX_NUMNODES )
> > + return from == to ? NUMA_LOCAL_DISTANCE : NUMA_NO_DISTANCE;
> > +
> > + return node_distance_map[from][to];
> > +}
> > +EXPORT_SYMBOL(__node_distance);
> > +
> > void __init numa_init(bool acpi_off)
> > {
> > uint32_t idx;
> > diff --git a/xen/include/asm-arm/numa.h b/xen/include/asm-arm/numa.h
> > index bb495a24e1..559b028a01 100644
> > --- a/xen/include/asm-arm/numa.h
> > +++ b/xen/include/asm-arm/numa.h
> > @@ -12,8 +12,19 @@ typedef u8 nodeid_t;
> > * set the number of NUMA memory block number to 128.
> > */
> > #define NODES_SHIFT 6
> > +/*
> > + * In ACPI spec, 0-9 are the reserved values for node distance,
> > + * 10 indicates local node distance, 20 indicates remote node
> > + * distance. Set node distance map in device tree will follow
> > + * the ACPI's definition.
> > + */
> > +#define NUMA_DISTANCE_UDF_MIN 0
> > +#define NUMA_DISTANCE_UDF_MAX 9
> > +#define NUMA_LOCAL_DISTANCE 10
> > +#define NUMA_REMOTE_DISTANCE 20
> >
> > extern void numa_init(bool acpi_off);
> > +extern void numa_set_distance(nodeid_t from, nodeid_t to, uint32_t
> distance);
> >
> > /*
> > * Temporary for fake NUMA node, when CPU, memory and distance
> > @@ -21,7 +32,6 @@ extern void numa_init(bool acpi_off);
> > * symbols will be removed.
> > */
> > extern mfn_t first_valid_mfn;
> > -#define __node_distance(a, b) (20)
> >
> > #else
> >
> > diff --git a/xen/include/asm-x86/numa.h b/xen/include/asm-x86/numa.h
> > index 5a57a51e26..e0253c20b7 100644
> > --- a/xen/include/asm-x86/numa.h
> > +++ b/xen/include/asm-x86/numa.h
> > @@ -21,7 +21,6 @@ extern nodeid_t apicid_to_node[];
> > extern void init_cpu_to_node(void);
> >
> > void srat_parse_regions(u64 addr);
> > -extern u8 __node_distance(nodeid_t a, nodeid_t b);
> > unsigned int arch_get_dma_bitsize(void);
> >
> > #endif
> > diff --git a/xen/include/xen/numa.h b/xen/include/xen/numa.h
> > index cb08d2eca9..0475823b13 100644
> > --- a/xen/include/xen/numa.h
> > +++ b/xen/include/xen/numa.h
> > @@ -58,7 +58,7 @@ static inline __attribute__((pure)) nodeid_t
> phys_to_nid(paddr_t addr)
> > #define node_spanned_pages(nid) (NODE_DATA(nid)->node_spanned_pages)
> > #define node_end_pfn(nid) (NODE_DATA(nid)->node_start_pfn + \
> > NODE_DATA(nid)->node_spanned_pages)
> > -
> > +extern u8 __node_distance(nodeid_t a, nodeid_t b);
> > extern void numa_add_cpu(int cpu);
> >
> > struct node {
> > --
> > 2.25.1
> >
