On 05/03/2018 02:57 PM, 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". > > There are still differences between the loop in this function and > the loop in vfs_copy_file_range(). Perhaps the differences could > be smoothed away, I did not check recently. > There is a difference. copy_file_range(2) or do_splice_direct() can return short writes. I think this loop is performed to cover short writes. I suppose we could remove the clone_file_range() before the loop and replace the do_splice_direct() with vfs_copy_file_range(). -- Goldwyn
