Re: [PATCH v2 4/4] ovl: use vfs_copy_file_range() to copy up file data

[Dave Chinner <david@fromorbit.com>]

On Mon, Sep 12, 2016 at 06:06:43PM +0300, Amir Goldstein wrote:
> Use vfs_copy_file_range() helper instead of calling do_splice_direct()
> when copying up file data.
> When copying up within the same fs, which supports copy_file_range(),
> fs implementation can be more efficient then do_splice_direct().
> vfs_copy_file_range() helper falls back to do_splice_direct() if
> it cannot use the file system's copy_file_range() implementation.
> 
> 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)
> 
> 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.

This is now stale, right? the reflink is done from the
vfs_clone_file_range() call added in an earlier patch, not this
change....

> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> ---
>  fs/overlayfs/copy_up.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c
> index e432d7e..32ea54f 100644
> --- a/fs/overlayfs/copy_up.c
> +++ b/fs/overlayfs/copy_up.c
> @@ -159,15 +159,16 @@ static int ovl_copy_up_data(struct path *old, struct path *new, loff_t len)
>  			break;
>  		}
>  
> -		bytes = do_splice_direct(old_file, &old_pos,
> -					 new_file, &new_pos,
> -					 this_len, SPLICE_F_MOVE);
> +		bytes = vfs_copy_file_range(old_file, old_pos,
> +					    new_file, new_pos,
> +					    this_len, 0);
>  		if (bytes <= 0) {
>  			error = bytes;
>  			break;
>  		}
> -		WARN_ON(old_pos != new_pos);
>  
> +		old_pos += bytes;
> +		new_pos += bytes;
>  		len -= bytes;
>  	}

Patch does not remove the

	/* FIXME: copy up sparse files efficiently */

comment. Efficient copying of sparse files is something
vfs_copy_file_range() should do internally....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com