[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Xen-devel] [RFC PATCH v1 08/21] ARM: NUMA: Parse NUMA distance information
Hello Vijay,
On 09/02/17 15:57, vijay.kilari@xxxxxxxxx wrote:
From: Vijaya Kumar K <Vijaya.Kumar@xxxxxxxxxx>
Parse distance-matrix and fetch node distance information.
Store distance information in node_distance[].
Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@xxxxxxxxxx>
---
xen/arch/arm/dt_numa.c | 90 ++++++++++++++++++++++++++++++++++++++++++++++
xen/arch/arm/numa.c | 19 +++++++++-
xen/include/asm-arm/numa.h | 1 +
3 files changed, 109 insertions(+), 1 deletion(-)
diff --git a/xen/arch/arm/dt_numa.c b/xen/arch/arm/dt_numa.c
index fce9e67..8979612 100644
--- a/xen/arch/arm/dt_numa.c
+++ b/xen/arch/arm/dt_numa.c
@@ -28,6 +28,19 @@
nodemask_t numa_nodes_parsed;
extern struct node node_memblk_range[NR_NODE_MEMBLKS];
+extern int _node_distance[MAX_NUMNODES * 2];
+extern int *node_distance;
I don't like this idea of having _node_distance and node_distance.
Looking at the code, I see little point to do that. You could just
initialize node_distance with the correct value.
Also the node distance can fit in u8, so you can save memory by using u8.
Lastly, I am not sure why you pre-allocate the memory. The distance
table could be quite big.
+
+static int numa_set_distance(u32 nodea, u32 nodeb, u32 distance)
Please avoid the use of u32 in favor of uint32_t.
Also, this function does not look very DT specific.
+{
+ if ( nodea >= MAX_NUMNODES || nodeb >= MAX_NUMNODES )
+ return -EINVAL;
+
I would have expected some sanity check here.
+ _node_distance[(nodea * MAX_NUMNODES) + nodeb] = distance;
+ node_distance = &_node_distance[0];
+
+ return 0;
+}
/*
* Even though we connect cpus to numa domains later in SMP
@@ -112,6 +125,66 @@ static int __init dt_numa_process_memory_node(const void
*fdt, int node,
return 0;
}
+static int __init dt_numa_parse_distance_map(const void *fdt, int node,
+ const char *name,
+ u32 address_cells,
+ u32 size_cells)
+{
+ const struct fdt_property *prop;
+ const __be32 *matrix;
+ int entry_count, len, i;
+
+ printk(XENLOG_INFO "NUMA: parsing numa-distance-map\n");
+
+ prop = fdt_get_property(fdt, node, "distance-matrix", &len);
+ if ( !prop )
+ {
+ printk(XENLOG_WARNING
+ "NUMA: No distance-matrix property in distance-map\n");
+
+ return -EINVAL;
+ }
+
+ if ( len % sizeof(u32) != 0 )
+ {
+ printk(XENLOG_WARNING
+ "distance-matrix in node is not a multiple of u32\n");
+
+ return -EINVAL;
+ }
+
+ entry_count = len / sizeof(u32);
+ if ( entry_count <= 0 )
+ {
+ printk(XENLOG_WARNING "NUMA: Invalid distance-matrix\n");
+
+ return -EINVAL;
+ }
+
+ matrix = (const __be32 *)prop->data;
+ for ( i = 0; i + 2 < entry_count; i += 3 )
+ {
+ u32 nodea, nodeb, distance;
+
+ nodea = dt_read_number(matrix, 1);
+ matrix++;
+ nodeb = dt_read_number(matrix, 1);
+ matrix++;
+ distance = dt_read_number(matrix, 1);
+ matrix++;
+
+ numa_set_distance(nodea, nodeb, distance);
What if numa_set_distance failed?
+ printk(XENLOG_INFO "NUMA: distance[node%d -> node%d] = %d\n",
+ nodea, nodeb, distance);
+
+ /* Set default distance of node B->A same as A->B */
+ if ( nodeb > nodea )
+ numa_set_distance(nodeb, nodea, distance);
+ }
+
+ return 0;
+}
+
static int __init dt_numa_scan_cpu_node(const void *fdt, int node,
const char *name, int depth,
u32 address_cells, u32 size_cells,
@@ -136,6 +209,18 @@ static int __init dt_numa_scan_memory_node(const void
*fdt, int node,
return 0;
}
+static int __init dt_numa_scan_distance_node(const void *fdt, int node,
+ const char *name, int depth,
+ u32 address_cells, u32 size_cells,
+ void *data)
+{
+ if ( device_tree_node_matches(fdt, node, "distance-map") )
Similar to memory and cpu, the name is not fixed. What you want to look
for is the compatible numa-distance-map-v1.
+ return dt_numa_parse_distance_map(fdt, node, name, address_cells,
+ size_cells);
+
+ return 0;
+}
+
int __init dt_numa_init(void)
{
int ret;
@@ -149,6 +234,11 @@ int __init dt_numa_init(void)
ret = device_tree_for_each_node((void *)device_tree_flattened,
dt_numa_scan_memory_node, NULL);
+ if ( ret )
+ return ret;
+
+ ret = device_tree_for_each_node((void *)device_tree_flattened,
+ dt_numa_scan_distance_node, NULL);
return ret;
}
diff --git a/xen/arch/arm/numa.c b/xen/arch/arm/numa.c
index 9a7f0bb..11d100b 100644
--- a/xen/arch/arm/numa.c
+++ b/xen/arch/arm/numa.c
@@ -22,14 +22,31 @@
#include <asm/mm.h>
#include <xen/numa.h>
#include <asm/acpi.h>
+#include <xen/errno.h>
Why did you add this include. I don't see any errno here.
+
+int _node_distance[MAX_NUMNODES * 2];
+int *node_distance;
+
+u8 __node_distance(nodeid_t a, nodeid_t b)
+{
+ if ( !node_distance )
+ return a == b ? 10 : 20;
Why does the 10/20 comes from?
+
+ return _node_distance[a * MAX_NUMNODES + b];
+}
+
+EXPORT_SYMBOL(__node_distance);
int __init numa_init(void)
{
- int ret = 0;
+ int i, ret = 0;
if ( numa_off )
goto no_numa;
+ for ( i = 0; i < MAX_NUMNODES * 2; i++ )
+ _node_distance[i] = 0;
+
Hmmmm... _node_distance will be zeroed by the compiler. So why that?
If you want to initialize correctly then use 10/20.
ret = dt_numa_init();
no_numa:
diff --git a/xen/include/asm-arm/numa.h b/xen/include/asm-arm/numa.h
index cdfeecd..b8857e2 100644
--- a/xen/include/asm-arm/numa.h
+++ b/xen/include/asm-arm/numa.h
@@ -11,6 +11,7 @@ typedef u8 nodeid_t;
int arch_numa_setup(char *opt);
int __init numa_init(void);
int __init dt_numa_init(void);
+u8 __node_distance(nodeid_t a, nodeid_t b);
This should be defined in common/numa.h as it is used by common code.
#else
static inline int arch_numa_setup(char *opt)
{
Regards,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel
|
