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

["Alejandro Colomar (man-pages)" <alx.manpages@gmail.com>]

Hello Amir, Luis,

On 2/24/21 5:10 PM, Amir Goldstein wrote:
> On Wed, Feb 24, 2021 at 4:22 PM Luis Henriques <lhenriques@suse.de> wrote:
>>
>> Update man-page with recent changes to this syscall.
>>
>> Signed-off-by: Luis Henriques <lhenriques@suse.de>
>> ---
>> 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)"

>> +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?

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.

> 
> 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.


> 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?

> 
> 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).
> 
>>   .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,

Alex

-- 
Alejandro Colomar
Linux man-pages comaintainer; https://www.kernel.org/doc/man-pages/
http://www.alejandro-colomar.es/