powerpc: support memoryless nodes

PATCH In review Nishanth Aravamudan 3 participants 12 messages Feb 19, 2014 – Feb 25, 2014

Subsystems (3)

Memory management PowerPC Page allocator

lore

Share & tools

lore

Nishanth Aravamudan

author

2014-02-19 23:16 UTC #0 lore raw

cover powerpc: support memoryless nodes

We have seen several issues recently on powerpc LPARs with memoryless node NUMA configurations, e.g. (an extreme case):

numactl --hardware
available: 2 nodes (0,3)
node 0 cpus:
node 0 size: 0 MB
node 0 free: 0 MB
node 3 cpus: 0 1 2 3
node 3 size: 8142 MB
node 3 free: 7765 MB
node distances:
node 0 3

  0:  10  20 
  3:  20  10

powerpc doesn't set CONFIG_HAVE_MEMORYLESS_NODES, so we are missing out on a lot of the core-kernel support necessary. This series attempts to fix this by enabling the config option, which requires a few other changes as well.

1/3: mm: return NUMA_NO_NODE in local_memory_node if zonelists are not setup
2/3: powerpc: enable CONFIG_HAVE_PERCPU_NUMA_NODE_ID
3/3: powerpc: enable CONFIG_HAVE_MEMORYLESS_NODES

I have tested this series with Christoph's patch (currently being discussed): http://www.spinics.net/lists/linux-mm/msg69452.html

Thanks,
Nish

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Nishanth Aravamudan

author

2014-02-19 23:17 UTC #1 lore raw

PATCH 1/3 mm: return NUMA_NO_NODE in local_memory_node if zonelists are not setup

We can call local_memory_node() before the zonelists are setup. In that case, first_zones_zonelist() will not set zone and the reference to zone->node will Oops. Catch this case, and, since we presumably running very early, just return that any node will do.

Signed-off-by Nishanth Aravamudan <[email protected]>

Cc Andrew Morton <[email protected]>

Cc Michal Hocko <[email protected]>

Cc Mel Gorman <[email protected]>

Cc [email protected]

Cc Christoph Lameter <[email protected]>

Cc David Rientjes <[email protected]>

Cc Joonsoo Kim <[email protected]>

Cc Ben Herrenschmidt <[email protected]>

Cc Anton Blanchard <[email protected]>

mm/page_alloc.c +2 -0

index e3758a0..5de4337 100644
@@ -3650,6 +3650,8 @@ int local_memory_node(int node)
 				   gfp_zone(GFP_KERNEL),
 				   NULL,
 				   &zone);
+	if (!zone)
+		return NUMA_NO_NODE;
 	return zone->node;
 }
 #endif

Nishanth Aravamudan

author

2014-02-19 23:18 UTC #2 lore raw

PATCH 2/3 powerpc: enable CONFIG_HAVE_PERCPU_NUMA_NODE_ID

In order to enable CONFIG_HAVE_MEMORYLESS_NODES, it is necessary to have somewhere to store the cpu <-> local-memory-node mapping. We could create another powerpc-specific lookup table, but the generic functions in include/linux/topology.h (protected by HAVE_PERCPU_NUMA_NODE_ID) are sufficient. This also allows us to remove the existing powerpc-specific cpu <-> node lookup table.

Signed-off-by Nishanth Aravamudan <[email protected]>

arch/powerpc/include/asm/mmzone.h +0 -1

index 7b58917..c8fbd1c 100644
@@ -29,7 +29,6 @@ extern struct pglist_data *node_data[];
  * Following are specific to this numa platform.
  */
 
-extern int numa_cpu_lookup_table[];
 extern cpumask_var_t node_to_cpumask_map[];
 #ifdef CONFIG_MEMORY_HOTPLUG
 extern unsigned long max_pfn;

arch/powerpc/include/asm/topology.h +0 ~~-13~~

index d0b5fca..8bbe8cc 100644
@@ -20,19 +20,6 @@ struct device_node;
 
 #include <asm/mmzone.h>
 
-static inline int cpu_to_node(int cpu)
-{
-	int nid;
-
-	nid = numa_cpu_lookup_table[cpu];
-
-	/*
-	 * During early boot, the numa-cpu lookup table might not have been
-	 * setup for all CPUs yet. In such cases, default to node 0.
-	 */
-	return (nid < 0) ? 0 : nid;
-}
-
 #define parent_node(node)	(node)
 
 #define cpumask_of_node(node) ((node) == -1 ?				\

arch/powerpc/kernel/smp.c +3 -0

index ac2621a..f45e68d 100644
@@ -739,6 +739,9 @@ void start_secondary(void *unused)
 	}
 	traverse_core_siblings(cpu, true);
 
+	set_cpu_numa_node(cpu, cpu_to_node(cpu));
+	set_cpu_numa_mem(cpu, local_memory_node(cpu_to_node(cpu)));
+
 	smp_wmb();
 	notify_cpu_starting(cpu);
 	set_cpu_online(cpu, true);

arch/powerpc/mm/numa.c +15 ~~-14~~

index 30a42e2..57e2809 100644
@@ -46,11 +46,9 @@ static char *cmdline __initdata;
 static int numa_debug;
 #define dbg(args...) if (numa_debug) { printk(KERN_INFO args); }
 
-int numa_cpu_lookup_table[NR_CPUS];
 cpumask_var_t node_to_cpumask_map[MAX_NUMNODES];
 struct pglist_data *node_data[MAX_NUMNODES];
 
-EXPORT_SYMBOL(numa_cpu_lookup_table);
 EXPORT_SYMBOL(node_to_cpumask_map);
 EXPORT_SYMBOL(node_data);

@@ -154,22 +152,25 @@ static void __init get_node_active_region(unsigned long pfn,
 	}
 }
 
-static void reset_numa_cpu_lookup_table(void)
+static void reset_numa_cpu_node(void)
 {
 	unsigned int cpu;
 
-	for_each_possible_cpu(cpu)
-		numa_cpu_lookup_table[cpu] = -1;
+	for_each_possible_cpu(cpu) {
+		set_cpu_numa_node(cpu, -1);
+		set_cpu_numa_mem(cpu, -1);
+	}
 }
 
