On 05/03/2018 05:11 PM, Dave Chinner wrote: > On Thu, May 03, 2018 at 10:57:09PM +0300, Amir Goldstein wrote: >> On Thu, May 3, 2018 at 6:26 PM, Goldwyn Rodrigues <rgoldwyn@xxxxxxx> wrote: >>> From: Goldwyn Rodrigues <rgoldwyn@xxxxxxxx> >>> >>> Signed-off-by: Goldwyn Rodrigues <rgoldwyn@xxxxxxxx> >>> --- >>> fs/overlayfs/copy_up.c | 2 +- >>> fs/read_write.c | 10 ++++++---- >>> include/linux/fs.h | 2 ++ >>> 3 files changed, 9 insertions(+), 5 deletions(-) >>> >>> diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c >>> index 8bede0742619..6634a85255ae 100644 >>> --- a/fs/overlayfs/copy_up.c >>> +++ b/fs/overlayfs/copy_up.c >>> @@ -175,7 +175,7 @@ static int ovl_copy_up_data(struct path *old, struct path *new, loff_t len) >>> break; >>> } >>> >>> - bytes = do_splice_direct(old_file, &old_pos, >>> + bytes = splice_with_holes(old_file, &old_pos, >>> new_file, &new_pos, >>> this_len, SPLICE_F_MOVE); >> >> >> Add.. you can remove this comment above :) >> /* FIXME: copy up sparse files efficiently */ >> >> For the record, when I added vfs_clone_file_range() above, >> Dave Chinner has suggested to replace the entire block with >> vfs_copy_file_range(), which would do all the fallbacks. >> Since then, vfs_copy_file_range() gained "try to clone first". > > Yup, we want the copy offload infrastructure to be used if at all > possible, so we get consistent behaviour for everyone trying to > optimise copy behaviour. In this case, I think that it is relevant > that we heard at LSFMM that userspace utils don't want to use > copy_file_range() because it doesn't "optimise for sparse files". Sorry, I did not cc you, but did you check the [2/3] of this patchset [2]? > > From that perspective, perhaps this needs "hole preserving copy" > behaviour needs to be moved inside copy-file_range() (and therefore > do_splice_direct()) and triggered by a new flag for > copy_file_range(). e.g. COPY_FILE_SPARSE. That way callers can tell > the kernel they want a sparse copy, and the kernel can attempt that > rather a copy that converts holes to zeros. In the patchset description [0], I ask if this should be the default feature (I think it should be) and holes should be converted to zeros as a special flag.. The only argument against it could be that applications are already using this interface, so this would be a change in behavior. > > In most cases, filesystems that implement efficient offloads already > preserve sparseness, but the do_splice_direct() fallback does not. > If we fix that, then we're a big step closer to getting utilities > like cp and rsync to use copy_file_range() instead of bit shuffling > through userspace to copy data..... Yes, I agree and hence the patchset. Besides, I think it should work across filesystems too [1]. > >> I am not asking that you do this as part of your work, simply >> pointing out an opportunity. > > *nod* > > Cheers, > > Dave. > [0] https://marc.info/?l=linux-fsdevel&m=152536120311694&w=2 [1] https://marc.info/?l=linux-fsdevel&m=152536121111700&w=2 [2] https://marc.info/?l=linux-fsdevel&m=152536121111700&w=2 -- Goldwyn
