xfs
[Top] [All Lists]

Re: Multi-CPU harmless lockdep on x86 while copying data

To: Dave Chinner <david@xxxxxxxxxxxxx>
Subject: Re: Multi-CPU harmless lockdep on x86 while copying data
From: Ben Myers <bpm@xxxxxxx>
Date: Mon, 10 Mar 2014 17:10:02 -0500
Cc: Christoph Hellwig <hch@xxxxxxxxxxxxx>, "Michael L. Semon" <mlsemon35@xxxxxxxxx>, xfs-oss <xfs@xxxxxxxxxxx>
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <20140310212430.GB6851@dastard>
References: <531BD8B9.1090400@xxxxxxxxx> <20140310025523.GV6851@dastard> <20140310103716.GA1431@xxxxxxxxxxxxx> <20140310204647.GW6851@dastard> <20140310211658.GT1935@xxxxxxx> <20140310212430.GB6851@dastard>
User-agent: Mutt/1.5.20 (2009-06-14)
On Tue, Mar 11, 2014 at 08:24:30AM +1100, Dave Chinner wrote:
> On Mon, Mar 10, 2014 at 04:16:58PM -0500, Ben Myers wrote:
> > Hi,
> > 
> > On Tue, Mar 11, 2014 at 07:46:47AM +1100, Dave Chinner wrote:
> > > On Mon, Mar 10, 2014 at 03:37:16AM -0700, Christoph Hellwig wrote:
> > > > On Mon, Mar 10, 2014 at 01:55:23PM +1100, Dave Chinner wrote:
> > > > > Changing the directory code to handle this sort of locking is going
> > > > > to require a bit of surgery. However, I can see advantages to moving
> > > > > directory data to the same locking strategy as regular file data -
> > > > > locking heirarchies are identical, directory ilock hold times are
> > > > > much reduced, we don't get lockdep whining about taking page faults
> > > > > with the ilock held, etc.
> > > > > 
> > > > > A quick hack at to demonstrate the high level, initial step of using
> > > > > the IOLOCK for readdir serialisation. I've done a little smoke
> > > > > testing on it, so it won't die immediately. It should get rid of all
> > > > > the nasty lockdep issues, but it doesn't start to address the deeper
> > > > > restructing that is needed.
> > > > 
> > > > What synchronization do we actually need from the iolock?  Pushing the
> > > > ilock down to where it's actually needed is a good idea either way,
> > > > though.
> > > 
> > > The issue is that if we push the ilock down to the just the block
> > > mapping routines, the directory can be modified while the readdir is
> > > in progress. That's the root problem that adding the ilock solved.
> > > Now, just pushing the ilock down to protect the bmbt lookups might
> > > result in a consistent lookup, but it won't serialise sanely against
> > > modifications.
> > > 
> > > i.e. readdir only walks one dir block at a time but
> > > it maps multiple blocks for readahead and keeps them in a local
> > > array and doesn't validate them again before issuing read o nthose
> > > buffers. Hence at a high level we currently have to serialise
> > > readdir against all directory modifications.
> > > 
> > > The only other option we might have is to completely rewrite the
> > > directory readahead code not to cache mappings. If we use the ilock
> > > purely for bmbt lookup and buffer read, then the ilock will
> > > serialise against modification, and the buffer lock will stabilise
> > > the buffer until the readdir moves to the next buffer and picks the
> > > ilock up again to read it.
> > > 
> > > That would avoid the need for high level serialisation, but it's a
> > > lot more work than using the iolock to provide the high level
> > > serialisation and i'm still not sure it's 100% safe. And I've got no
> > > idea if it would work for CXFS. Hopefully someone from SGI will
> > > chime in here....
> > 
> > Also in leaf and node formats a single modification can change multiple
> > buffers, so I suspect the buffer lock isn't enough serialization to 
> > maintain a
> > consistent directory in the face of multiple readers and writers.  The 
> > iolock
> > does resolve that issue.
> 
> Right, but we don't care about anything other than the leaf block
> that we are currently reading is consistent when the read starts and
> is consistent across the entire processing. i.e. if the leaf is locked by
> readdir, then the modification is completely stalled until the
> readdir lets it go. And readdir then can't get the next buffer until
> the modification is complete because it blocks on the ilock to get
> the next mapping and buffer....

As long as [you pointed out above] the readahead buffers aren't cached, and all
of the callers who do require that data/freeindex/node/leaf blocks be
consistent continue to take the ilock...  Yeah, I think that might work.

-Ben

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