| Message ID | 20170728235843.25298-1-tuomas.tynkkynen@iki.fi |
|---|---|
| State | Accepted |
| Headers | show |
On Sat, Jul 29, 2017 at 02:58:43AM +0300, Tuomas Tynkkynen wrote: > The for_each_child_of_node macro itself maintains the correct reference > count of the nodes so the explicit of_node_put() call causes a warning: > > [ 0.098960] OF: ERROR: Bad of_node_put() on /pmc@7000e400/powergates/xusba > [ 0.098981] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.11.3 #1-NixOS > [ 0.098996] Hardware name: NVIDIA Jetson TX1 Developer Kit (DT) > [ 0.099011] Call trace: > [ 0.099034] [<ffff00000808a048>] dump_backtrace+0x0/0x2a0 > [ 0.099051] [<ffff00000808a30c>] show_stack+0x24/0x30 > [ 0.099069] [<ffff0000084a6494>] dump_stack+0x9c/0xc0 > [ 0.099090] [<ffff000008992214>] of_node_release+0xa4/0xa8 > [ 0.099107] [<ffff0000084a9270>] kobject_put+0x90/0x1f8 > [ 0.099124] [<ffff0000089914ac>] of_node_put+0x24/0x30 > [ 0.099140] [<ffff00000898cec4>] __of_get_next_child+0x4c/0x70 > [ 0.099155] [<ffff00000898cf28>] of_get_next_child+0x40/0x68 > [ 0.099173] [<ffff0000090a099c>] tegra_pmc_early_init+0x4e8/0x5ac > [ 0.099189] [<ffff00000808399c>] do_one_initcall+0x5c/0x168 > [ 0.099206] [<ffff000009050c98>] kernel_init_freeable+0xd4/0x240 > [ 0.099224] [<ffff000008b2d658>] kernel_init+0x18/0x108 > [ 0.099238] [<ffff0000080836c0>] ret_from_fork+0x10/0x50 > > (It's not very apparent from the OF documentation that of_node_put() is > not needed; the macro itself has no docstring and of_get_next_child() > used in the implementation begins with "Returns a node pointer with > refcount incremented" but then only at the very end of the docstring > the crucial part "Decrements the refcount of prev" is mentioned.) Yeah, this kind of mistake has been going around for a while. There's even a semantic patch that fixes this kind of error (see (scripts/coccinelle/iterators/device_node_continue.cocci). I wonder if adding kerneldoc to for_each_child_of_node() may help avoid this kind of mistake in the future. > Fixes: a38045121bf42 ("soc/tegra: pmc: Add generic PM domain support") > Signed-off-by: Tuomas Tynkkynen <tuomas.tynkkynen@iki.fi> > --- > Compile tested only (backtrace received via a distro bug report). > --- > drivers/soc/tegra/pmc.c | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) Applied, thanks. Thierry
diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c index e233dd5dcab3..0453ff6839a7 100644 --- a/drivers/soc/tegra/pmc.c +++ b/drivers/soc/tegra/pmc.c @@ -918,10 +918,8 @@ static void tegra_powergate_init(struct tegra_pmc *pmc, if (!np) return; - for_each_child_of_node(np, child) { + for_each_child_of_node(np, child) tegra_powergate_add(pmc, child); - of_node_put(child); - } of_node_put(np); }
The for_each_child_of_node macro itself maintains the correct reference count of the nodes so the explicit of_node_put() call causes a warning: [ 0.098960] OF: ERROR: Bad of_node_put() on /pmc@7000e400/powergates/xusba [ 0.098981] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.11.3 #1-NixOS [ 0.098996] Hardware name: NVIDIA Jetson TX1 Developer Kit (DT) [ 0.099011] Call trace: [ 0.099034] [<ffff00000808a048>] dump_backtrace+0x0/0x2a0 [ 0.099051] [<ffff00000808a30c>] show_stack+0x24/0x30 [ 0.099069] [<ffff0000084a6494>] dump_stack+0x9c/0xc0 [ 0.099090] [<ffff000008992214>] of_node_release+0xa4/0xa8 [ 0.099107] [<ffff0000084a9270>] kobject_put+0x90/0x1f8 [ 0.099124] [<ffff0000089914ac>] of_node_put+0x24/0x30 [ 0.099140] [<ffff00000898cec4>] __of_get_next_child+0x4c/0x70 [ 0.099155] [<ffff00000898cf28>] of_get_next_child+0x40/0x68 [ 0.099173] [<ffff0000090a099c>] tegra_pmc_early_init+0x4e8/0x5ac [ 0.099189] [<ffff00000808399c>] do_one_initcall+0x5c/0x168 [ 0.099206] [<ffff000009050c98>] kernel_init_freeable+0xd4/0x240 [ 0.099224] [<ffff000008b2d658>] kernel_init+0x18/0x108 [ 0.099238] [<ffff0000080836c0>] ret_from_fork+0x10/0x50 (It's not very apparent from the OF documentation that of_node_put() is not needed; the macro itself has no docstring and of_get_next_child() used in the implementation begins with "Returns a node pointer with refcount incremented" but then only at the very end of the docstring the crucial part "Decrements the refcount of prev" is mentioned.) Fixes: a38045121bf42 ("soc/tegra: pmc: Add generic PM domain support") Signed-off-by: Tuomas Tynkkynen <tuomas.tynkkynen@iki.fi> --- Compile tested only (backtrace received via a distro bug report). --- drivers/soc/tegra/pmc.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)