From b3f95a6073163c41a533dcdcf084219c1549ce8f Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Sun, 30 Aug 2026 10:29:46 +0200 Subject: [PATCH] cp: open the source once per copy Every regular-file copy on Linux opened the source three times: once in check_for_data, once in check_sparse_detection, then again in the copy itself. Open it once and thread the descriptor through the probe and the copy, so the strategy and the bytes come from the same fd. before: open("src", O_RDONLY) x3 + open("dst", O_WRONLY|O_CREAT) x1 after: open("src", O_RDONLY) x1 + open("dst", O_WRONLY|O_CREAT) x1 Copying 3000 small files, 50 runs: 199.9ms +/- 14.9 -> 171.7ms +/- 23.6. The ratio's error bars overlap; the stable signal is system time, 177.3ms -> 146.9ms, with user time unchanged at ~18ms. --- src/uu/cp/src/platform/linux.rs | 232 +++++++++++++++----------------- tests/by-util/test_cp.rs | 30 ++++- 2 files changed, 136 insertions(+), 126 deletions(-) diff --git a/src/uu/cp/src/platform/linux.rs b/src/uu/cp/src/platform/linux.rs index 98afa6d95a8..fc32a4be5da 100644 --- a/src/uu/cp/src/platform/linux.rs +++ b/src/uu/cp/src/platform/linux.rs @@ -22,26 +22,15 @@ use crate::{ is_stream, }; -// Replacement for `std::fs::copy` that uses the safe-copy primitives but -// only applies `O_NOFOLLOW` to the *source* open. The destination is -// followed if it is a pre-existing symlink, matching GNU cp -d/-P which -// only forbid dereferencing on the source side. -fn fs_copy(source: P, dest: Q, source_nofollow: bool, context: &str) -> CopyResult<()> -where - P: AsRef, - Q: AsRef, -{ - let mut src = open_source(source, source_nofollow) - .map_err(|e| CpError::IoErrContext(e, context.to_owned()))?; - let mut dst = create_dest_restrictive(&dest, false).map_err(|e| { +// Create the destination. It is followed when it is a pre-existing symlink, +// matching GNU cp -d/-P which only forbid dereferencing on the source side. +fn create_dest(dest: &Path) -> CopyResult { + create_dest_restrictive(dest, false).map_err(|e| { CpError::IoErrContext( e, - translate!("cp-error-cannot-create-regular-file", "path" => dest.as_ref().quote()), + translate!("cp-error-cannot-create-regular-file", "path" => dest.quote()), ) - })?; - buf_copy::copy_fast(&mut src, &mut dst) - .map_err(|e| CpError::IoErrContext(e, context.to_owned()))?; - Ok(()) + }) } /// The fallback behavior for [`clone`] on failed system call. @@ -69,87 +58,89 @@ enum CloneFallback { enum CopyMethod { /// Do a sparse copy SparseCopy, - /// Use [`std::fs::copy`]. + /// Copy the bytes with [`buf_copy::copy_fast`]. FSCopy, /// Default (can either be [`CopyMethod::SparseCopy`] or [`CopyMethod::FSCopy`]) Default, - /// Use [`sparse_copy_without_hole`] + /// Use [`sparse_copy_without_hole_fd`] SparseCopyWithoutHole, } /// Use the Linux `ioctl_ficlone` API to do a copy-on-write clone. /// /// `fallback` controls what to do if the system call fails. -fn clone

