Messages in this thread | | | Subject | Re: [Patch 0/2] sysfs: fix s_active lockdep warning | From | (Eric W. Biederman) | Date | Fri, 05 Feb 2010 00:55:02 -0800 |
| |
Peter Zijlstra <peterz@infradead.org> writes:
> On Fri, 2010-01-29 at 12:30 -0800, Eric W. Biederman wrote: > >> We get false positives when the code of a sysfs attribute >> synchronously removes other sysfs attributes. In general that is not >> safe due to hotplug etc, but there are specific instances of static >> sysfs entries like the pm_core where it appears to be safe. >> >> I am not familiar with the device core lockdep issues. Are they similar? > > The device tree had the problem that we could basically hold a device > lock and an unspecified number of parent locks (iirc this was due to > device probing, where we hold the bus lock while probing/adding child > device, recursively). > > If we place each dev->lock into the same class (which would naively > happen), then this would lead to recursive lock warnings. The proposed > solution for this is to create MAX_LOCK_DEPTH classes and assign them to > the dev->lock depending on the depth in the device tree (Alan said that > MAX_LOCK_DEPTH is sufficient for all practical cases). > > static struct lock_class_key dev_tree_classes[MAX_LOCK_DEPTH]; > > device_add() or thereabouts would have something like: > > #ifdef CONFIG_PROVE_LOCKING > BUG_ON(dev->depth >= MAX_LOCK_DEPTH); > lockdep_set_class(dev->lock, &dev_tree_classes[dev->depth]); > #endif > > > Then there was a problem were we could lock all child devices while > holding the parent device lock (forgot why though), this would, on > taking the second child dev->lock, again lead to recursive lock > warnings. > > We have an annotation for that: lock_nest_lock (currently only > spin_lock_nest_lock exists, but mutex_lock_nest_lock is easily created), > and this would allow you to do things like: > > mutex_lock(&parent->lock); > for_each_device_child(child, parent) { > mutex_lock_nest_lock(&child->lock, &parent->lock); > ... > } > > > I hope this helps in figuring out the sysfs case..
Interesting. The sysfs attributes in general don't have this problem because the common case is effectively a reader/writer lock where we can have all of the readers we want.
In the sysfs case the classic problem is when an attribute grabs a lock and that lock is also held while the attribute is being deleted. I first found this with the rtnl_lock but there are other cases as well.
The particularly tricky case to handle in lockdep is when that other lock also the s_active lock from another part of the tree. There is an entire Alice style rabbit whole there, I am trying to figure out.
Eric
| |