[PATCH 2/5] splice: nest i_mutex outside pipe_lock

[Christoph Hellwig <hch@xxxxxxxxxxxxx>]

I can't find any obvious reason why we would want to nest i_mutex
inside the pipe lock, but two good reasons speak against it:

 - with i_mutex inside the pipe lock we have to unlock it every time we
   iterate to the next buffer, which prevents i_mutex from guaranteeing
   write atomicy similar to write(2), and also is ineffiecient for
   filesystems like ocfs2 that have additional cluster locking along
   i_mutex.
 - the ordering of pipe_lock outside i_mutex makes it very hard to
   share code with filesystems that have additional inode-wide locks
   that need to be taken in the right order with i_mutex.

In addition to changing the lock order this patch also removes the
useless I_MUTEX_CHILD annotations.

Signed-off-by: Christoph Hellwig <hch@xxxxxx>
---
 fs/ocfs2/file.c |   20 +++++++++++---------
 fs/splice.c     |    5 +++--
 2 files changed, 14 insertions(+), 11 deletions(-)

diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c
index a77ef6e..c68e111 100644
--- a/fs/ocfs2/file.c
+++ b/fs/ocfs2/file.c
@@ -2461,6 +2461,12 @@ static ssize_t ocfs2_file_splice_write(struct pipe_inode_info *pipe,
 			out->f_path.dentry->d_name.len,
 			out->f_path.dentry->d_name.name, len);
 
+	mutex_lock(&inode->i_mutex);
+	ret = ocfs2_rw_lock(inode, 1);
+	if (ret < 0) {
+		mlog_errno(ret);
+		goto out_unlock_inode;
+	}
 	pipe_lock(pipe);
 
 	splice_from_pipe_begin(&sd);
@@ -2469,20 +2475,16 @@ static ssize_t ocfs2_file_splice_write(struct pipe_inode_info *pipe,
 		if (ret <= 0)
 			break;
 
-		mutex_lock_nested(&inode->i_mutex, I_MUTEX_CHILD);
-		ret = ocfs2_rw_lock(inode, 1);
-		if (ret < 0)
-			mlog_errno(ret);
-		else {
-			ret = ocfs2_splice_to_file(pipe, out, &sd);
-			ocfs2_rw_unlock(inode, 1);
-		}
-		mutex_unlock(&inode->i_mutex);
+		ret = ocfs2_splice_to_file(pipe, out, &sd);
 	} while (ret > 0);
 	splice_from_pipe_end(pipe, &sd);
 
 	pipe_unlock(pipe);
 
+	ocfs2_rw_unlock(inode, 1);
+out_unlock_inode:
+	mutex_unlock(&inode->i_mutex);
+
 	if (sd.num_spliced)
 		ret = sd.num_spliced;
 
diff --git a/fs/splice.c b/fs/splice.c
index fcb459d..4fb6c1f 100644
--- a/fs/splice.c
+++ b/fs/splice.c
@@ -1005,6 +1005,8 @@ generic_file_splice_write(struct pipe_inode_info *pipe, struct file *out,
 	};
 	ssize_t ret;
 
+	mutex_lock(&inode->i_mutex);
+
 	pipe_lock(pipe);
 
 	splice_from_pipe_begin(&sd);
@@ -1013,7 +1015,6 @@ generic_file_splice_write(struct pipe_inode_info *pipe, struct file *out,
 		if (ret <= 0)
 			break;
 
-		mutex_lock_nested(&inode->i_mutex, I_MUTEX_CHILD);
 		ret = file_remove_suid(out);
 		if (!ret) {
 			ret = file_update_time(out);
@@ -1021,11 +1022,11 @@ generic_file_splice_write(struct pipe_inode_info *pipe, struct file *out,
 				ret = splice_from_pipe_feed(pipe, &sd,
 							    pipe_to_file);
 		}
-		mutex_unlock(&inode->i_mutex);
 	} while (ret > 0);
 	splice_from_pipe_end(pipe, &sd);
 
 	pipe_unlock(pipe);
+	mutex_unlock(&inode->i_mutex);
 
 	if (sd.num_spliced)
 		ret = sd.num_spliced;
-- 
1.7.10.4


--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html