Re: [PATCH 1/6] of: Do not free memory at of_node_release

[Guenter Roeck <linux@roeck-us.net>]

On Tue, Jun 24, 2014 at 11:23:35PM +0300, Pantelis Antoniou wrote:
> Hi Grant,
> 
> On Jun 24, 2014, at 11:21 PM, Grant Likely wrote:
> 
> > On Tue, 24 Jun 2014 17:23:35 +0300, Pantelis Antoniou <pantelis.antoniou@konsulko.com> wrote:
> >> Hi Grant,
> >> 
> >> On Jun 24, 2014, at 5:10 PM, Grant Likely wrote:
> >> 
> >>> On Sun, 22 Jun 2014 12:40:34 +0300, Pantelis Antoniou <pantelis.antoniou@konsulko.com> wrote:
> >>>> The life-cycle of nodes and properties does not allow us to release
> >>>> the memory taken by a device_node. Pointer to properties and nodes
> >>>> might still be in use in drivers, so any memory free'ing is dangerous.
> >>>> 
> >>>> Simply move all the properties to the deadprops list, and the node
> >>>> itself to of_alldeadnodes until the life-cycles issues are resolved.
> >>> 
> >>> Ummm. this looks wrong. The release function is supposed to be the place
> >>> to do the freeing, and with our discussion the other day about moving to
> >>> rcu, but keeping of_node_get/put() for anything that needs to hold a
> >>> long term reference, that means the lifecycle issues are pretty much
> >>> resolved. I don't think this patch is necessary.
> >>> 
> >> 
> >> Well, I thought about it too. This is the culmination of that process :)
> >> 
> >> The problem is not the node life-cycle, it's the properties. We can't
> >> tell for sure who and where has a pointer to the properties of the node,
> >> and when we free the node, the current code iterates over the properties
> >> list and frees them. This works only because in my tests, of_node_release
> >> only gets called when using overlays created nodes.
> > 
> > If a caller want's to use the value of a property, then it needs to
> > of_node_get() the node. Property lifecycle is strictly attached to the
> > lifecycle of it's node.
> > 
> 
> Taking bets if that's always the case?
> 
For my part I prefer to take the bet (and handle the consequences) over
a potential memory leak.

Guenter