[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Xen-devel] [RFC PATCH v1 10/21] ARM: NUMA: Add memory NUMA support
Hello Vijay,
On 09/02/17 15:57, vijay.kilari@xxxxxxxxx wrote:
From: Vijaya Kumar K <Vijaya.Kumar@xxxxxxxxxx>
For all banks in bootinfo.mem, update nodes[] with
corresponding nodeid and register these nodes by
calling setup_node_bootmem().
compute memnode_shift and initialize memnodemap[] to fetch
nodeid for a given physical address.
Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@xxxxxxxxxx>
---
xen/arch/arm/numa.c | 90 ++++++++++++++++++++++++++++++++++++++++++++++++++
xen/common/numa.c | 14 ++++++++
xen/include/xen/numa.h | 1 +
3 files changed, 105 insertions(+)
diff --git a/xen/arch/arm/numa.c b/xen/arch/arm/numa.c
index d4dbad4..aa34c82 100644
--- a/xen/arch/arm/numa.c
+++ b/xen/arch/arm/numa.c
@@ -24,10 +24,15 @@
#include <asm/acpi.h>
#include <xen/errno.h>
#include <xen/cpumask.h>
+#include <asm/setup.h>
int _node_distance[MAX_NUMNODES * 2];
int *node_distance;
extern nodemask_t numa_nodes_parsed;
+extern struct node nodes[MAX_NUMNODES] __initdata;
+extern int num_node_memblks;
+extern struct node node_memblk_range[NR_NODE_MEMBLKS];
+extern nodeid_t memblk_nodeid[NR_NODE_MEMBLKS];
void __init numa_set_cpu_node(int cpu, unsigned long hwid)
{
@@ -51,6 +56,88 @@ u8 __node_distance(nodeid_t a, nodeid_t b)
EXPORT_SYMBOL(__node_distance);
+static int __init numa_mem_init(void)
The function return only 2 different value either 0 or -EINVAL. It
should be better to use a boolean.
+{
+ nodemask_t memory_nodes_parsed;
+ int bank, nodeid;
+ struct node *nd;
+ paddr_t start, size, end;
+
+ nodes_clear(memory_nodes_parsed);
+ for ( bank = 0 ; bank < bootinfo.mem.nr_banks; bank++ )
+ {
+ start = bootinfo.mem.bank[bank].start;
+ size = bootinfo.mem.bank[bank].size;
+ end = start + size;
+
+ nodeid = get_numa_node(start, end);
+ if ( nodeid == -EINVAL || nodeid > MAX_NUMNODES )
If found, how the nodeid could be invalid? IHMO, this is a parsing bug.
+ {
+ printk(XENLOG_WARNING
+ "NUMA: node for mem bank start 0x%lx - 0x%lx not found\n",
+ start, end);
+
+ return -EINVAL;
+ }
+
+ nd = &nodes[nodeid];
+ if ( !node_test_and_set(nodeid, memory_nodes_parsed) )
+ {
+ nd->start = start;
+ nd->end = end;
+ }
+ else
+ {
+ if ( start < nd->start )
+ nd->start = start;
+ if ( nd->end < end )
+ nd->end = end;
+ }
+ }
This function is quite confusing. What is the purpose? Why do you go
through bootinfo.mem and not node_memblk_range?
+
+ return 0;
+}
+
+/* Use the information discovered above to actually set up the nodes. */
+static int __init numa_scan_mem_nodes(void)
This function looks very similar to acpi_scan_nodes on x86. This is call
to move more code in common.
+{
+ int i;
+
+ memnode_shift = compute_hash_shift(node_memblk_range, num_node_memblks,
+ memblk_nodeid);
+ if ( memnode_shift < 0 )
+ {
+ printk(XENLOG_WARNING "No NUMA hash found.\n");
+ memnode_shift = 0;
+ }
+
+ for_each_node_mask(i, numa_nodes_parsed)
+ {
+ u64 size = node_memblk_range[i].end - node_memblk_range[i].start;
+
+ if ( size == 0 )
+ printk(XENLOG_WARNING "NUMA: Node %u has no memory. \n", i);
+
+ printk(XENLOG_INFO
+ "NUMA: NODE[%d]: Start 0x%lx End 0x%lx\n",
+ i, nodes[i].start, nodes[i].end);
+ setup_node_bootmem(i, nodes[i].start, nodes[i].end);
+ }
+
+ return 0;
+}
+
+static int __init numa_initmem_init(void)
+{
+ if ( !numa_mem_init() )
+ {
+ if ( !numa_scan_mem_nodes() )
+ return 0;
+ }
This could be simplified with
if ( !numa_mem_init() && !num_scan_mem_nodes )
return 0;
However looking at the usage, this function seems rather pointless.
+
+ return -EINVAL;
+}
+
/*
* Setup early cpu_to_node.
*/
@@ -74,6 +161,9 @@ int __init numa_init(void)
ret = dt_numa_init();
+ if ( !ret )
+ ret = numa_initmem_init();
+
no_numa:
return ret;
}
diff --git a/xen/common/numa.c b/xen/common/numa.c
index 62c76af..2f5266a 100644
--- a/xen/common/numa.c
+++ b/xen/common/numa.c
@@ -63,6 +63,20 @@ void __init numa_add_memblk(nodeid_t nodeid, u64 start, u64
size)
num_node_memblks++;
}
+int __init get_numa_node(u64 start, u64 end)
I think this function will be confusing for the caller. Why using
num_node_memblks and not the node itself?
+{
+ int i;
Please unsigned int
+
+ for ( i = 0; i < num_node_memblks; i++ )
+ {
+ if ( start >= node_memblk_range[i].start &&
+ end <= node_memblk_range[i].end )
+ return memblk_nodeid[i];
+ }
+
+ return -EINVAL;
+}
+
int valid_numa_range(u64 start, u64 end, nodeid_t node)
{
#ifdef CONFIG_NUMA
diff --git a/xen/include/xen/numa.h b/xen/include/xen/numa.h
index 9392a89..4f04ab4 100644
--- a/xen/include/xen/numa.h
+++ b/xen/include/xen/numa.h
@@ -68,6 +68,7 @@ static inline __attribute__((pure)) nodeid_t
phys_to_nid(paddr_t addr)
#endif /* CONFIG_NUMA */
extern void numa_add_memblk(nodeid_t nodeid, u64 start, u64 size);
+extern int get_numa_node(u64 start, u64 end);
extern int valid_numa_range(u64 start, u64 end, nodeid_t node);
extern int conflicting_memblks(u64 start, u64 end);
extern void cutoff_node(int i, u64 start, u64 end);
Regards,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel
|
