(cc'ing a few possibly interested people)
Brian King <brk...@linux.vnet.ibm.com> writes: > While testing fixes to the hvcs hotplug code, kmemleak was reporting > potential memory leaks. This was tracked down to the struct device_node > object associated with the hvcs device. Looking at the leaked > object in crash showed that the kref in the kobject in the device_node > had a reference count of 1 still, and the release function was never > getting called as a result of this. This adds an of_node_put in > pSeries_reconfig_remove_node in order to balance the refcounting > so that we actually free the device_node in the case of it being > allocated in pSeries_reconfig_add_node. My concern here would be whether the additional put is the right thing to do in all cases. The questions it raises for me are: - Is it safe for nodes that were present at boot, instead of added dynamically? - Is it correct for all types of nodes, or is there something specific to hvcs that leaves a dangling refcount? Just hoping we're not stepping into a situation where we're preventing leaks in some situations but doing use-after-free in others. :-) > > Signed-off-by: Brian King <brk...@linux.vnet.ibm.com> > --- > arch/powerpc/platforms/pseries/reconfig.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/arch/powerpc/platforms/pseries/reconfig.c > b/arch/powerpc/platforms/pseries/reconfig.c > index 599bd2c78514..8cb7309b19a4 100644 > --- a/arch/powerpc/platforms/pseries/reconfig.c > +++ b/arch/powerpc/platforms/pseries/reconfig.c > @@ -77,6 +77,7 @@ static int pSeries_reconfig_remove_node(struct device_node > *np) > } > > of_detach_node(np); > + of_node_put(np); > of_node_put(parent); > return 0; In a situation like this where the of_node_put() call isn't obviously connected to one of the of_ iterator APIs or similar, I would prefer a comment indicating which "get" it balances. I suppose it corresponds to the node initialization itself, i.e. the of_node_init() call sites in pSeries_reconfig_add_node() and drivers/of/fdt.c::populate_node().