Re: [PATCH] sysfs: differentiate between locking links and non-links

[ebiederm@xmission.com (Eric W. Biederman)]

Greg KH <gregkh@suse.de> writes:

> On Wed, Feb 10, 2010 at 10:25:21AM -0800, Eric W. Biederman wrote:
>> Tejun Heo <tj@kernel.org> writes:
>> 
>> > On 02/10/2010 05:03 PM, Eric W. Biederman wrote:
>> >> Tejun Heo <tj@kernel.org> writes:
>> >> 
>> >>> Hello,
>> >>>
>> >>> On 02/10/2010 11:08 AM, Américo Wang wrote:
>> >>>> This bug report is new for me. Recently we received lots of sysfs lockdep
>> >>>> warnings, I am working on a patch to fix all the bogus ones.
>> >>>>
>> >>>> However, this one is _not_ similar to the other cases, as you decribed.
>> >>>> This patch could fix the problem, but not a good fix, IMO. We need more
>> >>>> work in sysfs layer to fix this kind of things. I will take care of this.
>> >>>
>> >>> Can't we just give each s_active lock a separate class?  Would that be
>> >>> too costly?
>> >> 
>> >> When I asked the question earlier I was told that that locking classes
>> >> require static storage.  Where would that static storage come from?
>> >
>> > Maybe I'm glossly misunderstanding it but wouldn't embedding struct
>> > lockdep_map into sysfs_node as in work_struct do the trick?
>> 
>> In lockdep_init_map there is the following check:
>> 
>> 	/*
>> 	 * Sanity check, the lock-class key must be persistent:
>> 	 */
>> 	if (!static_obj(key)) {
>> 		printk("BUG: key %p not in .data!\n", key);
>> 		DEBUG_LOCKS_WARN_ON(1);
>> 		return;
>> 	}
>> 
>> It needs playing with but I think we can embed something in struct
>> attribute, and simply disallow dynamically allocated instances of
>> struct attribute.
>
> I think some code dynamically creates attributes today, as this has
> never been a restriction.
>
> So I don't know if this is going to work :(

I need to see how locks do this, but they face the same problem.  For
normal locks this is resolved by having a requirement to call a special
initializer in the dynamic case.  Adding that requirement for the
rare dynamically allocated sysfs attributes seems reasonable.

Additionally sysfs attributes are exactly the right granularity for
lock classes because that is where the behavior is the same or
changes.

We have 845 instances of static struct attributes which certainly makes
that the dominant case and worth aiming at.


Eric