[PATCH] xfs: Re-initialise lockdep context for all inodes in reclaim

[PATCH] xfs: Re-initialise lockdep context for all inodes in reclaim

[Dave Chinner]

From: Dave Chinner <dchinner@redhat.com>

We re-initialise the lockdep context for inode iolocks when dropping an inode,
but not when we delete an inode. Now that we can reclaim inodes from a shrinker,
we can get get false lockdep warnings about inode iolock inversions during
reclaim of deleted inodes. Hence we need to re-initialise the iolock in the
delete path as well.

Signed-off-by: Dave Chinner <dchinner@redhat.com>
---
 fs/xfs/linux-2.6/xfs_super.c |   14 ++++++++++++++
 1 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/fs/xfs/linux-2.6/xfs_super.c b/fs/xfs/linux-2.6/xfs_super.c
index 8119d1b..ba0e884 100644
--- a/fs/xfs/linux-2.6/xfs_super.c
+++ b/fs/xfs/linux-2.6/xfs_super.c
@@ -911,6 +911,20 @@ xfs_fs_destroy_inode(
 
 	XFS_STATS_INC(vn_reclaim);
 
+	/*
+	 * The iolock is used by the file system to coordinate reads,
+	 * writes, and block truncates.  Up to this point the lock
+	 * protected concurrent accesses by users of the inode.  But
+	 * from here forward we're doing some final processing of the
+	 * inode because we're done with it, and although we reuse the
+	 * iolock for protection it is really a distinct lock class
+	 * (in the lockdep sense) from before.  To keep lockdep happy
+	 * (and basically indicate what we are doing), we explicitly
+	 * re-init the iolock here.
+	 */
+	ASSERT(!rwsem_is_locked(&ip->i_iolock.mr_lock));
+	mrlock_init(&ip->i_iolock, MRLOCK_BARRIER, "xfsio", ip->i_ino);
+
 	/* bad inode, get out here ASAP */
 	if (is_bad_inode(inode))
 		goto out_reclaim;
-- 
1.5.6.5

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] xfs: Re-initialise lockdep context for all inodes in reclaim

[Christoph Hellwig]

On Thu, May 27, 2010 at 11:59:06AM +1000, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
> 
> We re-initialise the lockdep context for inode iolocks when dropping an inode,
> but not when we delete an inode. Now that we can reclaim inodes from a shrinker,
> we can get get false lockdep warnings about inode iolock inversions during
> reclaim of deleted inodes. Hence we need to re-initialise the iolock in the
> delete path as well.

clear_inode also gets called when we delete an inode, so at least
the rationale for this isn't quite right.  It seems like we re-acquire
new lockdep dependecies between clear_inode and destroy_inode that your
shrinker doesn't like.  What lockdep report do you see?

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] xfs: Re-initialise lockdep context for all inodes in reclaim

[Dave Chinner]

On Thu, May 27, 2010 at 08:21:38AM -0400, Christoph Hellwig wrote:
> On Thu, May 27, 2010 at 11:59:06AM +1000, Dave Chinner wrote:
> > From: Dave Chinner <dchinner@redhat.com>
> > 
> > We re-initialise the lockdep context for inode iolocks when dropping an inode,
> > but not when we delete an inode. Now that we can reclaim inodes from a shrinker,
> > we can get get false lockdep warnings about inode iolock inversions during
> > reclaim of deleted inodes. Hence we need to re-initialise the iolock in the
> > delete path as well.
> 
> clear_inode also gets called when we delete an inode, so at least
> the rationale for this isn't quite right.  It seems like we re-acquire
> new lockdep dependecies between clear_inode and destroy_inode that your
> shrinker doesn't like.  What lockdep report do you see?

I'll have to regenerate it - I saw it a couple of times with the
current shrinker, and it went away with this patch. I'll put this
on hold for the moment.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs