Re: [RFC][PATCH 0/3] configfs: Make nested default groups lockdep-friendly

[Louis Rilling]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Peter Zijlstra a écrit :
> On Tue, 2008-05-20 at 15:13 -0700, Arjan van de Ven wrote:
>> On Tue, 20 May 2008 14:56:39 -0700
>> Joel Becker <Joel.Becker@xxxxxxxxxx> wrote:
>>
>>> On Tue, May 20, 2008 at 09:58:10AM -0700, Arjan van de Ven wrote:
>>>> On Tue, 20 May 2008 18:33:20 +0200
>>>> Louis Rilling <Louis.Rilling@xxxxxxxxxxx> wrote:
>>>>
>>>>> The following patches fix lockdep warnings resulting from
>>>>> (correct) recursive locking in configfs.
>>>>>
>>>>> ...
>>>>>
>>>>> Since lockdep does not handle such correct recursion, the idea is
>>>>> to insert lockdep_off()/lockdep_on() for inode mutexes as soon as
>>>>> the level of recursion of the I_MUTEX_PARENT -> I_MUTEX_CHILD
>>>>> dependency pattern increases.
>>>> I'm... not entirely happy with such a solution ;(
>>>>
>>>> there must be a better one.
>>> 	We're trying to find it.  I really appreciate Louis taking the
>>> time to approach the issue.  His first pass was to add 1 to
>>> MUTEX_CHILD for each level of recursion.  This has a very tight limit
>>> (4 or 5 levels), but probably covers all users that exist and perhaps
>>> all that ever will exist.  However, it means passing the lockdep
>>> annotation level throughout the entire call chain across multiple
>>> files.  It was definitely less readable.
>>> 	This approach takes a different tack - it's very readable, but
>>> it assumes that the currently correct locking will always remain so -
>>> a particular invariant that lockdep exists to verify :-)
>>> 	Louis, what about sticking the recursion level on
>>> configfs_dirent?  That is, you could add sd->s_level and then use it
>>> when needed.  THis would hopefully avoid having to pass the level as
>>> an argument to every function.  Then we can go back to your original
>>> scheme.  If they recurse too much and hit the lockdep limit, just
>>> rewind everything and return -ELOOP.
>> you can also make a new lockdep key for each level... not pretty but it
>> works
> 
> Yeah, that is what I've done in the past for trees:
> 
> http://programming.kicks-ass.net/kernel-patches/concurrent-pagecache/23-rc1-rt/radix-concurrent-lockdep.patch

Thanks for pointing this out.

Yes this could solve part of the issue, at the price of duplicating the
inode mutex class. However, this still does not solve the issue when
deleting config_groups, since in that case all nodes of the tree are
locked. Thinking about adding lockdep support for concurrent locking of
the direct children of a node in a tree...

Louis

- --
Dr Louis Rilling			Kerlabs
Skype: louis.rilling			Batiment Germanium
Phone: (+33|0) 6 80 89 08 23		80 avenue des Buttes de Coesmes
http://www.kerlabs.com/			35700 Rennes
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFIM/ifVKcRuvQ9Q1QRAmahAJ9n07/o3pgOteOgRnMR9a0iGDEFWQCgurOH
9MZ5E9iytPgn/v7QAxDTFrM=
=Q+7A
-----END PGP SIGNATURE-----
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/