Messages in this thread | | | Date | Fri, 19 Dec 2003 10:42:18 -0500 (EST) | From | Alan Stern <> | Subject | Re: [linux-kernel] Driver model: releasing parents before children |
| |
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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |