[Top] [All Lists]

Re: [3.0-stable PATCH 05/36] xfs: only take the ILOCK in xfs_reclaim_ino

To: Mark Tinguely <tinguely@xxxxxxx>
Subject: Re: [3.0-stable PATCH 05/36] xfs: only take the ILOCK in xfs_reclaim_inode()
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Wed, 12 Dec 2012 10:34:12 +1100
Cc: stable@xxxxxxxxxxxxxxx, xfs@xxxxxxxxxxx
In-reply-to: <20121203144309.267172499@xxxxxxx>
References: <20121203144208.143464631@xxxxxxx> <20121203144309.267172499@xxxxxxx>
User-agent: Mutt/1.5.21 (2010-09-15)
On Mon, Dec 03, 2012 at 05:42:13PM -0600, Mark Tinguely wrote:
> From: Alex Elder <elder@xxxxxxxxxxxxx>
> Upstream commit: ad637a10f444fc66b1f6d4a28fe30d4c61ed0161
> At the end of xfs_reclaim_inode(), the inode is locked in order to
> we wait for a possible concurrent lookup to complete before the
> inode is freed.  This synchronization step was taking both the ILOCK
> and the IOLOCK, but the latter was causing lockdep to produce
> reports of the possibility of deadlock.
> It turns out that there's no need to acquire the IOLOCK at this
> point anyway.  It may have been required in some earlier version of
> the code, but there should be no need to take the IOLOCK in
> xfs_iget(), so there's no (longer) any need to get it here for
> synchronization.  Add an assertion in xfs_iget() as a reminder
> of this assumption.
> Dave Chinner diagnosed this on IRC, and Christoph Hellwig suggested
> no longer including the IOLOCK.  I just put together the patch.
> Signed-off-by: Alex Elder <elder@xxxxxxxxxxxxx>
> Reviewed-by: Dave Chinner <dchinner@xxxxxxxxxx>
> Reviewed-by: Christoph Hellwig <hch@xxxxxx>
> Signed-off-by: Ben Myers <bpm@xxxxxxx>

I'm not sure we should be changing this just to keep lockdep quiet.
I'd prefer we don't make random locking changes to the stable


Dave Chinner

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