[PATCH] xfs: don't take the i_mutex in ->fsync()

[PATCH] xfs: don't take the i_mutex in ->fsync()

[Josef Bacik]

In pushing down the i_mutex into the ->fsync() handlers I introduced a deadlock
since xfs will call xfs_file_fsync in the O_SYNC case while still holding the
i_mutex case.  In talking with Dave he said that xfs doesn't really need to hold
the i_mutex in xfs_file_sync() so just drop the taking of i_mutex and we should
be good to go.  Thanks,

Signed-off-by: Josef Bacik <josef@redhat.com>
---
 fs/xfs/linux-2.6/xfs_file.c |    8 +-------
 1 files changed, 1 insertions(+), 7 deletions(-)

diff --git a/fs/xfs/linux-2.6/xfs_file.c b/fs/xfs/linux-2.6/xfs_file.c
index 9183f75..b928cb5 100644
--- a/fs/xfs/linux-2.6/xfs_file.c
+++ b/fs/xfs/linux-2.6/xfs_file.c
@@ -144,12 +144,8 @@ xfs_file_fsync(
 	if (error)
 		return error;
 
-	mutex_lock(&inode->i_mutex);
-
-	if (XFS_FORCED_SHUTDOWN(mp)) {
-		mutex_unlock(&inode->i_mutex);
+	if (XFS_FORCED_SHUTDOWN(mp))
 		return -XFS_ERROR(EIO);
-	}
 
 	xfs_iflags_clear(ip, XFS_ITRUNCATED);
 
@@ -205,7 +201,6 @@ xfs_file_fsync(
 				XFS_FSYNC_TS_LOG_RES(mp), 0, 0, 0);
 		if (error) {
 			xfs_trans_cancel(tp, 0);
-			mutex_unlock(&inode->i_mutex);
 			return -error;
 		}
 		xfs_ilock(ip, XFS_ILOCK_EXCL);
@@ -255,7 +250,6 @@ xfs_file_fsync(
 	    !log_flushed)
 		xfs_blkdev_issue_flush(mp->m_ddev_targp);
 
-	mutex_unlock(&inode->i_mutex);
 	return -error;
 }
 
-- 
1.7.5.2

Re: [PATCH] xfs: don't take the i_mutex in ->fsync()

[Christoph Hellwig]

On Tue, Jul 19, 2011 at 08:41:33AM -0400, Josef Bacik wrote:
> In pushing down the i_mutex into the ->fsync() handlers I introduced a deadlock
> since xfs will call xfs_file_fsync in the O_SYNC case while still holding the
> i_mutex case.  In talking with Dave he said that xfs doesn't really need to hold
> the i_mutex in xfs_file_sync() so just drop the taking of i_mutex and we should
> be good to go.  Thanks,

We need some serialized around the xfs_ioend_wait call to prvent new
ioends coming in from starving our wait, be that i_mutex or the iolock.

The right fix for you patch for now is fixing xfs_file_aio_write to
call xfs_file_fsync correctly, that is removing the
filemap_write_and_wait_range call, and instead moving the fsync call
into the section where we have dropped the iolock and i_mutex.

Dave and I can then later look into dropping i_mutex in fsync and
replacing it with the iolock and/or finding a way to reduce the number
of lock roundtrips in the O_SYNC path.