On Feb 16, 2021, at 6:51 AM, Amir Goldstein [off-list ref] wrote:

quoted

quoted

This is easy to solve with a flag COPY_FILE_SPLICE (or something) that
is internal to kernel users.

FWIW, you may want to look at the loop in ovl_copy_up_data()
for improvements to nfsd_copy_file_range().

We can move the check out to copy_file_range syscall:

       if (flags != 0)
               return -EINVAL;

Leave the fallback from all filesystems and check for the
COPY_FILE_SPLICE flag inside generic_copy_file_range().

Ok, the diff bellow is just to make sure I understood your suggestion.

The patch will also need to:

- change nfs and overlayfs calls to vfs_copy_file_range() so that they
  use the new flag.

- check flags in generic_copy_file_checks() to make sure only valid flags
  are used (COPY_FILE_SPLICE at the moment).

Also, where should this flag be defined?  include/uapi/linux/fs.h?

Cheers,
--
Luis

diff --git a/fs/read_write.c b/fs/read_write.c
index 75f764b43418..341d315d2a96 100644
--- a/fs/read_write.c
+++ b/fs/read_write.c

@@ -1383,6 +1383,13 @@ ssize_t generic_copy_file_range(struct file *file_in, loff_t pos_in,
                               struct file *file_out, loff_t pos_out,
                               size_t len, unsigned int flags)

{
+       if (!(flags & COPY_FILE_SPLICE)) {
+               if (!file_out->f_op->copy_file_range)
+                       return -EOPNOTSUPP;
+               else if (file_out->f_op->copy_file_range !=
+                        file_in->f_op->copy_file_range)
+                       return -EXDEV;
+       }

That looks strange, because you are duplicating the logic in
do_copy_file_range(). Maybe better:

if (WARN_ON_ONCE(flags & ~COPY_FILE_SPLICE))
       return -EINVAL;
if (flags & COPY_FILE_SPLICE)
      return do_splice_direct(file_in, &pos_in, file_out, &pos_out,
                                len > MAX_RW_COUNT ? MAX_RW_COUNT : len, 0);
if (!file_out->f_op->copy_file_range)
       return -EOPNOTSUPP;
return -EXDEV;

This shouldn't return -EINVAL to userspace if the flag is not set.

That implies there *is* some valid way for userspace to call this
function, which is AFAICS not possible if COPY_FILE_SPLICE is only
available to in-kernel callers.  Instead, it should continue
to return -EOPNOTSUPP to userspace if copy_file_range() is not valid
for this combination of file descriptors, so that applications will
fall back to the non-CFR implementation.

The WARN_ON_ONCE(ret == -EOPNOTSUPP) in vfs_copy_file_range() would
also need to be removed if this will be triggered from userspace.


Cheers, Andreas