From mboxrd@z Thu Jan 1 00:00:00 1970 From: Arjan van de Ven Date: Tue, 20 May 2008 22:14:37 -0000 Subject: [Ocfs2-devel] [RFC][PATCH 0/3] configfs: Make nested default groups lockdep-friendly In-Reply-To: <20080520215639.GG26609@mail.oracle.com> References: <20080520163320.025971210@kerlabs.com> <20080520095810.1d50d247@infradead.org> <20080520215639.GG26609@mail.oracle.com> Message-ID: <20080520151341.058f2df4@infradead.org> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Joel Becker Cc: Louis Rilling , linux-kernel@vger.kernel.org, ocfs2-devel@oss.oracle.com On Tue, 20 May 2008 14:56:39 -0700 Joel Becker 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 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 From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S934521AbYETWOV (ORCPT ); Tue, 20 May 2008 18:14:21 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1765828AbYETWN6 (ORCPT ); Tue, 20 May 2008 18:13:58 -0400 Received: from pentafluge.infradead.org ([213.146.154.40]:51523 "EHLO pentafluge.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1763773AbYETWN5 (ORCPT ); Tue, 20 May 2008 18:13:57 -0400 Date: Tue, 20 May 2008 15:13:41 -0700 From: Arjan van de Ven To: Joel Becker Cc: Louis Rilling , linux-kernel@vger.kernel.org, ocfs2-devel@oss.oracle.com Subject: Re: [RFC][PATCH 0/3] configfs: Make nested default groups lockdep-friendly Message-ID: <20080520151341.058f2df4@infradead.org> In-Reply-To: <20080520215639.GG26609@mail.oracle.com> References: <20080520163320.025971210@kerlabs.com> <20080520095810.1d50d247@infradead.org> <20080520215639.GG26609@mail.oracle.com> Organization: Intel X-Mailer: Claws Mail 3.3.1 (GTK+ 2.12.9; i386-redhat-linux-gnu) Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-SRS-Rewrite: SMTP reverse-path rewritten from by pentafluge.infradead.org See http://www.infradead.org/rpr.html Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, 20 May 2008 14:56:39 -0700 Joel Becker 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 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