-static void update_numa_cpu_lookup_table(unsigned int cpu, int node)
+static void update_numa_cpu_node(unsigned int cpu, int node)
 {
-	numa_cpu_lookup_table[cpu] = node;
+	set_cpu_numa_node(cpu, node);
+	set_cpu_numa_mem(cpu, local_memory_node(node));
 }
 
 static void map_cpu_to_node(int cpu, int node)
 {
-	update_numa_cpu_lookup_table(cpu, node);
+	update_numa_cpu_node(cpu, node);
 
 	dbg("adding cpu %d to node %d\n", cpu, node);

@@ -180,7 +181,7 @@ static void map_cpu_to_node(int cpu, int node)
 #if defined(CONFIG_HOTPLUG_CPU) || defined(CONFIG_PPC_SPLPAR)
 static void unmap_cpu_from_node(unsigned long cpu)
 {
-	int node = numa_cpu_lookup_table[cpu];
+	int node = cpu_to_node(cpu);
 
 	dbg("removing cpu %lu from node %d\n", cpu, node);

@@ -545,7 +546,7 @@ static int numa_setup_cpu(unsigned long lcpu)
 	 * directly instead of querying the firmware, since it represents
 	 * the most recent mapping notified to us by the platform (eg: VPHN).
 	 */
-	if ((nid = numa_cpu_lookup_table[lcpu]) >= 0) {
+	if ((nid = cpu_to_node(lcpu)) >= 0) {
 		map_cpu_to_node(lcpu, nid);
 		return nid;
 	}

@@ -1119,7 +1120,7 @@ void __init do_init_bootmem(void)
 	 */
 	setup_node_to_cpumask_map();
 
-	reset_numa_cpu_lookup_table();
+	reset_numa_cpu_node();
 	register_cpu_notifier(&ppc64_numa_nb);
 	cpu_numa_callback(&ppc64_numa_nb, CPU_UP_PREPARE,
 			  (void *)(unsigned long)boot_cpuid);

@@ -1518,7 +1519,7 @@ static int update_lookup_table(void *data)
 		base = cpu_first_thread_sibling(update->cpu);
 
 		for (j = 0; j < threads_per_core; j++) {
-			update_numa_cpu_lookup_table(base + j, nid);
+			update_numa_cpu_node(base + j, nid);
 		}
 	}

@@ -1571,7 +1572,7 @@ int arch_update_cpu_topology(void)
 		if (new_nid < 0 || !node_online(new_nid))
 			new_nid = first_online_node;
 
-		if (new_nid == numa_cpu_lookup_table[cpu]) {
+		if (new_nid == cpu_to_node(cpu)) {
 			cpumask_andnot(&cpu_associativity_changes_mask,
 					&cpu_associativity_changes_mask,
 					cpu_sibling_mask(cpu));

@@ -1583,7 +1584,7 @@ int arch_update_cpu_topology(void)
 			ud = &updates[i++];
 			ud->cpu = sibling;
 			ud->new_nid = new_nid;
-			ud->old_nid = numa_cpu_lookup_table[sibling];
+			ud->old_nid = cpu_to_node(sibling);
 			cpumask_set_cpu(sibling, &updated_cpus);
 			if (i < weight)
 				ud->next = &updates[i];

Nishanth Aravamudan

author

2014-02-19 23:24 UTC #3 lore raw

PATCH v2 2/3 powerpc: enable CONFIG_HAVE_PERCPU_NUMA_NODE_ID

[Apologies, I sent a stale version of this patch a moment ago...]

Signed-off-by Nishanth Aravamudan <[email protected]>

Cc Ben Herrenschmidt <[email protected]>

Cc Christoph Lameter <[email protected]>

Cc David Rientjes <[email protected]>

Cc Joonsoo Kim <[email protected]>

Cc Anton Blanchard <[email protected]>

Cc [email protected]

arch/powerpc/Kconfig +4 -0

index 957bf34..a84816c 100644
@@ -449,6 +449,10 @@ config NODES_SHIFT
 	default "4"
 	depends on NEED_MULTIPLE_NODES
 
+config USE_PERCPU_NUMA_NODE_ID
+	def_bool y
+	depends on NUMA
+
 config ARCH_SELECT_MEMORY_MODEL
 	def_bool y
 	depends on PPC64

arch/powerpc/include/asm/mmzone.h +0 -1

index 7b58917..c8fbd1c 100644
@@ -29,7 +29,6 @@ extern struct pglist_data *node_data[];
  * Following are specific to this numa platform.
  */
 
-extern int numa_cpu_lookup_table[];
 extern cpumask_var_t node_to_cpumask_map[];
 #ifdef CONFIG_MEMORY_HOTPLUG
 extern unsigned long max_pfn;

arch/powerpc/include/asm/topology.h +0 ~~-13~~

index d0b5fca..8bbe8cc 100644
@@ -20,19 +20,6 @@ struct device_node;
 
 #include <asm/mmzone.h>
 
-static inline int cpu_to_node(int cpu)
-{
-	int nid;
-
-	nid = numa_cpu_lookup_table[cpu];
-
-	/*
-	 * During early boot, the numa-cpu lookup table might not have been
-	 * setup for all CPUs yet. In such cases, default to node 0.
-	 */
-	return (nid < 0) ? 0 : nid;
-}
-
 #define parent_node(node)	(node)
 
 #define cpumask_of_node(node) ((node) == -1 ?				\

arch/powerpc/kernel/smp.c +3 -0

index ac2621a..f45e68d 100644
@@ -739,6 +739,9 @@ void start_secondary(void *unused)
 	}
 	traverse_core_siblings(cpu, true);
 
+	set_cpu_numa_node(cpu, cpu_to_node(cpu));
+	set_cpu_numa_mem(cpu, local_memory_node(cpu_to_node(cpu)));
+
 	smp_wmb();
 	notify_cpu_starting(cpu);
 	set_cpu_online(cpu, true);

arch/powerpc/mm/numa.c +15 ~~-14~~

index 30a42e2..57e2809 100644
@@ -46,11 +46,9 @@ static char *cmdline __initdata;
 static int numa_debug;
 #define dbg(args...) if (numa_debug) { printk(KERN_INFO args); }
 
-int numa_cpu_lookup_table[NR_CPUS];
 cpumask_var_t node_to_cpumask_map[MAX_NUMNODES];
 struct pglist_data *node_data[MAX_NUMNODES];
 
-EXPORT_SYMBOL(numa_cpu_lookup_table);
 EXPORT_SYMBOL(node_to_cpumask_map);
 EXPORT_SYMBOL(node_data);

@@ -154,22 +152,25 @@ static void __init get_node_active_region(unsigned long pfn,
 	}
 }
 
-static void reset_numa_cpu_lookup_table(void)
+static void reset_numa_cpu_node(void)
 {
 	unsigned int cpu;
 
-	for_each_possible_cpu(cpu)
-		numa_cpu_lookup_table[cpu] = -1;
+	for_each_possible_cpu(cpu) {
+		set_cpu_numa_node(cpu, -1);
+		set_cpu_numa_mem(cpu, -1);
+	}
 }
 
-static void update_numa_cpu_lookup_table(unsigned int cpu, int node)
+static void update_numa_cpu_node(unsigned int cpu, int node)
 {
-	numa_cpu_lookup_table[cpu] = node;
+	set_cpu_numa_node(cpu, node);
+	set_cpu_numa_mem(cpu, local_memory_node(node));
 }
 
 static void map_cpu_to_node(int cpu, int node)
 {
-	update_numa_cpu_lookup_table(cpu, node);
+	update_numa_cpu_node(cpu, node);
 
 	dbg("adding cpu %d to node %d\n", cpu, node);

@@ -180,7 +181,7 @@ static void map_cpu_to_node(int cpu, int node)
 #if defined(CONFIG_HOTPLUG_CPU) || defined(CONFIG_PPC_SPLPAR)
 static void unmap_cpu_from_node(unsigned long cpu)
 {
-	int node = numa_cpu_lookup_table[cpu];
+	int node = cpu_to_node(cpu);
 
 	dbg("removing cpu %lu from node %d\n", cpu, node);

@@ -545,7 +546,7 @@ static int numa_setup_cpu(unsigned long lcpu)
 	 * directly instead of querying the firmware, since it represents
 	 * the most recent mapping notified to us by the platform (eg: VPHN).
 	 */
-	if ((nid = numa_cpu_lookup_table[lcpu]) >= 0) {
+	if ((nid = cpu_to_node(lcpu)) >= 0) {
 		map_cpu_to_node(lcpu, nid);
 		return nid;
 	}

@@ -1119,7 +1120,7 @@ void __init do_init_bootmem(void)
 	 */
 	setup_node_to_cpumask_map();
 
-	reset_numa_cpu_lookup_table();
+	reset_numa_cpu_node();
 	register_cpu_notifier(&ppc64_numa_nb);
 	cpu_numa_callback(&ppc64_numa_nb, CPU_UP_PREPARE,
 			  (void *)(unsigned long)boot_cpuid);

@@ -1518,7 +1519,7 @@ static int update_lookup_table(void *data)
 		base = cpu_first_thread_sibling(update->cpu);
 
 		for (j = 0; j < threads_per_core; j++) {
-			update_numa_cpu_lookup_table(base + j, nid);
+			update_numa_cpu_node(base + j, nid);
 		}
 	}

@@ -1571,7 +1572,7 @@ int arch_update_cpu_topology(void)
 		if (new_nid < 0 || !node_online(new_nid))
 			new_nid = first_online_node;
 
-		if (new_nid == numa_cpu_lookup_table[cpu]) {
+		if (new_nid == cpu_to_node(cpu)) {
 			cpumask_andnot(&cpu_associativity_changes_mask,
 					&cpu_associativity_changes_mask,
 					cpu_sibling_mask(cpu));

@@ -1583,7 +1584,7 @@ int arch_update_cpu_topology(void)
 			ud = &updates[i++];
 			ud->cpu = sibling;
 			ud->new_nid = new_nid;
-			ud->old_nid = numa_cpu_lookup_table[sibling];
+			ud->old_nid = cpu_to_node(sibling);
 			cpumask_set_cpu(sibling, &updated_cpus);
 			if (i < weight)
 				ud->next = &updates[i];

Nishanth Aravamudan

author

2014-02-19 23:25 UTC #4 lore raw

PATCH 3/3 powerpc: enable CONFIG_HAVE_MEMORYLESS_NODES

Anton Blanchard found an issue with an LPAR that had no memory in Node
0. Christoph Lameter recommended, as one possible solution, to use numa_mem_id() for locality of the nearest memory node-wise. However, numa_mem_id() [and the other related APIs] are only useful if CONFIG_HAVE_MEMORYLESS_NODES is set. This is only the case for ia64 currently, but clearly we can have memoryless nodes on ppc64. Add the Kconfig option and define it to be the same value as CONFIG_NUMA.

On the LPAR in question, which was very inefficiently using slabs, this took the slab consumption at boot from roughly 7GB to roughly 4GB.

Signed-off-by Nishanth Aravamudan <[email protected]>

Reviewed-by Christoph Lameter <[email protected]>

Cc Ben Herrenschmidt <[email protected]>

Cc Anton Blanchard <[email protected]>

Cc David Rientjes <[email protected]>

Cc Joonsoo Kim <[email protected]>

Cc [email protected]

arch/powerpc/Kconfig +3 -0

index a84816c..0f5cd68 100644
@@ -449,6 +449,9 @@ config NODES_SHIFT
 	default "4"
 	depends on NEED_MULTIPLE_NODES
 
+config HAVE_MEMORYLESS_NODES
+	def_bool NUMA
+
 config USE_PERCPU_NUMA_NODE_ID
 	def_bool y
 	depends on NUMA

Nishanth Aravamudan

author

2014-02-19 23:32 UTC #5 lore raw

Re: [PATCH 1/3] mm: return NUMA_NO_NODE in local_memory_node if zonelists are not setup

In reply to Nishanth Aravamudan

[ Grr, sorry for not including you originally Andrew, if this ends up being ok with others, it will probably need to go through your tree. ]

Quoted 22 lines · Nishanth Aravamudan

We can call local_memory_node() before the zonelists are setup. In that case, first_zones_zonelist() will not set zone and the reference to zone->node will Oops. Catch this case, and, since we presumably running very early, just return that any node will do.
Signed-off-by Nishanth Aravamudan <[email protected]>
Cc Andrew Morton <[email protected]>
Cc Michal Hocko <[email protected]>
Cc Mel Gorman <[email protected]>
Cc [email protected]
Cc [email protected]
Cc Christoph Lameter <[email protected]>
Cc David Rientjes <[email protected]>
Cc Joonsoo Kim <[email protected]>
Cc Ben Herrenschmidt <[email protected]>
Cc Anton Blanchard <[email protected]>
mm/page_alloc.c +0 -0
index e3758a0..5de4337 100644

mm/page_alloc.c int local_memory_node(int node) · line 3657

Quoted 8 lines

mm/page_alloc.c +2 -0

@@ -3650,6 +3650,8 @@ int local_memory_node(int node)
 				   gfp_zone(GFP_KERNEL),
 				   NULL,
 				   &zone);
+	if (!zone)
+		return NUMA_NO_NODE;
 	return zone->node;
 }
 #endif

Christoph Lameter

2014-02-20 16:05 UTC #6 lore raw

Re: [PATCH 1/3] mm: return NUMA_NO_NODE in local_memory_node if zonelists are not setup

In reply to Nishanth Aravamudan

Quoted 4 lines · Nishanth Aravamudan

We can call local_memory_node() before the zonelists are setup. In that case, first_zones_zonelist() will not set zone and the reference to zone->node will Oops. Catch this case, and, since we presumably running very early, just return that any node will do.

Really? Isnt there some way to avoid this call if zonelists are not setup yet?

Nishanth Aravamudan

author

2014-02-20 18:29 UTC #7 lore raw

Re: [PATCH 1/3] mm: return NUMA_NO_NODE in local_memory_node if zonelists are not setup

Quoted 9 lines · Christoph Lameter

Quoted 4 lines · Nishanth Aravamudan
We can call local_memory_node() before the zonelists are setup. In that case, first_zones_zonelist() will not set zone and the reference to zone->node will Oops. Catch this case, and, since we presumably running very early, just return that any node will do.
Really? Isnt there some way to avoid this call if zonelists are not setup yet?

How do I best determine if zonelists aren't setup yet?

The call-path in question (after my series is applied) is:

arch/powerpc/kernel/setup_64.c::setup_arch ->

	arch/powerpc/mm/numa.c::do_init_bootmem() ->
		cpu_numa_callback() ->
			numa_setup_cpu() ->
				map_cpu_to_node() ->
					update_numa_cpu_node() ->
						set_cpu_numa_mem()

and setup_arch() is called before build_all_zonelists(NULL, NULL) in start_kernel(). This seemed like the most reasonable path, as it's used on hotplug as well.

I'm open to suggestsions!

Thanks,
Nish

Andrew Morton

maintainer

2014-02-21 22:42 UTC #8 lore raw

Re: [PATCH 1/3] mm: return NUMA_NO_NODE in local_memory_node if zonelists are not setup

Quoted 23 lines · On Thu, 20 Feb 2014 10:28:47 -0800 Nishanth Aravamudan…

Quoted 9 lines · Christoph Lameter
Quoted 4 lines · Nishanth Aravamudan
We can call local_memory_node() before the zonelists are setup. In that case, first_zones_zonelist() will not set zone and the reference to zone->node will Oops. Catch this case, and, since we presumably running very early, just return that any node will do.
Really? Isnt there some way to avoid this call if zonelists are not setup yet?
How do I best determine if zonelists aren't setup yet?
The call-path in question (after my series is applied) is:
arch/powerpc/kernel/setup_64.c::setup_arch ->
	arch/powerpc/mm/numa.c::do_init_bootmem() ->
		cpu_numa_callback() ->
			numa_setup_cpu() ->
				map_cpu_to_node() ->
					update_numa_cpu_node() ->
						set_cpu_numa_mem()

Quoted 4 lines

and setup_arch() is called before build_all_zonelists(NULL, NULL) in start_kernel(). This seemed like the most reasonable path, as it's used on hotplug as well.

But the call to local_memory_node() you added was in start_secondary(), which isn't in that trace.

I do agree that calling local_memory_node() too early then trying to fudge around the consequences seems rather wrong.

Nishanth Aravamudan

author

2014-02-21 23:56 UTC #9 lore raw

Re: [PATCH 1/3] mm: return NUMA_NO_NODE in local_memory_node if zonelists are not setup

Quoted 25 lines · Andrew Morton

Quoted 23 lines · On Thu, 20 Feb 2014 10:28:47 -0800 Nishanth Aravamudan…
Quoted 9 lines · Christoph Lameter
Quoted 4 lines · Nishanth Aravamudan
We can call local_memory_node() before the zonelists are setup. In that case, first_zones_zonelist() will not set zone and the reference to zone->node will Oops. Catch this case, and, since we presumably running very early, just return that any node will do.
Really? Isnt there some way to avoid this call if zonelists are not setup yet?
How do I best determine if zonelists aren't setup yet?
The call-path in question (after my series is applied) is:
arch/powerpc/kernel/setup_64.c::setup_arch ->
	arch/powerpc/mm/numa.c::do_init_bootmem() ->
		cpu_numa_callback() ->
			numa_setup_cpu() ->
				map_cpu_to_node() ->
					update_numa_cpu_node() ->
						set_cpu_numa_mem()

Quoted 7 lines

Quoted 4 lines
and setup_arch() is called before build_all_zonelists(NULL, NULL) in start_kernel(). This seemed like the most reasonable path, as it's used on hotplug as well.
But the call to local_memory_node() you added was in start_secondary(), which isn't in that trace.

I added two calls to local_memory_node(), I *think* both are necessary, but am willing to be corrected.

One is in map_cpu_to_node() and one is in start_secondary(). The start_secondary() path is fine, AFAICT, as we are up & running at that point. But in [the renamed function] update_numa_cpu_node() which is used by hotplug, we get called from do_init_bootmem(), which is before the zonelists are setup.

I think both calls are necessary because I believe the arch_update_cpu_topology() is used for supporting firmware-driven home-noding, which does not invoke start_secondary() again (the processor is already running, we're just updating the topology in that situation).

Then again, I could special-case the do_init_bootmem callpath, which is only called at kernel init time?

Quoted 2 lines

I do agree that calling local_memory_node() too early then trying to fudge around the consequences seems rather wrong.

If the answer is to simply not call local_memory_node() early, I'll submit a patch to at least add a comment, as there's nothing in the code itself to prevent this from happening and is guaranteed to oops.

Thanks,
Nish

Christoph Lameter

2014-02-24 19:43 UTC #10 lore raw

Re: [PATCH 1/3] mm: return NUMA_NO_NODE in local_memory_node if zonelists are not setup

Quoted 9 lines · Nishanth Aravamudan

I added two calls to local_memory_node(), I *think* both are necessary, but am willing to be corrected.
One is in map_cpu_to_node() and one is in start_secondary(). The start_secondary() path is fine, AFAICT, as we are up & running at that point. But in [the renamed function] update_numa_cpu_node() which is used by hotplug, we get called from do_init_bootmem(), which is before the zonelists are setup.

Quoted 8 lines

I think both calls are necessary because I believe the arch_update_cpu_topology() is used for supporting firmware-driven home-noding, which does not invoke start_secondary() again (the processor is already running, we're just updating the topology in that situation).
Then again, I could special-case the do_init_bootmem callpath, which is only called at kernel init time?

Well taht looks to be simpler.

Quoted 6 lines

Quoted 2 lines
I do agree that calling local_memory_node() too early then trying to fudge around the consequences seems rather wrong.
If the answer is to simply not call local_memory_node() early, I'll submit a patch to at least add a comment, as there's nothing in the code itself to prevent this from happening and is guaranteed to oops.

Ok.

Nishanth Aravamudan

author

2014-02-25 02:34 UTC #11 lore raw

Re: [PATCH 1/3] mm: return NUMA_NO_NODE in local_memory_node if zonelists are not setup

Quoted 17 lines · Christoph Lameter

Quoted 15 lines · Nishanth Aravamudan
I added two calls to local_memory_node(), I *think* both are necessary, but am willing to be corrected.
One is in map_cpu_to_node() and one is in start_secondary(). The start_secondary() path is fine, AFAICT, as we are up & running at that point. But in [the renamed function] update_numa_cpu_node() which is used by hotplug, we get called from do_init_bootmem(), which is before the zonelists are setup.
I think both calls are necessary because I believe the arch_update_cpu_topology() is used for supporting firmware-driven home-noding, which does not invoke start_secondary() again (the processor is already running, we're just updating the topology in that situation).

Quoted 4 lines

Quoted 2 lines
Then again, I could special-case the do_init_bootmem callpath, which is only called at kernel init time?
Well taht looks to be simpler.

Ok, I'll work on this.

Quoted 8 lines

Quoted 6 lines
Quoted 2 lines
I do agree that calling local_memory_node() too early then trying to fudge around the consequences seems rather wrong.
If the answer is to simply not call local_memory_node() early, I'll submit a patch to at least add a comment, as there's nothing in the code itself to prevent this from happening and is guaranteed to oops.
Ok.

Thanks!
-Nish