Thread (24 messages) 24 messages, 3 authors, 2016-09-14

Re: [PATCH v2 3/4] vfs: allow vfs_copy_file_range() across file systems

From: Amir Goldstein <amir73il@gmail.com>
Date: 2016-09-13 07:01:06
Also in: linux-fsdevel, linux-unionfs

On Tue, Sep 13, 2016 at 3:08 AM, Dave Chinner [off-list ref] wrote:
On Mon, Sep 12, 2016 at 06:06:42PM +0300, Amir Goldstein wrote:
quoted
copy_file_range syscall returns -EXDEV if src and dest
file are not on the same file system.
The vfs_copy_file_range() helper, however, knows how to copy
across file systems with do_splice_direct().

Move the enforcement of same file system from the vfs helper
to the syscall code.

A following patch is going to use the vfs_copy_file_range()
helper in overlayfs to copy up between lower and upper
not on the same file system.

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 fs/read_write.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/fs/read_write.c b/fs/read_write.c
index 9dc6e52..c4675c6 100644
--- a/fs/read_write.c
+++ b/fs/read_write.c
@@ -1502,10 +1502,6 @@ ssize_t vfs_copy_file_range(struct file *file_in, loff_t pos_in,
          (file_out->f_flags & O_APPEND))
              return -EBADF;

-     /* this could be relaxed once a method supports cross-fs copies */
-     if (inode_in->i_sb != inode_out->i_sb)
-             return -EXDEV;
-
      if (len == 0)
              return 0;
@@ -1514,7 +1510,8 @@ ssize_t vfs_copy_file_range(struct file *file_in, loff_t pos_in,
              return ret;

      ret = -EOPNOTSUPP;
-     if (file_out->f_op->copy_file_range)
+     if (inode_in->i_sb == inode_out->i_sb &&
+                     file_out->f_op->copy_file_range)
              ret = file_out->f_op->copy_file_range(file_in, pos_in, file_out,
                                                    pos_out, len, flags);
Indenting is wrong, and you dropped an important comment. i.e
copy_file_range() still doesn't support cross fs copies.....
OK. moved the comment down this this same i_sb test
quoted
      if (ret == -EOPNOTSUPP)
@@ -1569,6 +1566,14 @@ SYSCALL_DEFINE6(copy_file_range, int, fd_in, loff_t __user *, off_in,
              pos_out = f_out.file->f_pos;
      }

+     /*
+      * FIXME: should copy_file_range syscall enforce that src and
+      * dest files are on the same mount point or only on the same
+      * file system? none of the above?
+      */
+     if (file_inode(f_in.file)->i_sb != file_inode(f_out.file)->i_sb)
+             return -EXDEV;
For the purposes of this patch, it should simply do what it already
does. If there's a API semantic change that needs to be made, then
get that sorted out now rather than adding a "fixme" comment that
will simply be ignored....
How about:

        /*
         * vfs_copy_file_range() can do cross-fs copy, but we want to
         * fulfill the guaranty to userland that copy_file_range syscall
         * does not allow cross-fs copy
         */

Cheers,
Amir.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help