Re: [PATCH] remap_range: merge do_clone_file_range() into vfs_clone_file_range()

[Jan Kara <jack@suse.cz>]

On Fri 02-02-24 14:08:07, Amir Goldstein wrote:
> On Fri, Feb 2, 2024 at 1:44 PM Jan Kara <jack@suse.cz> wrote:
> > > +     /* Try to use clone_file_range to clone up within the same fs */
> > > +     cloned = vfs_clone_file_range(old_file, 0, new_file, 0, len, 0);
> > > +     if (cloned == len)
> > > +             goto out_fput;
> > > +
> > > +     /* Couldn't clone, so now we try to copy the data */
> > >       error = rw_verify_area(READ, old_file, &old_pos, len);
> > >       if (!error)
> > >               error = rw_verify_area(WRITE, new_file, &new_pos, len);
> > >       if (error)
> > >               goto out_fput;
> >
> > Do we need to keep these rw_verify_area() checks here when
> > vfs_clone_file_range() already did remap_verify_area()?
> 
> Yes, because in the common case of no clone support (e.g. ext4),
> the permission hooks in vfs_clone_file_range() will not be called.
> 
> There is a corner case where fs supports clone, but for some reason
> rejects this specific clone request, although there is no apparent
> reason to reject a clone request for the entire file range.
> 
> In that case, permission hooks will be called twice - no big deal -
> that is exactly like a fallback in userspace cp --reflink=auto that
> will end up calling permission hooks twice in this corner case.

I see. Thanks for explanation!

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR