On Fri 02-02-24 14:08:07, Amir Goldstein wrote: > On Fri, Feb 2, 2024 at 1:44 PM Jan Kara <jack@xxxxxxx> 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@xxxxxxxx> SUSE Labs, CR
