linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/3] powerpc: numa: Remove double of_node_put in hot_add_node_scn_to_nid
@ 2011-08-10  6:49 Anton Blanchard
  2011-08-10  6:49 ` [PATCH 2/3] powerpc: Use for_each_node_by_type instead of open coding it Anton Blanchard
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Anton Blanchard @ 2011-08-10  6:49 UTC (permalink / raw)
  To: benh, paulus; +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 of_node_put for us so remove the
duplicate one inside the loop.

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-10 11:31:59.723379868 +1000
@@ -1214,7 +1214,6 @@ int hot_add_node_scn_to_nid(unsigned lon
 			break;
 		}
 
-		of_node_put(memory);
 		if (nid >= 0)
 			break;
 	}

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

* [PATCH 2/3] powerpc: Use for_each_node_by_type instead of open coding it
  2011-08-10  6:49 [PATCH 1/3] powerpc: numa: Remove double of_node_put in hot_add_node_scn_to_nid Anton Blanchard
@ 2011-08-10  6:49 ` Anton Blanchard
  2011-08-10  6:49 ` [PATCH 3/3] powerpc: Coding style cleanups Anton Blanchard
  2011-08-10  8:33 ` [PATCH 1/3] powerpc: numa: Remove double of_node_put in hot_add_node_scn_to_nid Stephen Rothwell
  2 siblings, 0 replies; 5+ messages in thread
From: Anton Blanchard @ 2011-08-10  6:49 UTC (permalink / raw)
  To: benh, paulus; +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] 5+ messages in thread

* [PATCH 3/3] powerpc: Coding style cleanups
  2011-08-10  6:49 [PATCH 1/3] powerpc: numa: Remove double of_node_put in hot_add_node_scn_to_nid Anton Blanchard
  2011-08-10  6:49 ` [PATCH 2/3] powerpc: Use for_each_node_by_type instead of open coding it Anton Blanchard
@ 2011-08-10  6:49 ` Anton Blanchard
  2011-08-10  8:33 ` [PATCH 1/3] powerpc: numa: Remove double of_node_put in hot_add_node_scn_to_nid Stephen Rothwell
  2 siblings, 0 replies; 5+ messages in thread
From: Anton Blanchard @ 2011-08-10  6:49 UTC (permalink / raw)
  To: benh, paulus; +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] 5+ messages in thread

* Re: [PATCH 1/3] powerpc: numa: Remove double of_node_put in hot_add_node_scn_to_nid
  2011-08-10  6:49 [PATCH 1/3] powerpc: numa: Remove double of_node_put in hot_add_node_scn_to_nid Anton Blanchard
  2011-08-10  6:49 ` [PATCH 2/3] powerpc: Use for_each_node_by_type instead of open coding it Anton Blanchard
  2011-08-10  6:49 ` [PATCH 3/3] powerpc: Coding style cleanups Anton Blanchard
@ 2011-08-10  8:33 ` Stephen Rothwell
  2011-08-10 11:28   ` Anton Blanchard
  2 siblings, 1 reply; 5+ messages in thread
From: Stephen Rothwell @ 2011-08-10  8:33 UTC (permalink / raw)
  To: Anton Blanchard; +Cc: paulus, linuxppc-dev

[-- Attachment #1: Type: text/plain, Size: 1198 bytes --]

Hi Anton,

On Wed, 10 Aug 2011 16:49:34 +1000 Anton Blanchard <anton@samba.org> wrote:
>
> During memory hotplug testing, I got the following warning:
> 
> 
> ERROR: Bad of_node_put() on /memory@0
> 
> of_find_node_by_type() loop does of_node_put for us so remove the
> duplicate one inside the loop.

But does an of_node_get() on its return value ..

> 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-10 11:31:59.723379868 +1000
> @@ -1214,7 +1214,6 @@ int hot_add_node_scn_to_nid(unsigned lon
>  			break;
>  		}
>  
> -		of_node_put(memory);
>  		if (nid >= 0)
>  			break;
>  	}

Won't that need an of_node_put(memory) after the loop if (nid >= 0) ?  In
fact you should be able to just move the of_node_put(memory) to after the
loop since of_node_put(NULL) is fine.

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 490 bytes --]

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

* Re: [PATCH 1/3] powerpc: numa: Remove double of_node_put in hot_add_node_scn_to_nid
  2011-08-10  8:33 ` [PATCH 1/3] powerpc: numa: Remove double of_node_put in hot_add_node_scn_to_nid Stephen Rothwell
@ 2011-08-10 11:28   ` Anton Blanchard
  0 siblings, 0 replies; 5+ messages in thread
From: Anton Blanchard @ 2011-08-10 11:28 UTC (permalink / raw)
  To: Stephen Rothwell; +Cc: paulus, linuxppc-dev

Hi Stephen,

> > 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-10
> > 11:31:59.723379868 +1000 @@ -1214,7 +1214,6 @@ int
> > hot_add_node_scn_to_nid(unsigned lon break; }
> >  
> > -		of_node_put(memory);
> >  		if (nid >= 0)
> >  			break;
> >  	}
> 
> Won't that need an of_node_put(memory) after the loop if (nid >=
> 0) ?  In fact you should be able to just move the of_node_put(memory)
> to after the loop since of_node_put(NULL) is fine.

Nice catch! Will respin the patch.

Anton

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

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

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-08-10  6:49 [PATCH 1/3] powerpc: numa: Remove double of_node_put in hot_add_node_scn_to_nid Anton Blanchard
2011-08-10  6:49 ` [PATCH 2/3] powerpc: Use for_each_node_by_type instead of open coding it Anton Blanchard
2011-08-10  6:49 ` [PATCH 3/3] powerpc: Coding style cleanups Anton Blanchard
2011-08-10  8:33 ` [PATCH 1/3] powerpc: numa: Remove double of_node_put in hot_add_node_scn_to_nid Stephen Rothwell
2011-08-10 11:28   ` 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).