Messages in this thread | | | Subject | Re: [Patch 0/2] sysfs: fix s_active lockdep warning | From | Peter Zijlstra <> | Date | Thu, 04 Feb 2010 12:38:16 +0100 |
| |
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..
| |