linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/4] [PATCH] powerpc: numa: Remove double of_node_put in hot_add_node_scn_to_nid
@ 2011-08-11  6:44 Anton Blanchard
  2011-08-11  6:44 ` [PATCH 2/4] [PATCH] powerpc: Use for_each_node_by_type instead of open coding it Anton Blanchard
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Anton Blanchard @ 2011-08-11  6:44 UTC (permalink / raw)
  To: benh, paulus, sfr, nfont; +Cc: linuxppc-dev

During memory hotplug testing, I got the following warning:


ERROR: Bad of_node_put() on /memory@0

of_node_release
kref_put
of_node_put
of_find_node_by_type
hot_add_node_scn_to_nid
hot_add_scn_to_nid
memory_add_physaddr_to_nid
...


of_find_node_by_type() loop does the of_node_put for us so we only
need the handle the case where we terminate the loop early.

As suggested by Stephen Rothwell we can do the of_node_put
unconditionally outside of the loop since of_node_put handles a
NULL argument fine.

Signed-off-by: Anton Blanchard <anton@samba.org>
Cc: stable@kernel.org
---

Index: linux-powerpc/arch/powerpc/mm/numa.c
===================================================================
--- linux-powerpc.orig/arch/powerpc/mm/numa.c	2011-06-06 08:07:35.148708089 +1000
+++ linux-powerpc/arch/powerpc/mm/numa.c	2011-08-11 09:07:07.329584634 +1000
@@ -1214,11 +1214,12 @@ int hot_add_node_scn_to_nid(unsigned lon
 			break;
 		}
 
-		of_node_put(memory);
 		if (nid >= 0)
 			break;
 	}
 
+	of_node_put(memory);
+
 	return nid;
 }
 

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH 2/4] [PATCH] powerpc: Use for_each_node_by_type instead of open coding it
  2011-08-11  6:44 [PATCH 1/4] [PATCH] powerpc: numa: Remove double of_node_put in hot_add_node_scn_to_nid Anton Blanchard
@ 2011-08-11  6:44 ` Anton Blanchard
  2011-08-11  6:44 ` [PATCH 3/4] [PATCH] powerpc: Coding style cleanups Anton Blanchard
  2011-08-11  6:44 ` [PATCH 4/4] powerpc: Fix oops when echoing bad values to /sys/devices/system/memory/probe Anton Blanchard
  2 siblings, 0 replies; 4+ messages in thread
From: Anton Blanchard @ 2011-08-11  6:44 UTC (permalink / raw)
  To: benh, paulus, sfr; +Cc: linuxppc-dev

Use for_each_node_by_type instead of open coding it.

Signed-off-by: Anton Blanchard <anton@samba.org>
---

Index: linux-powerpc/arch/powerpc/kernel/machine_kexec_64.c
===================================================================
--- linux-powerpc.orig/arch/powerpc/kernel/machine_kexec_64.c	2011-08-10 16:17:52.508167607 +1000
+++ linux-powerpc/arch/powerpc/kernel/machine_kexec_64.c	2011-08-10 16:18:03.000000000 +1000
@@ -74,8 +74,7 @@ int default_machine_kexec_prepare(struct
 	}
 
 	/* We also should not overwrite the tce tables */
