lkml.org 
[lkml]   [2010]   [Feb]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [Patch 0/2] sysfs: fix s_active lockdep warning
From
Date
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


\
 
 \ /
  Last update: 2010-02-05 09:57    [W:1.252 / U:0.260 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site