GNU bug report logs - #60416
Be more liberal using copy_file_range() with NFS mounted filesystems

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 60416 in the body.
You can then email your comments to 60416 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Braiam <braiamp <at> gmail.com>
To: bug-coreutils <at> gnu.org
Subject: Be more liberal using copy_file_range() with NFS mounted filesystems
Date: Thu, 29 Dec 2022 12:04:38 -0400

When using a nfs export, cp seems to not try hard enough using
copy_file_range(). This was
the conclusion we arrived in this forum thread[1] at Truenas forums.
It was found a way to force
cp to use it, but it should not be necessary, since there are supposed
to be fallbacks.

I'm unsure if we found something that triggered the undesired behavior.

[1]: https://www.truenas.com/community/threads/nfs-does-not-support-server-side-copy-with-zfs.103309/post-712071

-- 
Braiam

Message #8 received at 60416 <at> debbugs.gnu.org (full text, mbox):

From: Pádraig Brady <P <at> draigBrady.com>
To: Braiam <braiamp <at> gmail.com>, 60416 <at> debbugs.gnu.org
Subject: Re: bug#60416: Be more liberal using copy_file_range() with NFS
 mounted filesystems
Date: Fri, 30 Dec 2022 15:33:12 +0000

On 29/12/2022 16:04, Braiam wrote:
> When using a nfs export, cp seems to not try hard enough using
> copy_file_range(). This was
> the conclusion we arrived in this forum thread[1] at Truenas forums.
> It was found a way to force
> cp to use it, but it should not be necessary, since there are supposed
> to be fallbacks.
> 
> I'm unsure if we found something that triggered the undesired behavior.
> 
> [1]: https://www.truenas.com/community/threads/nfs-does-not-support-server-side-copy-with-zfs.103309/post-712071
> 

Currently we don't use copy_file_range() with sparse files,
as per the Linux man page "copy_file_range() may expand any
holes existing in the requested range".
I confirmed that copy_file_range() definitely expands holes
on ext4 on Linux 5.7 at least.
Also the FreeBSD man page says "this system call attempts
to maintain holes in the output file for the byte range being copied.
However, this does not always work well."

Now maybe we should give precedence to server side copy for remote file systems,
though that would be optimizing runtime and network usage
while potentially pessimizing storage usage
if holes were expanded in the destination.

Now fundamentally copy_file_range() isn't restricted from
maintaining holes from source to destination,
which suggests we could give precedence to copy_file_range()
on remote file systems.

Also perhaps we can improve the heuristic somehow,
even again just for remote file systems.
Maybe determine a file is sparse on remote systems
only if it seems that more than half of the file is sparse.

For completeness, one might be wondering why we're using
copy_file_range() at all with --sparse=never, as that syscall
doesn't guarantee sparseness.  However in this case we
only use copy_file_range() with the portion of the file
considered non sparse (and manually write the zeros)¹.

So in summary pseudo code could be:

sparse_file = blocks < size/blocksize;
very_sparse_file = blocks < (size/2)/blocksize;

if (   (!possible_hole_in_range || sparse_mode=auto)
    && reflinks_allowed
    && (
         (remote_file && !very_sparse_file)
      || (!remote_file && !sparse_file)
       )
   )
     copy_file_range(...);


Note `stat <your files>; df -T <your files>` would
give us some concrete heuristics for your case at least.
Note it would be useful to get such stats from files
that were completely copied by copy_file_range() on your systems
(i.e. not by cp), to see if holes were expanded for your case.

cheers,
Pádraig

¹ Actually I now notice that where SEEK_HOLE is _not_ available
and copy_file_range() is, then `cp --sparse=never` would not be honored,
as copy_file_range() would expand the holes (confirmed on ext4 at least).
Now I don't know of any practical systems having copy_file_range()
and not having SEEK_HOLE, but I might add the constraint to the code,
at least for clarification reasons.

Message #11 received at 60416 <at> debbugs.gnu.org (full text, mbox):

From: Pádraig Brady <P <at> draigBrady.com>
To: Braiam <braiamp <at> gmail.com>, 60416 <at> debbugs.gnu.org
Subject: [PATCH] copy: attempt copy offload with sparse files
Date: Fri, 30 Dec 2022 19:52:02 +0000

[Message part 1 (text/plain, inline)]

On 30/12/2022 15:33, Pádraig Brady wrote:
> On 29/12/2022 16:04, Braiam wrote:
>> When using a nfs export, cp seems to not try hard enough using
>> copy_file_range(). This was
>> the conclusion we arrived in this forum thread[1] at Truenas forums.
>> It was found a way to force
>> cp to use it, but it should not be necessary, since there are supposed
>> to be fallbacks.
>>
>> I'm unsure if we found something that triggered the undesired behavior.
>>
>> [1]: https://www.truenas.com/community/threads/nfs-does-not-support-server-side-copy-with-zfs.103309/post-712071
>>
> 
> Currently we don't use copy_file_range() with sparse files,
> as per the Linux man page "copy_file_range() may expand any
> holes existing in the requested range".
> I confirmed that copy_file_range() definitely expands holes
> on ext4 on Linux 5.7 at least.
> Also the FreeBSD man page says "this system call attempts
> to maintain holes in the output file for the byte range being copied.
> However, this does not always work well."
> 
> Now maybe we should give precedence to server side copy for remote file systems,
> though that would be optimizing runtime and network usage
> while potentially pessimizing storage usage
> if holes were expanded in the destination.
> 
> Now fundamentally copy_file_range() isn't restricted from
> maintaining holes from source to destination,
> which suggests we could give precedence to copy_file_range()
> on remote file systems.
> 
> Also perhaps we can improve the heuristic somehow,
> even again just for remote file systems.
> Maybe determine a file is sparse on remote systems
> only if it seems that more than half of the file is sparse.
> 
> For completeness, one might be wondering why we're using
> copy_file_range() at all with --sparse=never, as that syscall
> doesn't guarantee sparseness.  However in this case we
> only use copy_file_range() with the portion of the file
> considered non sparse (and manually write the zeros)¹.
> 
> So in summary pseudo code could be:
> 
> sparse_file = blocks < size/blocksize;
> very_sparse_file = blocks < (size/2)/blocksize;
> 
> if (   (!possible_hole_in_range || sparse_mode=auto)
>       && reflinks_allowed
>       && (
>            (remote_file && !very_sparse_file)
>         || (!remote_file && !sparse_file)
>          )
>      )
>        copy_file_range(...);
> 
> 
> Note `stat <your files>; df -T <your files>` would
> give us some concrete heuristics for your case at least.
> Note it would be useful to get such stats from files
> that were completely copied by copy_file_range() on your systems
> (i.e. not by cp), to see if holes were expanded for your case.
> 
> cheers,
> Pádraig
> 
> ¹ Actually I now notice that where SEEK_HOLE is _not_ available
> and copy_file_range() is, then `cp --sparse=never` would not be honored,
> as copy_file_range() would expand the holes (confirmed on ext4 at least).
> Now I don't know of any practical systems having copy_file_range()
> and not having SEEK_HOLE, but I might add the constraint to the code,
> at least for clarification reasons.

Looking a bit closer at things, the current code does something like:

if file appears sparse
    use SEEK_DATA to iterate over data parts,
      try writing data with copy_file_range() if sparse_mode == never
        (as thinking was there is no need to look for additional holes)
else
    use copy_file_range if sparse_never || sparse_auto


Now the use of copy_file_range() with sparse=never may be problematic,
as assumes copy_file_range() won't add additional holes.
This is problematic in first case as SEEK_HOLE is best effort,
and in the second (else) case as file sparseness is a heuristic.

Also from above analysis the OP's case seems to be first sparse case,
as in second (else) case, with --sparse=auto, copy_file_range()
would have been used (as file appears non sparse (so plain scan used)).

