[Top] [All Lists]

Re: [PATCH 3/6] xfs: introduce inode cluster buffer trylocks for xfs_ifl

To: Dave Chinner <david@xxxxxxxxxxxxx>
Subject: Re: [PATCH 3/6] xfs: introduce inode cluster buffer trylocks for xfs_iflush
From: Alex Elder <aelder@xxxxxxx>
Date: Fri, 25 Mar 2011 16:00:50 -0500
Cc: xfs@xxxxxxxxxxx
In-reply-to: <1300860870-15471-4-git-send-email-david@xxxxxxxxxxxxx>
References: <1300860870-15471-1-git-send-email-david@xxxxxxxxxxxxx> <1300860870-15471-4-git-send-email-david@xxxxxxxxxxxxx>
Reply-to: aelder@xxxxxxx
On Wed, 2011-03-23 at 17:14 +1100, Dave Chinner wrote:
> From: Dave Chinner <dchinner@xxxxxxxxxx>
> There is an ABBA deadlock between synchronous inode flushing in
> xfs_reclaim_inode and xfs_icluster_free. xfs_icluster_free locks the
> buffer, then takes inode ilocks, whilst synchronous reclaim takes
> the ilock followed by the buffer lock in xfs_iflush().
> To avoid this deadlock, separate the inode cluster buffer locking
> semantics from the synchronous inode flush semantics, allowing
> callers to attempt to lock the buffer but still issue synchronous IO
> if it can get the buffer. This requires xfs_iflush() calls that
> currently use non-blocking semantics to pass SYNC_TRYLOCK rather
> than 0 as the flags parameter.
> This allows xfs_reclaim_inode to avoid the deadlock on the buffer
> lock and detect the failure so that it can drop the inode ilock and
> restart the reclaim attempt on the inode. This allows
> xfs_ifree_cluster to obtain the inode lock, mark the inode stale and
> release it and hence defuse the deadlock situation. It also has the
> pleasant side effect of avoiding IO in xfs_reclaim_inode when it
> tries to next reclaim the inode as it is now marked stale.

Looks good.

Reviewed-by: Alex Elder <aelder@xxxxxxx>

> Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx>
> Reviewed-by: Christoph Hellwig <hch@xxxxxx>

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