[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
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |