Re: [PATCH] copy_file_range.2: Kernel v5.12 updates

On Sat, Feb 27, 2021 at 12:19 AM Alejandro Colomar (man-pages)
[off-list ref] wrote:

Hello Amir, Luis,

On 2/24/21 5:10 PM, Amir Goldstein wrote:

quoted

On Wed, Feb 24, 2021 at 4:22 PM Luis Henriques [off-list ref] wrote:

quoted

Update man-page with recent changes to this syscall.

Signed-off-by: Luis Henriques <redacted>
---
Hi!

Here's a suggestion for fixing the manpage for copy_file_range().  Note that
I've assumed the fix will hit 5.12.

  man2/copy_file_range.2 | 10 +++++++++-
  1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/man2/copy_file_range.2 b/man2/copy_file_range.2
index 611a39b8026b..b0fd85e2631e 100644
--- a/man2/copy_file_range.2
+++ b/man2/copy_file_range.2

@@ -169,6 +169,9 @@ Out of memory.
  .B ENOSPC
  There is not enough space on the target filesystem to complete the copy.
  .TP
+.B EOPNOTSUPP

I'll add the kernel version here:

.BR EOPNOTSUPP " (since Linux 5.12)"

Error could be returned prior to 5.3 and would be probably returned
by future stable kernels 5.3..5.12 too

quoted

quoted

+The filesystem does not support this operation >> +.TP
  .B EOVERFLOW
  The requested source or destination range is too large to represent in the
  specified data types.

@@ -187,7 +190,7 @@ refers to an active swap file.
  .B EXDEV
  The files referred to by
  .IR fd_in " and " fd_out
-are not on the same mounted filesystem (pre Linux 5.3).
+are not on the same mounted filesystem (pre Linux 5.3 and post Linux 5.12).

I'm not sure that 'mounted' adds any value here.  Would you remove the
word here?

See rename(2). 'mounted' in this context is explained there.
HOWEVER, it does not fit here.
copy_file_range() IS allowed between two mounts of the same filesystem instance.

To make things more complicated, it appears that cross mount clone is not
allowed via FICLONE/FICLONERANGE ioctl, so ioctl_ficlonerange(2) man page
also uses the 'mounted filesystem' terminology for EXDEV

As things stand now, because of the fallback to clone logic,
copy_file_range() provides a way for users to clone across different mounts
of the same filesystem instance, which they cannot do with the FICLONE ioctl.

Fun :)

BTW, I don't know if preventing cross mount clone was done intentionally,
but as I wrote in a comment in the code once:

        /*
         * FICLONE/FICLONERANGE ioctls enforce that src and dest files are on
         * the same mount. Practically, they only need to be on the same file
         * system.
         */

It reads as if two separate devices with the same filesystem type would
still give this error.

Per the LWN.net article Amir shared, this is permitted ("When called
from user space, copy_file_range() will only try to copy a file across
filesystems if the two are of the same type").

This behavior was slightly different before 5.3 AFAICR (was it?) ("until
then, copy_file_range() refused to copy between files that were not
located on the same filesystem.").  If that's the case, I'd specify the
difference, or more probably split the error into two, one before 5.3,
and one since 5.12.

True.

quoted

I think you need to drop the (Linux range) altogether.

I'll keep the range.  Users of 5.3..5.11 might be surprised if the
filesystems are different and they don't get an error, I think.

I reworded it to follow other pages conventions:

.BR EXDEV " (before Linux 5.3; or since Linux 5.12)"

which renders as:

        EXDEV (before Linux 5.3; or since Linux 5.12)
               The files referred to by fd_in and fd_out are not on
               the same mounted filesystem.

drop 'mounted'

quoted

What's missing here is the NFS cross server copy use case.
Maybe:

...are not on the same mounted filesystem and the source and target filesystems
do not support cross-filesystem copy.

Yes.

Again, this wasn't true before 5.3, right?

Right.
Actually, v5.3 provides the vfs capabilities for filesystems to support
cross fs copy. I am not sure if NFS already implements cross fs copy in
v5.3 and not sure about cifs. Need to get input from nfs/cis developers
or dig in the release notes for server-side copy.

quoted

You may refer the reader to VERSIONS section where it will say which
filesystems support cross-fs copy as of kernel version XXX (i.e. cifs and nfs).

quoted

  .SH VERSIONS
  The
  .BR copy_file_range ()

@@ -202,6 +205,11 @@ Applications should target the behaviour and requirements of 5.3 kernels.
  .PP
  First support for cross-filesystem copies was introduced in Linux 5.3.
  Older kernels will return -EXDEV when cross-filesystem copies are attempted.
+.PP
+After Linux 5.12, support for copies between different filesystems was dropped.
+However, individual filesystems may still provide
+.BR copy_file_range ()
+implementations that allow copies across different devices.

Again, this is not likely to stay uptodate for very long.
The stable kernels are expected to apply your patch (because it fixes
a regression)
so this should be phrased differently.
If it were me, I would provide all the details of the situation to
Michael and ask him
to write the best description for this section.

I'll look into more detail at this part in a later review.


On 2/26/21 11:34 AM, Amir Goldstein wrote:
 > Is this detailed enough? ;-)
 >
 > https://lwn.net/Articles/846403/

Yes, it is!

Thanks to LWN :)

Thanks,
Amir.