bug#79267: cp --sparse=auto heuristic fails on a squashfs mounted drive.
Paul Eggert <[email protected]>
| Newsgroups | gmane.comp.gnu.core-utils.bugs |
|---|---|
| Message-ID | <[email protected]> |
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. > BTW commit 26bf557 also changed this a couple of weeks ago > without updating the comment, so the comment relates to sparse_mode != > SPARSE_ALWAYS. Not quite following but I hope the comment is OK now with the first patch installed. > 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.
0001-cp-go-back-to-copy_file_range-optimization.patch
(text/x-patch, 1.1 KB)
From 306de6c2619e2a9339ade9a88d55c4940942d516 Mon Sep 17 00:00:00 2001 From: Paul Eggert <[email protected]> Date: Fri, 22 Aug 2025 10:37:50 -0700 Subject: [PATCH 1/2] cp: go back to copy_file_range optimization This reverts part of the previous change. * src/copy.c (lseek_copy): When calling sparse_copy, do not ask it to scan for zeros unless --sparse=always, so that it can use copy_file_range which can be far more efficient. --- src/copy.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/copy.c b/src/copy.c index e328bd886..134c57c8b 100644 --- a/src/copy.c +++ b/src/copy.c @@ -617,7 +617,7 @@ lseek_copy (int src_fd, int dest_fd, char **abuf, size_t buf_size, if ( ! sparse_copy (src_fd, dest_fd, abuf, buf_size, allow_reflink, src_name, dst_name, ext_len, - sparse_mode != SPARSE_NEVER ? hole_size : nullptr, + sparse_mode == SPARSE_ALWAYS ? hole_size : nullptr, &n_read)) return false; -- 2.50.1
0002-cp-improve-hole-handling-on-squashfs.patch
(text/x-patch, 6.1 KB)
From 39f22fe687ea0c226e3fb35e86cd5ea329180b80 Mon Sep 17 00:00:00 2001 From: Paul Eggert <[email protected]> Date: Fri, 22 Aug 2025 17:34:04 -0700 Subject: [PATCH 2/2] cp: improve hole handling on squashfs Better fix for problem reported by Jeremy Allison <https://bugs.gnu.org/79267>. * src/copy.c (struct scan_inference): New type, replacing union scan_inference. All uses changed. This is so infer_scantype can report the first hole's offset when known. (lseek_copy): 5th arg is now struct scan_inference const *, not just off_t. All uses changed. (infer_scantype): If SEEK_SET+SEEK_HOLE do not find a hole, fall back on ZERO_SCANTYPE. --- NEWS | 4 ++++ src/copy.c | 60 +++++++++++++++++++++++++++++++++++------------------- 2 files changed, 43 insertions(+), 21 deletions(-) diff --git a/NEWS b/NEWS index 5724698bf..2a8056658 100644 --- a/NEWS +++ b/NEWS @@ -17,6 +17,10 @@ GNU coreutils NEWS -*- outline -*- data zeros used extents rather than holes. [This bug was present in "the beginning".] + cp missed opportunities to create holes when copying from file + systems like squashfs that support SEEK_HOLE only trivially. + [bug introduced in coreutils-9.0] + cp, install, and mv now avoid possible data corruption on glibc 2.41 and 2.42 systems when copy_file_range is used with ranges > 2GiB, avoiding https://sourceware.org/PR33245 diff --git a/src/copy.c b/src/copy.c index 134c57c8b..73ded6857 100644 --- a/src/copy.c +++ b/src/copy.c @@ -106,6 +106,16 @@ # define CAN_HARDLINK_SYMLINKS 0 #endif +/* Result of infer_scantype when it returns LSEEK_SCANTYPE. */ +struct scan_inference +{ + /* The offset of the first data block, or -1 if the file has no data. */ + off_t ext_start; + + /* The offset of the first hole. */ + off_t hole_start; +}; + struct dir_list { struct dir_list *parent; @@ -540,7 +550,7 @@ write_zeros (int fd, off_t n_bytes) static bool lseek_copy (int src_fd, int dest_fd, char **abuf, size_t buf_size, - off_t ext_start, off_t src_total_size, + struct scan_inference const *scan_inference, off_t src_total_size, enum Sparse_type sparse_mode, bool allow_reflink, char const *src_name, char const *dst_name, @@ -552,9 +562,11 @@ lseek_copy (int src_fd, int dest_fd, char **abuf, size_t buf_size, copy_debug.sparse_detection = COPY_DEBUG_EXTERNAL; - while (0 <= ext_start) + for (off_t ext_start = scan_inference->ext_start; 0 <= ext_start; ) { - off_t ext_end = lseek (src_fd, ext_start, SEEK_HOLE); + off_t ext_end = (ext_start == 0 + ? scan_inference->hole_start + : lseek (src_fd, ext_start, SEEK_HOLE)); if (ext_end < 0) { if (errno != ENXIO) @@ -1085,23 +1097,13 @@ enum scantype LSEEK_SCANTYPE, }; -/* Result of infer_scantype. */ -union scan_inference -{ - /* Used if infer_scantype returns LSEEK_SCANTYPE. This is the - offset of the first data block, or -1 if the file has no data. */ - off_t ext_start; -}; - /* Return how to scan a file with descriptor FD and stat buffer SB. - *SCAN_INFERENCE is set to a valid value if returning LSEEK_SCANTYPE. */ + Set *SCAN_INFERENCE if returning LSEEK_SCANTYPE. */ static enum scantype infer_scantype (int fd, struct stat const *sb, - union scan_inference *scan_inference) + struct scan_inference *scan_inference) { - scan_inference->ext_start = -1; /* avoid -Wmaybe-uninitialized */ - - /* Only attempt SEEK_HOLE if this heuristic + /* Try SEEK_HOLE only if this heuristic suggests the file is sparse. */ if (! (HAVE_STRUCT_STAT_ST_BLOCKS && S_ISREG (sb->st_mode) @@ -1109,10 +1111,26 @@ infer_scantype (int fd, struct stat const *sb, return PLAIN_SCANTYPE; #ifdef SEEK_HOLE - off_t ext_start = lseek (fd, 0, SEEK_DATA); - if (0 <= ext_start || errno == ENXIO) + scan_inference->ext_start = lseek (fd, 0, SEEK_DATA); + if (scan_inference->ext_start == 0) + { + scan_inference->hole_start = lseek (fd, 0, SEEK_HOLE); + if (0 <= scan_inference->hole_start) + { + if (scan_inference->hole_start < sb->st_size) + return LSEEK_SCANTYPE; + + /* Though the file likely has 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. */ + if (lseek (fd, 0, SEEK_SET) < 0) + return ERROR_SCANTYPE; + } + } + else if (0 < scan_inference->ext_start || errno == ENXIO) { - scan_inference->ext_start = ext_start; + scan_inference->hole_start = 0; /* Pacify -Wmaybe-uninitialized. */ return LSEEK_SCANTYPE; } else if (errno != EINVAL && !is_ENOTSUP (errno)) @@ -1209,7 +1227,6 @@ copy_reg (char const *src_name, char const *dst_name, mode_t extra_permissions; struct stat sb; struct stat src_open_sb; - union scan_inference scan_inference; bool return_val = true; bool data_copy_required = x->data_copy_required; bool preserve_xattr = USE_XATTR & x->preserve_xattr; @@ -1518,6 +1535,7 @@ copy_reg (char const *src_name, char const *dst_name, size_t buf_size = io_blksize (&sb); /* Deal with sparse files. */ + struct scan_inference scan_inference; enum scantype scantype = infer_scantype (source_desc, &src_open_sb, &scan_inference); if (scantype == ERROR_SCANTYPE) @@ -1565,7 +1583,7 @@ copy_reg (char const *src_name, char const *dst_name, #ifdef SEEK_HOLE scantype == LSEEK_SCANTYPE ? lseek_copy (source_desc, dest_desc, &buf, buf_size, - scan_inference.ext_start, src_open_sb.st_size, + &scan_inference, src_open_sb.st_size, make_holes ? x->sparse_mode : SPARSE_NEVER, x->reflink_mode != REFLINK_NEVER, src_name, dst_name, &hole_size, &n_read) -- 2.50.1