Re: [linux-kernel] Driver model: releasing parents before children

[Alan Stern]

Greg:

The more I think about your recent change to sysfs (to prevent parent 
kobjects from being deleted before their children) the more problems I 
see with it.

There's already the classic problem of having duplicate ->parent pointers
in struct device and struct device->kobj.  The difficulties this can cause
are well known...  If the two parent pointers disagree, which one is
right?  But if they always agree, what reason is there for having two of
them?  And if they're meant to always agree, what if you miss one spot in
the code which changes one of the pointers but not the other?

But let's leave that aside.  This is really about problems caused 
specifically by your patch.

There's a regrettable lack of symmetry now.  In a struct device's embedded
kobject, the parent pointer is set (and kobject_get() is called on the
parent) when the kobject is _registered_.  But the pointer and the
reference aren't dropped until the kobject is _cleaned up_ (as opposed to
_unregistered_).  This means that if a struct device is registered, then
unregistered, then registered again, there will be a dangling reference to
the original parent's kobject.

Another problem can occur if kobject_add() fails, say because of an error
during create_dir().  Although the reference to the parent is not
retained, kobj->parent remains set.  Consequently when the kobject is
eventually cleaned up, it will try to release a non-existent reference to
the parent.  This problem crops up whenever a kobject's parent pointer is
set without getting a reference to the parent.


It's not at all clear to me what the reason was for your patch in the
first place.  I guess you an across some situation where freeing a parent
before freeing its child would cause an error.  Normally this wouldn't be
a problem, since once the child was unregistered it would lose both the
reference and the pointer to its parent.  In your situation, the child
kobject must have been embedded in a larger structure that _did_ retain a
pointer to the parent even after the child was unregistered.  Maybe that
larger structure actually contained the parent kobject as well as the
child.

There are other ways of dealing with this situation that don't involve 
changing the driver model core.  Here are two possible approaches:

	When the driver initializes the child kobject, have it do 
	kobject_get() on the parent.  In the child's release routine 
	do kobject_put().

	When you unregister or release the parent, wait until each
	child has been released.

Neither of these would cause the problems that your patch raises.

Hoping I'm not totally out of line...

Alan Stern

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/