[v3,3/4] vfs: allow vfs_copy_file_range() across file systems
diff mbox

Message ID 1473856994-27463-4-git-send-email-amir73il@gmail.com
State New
Headers show

Commit Message

Amir Goldstein Sept. 14, 2016, 12:43 p.m. UTC
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 | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

Comments

Amir Goldstein Sept. 23, 2016, 7:57 a.m. UTC | #1
On Wed, Sep 14, 2016 at 3:43 PM, Amir Goldstein <amir73il@gmail.com> wrote:
> 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 | 16 +++++++++++-----
>  1 file changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/fs/read_write.c b/fs/read_write.c
> index 9dc6e52..6975fe8 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,9 @@ 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)
> +       /* copy_file_range() method does not support cross-fs copies */
> +       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);
>         if (ret == -EOPNOTSUPP)
> @@ -1569,6 +1567,14 @@ SYSCALL_DEFINE6(copy_file_range, int, fd_in, loff_t __user *, off_in,
>                 pos_out = f_out.file->f_pos;
>         }
>
> +       /*
> +        * 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
> +        */
> +       if (file_inode(f_in.file)->i_sb != file_inode(f_out.file)->i_sb)
> +               return -EXDEV;

Oops, that was supposed to be goto out;
Anyway, I am holding back on the vfs_copy_file_range() patches sub set
until I have a reliable test on xfs to fall back from clone to copy range

> +
>         ret = vfs_copy_file_range(f_in.file, pos_in, f_out.file, pos_out, len,
>                                   flags);
>         if (ret > 0) {
> --
> 2.7.4
>
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Darrick J. Wong Sept. 23, 2016, 3:19 p.m. UTC | #2
On Fri, Sep 23, 2016 at 10:57:56AM +0300, Amir Goldstein wrote:
> On Wed, Sep 14, 2016 at 3:43 PM, Amir Goldstein <amir73il@gmail.com> wrote:
> > 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 | 16 +++++++++++-----
> >  1 file changed, 11 insertions(+), 5 deletions(-)
> >
> > diff --git a/fs/read_write.c b/fs/read_write.c
> > index 9dc6e52..6975fe8 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,9 @@ 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)
> > +       /* copy_file_range() method does not support cross-fs copies */
> > +       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);
> >         if (ret == -EOPNOTSUPP)
> > @@ -1569,6 +1567,14 @@ SYSCALL_DEFINE6(copy_file_range, int, fd_in, loff_t __user *, off_in,
> >                 pos_out = f_out.file->f_pos;
> >         }
> >
> > +       /*
> > +        * 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
> > +        */
> > +       if (file_inode(f_in.file)->i_sb != file_inode(f_out.file)->i_sb)
> > +               return -EXDEV;
> 
> Oops, that was supposed to be goto out;
> Anyway, I am holding back on the vfs_copy_file_range() patches sub set
> until I have a reliable test on xfs to fall back from clone to copy range

I could just create another error injection point that'll make XFS pretend
that it's out of space.

--D

> 
> > +
> >         ret = vfs_copy_file_range(f_in.file, pos_in, f_out.file, pos_out, len,
> >                                   flags);
> >         if (ret > 0) {
> > --
> > 2.7.4
> >
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch
diff mbox

diff --git a/fs/read_write.c b/fs/read_write.c
index 9dc6e52..6975fe8 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,9 @@  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)
+	/* copy_file_range() method does not support cross-fs copies */
+	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);
 	if (ret == -EOPNOTSUPP)
@@ -1569,6 +1567,14 @@  SYSCALL_DEFINE6(copy_file_range, int, fd_in, loff_t __user *, off_in,
 		pos_out = f_out.file->f_pos;
 	}
 
+	/*
+	 * 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
+	 */
+	if (file_inode(f_in.file)->i_sb != file_inode(f_out.file)->i_sb)
+		return -EXDEV;
+
 	ret = vfs_copy_file_range(f_in.file, pos_in, f_out.file, pos_out, len,
 				  flags);
 	if (ret > 0) {