Re: [PATCH] Fix fake numa on ppc

# cat /sys/devices/system/node/node4/cpulist
4-7

quoted

quoted

Signed-off-by: Ankita Garg <redacted>

Index: linux-2.6.31-rc5/arch/powerpc/mm/numa.c
===================================================================

--- linux-2.6.31-rc5.orig/arch/powerpc/mm/numa.c
+++ linux-2.6.31-rc5/arch/powerpc/mm/numa.c

@@ -26,6 +26,11 @@
 #include <asm/smp.h>

 static int numa_enabled = 1;
+static int fake_enabled = 1;
+
+/* The array maps a real numa node to the first fake node that gets
+created on it */

Coding style is broken

Fixed.

quoted

quoted

+int fake_numa_node_mapping[MAX_NUMNODES];

 static char *cmdline __initdata;

@@ -49,14 +54,24 @@ static int __cpuinit fake_numa_create_ne
 	unsigned long long mem;
 	char *p = cmdline;
 	static unsigned int fake_nid;
+	static unsigned int orig_nid = 0;

Should we call this prev_nid?

Yes, makes sense.

quoted

quoted

 	static unsigned long long curr_boundary;

 	/*
 	 * Modify node id, iff we started creating NUMA nodes
 	 * We want to continue from where we left of the last time
 	 */
-	if (fake_nid)
+	if (fake_nid) {
+		if (orig_nid != *nid) {

OK, so this is called when the real NUMA node changes - comments would
be nice

Thanks, have added the comment.

quoted

quoted

+			fake_nid++;
+			fake_numa_node_mapping[*nid] = fake_nid;
+			orig_nid = *nid;
+			*nid = fake_nid;
+			return 0;
+		}
 		*nid = fake_nid;
+	}
+
 	/*
 	 * In case there are no more arguments to parse, the
 	 * node_id should be the same as the last fake node id

@@ -440,7 +455,7 @@ static int of_drconf_to_nid_single(struc
  */
 static int __cpuinit numa_setup_cpu(unsigned long lcpu)
 {
-	int nid = 0;
+	int nid = 0, new_nid;
 	struct device_node *cpu = of_get_cpu_node(lcpu, NULL);

 	if (!cpu) {

@@ -450,8 +465,15 @@ static int __cpuinit numa_setup_cpu(unsi

 	nid = of_node_to_nid_single(cpu);

+	if (fake_enabled && nid) {
+		new_nid = fake_numa_node_mapping[nid];
+		if (new_nid > 0)
+			nid = new_nid;
+	}
+
 	if (nid < 0 || !node_online(nid))
 		nid = any_online_node(NODE_MASK_ALL);
+
 out:
 	map_cpu_to_node(lcpu, nid);

@@ -1005,8 +1027,11 @@ static int __init early_numa(char *p)
 		numa_debug = 1;

 	p = strstr(p, "fake=");
-	if (p)
+	if (p) {
 		cmdline = p + strlen("fake=");
+		if (numa_enabled)
+			fake_enabled = 1;

Have you tried passing just numa=fake= without any commandline?
That should enable fake_enabled, but I wonder if that negatively
impacts numa_setup_cpu(). I wonder if you should look at cmdline
to decide on fake_enabled.

fake_enabled does get set even for numa=fake=. However, it does not
impact numa_setup_cpu, since fake_numa_node_mapping array would have no
mapping stored and there is a condition there already to check for the
value of the mapping. I confirmed this by booting with the above
parameter as well.

quoted

quoted

+	}

 	return 0;
 }

Overall, I think this is the right thing to do, we need to move in
this direction. 

Heres the updated patch:

Signed-off-by: Ankita Garg <redacted> 

Index: linux-2.6.31-rc5/arch/powerpc/mm/numa.c
===================================================================

--- linux-2.6.31-rc5.orig/arch/powerpc/mm/numa.c
+++ linux-2.6.31-rc5/arch/powerpc/mm/numa.c

@@ -26,6 +26,13 @@
 #include <asm/smp.h>

 static int numa_enabled = 1;
+static int fake_enabled = 1;
+
+/*
+ * The array maps a real numa node to the first fake node that gets
+ * created on it
+ */
+int fake_numa_node_mapping[MAX_NUMNODES];

 static char *cmdline __initdata;

@@ -49,14 +56,29 @@ static int __cpuinit fake_numa_create_ne
 	unsigned long long mem;
 	char *p = cmdline;
 	static unsigned int fake_nid;
+	static unsigned int prev_nid = 0;
 	static unsigned long long curr_boundary;

 	/*
 	 * Modify node id, iff we started creating NUMA nodes
 	 * We want to continue from where we left of the last time
 	 */
-	if (fake_nid)
+	if (fake_nid) {
+		/*
+		 * Moved over to the next real numa node, increment fake
+		 * node number and store the mapping of the real node to
+		 * the fake node
+		 */
+		if (prev_nid != *nid) {
+			fake_nid++;
+			fake_numa_node_mapping[*nid] = fake_nid;
+			prev_nid = *nid;
+			*nid = fake_nid;
+			return 0;
+		}
 		*nid = fake_nid;
+	}
+
 	/*
 	 * In case there are no more arguments to parse, the
 	 * node_id should be the same as the last fake node id

@@ -440,7 +462,7 @@ static int of_drconf_to_nid_single(struc
  */
 static int __cpuinit numa_setup_cpu(unsigned long lcpu)
 {
-	int nid = 0;
+	int nid = 0, new_nid;
 	struct device_node *cpu = of_get_cpu_node(lcpu, NULL);

 	if (!cpu) {

@@ -450,8 +472,15 @@ static int __cpuinit numa_setup_cpu(unsi

 	nid = of_node_to_nid_single(cpu);

+	if (fake_enabled && nid) {
+		new_nid = fake_numa_node_mapping[nid];
+		if (new_nid > 0)
+			nid = new_nid;
+	}
+
 	if (nid < 0 || !node_online(nid))
 		nid = any_online_node(NODE_MASK_ALL);
+
 out:
 	map_cpu_to_node(lcpu, nid);

@@ -1005,8 +1034,12 @@ static int __init early_numa(char *p)
 		numa_debug = 1;

 	p = strstr(p, "fake=");
-	if (p)
+	if (p) {
 		cmdline = p + strlen("fake=");
+		if (numa_enabled) {
+			fake_enabled = 1;
+		}
+	}

 	return 0;
 }