[Top] [All Lists]

Re: [PATCH] xfs: provide a inode iolock lockdep class

To: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Subject: Re: [PATCH] xfs: provide a inode iolock lockdep class
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Thu, 16 Dec 2010 09:57:10 +1100
Cc: xfs@xxxxxxxxxxx
In-reply-to: <20101215121811.GA3122@xxxxxxxxxxxxx>
References: <1292202431-15320-1-git-send-email-david@xxxxxxxxxxxxx> <20101215121811.GA3122@xxxxxxxxxxxxx>
User-agent: Mutt/1.5.20 (2009-06-14)
On Wed, Dec 15, 2010 at 07:18:11AM -0500, Christoph Hellwig wrote:
> On Mon, Dec 13, 2010 at 12:07:11PM +1100, Dave Chinner wrote:
> > From: Dave Chinner <dchinner@xxxxxxxxxx>
> > 
> > The XFS iolock needs to be re-initialised to a new lock class before
> > it enters reclaim to prevent lockdep false positives. Unfortunately,
> > this is not sufficient protection as inodes in the XFS_IRECLAIMABLE
> > state can be recycled and not re-initialised before being reused.
> > 
> > We need to re-initialise the lock state when transfering out of
> > XFS_IRECLAIMABLE state to XFS_INEW, but we need to keep the same
> > class as if the inode was just allocated. Hence we need a specific
> > lockdep class variable for the iolock so that both initialisations
> > use the same class.
> > 
> > While there, add a specific class for inodes in the reclaim state so
> > that it is easy to tell from lockdep reports what state the inode
> > was in that generated the report.
> Looks good to me.  As long as we have the mrlock abstraction we might as
> well hide this behind it, but as I plan on killing the abstraction
> that's probably not worth the effort.

I thought about doing that, too, after looking at what the abstraction
still provides us with. I think it only provides an extra "is write
locked" debug check which I think could be added to the generic
rwsem code pretty easily. But I've got a big enough patch stack out
for review right now. ;)

> Reviewed-by: Christoph Hellwig <hch@xxxxxx>

Thanks, I'll add it to the for-2.6.38 stack.


Dave Chinner

<Prev in Thread] Current Thread [Next in Thread>