[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [RFC PATCH v3 16/24] ARM: NUMA: Add memory NUMA support



Hi Vijay,

On 18/07/17 12:41, vijay.kilari@xxxxxxxxx wrote:
From: Vijaya Kumar K <Vijaya.Kumar@xxxxxxxxxx>

Implement arch_sanitize_nodes_memory() which looks at all banks
in bootinfo.mem, update nodes[] with corresponding nodeid.
Call numa_scan_nodes() generic function with ram start and
end address, which takes care of further computing memnodeshift
and populating memnodemap[] using generic implementation.

Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@xxxxxxxxxx>
---
v3: - Dropped common code from asm-arm/numa.h
    - Re-used numa_initmem_init() from common code.
---
 xen/arch/arm/numa/numa.c | 77 +++++++++++++++++++++++++++++++++++++++++++++++-
 xen/common/numa.c        | 14 +++++++++
 xen/include/xen/numa.h   |  1 +
 3 files changed, 91 insertions(+), 1 deletion(-)

diff --git a/xen/arch/arm/numa/numa.c b/xen/arch/arm/numa/numa.c
index dc80aa5..85352dc 100644
--- a/xen/arch/arm/numa/numa.c
+++ b/xen/arch/arm/numa/numa.c
@@ -18,6 +18,7 @@
 #include <xen/ctype.h>
 #include <xen/nodemask.h>
 #include <xen/numa.h>
+#include <xen/pfn.h>
 #include <asm/acpi.h>

 static uint8_t (*node_distance_fn)(nodeid_t a, nodeid_t b);
@@ -64,9 +65,66 @@ void register_node_distance(uint8_t (fn)(nodeid_t a, 
nodeid_t b))
     node_distance_fn = fn;
 }

+bool __init arch_sanitize_nodes_memory(void)

Likely when I say a function is confusing on a previous version, it means you have to add more comments in the function...

+{
+    nodemask_t mem_nodes_parsed;
+    int bank, nodeid;
+    struct node *nd;
+    paddr_t start, size, end;
+
+    nodes_clear(mem_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_mem_nodeid(start, end);
+        if ( nodeid >= NUMA_NO_NODE )

This check is very confusing.

+        {
+            printk(XENLOG_WARNING
+                   "NUMA: node for mem bank start 0x%lx - 0x%lx not found\n",

start and end are paddr_t should it should be PRI_paddr.

+                   start, end);
+
+            return false;
+        }
+
+        nd = get_numa_node(nodeid);
+        if ( !node_test_and_set(nodeid, mem_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 called "sanitize nodes", but here you also update start/end. Mind explaining why you need this on ARM when it is not necessary on x86?

+        }
+    }
+
+    return true;
+}
+
+static void __init numa_reset_numa_nodes(void)
+{
+    int i;
+    struct node *nd;
+
+    for ( i = 0; i < MAX_NUMNODES; i++ )
+    {
+        nd = get_numa_node(i);
+        nd->start = 0;
+        nd->end = 0;
+    }
+}
+
 void __init numa_init(void)
 {
-    int ret = 0;
+    int ret = 0, bank;
+    paddr_t ram_start = ~0;
+    paddr_t ram_end = 0;

     nodes_clear(processor_nodes_parsed);
     init_cpu_to_node();
@@ -83,6 +141,23 @@ void __init numa_init(void)
     }

 no_numa:
+    for ( bank = 0 ; bank < bootinfo.mem.nr_banks; bank++ )
+    {
+        paddr_t bank_start = bootinfo.mem.bank[bank].start;
+        paddr_t bank_end = bank_start + bootinfo.mem.bank[bank].size;
+
+        ram_start = min(ram_start, bank_start);
+        ram_end = max(ram_end, bank_end);
+    }
+
+    /*
+     * In arch_sanitize_nodes_memory() we update nodes[] properly.
+     * Hence we reset the nodes[] before calling numa_scan_nodes().
+     */
+    numa_reset_numa_nodes();
+
+    numa_initmem_init(PFN_UP(ram_start), PFN_DOWN(ram_end));

I might miss something. numa_initmem_init is here to scan the NUMA nodes and set them up. So why are you calling it here?

+
     return;
 }

diff --git a/xen/common/numa.c b/xen/common/numa.c
index 5e985d2..0f79a07 100644
--- a/xen/common/numa.c
+++ b/xen/common/numa.c
@@ -76,6 +76,20 @@ nodeid_t get_memblk_nodeid(unsigned int id)
     return memblk_nodeid[id];
 }

+int __init get_mem_nodeid(paddr_t start, paddr_t end)
+{
+    unsigned int i;
+
+    for ( i = 0; i < get_num_node_memblks(); i++ )
+    {
+        if ( start >= node_memblk_range[i].start &&
+             end <= node_memblk_range[i].end )
+            return memblk_nodeid[i];
+    }
+
+    return -EINVAL;
+}
+
 static nodeid_t *get_memblk_nodeid_map(void)
 {
     return &memblk_nodeid[0];
diff --git a/xen/include/xen/numa.h b/xen/include/xen/numa.h
index 8a306e7..a541eb7 100644
--- a/xen/include/xen/numa.h
+++ b/xen/include/xen/numa.h
@@ -70,6 +70,7 @@ struct node *get_numa_node(unsigned int id);
 nodeid_t get_memblk_nodeid(unsigned int memblk);
 struct node *get_node_memblk_range(unsigned int memblk);
 int numa_add_memblk(nodeid_t nodeid, paddr_t start, uint64_t size);
+int get_mem_nodeid(paddr_t start, paddr_t end);
 int get_num_node_memblks(void);
 bool arch_sanitize_nodes_memory(void);
 void numa_failed(void);


Cheers,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.