On Fri, Sep 09, 2016 at 10:31:02AM +0300, Amir Goldstein wrote:
> On Thu, Sep 8, 2016 at 11:25 PM, Dave Chinner <david@xxxxxxxxxxxxx> wrote:
> > On Thu, Sep 08, 2016 at 06:29:54PM +0300, Amir Goldstein wrote:
> >> When copying up within the same fs, try to use f_op->copy_file_range().
> >> This becomes very efficient when lower and upper are on the same fs
> >> with file reflink support.
> >>
> >> Tested correct behavior when lower and upper are on:
> >> 1. same ext4 (copy)
> >> 2. same xfs + reflink patches + mkfs.xfs (copy)
> >> 3. same xfs + reflink patches + mkfs.xfs -m reflink=1 (clone)
> >> 4. different xfs + reflink patches + mkfs.xfs -m reflink=1 (copy)
> >>
> >> Verified that all the overlay xfstests pass in the 'same xfs+reflink'
> >> setup.
> >>
> >> For comparison, on my laptop, xfstest overlay/001 (copy up of large
> >> sparse files) takes less than 1 second in the xfs reflink setup vs.
> >> 25 seconds on the rest of the setups.
> >>
> >> Signed-off-by: Amir Goldstein <amir73il@xxxxxxxxx>
> >> ---
> >> fs/overlayfs/copy_up.c | 28 +++++++++++++++++++++++++---
> >> 1 file changed, 25 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c
> >> index 43fdc27..400567b 100644
> >> --- a/fs/overlayfs/copy_up.c
> >> +++ b/fs/overlayfs/copy_up.c
> >> @@ -121,6 +121,7 @@ static int ovl_copy_up_data(struct path *old, struct path *new, loff_t len)
> >> struct file *new_file;
> >> loff_t old_pos = 0;
> >> loff_t new_pos = 0;
> >> + int try_copy_file = 0;
> >> int error = 0;
> >>
> >> if (len == 0)
> >> @@ -136,6 +137,13 @@ static int ovl_copy_up_data(struct path *old, struct path *new, loff_t len)
> >> goto out_fput;
> >> }
> >>
> >> + /*
> >> + * When copying up within the same fs, try to use fs's copy_file_range
> >> + */
> >> + if (file_inode(old_file)->i_sb == file_inode(new_file)->i_sb) {
> >> + try_copy_file = (new_file->f_op->copy_file_range != NULL);
> >> + }
> >
> > You don't need this. .copy_file_range() should return -EXDEV when
> > you try to use it to copy files across different mount points or
> > superblocks.
> >
>
> Right.
>
> > i.e. you should probably be calling vfs_copy_file_range() here to do
> > the copy up, and if that fails (for whatever reason) then fall back
> > to the existing data copying code.
> >
>
> Yes, I considered that. With this V0 patch, copy_file_range() is
> called inside the copy data 'killable loop'
> but, unlike the slower splice, it tries to copy the entire remaining
> len on every cycle and will most likely get all or nothing without
> causing any major stalls.
> So my options for V1 are:
> 1. use the existing loop only fallback to splice on any
> copy_file_range() failure.
> 2. add another (non killable?) loop before the splice killable loop to
> try and copy up as much data with copy_file_range()
> 3. implement ovl_copy_up_file_range() and do the fallback near the
> call site of ovl_copy_up_data()
vfs_copy_file_range() already has a fallback to call
do_splice_direct() itself if ->copy_file_range() is not supported.
i.e. it will behave identically to the existing code if
copy_file_range is not supported by the underlying fs.
If copy_file_range() fails, then it's for a reason that will cause
do_splice_direct() to fail as well.
vfs_copy_file_range() should really be a direct replacement for any
code that calls do_splice_direct(). If it's not, we should make it
so (e.g call do_splice direct for cross-fs copies automatically
rather than returning EXDEV) and then replace all the calls in the
kernel to do_splice_direct() with vfs_copy_file_range()....
Cheers,
Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
--
To unsubscribe from this list: send the line "unsubscribe linux-unionfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