-	for (node = of_find_node_by_type(NULL, "pci"); node != NULL;
-			node = of_find_node_by_type(node, "pci")) {
+	for_each_node_by_type(node, "pci") {
 		basep = of_get_property(node, "linux,tce-base", NULL);
 		sizep = of_get_property(node, "linux,tce-size", NULL);
 		if (basep == NULL || sizep == NULL)
Index: linux-powerpc/arch/powerpc/kernel/setup_64.c
===================================================================
--- linux-powerpc.orig/arch/powerpc/kernel/setup_64.c	2011-08-10 16:17:52.518167781 +1000
+++ linux-powerpc/arch/powerpc/kernel/setup_64.c	2011-08-10 16:19:00.629356097 +1000
@@ -278,7 +278,7 @@ static void __init initialize_cache_info
 
 	DBG(" -> initialize_cache_info()\n");
 
-	for (np = NULL; (np = of_find_node_by_type(np, "cpu"));) {
+	for_each_node_by_type(np, "cpu") {
 		num_cpus += 1;
 
 		/* We're assuming *all* of the CPUs have the same
Index: linux-powerpc/arch/powerpc/mm/numa.c
===================================================================
--- linux-powerpc.orig/arch/powerpc/mm/numa.c	2011-08-10 16:17:52.508167607 +1000
+++ linux-powerpc/arch/powerpc/mm/numa.c	2011-08-10 16:19:57.080356036 +1000
@@ -710,7 +710,7 @@ static void __init parse_drconf_memory(s
 static int __init parse_numa_properties(void)
 {
 	struct device_node *cpu = NULL;
-	struct device_node *memory = NULL;
+	struct device_node *memory;
 	int default_nid = 0;
 	unsigned long i;
 
@@ -750,8 +750,8 @@ static int __init parse_numa_properties(
 	}
 
 	get_n_mem_cells(&n_mem_addr_cells, &n_mem_size_cells);
-	memory = NULL;
-	while ((memory = of_find_node_by_type(memory, "memory")) != NULL) {
+
+	for_each_node_by_type(memory, "memory") {
 		unsigned long start;
 		unsigned long size;
 		int nid;
@@ -1187,10 +1187,10 @@ static int hot_add_drconf_scn_to_nid(str
  */
 int hot_add_node_scn_to_nid(unsigned long scn_addr)
 {
-	struct device_node *memory = NULL;
+	struct device_node *memory;
 	int nid = -1;
 
-	while ((memory = of_find_node_by_type(memory, "memory")) != NULL) {
+	for_each_node_by_type(memory, "memory") {
 		unsigned long start, size;
 		int ranges;
 		const unsigned int *memcell_buf;

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH 3/4] [PATCH] powerpc: Coding style cleanups
  2011-08-11  6:44 [PATCH 1/4] [PATCH] powerpc: numa: Remove double of_node_put in hot_add_node_scn_to_nid Anton Blanchard
  2011-08-11  6:44 ` [PATCH 2/4] [PATCH] powerpc: Use for_each_node_by_type instead of open coding it Anton Blanchard
@ 2011-08-11  6:44 ` Anton Blanchard
  2011-08-11  6:44 ` [PATCH 4/4] powerpc: Fix oops when echoing bad values to /sys/devices/system/memory/probe Anton Blanchard
  2 siblings, 0 replies; 4+ messages in thread
From: Anton Blanchard @ 2011-08-11  6:44 UTC (permalink / raw)
  To: benh, paulus, sfr; +Cc: linuxppc-dev

While converting code to use for_each_node_by_type I noticed a
number of coding style issues.

Signed-off-by: Anton Blanchard <anton@samba.org>
---

Index: linux-powerpc/arch/powerpc/kernel/setup_64.c
===================================================================
--- linux-powerpc.orig/arch/powerpc/kernel/setup_64.c	2011-08-10 16:19:00.629356097 +1000
+++ linux-powerpc/arch/powerpc/kernel/setup_64.c	2011-08-10 16:21:31.642031299 +1000
@@ -281,11 +281,11 @@ static void __init initialize_cache_info
 	for_each_node_by_type(np, "cpu") {
 		num_cpus += 1;
 
-		/* We're assuming *all* of the CPUs have the same
+		/*
+		 * We're assuming *all* of the CPUs have the same
 		 * d-cache and i-cache sizes... -Peter
 		 */
-
-		if ( num_cpus == 1 ) {
+		if (num_cpus == 1) {
 			const u32 *sizep, *lsizep;
 			u32 size, lsize;
 
@@ -294,10 +294,13 @@ static void __init initialize_cache_info
 			sizep = of_get_property(np, "d-cache-size", NULL);
 			if (sizep != NULL)
 				size = *sizep;
-			lsizep = of_get_property(np, "d-cache-block-size", NULL);
+			lsizep = of_get_property(np, "d-cache-block-size",
+						 NULL);
 			/* fallback if block size missing */
 			if (lsizep == NULL)
-				lsizep = of_get_property(np, "d-cache-line-size", NULL);
+				lsizep = of_get_property(np,
+							 "d-cache-line-size",
+							 NULL);
 			if (lsizep != NULL)
 				lsize = *lsizep;
 			if (sizep == 0 || lsizep == 0)
@@ -314,9 +317,12 @@ static void __init initialize_cache_info
 			sizep = of_get_property(np, "i-cache-size", NULL);
 			if (sizep != NULL)
 				size = *sizep;
-			lsizep = of_get_property(np, "i-cache-block-size", NULL);
+			lsizep = of_get_property(np, "i-cache-block-size",
+						 NULL);
 			if (lsizep == NULL)
-				lsizep = of_get_property(np, "i-cache-line-size", NULL);
+				lsizep = of_get_property(np,
+							 "i-cache-line-size",
+							 NULL);
 			if (lsizep != NULL)
 				lsize = *lsizep;
 			if (sizep == 0 || lsizep == 0)
Index: linux-powerpc/arch/powerpc/mm/numa.c
===================================================================
--- linux-powerpc.orig/arch/powerpc/mm/numa.c	2011-08-10 16:19:57.080356036 +1000
+++ linux-powerpc/arch/powerpc/mm/numa.c	2011-08-10 16:20:46.981240046 +1000
@@ -709,7 +709,6 @@ static void __init parse_drconf_memory(s
 
 static int __init parse_numa_properties(void)
 {
-	struct device_node *cpu = NULL;
 	struct device_node *memory;
 	int default_nid = 0;
 	unsigned long i;
@@ -732,6 +731,7 @@ static int __init parse_numa_properties(
 	 * each node to be onlined must have NODE_DATA etc backing it.
 	 */
 	for_each_present_cpu(i) {
+		struct device_node *cpu;
 		int nid;
 
 		cpu = of_get_cpu_node(i, NULL);
@@ -800,8 +800,9 @@ new_range:
 	}
 
 	/*
-	 * Now do the same thing for each MEMBLOCK listed in the ibm,dynamic-memory
-	 * property in the ibm,dynamic-reconfiguration-memory node.
+	 * Now do the same thing for each MEMBLOCK listed in the
+	 * ibm,dynamic-memory property in the
+	 * ibm,dynamic-reconfiguration-memory node.
 	 */
 	memory = of_find_node_by_path("/ibm,dynamic-reconfiguration-memory");
 	if (memory)

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH 4/4] powerpc: Fix oops when echoing bad values to /sys/devices/system/memory/probe
  2011-08-11  6:44 [PATCH 1/4] [PATCH] powerpc: numa: Remove double of_node_put in hot_add_node_scn_to_nid Anton Blanchard
  2011-08-11  6:44 ` [PATCH 2/4] [PATCH] powerpc: Use for_each_node_by_type instead of open coding it Anton Blanchard
  2011-08-11  6:44 ` [PATCH 3/4] [PATCH] powerpc: Coding style cleanups Anton Blanchard
@ 2011-08-11  6:44 ` Anton Blanchard
  2 siblings, 0 replies; 4+ messages in thread
From: Anton Blanchard @ 2011-08-11  6:44 UTC (permalink / raw)
  To: benh, paulus, sfr, nfont; +Cc: linuxppc-dev

If we echo an address the hypervisor doesn't like to
/sys/devices/system/memory/probe we oops the box:

# echo 0x10000000000 > /sys/devices/system/memory/probe

kernel BUG at arch/powerpc/mm/hash_utils_64.c:541!

The backtrace is:

create_section_mapping
arch_add_memory
add_memory
memory_probe_store
sysdev_class_store
sysfs_write_file
vfs_write
SyS_write

In create_section_mapping we BUG if htab_bolt_mapping returned
an error. A better approach is to return an error which will
propagate back to userspace.

Rerunning the test with this patch applied:

# echo 0x10000000000 > /sys/devices/system/memory/probe
-bash: echo: write error: Invalid argument

Signed-off-by: Anton Blanchard <anton@samba.org>
Cc: stable@kernel.org
---

Index: linux-build/arch/powerpc/mm/hash_utils_64.c
===================================================================
--- linux-build.orig/arch/powerpc/mm/hash_utils_64.c	2011-08-10 17:00:42.603007554 +1000
+++ linux-build/arch/powerpc/mm/hash_utils_64.c	2011-08-10 17:05:56.338457482 +1000
@@ -534,11 +534,11 @@ static unsigned long __init htab_get_tab
 }
 
 #ifdef CONFIG_MEMORY_HOTPLUG
-void create_section_mapping(unsigned long start, unsigned long end)
+int create_section_mapping(unsigned long start, unsigned long end)
 {
-	BUG_ON(htab_bolt_mapping(start, end, __pa(start),
+	return htab_bolt_mapping(start, end, __pa(start),
 				 pgprot_val(PAGE_KERNEL), mmu_linear_psize,
-				 mmu_kernel_ssize));
+				 mmu_kernel_ssize);
 }
 
 int remove_section_mapping(unsigned long start, unsigned long end)
Index: linux-build/arch/powerpc/include/asm/sparsemem.h
===================================================================
--- linux-build.orig/arch/powerpc/include/asm/sparsemem.h	2011-08-10 17:06:11.948728948 +1000
+++ linux-build/arch/powerpc/include/asm/sparsemem.h	2011-08-10 17:06:21.878901648 +1000
@@ -16,7 +16,7 @@
 #endif /* CONFIG_SPARSEMEM */
 
 #ifdef CONFIG_MEMORY_HOTPLUG
-extern void create_section_mapping(unsigned long start, unsigned long end);
+extern int create_section_mapping(unsigned long start, unsigned long end);
 extern int remove_section_mapping(unsigned long start, unsigned long end);
 #ifdef CONFIG_NUMA
 extern int hot_add_scn_to_nid(unsigned long scn_addr);
Index: linux-build/arch/powerpc/mm/mem.c
===================================================================
--- linux-build.orig/arch/powerpc/mm/mem.c	2011-08-10 17:06:42.539260996 +1000
+++ linux-build/arch/powerpc/mm/mem.c	2011-08-10 17:06:45.269308484 +1000
@@ -123,7 +123,8 @@ int arch_add_memory(int nid, u64 start,
 	pgdata = NODE_DATA(nid);
 
 	start = (unsigned long)__va(start);
-	create_section_mapping(start, start + size);
+	if (create_section_mapping(start, start + size))
+		return -EINVAL;
 
 	/* this should work for most non-highmem platforms */
 	zone = pgdata->node_zones;

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2011-08-11  6:44 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-08-11  6:44 [PATCH 1/4] [PATCH] powerpc: numa: Remove double of_node_put in hot_add_node_scn_to_nid Anton Blanchard
2011-08-11  6:44 ` [PATCH 2/4] [PATCH] powerpc: Use for_each_node_by_type instead of open coding it Anton Blanchard
2011-08-11  6:44 ` [PATCH 3/4] [PATCH] powerpc: Coding style cleanups Anton Blanchard
2011-08-11  6:44 ` [PATCH 4/4] powerpc: Fix oops when echoing bad values to /sys/devices/system/memory/probe Anton Blanchard

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).