[PATCH] powerpc: add for_each_node_by_foo helpers

[Nathan Lynch]

Benjamin Herrenschmidt wrote:
> On Sun, 2006-03-12 at 16:18 -0600, Nathan Lynch wrote:
> > Benjamin Herrenschmidt wrote:
> > > On Wed, 2006-03-08 at 16:47 +0100, Christoph Hellwig wrote:
> > > > Typical use for of_find_node_by_name and of_find_node_by_type is to
> > > > iterate over all nodes of a given type/name.  Add a helper macro to
> > > > do that (in spirit of the list_for_each* macros).
> > > 
> > > Looks good. Maybe you have noted however that we aren't entirely safe
> > > yet vs. device-tree being dynamic however. There is a per-node lock, but
> > > no lock protecting the global node chain, which as usual means iterating
> > > the list isn't entirely safe vs. removal...
> > 
> > Eh?  There isn't any per-node lock, and the global chain is protected
> > by a rwlock (devtree_lock).
> 
> I recently showed a variety of scenarios that would blow up in funny
> ways.. It's not enough.

A variety of scenarios?  I'm aware of only one [1], which is kind of a
corner case, and has never come up in practice to my knowledge.

If you know of others, I'd like to know the details.


> It would only work if the global chain lock was
> taken by callers before iterating.

Are we talking about the same thing?  I'm talking about the
of_find_node_by_foo iterators and they all hold the lock while
traversing the tree.


[1] IIRC, it's something like this:

Nodes A, B, and C are adjacent in the allnext list -- A->allnext = B,
B->allnext = C.

A is unplugged from the tree (of_detach_node), but something still
holds a reference to it so A is not freed yet.

B is then unplugged from the tree, but we can't get to A to fix up
its allnext field to point to C.  B has no outstanding references
and is freed.

The user of A attempts to dereference A->allnext, but B isn't
there anymore.

This could happen with the sibling list and parent pointer as well.

So the basic issue is that traversing back into the tree from a node
that has been unplugged from the tree is invalid -- the state of the
tree is indeterminate with respect to the removed node's links into
it.

Seems to me that one way to address this is to keep a node from being
unplugged until all other users have dropped their references to it.