So while we theoretically should only be allowing copy_file_range()
with --sparse=auto, we definitely should not be precluding it in the first
case with that sparse mode.

I.e. for the first case we should only disallow copy_file_range() with
sparse=ALWAYS, because --sparse=auto is best effort, and we've enough
signal from SEEK_DATA as to the likelihood of processing holes or not.

The attached makes that change.
It would be great if you could test that on your systems.
To that end you might find the following tarball useful,
which has the patch already applied:
  https://pixelbeat.org/cu/coreutils-9.1.109-bd17b.tar.xz
which can be built like:
  tar -xf coreutils-9.1.109-bd17b.tar.xz
  cd coreutils-9.1.109-bd17b/
  ./configure && make -j $(nproc)
then tested with:
  src/cp ...

cheers,
Pádraig

[copy-sparse-offload.patch (text/x-patch, attachment)]

Message #14 received at 60416 <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Pádraig Brady <P <at> draigBrady.com>,
 Braiam <braiamp <at> gmail.com>, 60416 <at> debbugs.gnu.org
Subject: Re: bug#60416: [PATCH] copy: attempt copy offload with sparse files
Date: Fri, 30 Dec 2022 14:32:14 -0800

Thanks, looks good, please install.

Message #19 received at 60416-done <at> debbugs.gnu.org (full text, mbox):

From: Pádraig Brady <P <at> draigBrady.com>
To: Paul Eggert <eggert <at> cs.ucla.edu>, Braiam <braiamp <at> gmail.com>,
 60416-done <at> debbugs.gnu.org
Subject: Re: bug#60416: [PATCH] copy: attempt copy offload with sparse files
Date: Sat, 31 Dec 2022 00:23:30 +0000

On 30/12/2022 22:32, Paul Eggert wrote:
> Thanks, looks good, please install.

Thanks for the review.
OP also posted good results.
Pushed with NEWS entry.
Marking this as done.

I may follow up with a related patch to
ensure we _don't_ use copy_file_range with --sparse=never.

cheers,
Pádraig

Message #22 received at 60416-done <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Pádraig Brady <P <at> draigBrady.com>,
 Braiam <braiamp <at> gmail.com>, 60416-done <at> debbugs.gnu.org
Subject: Re: bug#60416: [PATCH] copy: attempt copy offload with sparse files
Date: Fri, 30 Dec 2022 17:36:24 -0800

On 2022-12-30 16:23, Pádraig Brady wrote:
> 
> I may follow up with a related patch to
> ensure we _don't_ use copy_file_range with --sparse=never.

Not sure it's worth the bother. A file system that would create holes 
with copy_file_range could also do so with plain 'write', no?

Message #25 received at 60416-done <at> debbugs.gnu.org (full text, mbox):

From: Pádraig Brady <P <at> draigBrady.com>
To: Paul Eggert <eggert <at> cs.ucla.edu>, Braiam <braiamp <at> gmail.com>,
 60416-done <at> debbugs.gnu.org
Subject: Re: bug#60416: [PATCH] copy: attempt copy offload with sparse files
Date: Sat, 31 Dec 2022 12:04:27 +0000

On 31/12/2022 01:36, Paul Eggert wrote:
> On 2022-12-30 16:23, Pádraig Brady wrote:
>>
>> I may follow up with a related patch to
>> ensure we _don't_ use copy_file_range with --sparse=never.
> 
> Not sure it's worth the bother. A file system that would create holes
> with copy_file_range could also do so with plain 'write', no?

Well copy_file_range() would be lower level and within its
remit to propagate holes. The FreeBSD docs allude to this also.

But yes, our use of copy_file_range() is generally restricted
to non holes, so less of an issue for us.

Also there may be users using `cp --sparse=never` to avoid
the perf issue currently, like we saw on the OP web thread.

I'll hold off on that change so.

Hopefully copy_file_range() flags are introduced to give
control over this (like I suggested on lkml years ago).

cheers,
Pádraig

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.