( - source: P, - dest: P, +fn clone( + src_file: &mut File, + dest: &Path, fallback: CloneFallback, - nofollow: bool, context: &str, -) -> CopyResult<()> -where - P: AsRef, -{ +) -> CopyResult<()> { // Only needed to decide whether a failed --reflink=always clone should // clean up the dest, so skip the lstat for the other fallbacks. - let dest_existed = - matches!(fallback, CloneFallback::Error) && dest.as_ref().symlink_metadata().is_ok(); - let mut src_file = - open_source(&source, nofollow).map_err(|e| CpError::IoErrContext(e, context.to_owned()))?; - let mut dst_file = create_dest_restrictive(&dest, false).map_err(|e| { - CpError::IoErrContext( - e, - translate!("cp-error-cannot-create-regular-file", "path" => dest.as_ref().quote()), - ) - })?; - if let Err(err) = ioctl_ficlone(&dst_file, &src_file) { + let dest_existed = matches!(fallback, CloneFallback::Error) && dest.symlink_metadata().is_ok(); + let mut dst_file = create_dest(dest)?; + if let Err(err) = ioctl_ficlone(&dst_file, &*src_file) { // Reuse the already-open descriptors: the dest was just created with // a restrictive mode that the umask may have stripped of write bits, // so re-opening it by path can fail with EACCES (LP: #2164777). return match fallback { CloneFallback::Error => { // GNU cp removes a dest it created itself, but keeps a - // pre-existing (now truncated) one. - if !dest_existed { - let _ = std::fs::remove_file(&dest); + // pre-existing (now truncated) one. Only unlink while the + // path still names the inode we created: it may have been + // replaced since, and removing blindly would drop an + // unrelated file. + if !dest_existed && path_still_refers_to(dest, &dst_file) { + let _ = std::fs::remove_file(dest); } Err(CpError::IoErrContext(err.into(), context.to_owned())) } - CloneFallback::FSCopy => buf_copy::copy_fast(&mut src_file, &mut dst_file) + CloneFallback::FSCopy => buf_copy::copy_fast(src_file, &mut dst_file) .map_err(|e| CpError::IoErrContext(e, context.to_owned())), - CloneFallback::SparseCopy => sparse_copy_fd(&mut src_file, &dst_file, context), + CloneFallback::SparseCopy => sparse_copy_fd(src_file, &dst_file, context), CloneFallback::SparseCopyWithoutHole => { - sparse_copy_without_hole_fd(&src_file, &dst_file, context) + sparse_copy_without_hole_fd(src_file, &dst_file, context) } }; } Ok(()) } +/// Whether `path` still resolves to the inode behind `file`. +fn path_still_refers_to(path: &Path, file: &File) -> bool { + let (Ok(current), Ok(opened)) = (path.symlink_metadata(), file.metadata()) else { + return false; + }; + current.dev() == opened.dev() && current.ino() == opened.ino() +} + /// Checks whether a file contains any non null bytes i.e. any byte != 0x0 /// This function returns a tuple of (bool, u64, u64) signifying a tuple of (whether a file has /// data, its size, no of blocks it has allocated in disk) -fn check_for_data(source: &Path, nofollow: bool) -> io::Result<(bool, u64, u64)> { - let mut src_file = open_source(source, nofollow)?; +fn check_for_data(src_file: &mut File) -> io::Result<(bool, u64, u64)> { let metadata = src_file.metadata()?; let size = metadata.size(); let blocks = metadata.blocks(); // checks edge case of virtual files in /proc which have a size of zero but contains data - if size == 0 { + let (has_data, blocks) = if size == 0 { let mut buf: Vec = vec![0; metadata.blksize() as usize]; // Directly use metadata.blksize() - let _ = src_file.read(&mut buf)?; - return Ok((buf.iter().any(|&x| x != 0x0), size, 0)); - } - let has_data = seek(src_file, SeekFrom::Data(0)).is_ok(); + let read = src_file.read(&mut buf)?; + (buf[..read].iter().any(|&x| x != 0x0), 0) + } else { + (seek(&*src_file, SeekFrom::Data(0)).is_ok(), blocks) + }; + + // The probe moved the descriptor; the copy that follows reads it + // sequentially, so hand it back positioned at the start. + seek(&*src_file, SeekFrom::Start(0))?; Ok((has_data, size, blocks)) } /// Checks whether a file is sparse i.e. it contains holes, uses the crude heuristic blocks < size / 512 /// Reference:`` -fn check_sparse_detection(source: &Path, nofollow: bool) -> io::Result { - let src_file = open_source(source, nofollow)?; +fn check_sparse_detection(src_file: &File) -> io::Result { let metadata = src_file.metadata()?; let size = metadata.size(); let blocks = metadata.blocks(); @@ -157,23 +148,8 @@ fn check_sparse_detection(source: &Path, nofollow: bool) -> io::Result { Ok(blocks < size / 512) } -/// Optimized [`sparse_copy`] doesn't create holes for large sequences of zeros in non `sparse_files` +/// Optimized [`sparse_copy_fd`] doesn't create holes for large sequences of zeros in non `sparse_files` /// Used when `--sparse=auto` -fn sparse_copy_without_hole

(source: P, dest: P, nofollow: bool, context: &str) -> CopyResult<()> -where - P: AsRef, -{ - let src_file = - open_source(&source, nofollow).map_err(|e| CpError::IoErrContext(e, context.to_owned()))?; - let dst_file = create_dest_restrictive(&dest, false).map_err(|e| { - CpError::IoErrContext( - e, - translate!("cp-error-cannot-create-regular-file", "path" => dest.as_ref().quote()), - ) - })?; - sparse_copy_without_hole_fd(&src_file, &dst_file, context) -} - fn sparse_copy_without_hole_fd(src_file: &File, dst_file: &File, context: &str) -> CopyResult<()> { let ctx_err = |e: io::Error| CpError::IoErrContext(e, context.to_owned()); @@ -212,21 +188,6 @@ fn sparse_copy_without_hole_fd(src_file: &File, dst_file: &File, context: &str) } /// Perform a sparse copy from one file to another. /// Creates a holes for large sequences of zeros in `non_sparse_files`, used for `--sparse=always` -fn sparse_copy

(source: P, dest: P, nofollow: bool, context: &str) -> CopyResult<()> -where - P: AsRef, -{ - let mut src_file = - open_source(&source, nofollow).map_err(|e| CpError::IoErrContext(e, context.to_owned()))?; - let dst_file = create_dest_restrictive(&dest, false).map_err(|e| { - CpError::IoErrContext( - e, - translate!("cp-error-cannot-create-regular-file", "path" => dest.as_ref().quote()), - ) - })?; - sparse_copy_fd(&mut src_file, &dst_file, context) -} - fn sparse_copy_fd(src_file: &mut File, dst_file: &File, context: &str) -> CopyResult<()> { let ctx_err = |e: io::Error| CpError::IoErrContext(e, context.to_owned()); @@ -321,6 +282,12 @@ where } /// Copies `source` to `dest` using copy-on-write if possible. +/// +/// The source is opened once and the descriptor is threaded through both the +/// sparseness probe and the copy itself. Probing and copying used to open the +/// path separately, costing three opens per copy and letting the strategy be +/// decided from one file while the bytes came from whatever the path named +/// later (#13185). pub(crate) fn copy_on_write( source: &Path, dest: &Path, @@ -344,16 +311,20 @@ pub(crate) fn copy_on_write( copy_debug.offload = OffloadReflinkDebug::Avoided; copy_stream(source, dest, nofollow, context) } else { + let mut src_file = open_source(source, nofollow) + .map_err(|e| CpError::IoErrContext(e, context.to_owned()))?; let mut copy_method = CopyMethod::Default; - let result = handle_reflink_never_sparse_always(source, dest, nofollow); + let result = handle_reflink_never_sparse_always(&mut src_file, dest); if let Ok((debug, method)) = result { copy_debug = debug; copy_method = method; } + let mut dst_file = create_dest(dest)?; match copy_method { - CopyMethod::FSCopy => fs_copy(source, dest, nofollow, context), - _ => sparse_copy(source, dest, nofollow, context), + CopyMethod::FSCopy => buf_copy::copy_fast(&mut src_file, &mut dst_file) + .map_err(|e| CpError::IoErrContext(e, context.to_owned())), + _ => sparse_copy_fd(&mut src_file, &dst_file, context), } } } @@ -364,11 +335,15 @@ pub(crate) fn copy_on_write( copy_debug.offload = OffloadReflinkDebug::Avoided; copy_stream(source, dest, nofollow, context) } else { - let result = handle_reflink_never_sparse_never(source, nofollow); + let mut src_file = open_source(source, nofollow) + .map_err(|e| CpError::IoErrContext(e, context.to_owned()))?; + let result = handle_reflink_never_sparse_never(&mut src_file); if let Ok(debug) = result { copy_debug = debug; } - fs_copy(source, dest, nofollow, context) + let mut dst_file = create_dest(dest)?; + buf_copy::copy_fast(&mut src_file, &mut dst_file) + .map_err(|e| CpError::IoErrContext(e, context.to_owned())) } } (ReflinkMode::Never, SparseMode::Auto) => { @@ -378,18 +353,22 @@ pub(crate) fn copy_on_write( copy_debug.offload = OffloadReflinkDebug::Avoided; copy_stream(source, dest, nofollow, context) } else { + let mut src_file = open_source(source, nofollow) + .map_err(|e| CpError::IoErrContext(e, context.to_owned()))?; let mut copy_method = CopyMethod::Default; - let result = handle_reflink_never_sparse_auto(source, dest, nofollow); + let result = handle_reflink_never_sparse_auto(&mut src_file, dest); if let Ok((debug, method)) = result { copy_debug = debug; copy_method = method; } + let mut dst_file = create_dest(dest)?; match copy_method { CopyMethod::SparseCopyWithoutHole => { - sparse_copy_without_hole(source, dest, nofollow, context) + sparse_copy_without_hole_fd(&src_file, &dst_file, context) } - _ => fs_copy(source, dest, nofollow, context), + _ => buf_copy::copy_fast(&mut src_file, &mut dst_file) + .map_err(|e| CpError::IoErrContext(e, context.to_owned())), } } } @@ -400,8 +379,10 @@ pub(crate) fn copy_on_write( copy_debug.offload = OffloadReflinkDebug::Avoided; copy_stream(source, dest, nofollow, context) } else { + let mut src_file = open_source(source, nofollow) + .map_err(|e| CpError::IoErrContext(e, context.to_owned()))?; let mut copy_method = CopyMethod::Default; - let result = handle_reflink_auto_sparse_always(source, dest, nofollow); + let result = handle_reflink_auto_sparse_always(&mut src_file, dest); if let Ok((debug, method)) = result { copy_debug = debug; copy_method = method; @@ -409,9 +390,9 @@ pub(crate) fn copy_on_write( match copy_method { CopyMethod::FSCopy => { - clone(source, dest, CloneFallback::FSCopy, nofollow, context) + clone(&mut src_file, dest, CloneFallback::FSCopy, context) } - _ => clone(source, dest, CloneFallback::SparseCopy, nofollow, context), + _ => clone(&mut src_file, dest, CloneFallback::SparseCopy, context), } } } @@ -422,12 +403,14 @@ pub(crate) fn copy_on_write( copy_debug.offload = OffloadReflinkDebug::Avoided; copy_stream(source, dest, nofollow, context) } else { - let result = handle_reflink_auto_sparse_never(source, nofollow); + let mut src_file = open_source(source, nofollow) + .map_err(|e| CpError::IoErrContext(e, context.to_owned()))?; + let result = handle_reflink_auto_sparse_never(&mut src_file); if let Ok(debug) = result { copy_debug = debug; } - clone(source, dest, CloneFallback::FSCopy, nofollow, context) + clone(&mut src_file, dest, CloneFallback::FSCopy, context) } } (ReflinkMode::Auto, SparseMode::Auto) => { @@ -435,8 +418,10 @@ pub(crate) fn copy_on_write( copy_debug.offload = OffloadReflinkDebug::Unsupported; copy_stream(source, dest, nofollow, context) } else { + let mut src_file = open_source(source, nofollow) + .map_err(|e| CpError::IoErrContext(e, context.to_owned()))?; let mut copy_method = CopyMethod::Default; - let result = handle_reflink_auto_sparse_auto(source, dest, nofollow); + let result = handle_reflink_auto_sparse_auto(&mut src_file, dest); if let Ok((debug, method)) = result { copy_debug = debug; copy_method = method; @@ -444,13 +429,12 @@ pub(crate) fn copy_on_write( match copy_method { CopyMethod::SparseCopyWithoutHole => clone( - source, + &mut src_file, dest, CloneFallback::SparseCopyWithoutHole, - nofollow, context, ), - _ => clone(source, dest, CloneFallback::FSCopy, nofollow, context), + _ => clone(&mut src_file, dest, CloneFallback::FSCopy, context), } } } @@ -459,7 +443,9 @@ pub(crate) fn copy_on_write( copy_debug.sparse_detection = SparseDebug::No; copy_debug.reflink = OffloadReflinkDebug::Yes; - clone(source, dest, CloneFallback::Error, nofollow, context) + let mut src_file = open_source(source, nofollow) + .map_err(|e| CpError::IoErrContext(e, context.to_owned()))?; + clone(&mut src_file, dest, CloneFallback::Error, context) } (ReflinkMode::Always, _) => { return Err(translate!("cp-error-reflink-always-sparse-auto").into()); @@ -472,9 +458,8 @@ pub(crate) fn copy_on_write( /// Handles debug results when flags are "--reflink=auto" and "--sparse=always" and specifies what /// type of copy should be used fn handle_reflink_auto_sparse_always( - source: &Path, + src_file: &mut File, dest: &Path, - nofollow: bool, ) -> io::Result<(CopyDebug, CopyMethod)> { let mut copy_debug = CopyDebug { offload: OffloadReflinkDebug::Unknown, @@ -482,8 +467,8 @@ fn handle_reflink_auto_sparse_always( sparse_detection: SparseDebug::Zeros, }; let mut copy_method = CopyMethod::Default; - let (data_flag, size, blocks) = check_for_data(source, nofollow)?; - let sparse_flag = check_sparse_detection(source, nofollow)?; + let (data_flag, size, blocks) = check_for_data(src_file)?; + let sparse_flag = check_sparse_detection(src_file)?; if data_flag || size < 512 { copy_debug.offload = OffloadReflinkDebug::Avoided; @@ -508,14 +493,14 @@ fn handle_reflink_auto_sparse_always( /// Handles debug results when flags are "--reflink=auto" and "--sparse=auto" and specifies what /// type of copy should be used -fn handle_reflink_never_sparse_never(source: &Path, nofollow: bool) -> io::Result { +fn handle_reflink_never_sparse_never(src_file: &mut File) -> io::Result { let mut copy_debug = CopyDebug { offload: OffloadReflinkDebug::Unknown, reflink: OffloadReflinkDebug::No, sparse_detection: SparseDebug::No, }; - let (data_flag, size, _blocks) = check_for_data(source, nofollow)?; - let sparse_flag = check_sparse_detection(source, nofollow)?; + let (data_flag, size, _blocks) = check_for_data(src_file)?; + let sparse_flag = check_sparse_detection(src_file)?; if sparse_flag { copy_debug.sparse_detection = SparseDebug::SeekHole; @@ -528,16 +513,16 @@ fn handle_reflink_never_sparse_never(source: &Path, nofollow: bool) -> io::Resul } /// Handles debug results when flags are "--reflink=auto" and "--sparse=never", files will be copied -/// through cloning them with fallback switching to [`std::fs::copy`] -fn handle_reflink_auto_sparse_never(source: &Path, nofollow: bool) -> io::Result { +/// through cloning them with fallback switching to [`buf_copy::copy_fast`] +fn handle_reflink_auto_sparse_never(src_file: &mut File) -> io::Result { let mut copy_debug = CopyDebug { offload: OffloadReflinkDebug::Unknown, reflink: OffloadReflinkDebug::No, sparse_detection: SparseDebug::No, }; - let (data_flag, size, _blocks) = check_for_data(source, nofollow)?; - let sparse_flag = check_sparse_detection(source, nofollow)?; + let (data_flag, size, _blocks) = check_for_data(src_file)?; + let sparse_flag = check_sparse_detection(src_file)?; if sparse_flag { copy_debug.sparse_detection = SparseDebug::SeekHole; @@ -552,9 +537,8 @@ fn handle_reflink_auto_sparse_never(source: &Path, nofollow: bool) -> io::Result /// Handles debug results when flags are "--reflink=auto" and "--sparse=auto" and specifies what /// type of copy should be used fn handle_reflink_auto_sparse_auto( - source: &Path, + src_file: &mut File, dest: &Path, - nofollow: bool, ) -> io::Result<(CopyDebug, CopyMethod)> { let mut copy_debug = CopyDebug { offload: OffloadReflinkDebug::Unknown, @@ -563,8 +547,8 @@ fn handle_reflink_auto_sparse_auto( }; let mut copy_method = CopyMethod::Default; - let (data_flag, size, blocks) = check_for_data(source, nofollow)?; - let sparse_flag = check_sparse_detection(source, nofollow)?; + let (data_flag, size, blocks) = check_for_data(src_file)?; + let sparse_flag = check_sparse_detection(src_file)?; if (data_flag && size != 0) || (size > 0 && size < 512) { copy_debug.offload = OffloadReflinkDebug::Yes; @@ -596,9 +580,8 @@ fn handle_reflink_auto_sparse_auto( /// Handles debug results when flags are "--reflink=never" and "--sparse=auto" and specifies what /// type of copy should be used fn handle_reflink_never_sparse_auto( - source: &Path, + src_file: &mut File, dest: &Path, - nofollow: bool, ) -> io::Result<(CopyDebug, CopyMethod)> { let mut copy_debug = CopyDebug { offload: OffloadReflinkDebug::Unknown, @@ -606,8 +589,8 @@ fn handle_reflink_never_sparse_auto( sparse_detection: SparseDebug::No, }; - let (data_flag, size, blocks) = check_for_data(source, nofollow)?; - let sparse_flag = check_sparse_detection(source, nofollow)?; + let (data_flag, size, blocks) = check_for_data(src_file)?; + let sparse_flag = check_sparse_detection(src_file)?; let mut copy_method = CopyMethod::Default; if data_flag || size < 512 { @@ -633,9 +616,8 @@ fn handle_reflink_never_sparse_auto( /// Handles debug results when flags are "--reflink=never" and "--sparse=always" and specifies what /// type of copy should be used fn handle_reflink_never_sparse_always( - source: &Path, + src_file: &mut File, dest: &Path, - nofollow: bool, ) -> io::Result<(CopyDebug, CopyMethod)> { let mut copy_debug = CopyDebug { offload: OffloadReflinkDebug::Unknown, @@ -644,8 +626,8 @@ fn handle_reflink_never_sparse_always( }; let mut copy_method = CopyMethod::SparseCopy; - let (data_flag, size, blocks) = check_for_data(source, nofollow)?; - let sparse_flag = check_sparse_detection(source, nofollow)?; + let (data_flag, size, blocks) = check_for_data(src_file)?; + let sparse_flag = check_sparse_detection(src_file)?; if data_flag || size < 512 { copy_debug.offload = OffloadReflinkDebug::Avoided; diff --git a/tests/by-util/test_cp.rs b/tests/by-util/test_cp.rs index 84a8e8dcf21..91a85fc10be 100644 --- a/tests/by-util/test_cp.rs +++ b/tests/by-util/test_cp.rs @@ -4,7 +4,7 @@ // file that was distributed with this source code. // spell-checker:ignore (flags) reflink (fs) tmpfs (linux) filefrag rlimit Rlim NOFILE clob btrfs neve ROOTDIR USERDIR outfile subvolume uufs xattrs ELOOP -// spell-checker:ignore bdfl hlsl IRWXO IRWXG nconfined matchpathcon libselinux-devel prwx doesnotexist reftests subdirs mksocket srwx dstlink +// spell-checker:ignore bdfl hlsl IRWXO IRWXG nconfined matchpathcon libselinux-devel prwx doesnotexist reftests subdirs mksocket srwx dstlink mcstransd #[cfg(unix)] use rstest::rstest; use uucore::display::Quotable; @@ -5593,6 +5593,34 @@ fn test_cp_debug_sparse_never_zero_sized_virtual_file() { .stdout_contains("copy offload: avoided, reflink: no, sparse detection: no"); } +// A file that stats as zero-sized but still returns data (/proc entries) +// is probed for content by reading from it. The copy reuses that same +// descriptor, so it has to start over at offset 0 -- otherwise everything +// the probe consumed is dropped and the destination ends up empty. The +// --debug tests above only look at the reported strategy, not the bytes. +#[test] +#[cfg(target_os = "linux")] +fn test_cp_zero_sized_virtual_file_contents() { + let expected = std::fs::read_to_string("/proc/version").unwrap(); + assert!(!expected.is_empty()); + + for extra in [ + &[][..], + &["--sparse=never"], + &["--sparse=always"], + &["--sparse=auto"], + &["--reflink=never"], + &["--reflink=never", "--sparse=always"], + ] { + let (at, mut ucmd) = at_and_ucmd!(); + ucmd.args(extra) + .args(&["/proc/version", "copied"]) + .succeeds() + .no_output(); + assert_eq!(at.read("copied"), expected, "with {extra:?}"); + } +} + #[test] #[cfg(target_os = "linux")] fn test_cp_debug_default_zero_sized_virtual_file() {