* [PATCH] of/irq: Fix device_node refcount in of_irq_parse_raw()
@ 2013-12-17 17:32 Cédric Le Goater
2013-12-30 2:37 ` Benjamin Herrenschmidt
0 siblings, 1 reply; 4+ messages in thread
From: Cédric Le Goater @ 2013-12-17 17:32 UTC (permalink / raw)
To: grant.likely; +Cc: Cédric Le Goater, linuxppc-dev, devicetree
Commit 2361613206e6, "of/irq: Refactor interrupt-map parsing" changed
the refcount on the device_node causing an error in of_node_put():
ERROR: Bad of_node_put() on /pci@800000020000000
CPU: 0 PID: 1 Comm: swapper/0 Not tainted 3.13.0-rc3-dirty #2
Call Trace:
[c00000003e403500] [c0000000000144fc] .show_stack+0x7c/0x1f0 (unreliable)
[c00000003e4035d0] [c00000000070f250] .dump_stack+0x88/0xb4
[c00000003e403650] [c0000000005e8768] .of_node_release+0xd8/0xf0
[c00000003e4036e0] [c0000000005eeafc] .of_irq_parse_one+0x10c/0x280
[c00000003e4037a0] [c0000000005efd4c] .of_irq_parse_pci+0x3c/0x1d0
[c00000003e403840] [c000000000038240] .pcibios_setup_device+0xa0/0x2e0
[c00000003e403910] [c0000000000398f0] .pcibios_setup_bus_devices+0x60/0xd0
[c00000003e403990] [c00000000003b3a4] .__of_scan_bus+0x1a4/0x2b0
[c00000003e403a80] [c00000000003a62c] .pcibios_scan_phb+0x30c/0x410
[c00000003e403b60] [c0000000009fe430] .pcibios_init+0x7c/0xd4
This patch adjusts the refcount in the walk of the interrupt tree.
When a match is found, there is no need to increase the refcount
on 'out_irq->np' as 'newpar' is already holding a ref. The refcount
balance between 'ipar' and 'newpar' is maintained in the skiplevel:
goto label.
This patch also removes the usage of the device_node variable 'old'
which seems useless after the latest changes.
Signed-off-by: Cédric Le Goater <clg@fr.ibm.com>
---
This patch was tested on powerpc, pseries and powernv. This is a
new area for me so I might have missed a path. Please take a look.
We could now introduce an helper routine to look for #address-cells in
of_irq_parse_raw(). This can be the subject of another patch.
Thanks,
C.
drivers/of/irq.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/of/irq.c b/drivers/of/irq.c
index 786b0b47fae4..27212402c532 100644
--- a/drivers/of/irq.c
+++ b/drivers/of/irq.c
@@ -165,7 +165,6 @@ int of_irq_parse_raw(const __be32 *addr, struct of_phandle_args *out_irq)
if (of_get_property(ipar, "interrupt-controller", NULL) !=
NULL) {
pr_debug(" -> got it !\n");
- of_node_put(old);
return 0;
}
@@ -250,8 +249,7 @@ int of_irq_parse_raw(const __be32 *addr, struct of_phandle_args *out_irq)
* Successfully parsed an interrrupt-map translation; copy new
* interrupt specifier into the out_irq structure
*/
- of_node_put(out_irq->np);
- out_irq->np = of_node_get(newpar);
+ out_irq->np = newpar;
match_array = imap - newaddrsize - newintsize;
for (i = 0; i < newintsize; i++)
@@ -268,7 +266,6 @@ int of_irq_parse_raw(const __be32 *addr, struct of_phandle_args *out_irq)
}
fail:
of_node_put(ipar);
- of_node_put(out_irq->np);
of_node_put(newpar);
return -EINVAL;
--
1.7.10.4
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] of/irq: Fix device_node refcount in of_irq_parse_raw()
2013-12-17 17:32 [PATCH] of/irq: Fix device_node refcount in of_irq_parse_raw() Cédric Le Goater
@ 2013-12-30 2:37 ` Benjamin Herrenschmidt
2013-12-30 2:42 ` Rob Herring
0 siblings, 1 reply; 4+ messages in thread
From: Benjamin Herrenschmidt @ 2013-12-30 2:37 UTC (permalink / raw)
To: Cédric Le Goater
Cc: grant.likely, devicetree, Thierry Reding, linuxppc-dev,
Linux Kernel list
On Tue, 2013-12-17 at 18:32 +0100, Cédric Le Goater wrote:
> Commit 2361613206e6, "of/irq: Refactor interrupt-map parsing" changed
> the refcount on the device_node causing an error in of_node_put():
Grant, Thierry, this is a regression, please send to Linus ASAP...
Cheers,
Ben.
> ERROR: Bad of_node_put() on /pci@800000020000000
> CPU: 0 PID: 1 Comm: swapper/0 Not tainted 3.13.0-rc3-dirty #2
> Call Trace:
> [c00000003e403500] [c0000000000144fc] .show_stack+0x7c/0x1f0 (unreliable)
> [c00000003e4035d0] [c00000000070f250] .dump_stack+0x88/0xb4
> [c00000003e403650] [c0000000005e8768] .of_node_release+0xd8/0xf0
> [c00000003e4036e0] [c0000000005eeafc] .of_irq_parse_one+0x10c/0x280
> [c00000003e4037a0] [c0000000005efd4c] .of_irq_parse_pci+0x3c/0x1d0
> [c00000003e403840] [c000000000038240] .pcibios_setup_device+0xa0/0x2e0
> [c00000003e403910] [c0000000000398f0] .pcibios_setup_bus_devices+0x60/0xd0
> [c00000003e403990] [c00000000003b3a4] .__of_scan_bus+0x1a4/0x2b0
> [c00000003e403a80] [c00000000003a62c] .pcibios_scan_phb+0x30c/0x410
> [c00000003e403b60] [c0000000009fe430] .pcibios_init+0x7c/0xd4
>
> This patch adjusts the refcount in the walk of the interrupt tree.
> When a match is found, there is no need to increase the refcount
> on 'out_irq->np' as 'newpar' is already holding a ref. The refcount
> balance between 'ipar' and 'newpar' is maintained in the skiplevel:
> goto label.
>
> This patch also removes the usage of the device_node variable 'old'
> which seems useless after the latest changes.
>
> Signed-off-by: Cédric Le Goater <clg@fr.ibm.com>
> ---
>
> This patch was tested on powerpc, pseries and powernv. This is a
> new area for me so I might have missed a path. Please take a look.
>
> We could now introduce an helper routine to look for #address-cells in
> of_irq_parse_raw(). This can be the subject of another patch.
>
> Thanks,
>
> C.
>
> drivers/of/irq.c | 5 +----
> 1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/drivers/of/irq.c b/drivers/of/irq.c
> index 786b0b47fae4..27212402c532 100644
> --- a/drivers/of/irq.c
> +++ b/drivers/of/irq.c
> @@ -165,7 +165,6 @@ int of_irq_parse_raw(const __be32 *addr, struct of_phandle_args *out_irq)
> if (of_get_property(ipar, "interrupt-controller", NULL) !=
> NULL) {
> pr_debug(" -> got it !\n");
> - of_node_put(old);
> return 0;
> }
>
> @@ -250,8 +249,7 @@ int of_irq_parse_raw(const __be32 *addr, struct of_phandle_args *out_irq)
> * Successfully parsed an interrrupt-map translation; copy new
> * interrupt specifier into the out_irq structure
> */
> - of_node_put(out_irq->np);
> - out_irq->np = of_node_get(newpar);
> + out_irq->np = newpar;
>
> match_array = imap - newaddrsize - newintsize;
> for (i = 0; i < newintsize; i++)
> @@ -268,7 +266,6 @@ int of_irq_parse_raw(const __be32 *addr, struct of_phandle_args *out_irq)
> }
> fail:
> of_node_put(ipar);
> - of_node_put(out_irq->np);
> of_node_put(newpar);
>
> return -EINVAL;
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] of/irq: Fix device_node refcount in of_irq_parse_raw()
2013-12-30 2:37 ` Benjamin Herrenschmidt
@ 2013-12-30 2:42 ` Rob Herring
2013-12-30 3:59 ` Benjamin Herrenschmidt
0 siblings, 1 reply; 4+ messages in thread
From: Rob Herring @ 2013-12-30 2:42 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: devicetree@vger.kernel.org, Linux Kernel list, Thierry Reding,
Grant Likely, linuxppc-dev, Cédric Le Goater
[-- Attachment #1: Type: text/plain, Size: 3817 bytes --]
On Sun, Dec 29, 2013 at 8:37 PM, Benjamin Herrenschmidt <
benh@kernel.crashing.org> wrote:
> On Tue, 2013-12-17 at 18:32 +0100, Cédric Le Goater wrote:
> > Commit 2361613206e6, "of/irq: Refactor interrupt-map parsing" changed
> > the refcount on the device_node causing an error in of_node_put():
>
> Grant, Thierry, this is a regression, please send to Linus ASAP...
>
>
I'm working on doing that right this moment.
Rob
> Cheers,
> Ben.
>
> > ERROR: Bad of_node_put() on /pci@800000020000000
> > CPU: 0 PID: 1 Comm: swapper/0 Not tainted 3.13.0-rc3-dirty #2
> > Call Trace:
> > [c00000003e403500] [c0000000000144fc] .show_stack+0x7c/0x1f0 (unreliable)
> > [c00000003e4035d0] [c00000000070f250] .dump_stack+0x88/0xb4
> > [c00000003e403650] [c0000000005e8768] .of_node_release+0xd8/0xf0
> > [c00000003e4036e0] [c0000000005eeafc] .of_irq_parse_one+0x10c/0x280
> > [c00000003e4037a0] [c0000000005efd4c] .of_irq_parse_pci+0x3c/0x1d0
> > [c00000003e403840] [c000000000038240] .pcibios_setup_device+0xa0/0x2e0
> > [c00000003e403910] [c0000000000398f0]
> .pcibios_setup_bus_devices+0x60/0xd0
> > [c00000003e403990] [c00000000003b3a4] .__of_scan_bus+0x1a4/0x2b0
> > [c00000003e403a80] [c00000000003a62c] .pcibios_scan_phb+0x30c/0x410
> > [c00000003e403b60] [c0000000009fe430] .pcibios_init+0x7c/0xd4
> >
> > This patch adjusts the refcount in the walk of the interrupt tree.
> > When a match is found, there is no need to increase the refcount
> > on 'out_irq->np' as 'newpar' is already holding a ref. The refcount
> > balance between 'ipar' and 'newpar' is maintained in the skiplevel:
> > goto label.
> >
> > This patch also removes the usage of the device_node variable 'old'
> > which seems useless after the latest changes.
> >
> > Signed-off-by: Cédric Le Goater <clg@fr.ibm.com>
> > ---
> >
> > This patch was tested on powerpc, pseries and powernv. This is a
> > new area for me so I might have missed a path. Please take a look.
> >
> > We could now introduce an helper routine to look for #address-cells in
> > of_irq_parse_raw(). This can be the subject of another patch.
> >
> > Thanks,
> >
> > C.
> >
> > drivers/of/irq.c | 5 +----
> > 1 file changed, 1 insertion(+), 4 deletions(-)
> >
> > diff --git a/drivers/of/irq.c b/drivers/of/irq.c
> > index 786b0b47fae4..27212402c532 100644
> > --- a/drivers/of/irq.c
> > +++ b/drivers/of/irq.c
> > @@ -165,7 +165,6 @@ int of_irq_parse_raw(const __be32 *addr, struct
> of_phandle_args *out_irq)
> > if (of_get_property(ipar, "interrupt-controller", NULL) !=
> > NULL) {
> > pr_debug(" -> got it !\n");
> > - of_node_put(old);
> > return 0;
> > }
> >
> > @@ -250,8 +249,7 @@ int of_irq_parse_raw(const __be32 *addr, struct
> of_phandle_args *out_irq)
> > * Successfully parsed an interrrupt-map translation; copy
> new
> > * interrupt specifier into the out_irq structure
> > */
> > - of_node_put(out_irq->np);
> > - out_irq->np = of_node_get(newpar);
> > + out_irq->np = newpar;
> >
> > match_array = imap - newaddrsize - newintsize;
> > for (i = 0; i < newintsize; i++)
> > @@ -268,7 +266,6 @@ int of_irq_parse_raw(const __be32 *addr, struct
> of_phandle_args *out_irq)
> > }
> > fail:
> > of_node_put(ipar);
> > - of_node_put(out_irq->np);
> > of_node_put(newpar);
> >
> > return -EINVAL;
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
[-- Attachment #2: Type: text/html, Size: 5511 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] of/irq: Fix device_node refcount in of_irq_parse_raw()
2013-12-30 2:42 ` Rob Herring
@ 2013-12-30 3:59 ` Benjamin Herrenschmidt
0 siblings, 0 replies; 4+ messages in thread
From: Benjamin Herrenschmidt @ 2013-12-30 3:59 UTC (permalink / raw)
To: Rob Herring
Cc: devicetree@vger.kernel.org, Linux Kernel list, Thierry Reding,
Grant Likely, linuxppc-dev, Cédric Le Goater
On Sun, 2013-12-29 at 20:42 -0600, Rob Herring wrote:
> On Sun, Dec 29, 2013 at 8:37 PM, Benjamin Herrenschmidt
> <benh@kernel.crashing.org> wrote:
>
> On Tue, 2013-12-17 at 18:32 +0100, Cédric Le Goater wrote:
> > Commit 2361613206e6, "of/irq: Refactor interrupt-map
> parsing" changed
> > the refcount on the device_node causing an error in
> of_node_put():
>
>
> Grant, Thierry, this is a regression, please send to Linus
> ASAP...
>
>
>
> I'm working on doing that right this moment.
Thanks, looks like I'm not the only trying to get some work done
today :)
Cheers,
Ben.
>
> Rob
>
> Cheers,
> Ben.
>
> > ERROR: Bad of_node_put() on /pci@800000020000000
> > CPU: 0 PID: 1 Comm: swapper/0 Not tainted 3.13.0-rc3-dirty
> #2
> > Call Trace:
> > [c00000003e403500] [c0000000000144fc] .show_stack+0x7c/0x1f0
> (unreliable)
> > [c00000003e4035d0] [c00000000070f250] .dump_stack+0x88/0xb4
> > [c00000003e403650] [c0000000005e8768] .of_node_release
> +0xd8/0xf0
> > [c00000003e4036e0] [c0000000005eeafc] .of_irq_parse_one
> +0x10c/0x280
> > [c00000003e4037a0] [c0000000005efd4c] .of_irq_parse_pci
> +0x3c/0x1d0
> > [c00000003e403840] [c000000000038240] .pcibios_setup_device
> +0xa0/0x2e0
> > [c00000003e403910]
> [c0000000000398f0] .pcibios_setup_bus_devices+0x60/0xd0
> > [c00000003e403990] [c00000000003b3a4] .__of_scan_bus
> +0x1a4/0x2b0
> > [c00000003e403a80] [c00000000003a62c] .pcibios_scan_phb
> +0x30c/0x410
> > [c00000003e403b60] [c0000000009fe430] .pcibios_init
> +0x7c/0xd4
> >
> > This patch adjusts the refcount in the walk of the interrupt
> tree.
> > When a match is found, there is no need to increase the
> refcount
> > on 'out_irq->np' as 'newpar' is already holding a ref. The
> refcount
> > balance between 'ipar' and 'newpar' is maintained in the
> skiplevel:
> > goto label.
> >
> > This patch also removes the usage of the device_node
> variable 'old'
> > which seems useless after the latest changes.
> >
> > Signed-off-by: Cédric Le Goater <clg@fr.ibm.com>
> > ---
> >
> > This patch was tested on powerpc, pseries and powernv. This
> is a
> > new area for me so I might have missed a path. Please take a
> look.
> >
> > We could now introduce an helper routine to look for
> #address-cells in
> > of_irq_parse_raw(). This can be the subject of another
> patch.
> >
> > Thanks,
> >
> > C.
> >
> > drivers/of/irq.c | 5 +----
> > 1 file changed, 1 insertion(+), 4 deletions(-)
> >
> > diff --git a/drivers/of/irq.c b/drivers/of/irq.c
> > index 786b0b47fae4..27212402c532 100644
> > --- a/drivers/of/irq.c
> > +++ b/drivers/of/irq.c
> > @@ -165,7 +165,6 @@ int of_irq_parse_raw(const __be32 *addr,
> struct of_phandle_args *out_irq)
> > if (of_get_property(ipar,
> "interrupt-controller", NULL) !=
> > NULL) {
> > pr_debug(" -> got it !\n");
> > - of_node_put(old);
> > return 0;
> > }
> >
> > @@ -250,8 +249,7 @@ int of_irq_parse_raw(const __be32 *addr,
> struct of_phandle_args *out_irq)
> > * Successfully parsed an interrrupt-map
> translation; copy new
> > * interrupt specifier into the out_irq
> structure
> > */
> > - of_node_put(out_irq->np);
> > - out_irq->np = of_node_get(newpar);
> > + out_irq->np = newpar;
> >
> > match_array = imap - newaddrsize - newintsize;
> > for (i = 0; i < newintsize; i++)
> > @@ -268,7 +266,6 @@ int of_irq_parse_raw(const __be32 *addr,
> struct of_phandle_args *out_irq)
> > }
> > fail:
> > of_node_put(ipar);
> > - of_node_put(out_irq->np);
> > of_node_put(newpar);
> >
> > return -EINVAL;
>
>
> --
>
> To unsubscribe from this list: send the line "unsubscribe
> devicetree" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at
> http://vger.kernel.org/majordomo-info.html
>
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2013-12-30 3:59 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-12-17 17:32 [PATCH] of/irq: Fix device_node refcount in of_irq_parse_raw() Cédric Le Goater
2013-12-30 2:37 ` Benjamin Herrenschmidt
2013-12-30 2:42 ` Rob Herring
2013-12-30 3:59 ` Benjamin Herrenschmidt
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).