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

["Darrick J. Wong" <darrick.wong@oracle.com>]

On Fri, Sep 23, 2016 at 09:52:42PM +0300, Amir Goldstein wrote:
> On Fri, Sep 23, 2016 at 7:13 PM, Darrick J. Wong
> <darrick.wong@oracle.com> wrote:
> > 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
> >
> > Ok, attached are two rough patches -- one to add the error injection point
> > into the kernel, and a second one to add it to the xfs_io 'inject' command.
> > Note that you'll have to format the XFS filesystem with rmapbt=1 since we
> > can't otherwise avoid per-AG ENOSPC if rmap is enabled.
> >
> > The relevant xfstests commands are:
> >
> > _require_xfs_io_error_injection "ag_resv_critical"
> > _scratch_inject_error "ag_resv_critical"
> >
> > See the xfs/325 test for a rough framework.  I'll work on cleaning up the
> > patches and trying to get them into 4.9.
> >
> 
> Thanks, Darrick, but I'm not sure that's enough. does the framework allow
> to inject an error for a specific AG? otherwise, the code will not
> fall back from
> failing full reflink to partial copy partial reflink.

You could change XFS_RANDOM_AG_RESV_CRITICAL to a non-1 value to make it
fail randomly (instead of all the time) and reflink a sparse file with a
large number of extents.  This...

#define XFS_RANDOM_AG_RESV_CRITICAL 2

...would make it fail half the time.

--D

> --
> 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