On 23/08/2025 01:48, Paul Eggert wrote:
On 8/22/25 07:04, Pádraig Brady wrote:
/* Copy this extent, looking for further opportunities to not
bother to write zeros if --sparse=always, since SEEK_HOLE
is conservative and may miss some holes. */
So the comment needs to be tweaked, but a more general issue
is that it disables copy offloading (copy_file_range) for sparse files.
Ouch, I didn't see that. That's a real loss. I installed the first
attached patch to revert that part of my recent change.
I assume the part of the change that always punches holes is OK. I
couldn't see why one would not want to punch a hole if one has already
taken the trouble to find and create the hole.
>> If we can't handle all cases optimally, I'd be inclined to err on being
>> as performant as possible by default, and only try harder to look for holes
>> with --sparse=always.>> squashfs is giving the wrong info here after all,
right?
Yes, that's the actual space-performance bug here. I installed the
second attached patch to try to work around it.
Jeremy, can you please try these two further patches? Thanks.
Note this squashfs bug avoidance patch is interacting very negatively
with performance on OpenZFS with transparent compression enabled.
Copying about 1G of files went from 15s to over 6 minutes.
Copy offloading really does have huge advantages.
With ZFS our reflinking is not supported and with compression it also
triggers our sparse heuristic due to allocated blocks being < size/512.
That triggers the SEEK_HOLE check, which due to the squashfs adjustment
is falling back to scanning for zeros when it determines the
file might not be in fact sparse.
So I'm applying the attached for the upcoming coreutils 9.9 release,
which will regress the squashfs case again, but is the right compromise
unless someone can come up with a way to distinguish these two cases.
thanks,
Padraig
From d16aeffacfb048c13c164962d8469f0b3bea5164 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C3=A1draig=20Brady?= <[email protected]>
Date: Thu, 30 Oct 2025 13:02:48 +0000
Subject: [PATCH] copy: don't avoid copy-offload upon SEEK_HOLE indicating
non-sparse
* src/copy-file-data.c (infer_scantype): Fall back to a plain copy
if SEEK_HOLE indicates non-sparse, as zero copy avoids copy offload.
This was seen with transparently compressed files on OpenZFS.
* tests/cp/sparse-perf.sh: Add a test case.
* NEWS: Mention the bug fix.
Fixes https://github.com/coreutils/coreutils/issues/122
---
NEWS | 6 ++++++
src/copy-file-data.c | 11 +++++++++--
tests/cp/sparse-perf.sh | 10 ++++++++++
3 files changed, 25 insertions(+), 2 deletions(-)
diff --git a/NEWS b/NEWS
index 0cc760aee..5461de2f1 100644
--- a/NEWS
+++ b/NEWS
@@ -18,6 +18,12 @@ GNU coreutils NEWS -*- outline -*-
Also non standard SHA2 tags with a bad length resulted in undefined behavior.
[bug introduced in coreutils-9.8]
+ 'cp' restores performance with transparently compressed files, which
+ regressed due to the avoidance of copy offload, seen with OpenZFS at least.
+ Note file systems like squashfs that support SEEK_HOLE only trivially, may
+ again, like <= v9.7 with default options, miss opportunities to create holes.
+ [bug introduced in coreutils-9.8]
+
'numfmt' no longer reads out-of-bounds memory with trailing blanks in input.
[bug introduced with numfmt in coreutils-8.21]
diff --git a/src/copy-file-data.c b/src/copy-file-data.c
index c3abe41e2..c82a4b386 100644
--- a/src/copy-file-data.c
+++ b/src/copy-file-data.c
@@ -481,12 +481,19 @@ infer_scantype (int fd, struct stat const *sb, off_t pos,
if (scan_inference->hole_start < sb->st_size)
return LSEEK_SCANTYPE;
- /* Though the file likely has holes, SEEK_DATA and SEEK_HOLE
+ /* Though the file may have holes, SEEK_DATA and SEEK_HOLE
didn't find any. This can happen with file systems like
circa-2025 squashfs that support SEEK_HOLE only trivially.
- Fall back on ZERO_SCANTYPE. */
+ This can also happen due to transparent file compression,
+ which can also indicate fewer than the usual number of blocks. */
+
if (lseek (fd, pos, SEEK_SET) < 0)
return ERROR_SCANTYPE;
+
+ /* we prefer to return PLAIN_SCANTYPE here so that copy offload
+ continues to be used. Falling through to ZERO_SCANTYPE would be
+ less performant in the compressed file case. */
+ return PLAIN_SCANTYPE;
}
}
else if (pos < scan_inference->ext_start || errno == ENXIO)
diff --git a/tests/cp/sparse-perf.sh b/tests/cp/sparse-perf.sh
index 5a283c1fe..5ee984c52 100755
--- a/tests/cp/sparse-perf.sh
+++ b/tests/cp/sparse-perf.sh
@@ -35,6 +35,16 @@ cmp $other_partition_sparse k2 || fail=1
grep ': avoided' cp.out && { cat cp.out; fail=1; }
+# Create a large-non-sparse-but-compressible file
+# Ensure we don't avoid copy offload which we saw with
+# transparent compression on OpenZFS at least
+# (as that triggers our sparse heuristic).
+mls='might-look-sparse'
+yes | head -n1M > "$mls" || framework_failure_
+cp --debug "$mls" "$mls.cp" >cp.out || fail=1
+cmp "$mls" "$mls.cp" || fail=1
+grep ': avoided' cp.out && { cat cp.out; fail=1; }
+
# Create a large-but-sparse file on the current partition.
# We disable relinking below, thus verifying SEEK_HOLE support
--
2.51.0
