From 834ef9f08ae3429a05dead80237bb4bd04769895 Mon Sep 17 00:00:00 2001 From: Nicolas Koch Date: Tue, 15 May 2018 15:25:09 +0200 Subject: [PATCH 1/9] fs: use copy_file_range on linux --- src/libstd/sys/unix/fs.rs | 67 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 67 insertions(+) diff --git a/src/libstd/sys/unix/fs.rs b/src/libstd/sys/unix/fs.rs index a1ca839dc187..b9f0f39bbe2a 100644 --- a/src/libstd/sys/unix/fs.rs +++ b/src/libstd/sys/unix/fs.rs @@ -761,6 +761,7 @@ pub fn canonicalize(p: &Path) -> io::Result { Ok(PathBuf::from(OsString::from_vec(buf))) } +#[cfg(not(target_os = "linux"))] pub fn copy(from: &Path, to: &Path) -> io::Result { use fs::{File, set_permissions}; if !from.is_file() { @@ -776,3 +777,69 @@ pub fn copy(from: &Path, to: &Path) -> io::Result { set_permissions(to, perm)?; Ok(ret) } + +#[cfg(target_os = "linux")] +pub fn copy(from: &Path, to: &Path) -> io::Result { + use fs::{File, set_permissions}; + + unsafe fn copy_file_range( + fd_in: libc::c_int, + off_in: *mut libc::loff_t, + fd_out: libc::c_int, + off_out: *mut libc::loff_t, + len: libc::size_t, + flags: libc::c_uint, + ) -> libc::c_long { + libc::syscall( + libc::SYS_copy_file_range, + fd_in, + off_in, + fd_out, + off_out, + len, + flags, + ) + } + + if !from.is_file() { + return Err(Error::new(ErrorKind::InvalidInput, + "the source path is not an existing regular file")) + } + + let mut reader = File::open(from)?; + let mut writer = File::create(to)?; + let (perm, len) = { + let metadata = reader.metadata()?; + (metadata.permissions(), metadata.size()) + }; + + let mut written = 0u64; + while written < len { + let copy_result = unsafe { + cvt(copy_file_range(reader.as_raw_fd(), + ptr::null_mut(), + writer.as_raw_fd(), + ptr::null_mut(), + len as usize, + 0) + ) + }; + match copy_result { + Ok(ret) => written += ret as u64, + Err(err) => { + match err.raw_os_error() { + Some(os_err) if os_err == libc::ENOSYS || os_err == libc::EXDEV => { + // Either kernel is too old or the files are not mounted on the same fs. + // Try again with fallback method + let ret = io::copy(&mut reader, &mut writer)?; + set_permissions(to, perm)?; + return Ok(ret) + }, + _ => return Err(err), + } + } + } + } + set_permissions(to, perm)?; + Ok(written) +} From 00ec3cf2a0449f41bd3cb873bc9b2cf1b6241095 Mon Sep 17 00:00:00 2001 From: Nicolas Koch Date: Wed, 16 May 2018 10:17:06 +0200 Subject: [PATCH 2/9] Use copy_file_range on android also --- src/libstd/sys/unix/fs.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libstd/sys/unix/fs.rs b/src/libstd/sys/unix/fs.rs index b9f0f39bbe2a..42dc7b83da96 100644 --- a/src/libstd/sys/unix/fs.rs +++ b/src/libstd/sys/unix/fs.rs @@ -761,7 +761,7 @@ pub fn canonicalize(p: &Path) -> io::Result { Ok(PathBuf::from(OsString::from_vec(buf))) } -#[cfg(not(target_os = "linux"))] +#[cfg(not(any(target_os = "linux", target_os = "android")))] pub fn copy(from: &Path, to: &Path) -> io::Result { use fs::{File, set_permissions}; if !from.is_file() { @@ -778,7 +778,7 @@ pub fn copy(from: &Path, to: &Path) -> io::Result { Ok(ret) } -#[cfg(target_os = "linux")] +#[cfg(any(target_os = "linux", target_os = "android"))] pub fn copy(from: &Path, to: &Path) -> io::Result { use fs::{File, set_permissions}; @@ -812,7 +812,7 @@ pub fn copy(from: &Path, to: &Path) -> io::Result { let metadata = reader.metadata()?; (metadata.permissions(), metadata.size()) }; - + let mut written = 0u64; while written < len { let copy_result = unsafe { From b605923cc83a444f6be52542b891d31b00c87662 Mon Sep 17 00:00:00 2001 From: Nicolas Koch Date: Wed, 16 May 2018 10:21:34 +0200 Subject: [PATCH 3/9] Add clarifying comment about offset argument --- src/libstd/sys/unix/fs.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/libstd/sys/unix/fs.rs b/src/libstd/sys/unix/fs.rs index 42dc7b83da96..0649a147ea3c 100644 --- a/src/libstd/sys/unix/fs.rs +++ b/src/libstd/sys/unix/fs.rs @@ -816,6 +816,8 @@ pub fn copy(from: &Path, to: &Path) -> io::Result { let mut written = 0u64; while written < len { let copy_result = unsafe { + // We actually don't have to adjust the offsets, + // because copy_file_range adjusts the file offset automatically cvt(copy_file_range(reader.as_raw_fd(), ptr::null_mut(), writer.as_raw_fd(), From f4c2825c8f80eae6ef18eb3fa30464a18a588e0f Mon Sep 17 00:00:00 2001 From: Nicolas Koch Date: Wed, 16 May 2018 10:27:14 +0200 Subject: [PATCH 4/9] Adjust len in every iteration --- src/libstd/sys/unix/fs.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/libstd/sys/unix/fs.rs b/src/libstd/sys/unix/fs.rs index 0649a147ea3c..c9d187f2ff27 100644 --- a/src/libstd/sys/unix/fs.rs +++ b/src/libstd/sys/unix/fs.rs @@ -815,6 +815,7 @@ pub fn copy(from: &Path, to: &Path) -> io::Result { let mut written = 0u64; while written < len { + let bytes_to_copy = len - written; let copy_result = unsafe { // We actually don't have to adjust the offsets, // because copy_file_range adjusts the file offset automatically @@ -822,7 +823,7 @@ pub fn copy(from: &Path, to: &Path) -> io::Result { ptr::null_mut(), writer.as_raw_fd(), ptr::null_mut(), - len as usize, + bytes_to_copy as usize, 0) ) }; From a5e2942861324493c2cc5a32cb1473e656857b98 Mon Sep 17 00:00:00 2001 From: Nicolas Koch Date: Wed, 16 May 2018 10:35:19 +0200 Subject: [PATCH 5/9] Fix large file copies on 32 bit platforms --- src/libstd/sys/unix/fs.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/libstd/sys/unix/fs.rs b/src/libstd/sys/unix/fs.rs index c9d187f2ff27..d8739d653269 100644 --- a/src/libstd/sys/unix/fs.rs +++ b/src/libstd/sys/unix/fs.rs @@ -815,7 +815,11 @@ pub fn copy(from: &Path, to: &Path) -> io::Result { let mut written = 0u64; while written < len { - let bytes_to_copy = len - written; + let bytes_to_copy = if len - written > usize::max_value() as u64 { + usize::max_value() + } else { + (len - written) as usize + }; let copy_result = unsafe { // We actually don't have to adjust the offsets, // because copy_file_range adjusts the file offset automatically @@ -823,7 +827,7 @@ pub fn copy(from: &Path, to: &Path) -> io::Result { ptr::null_mut(), writer.as_raw_fd(), ptr::null_mut(), - bytes_to_copy as usize, + bytes_to_copy, 0) ) }; From 09d03bc245b27728c9cdd4976114506ae20208a7 Mon Sep 17 00:00:00 2001 From: Nicolas Koch Date: Thu, 17 May 2018 14:10:14 +0200 Subject: [PATCH 6/9] Store ENOSYS in a global to avoid unnecessary system calls --- src/libstd/sys/unix/fs.rs | 35 +++++++++++++++++++++++++---------- 1 file changed, 25 insertions(+), 10 deletions(-) diff --git a/src/libstd/sys/unix/fs.rs b/src/libstd/sys/unix/fs.rs index d8739d653269..8412540934eb 100644 --- a/src/libstd/sys/unix/fs.rs +++ b/src/libstd/sys/unix/fs.rs @@ -781,6 +781,11 @@ pub fn copy(from: &Path, to: &Path) -> io::Result { #[cfg(any(target_os = "linux", target_os = "android"))] pub fn copy(from: &Path, to: &Path) -> io::Result { use fs::{File, set_permissions}; + use sync::atomic::{AtomicBool, Ordering}; + + // Kernel prior to 4.5 don't have copy_file_range + // We store the availability in a global to avoid unneccessary syscalls + static HAS_COPY_FILE_RANGE: AtomicBool = AtomicBool::new(true); unsafe fn copy_file_range( fd_in: libc::c_int, @@ -820,16 +825,26 @@ pub fn copy(from: &Path, to: &Path) -> io::Result { } else { (len - written) as usize }; - let copy_result = unsafe { - // We actually don't have to adjust the offsets, - // because copy_file_range adjusts the file offset automatically - cvt(copy_file_range(reader.as_raw_fd(), - ptr::null_mut(), - writer.as_raw_fd(), - ptr::null_mut(), - bytes_to_copy, - 0) - ) + let copy_result = if HAS_COPY_FILE_RANGE.load(Ordering::Relaxed) { + let copy_result = unsafe { + // We actually don't have to adjust the offsets, + // because copy_file_range adjusts the file offset automatically + cvt(copy_file_range(reader.as_raw_fd(), + ptr::null_mut(), + writer.as_raw_fd(), + ptr::null_mut(), + bytes_to_copy, + 0) + ) + }; + if let Err(ref copy_err) = copy_result { + if let Some(libc::ENOSYS) = copy_err.raw_os_error() { + HAS_COPY_FILE_RANGE.store(false, Ordering::Relaxed); + } + } + copy_result + } else { + Err(io::Error::from_raw_os_error(libc::ENOSYS)) }; match copy_result { Ok(ret) => written += ret as u64, From 3f392abdfb2ec0f436352d349eedd5f6707bf817 Mon Sep 17 00:00:00 2001 From: Nicolas Koch Date: Thu, 24 May 2018 14:51:59 +0200 Subject: [PATCH 7/9] Implement suggestions from the PR - Move loading of atomic bool outside the loop - Add comment about TryFrom for future improvement --- src/libstd/sys/unix/fs.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/libstd/sys/unix/fs.rs b/src/libstd/sys/unix/fs.rs index 8412540934eb..6624c48cbe0d 100644 --- a/src/libstd/sys/unix/fs.rs +++ b/src/libstd/sys/unix/fs.rs @@ -818,14 +818,16 @@ pub fn copy(from: &Path, to: &Path) -> io::Result { (metadata.permissions(), metadata.size()) }; + let has_copy_file_range = HAS_COPY_FILE_RANGE.load(Ordering::Relaxed); let mut written = 0u64; while written < len { + // TODO should ideally use TryFrom let bytes_to_copy = if len - written > usize::max_value() as u64 { usize::max_value() } else { (len - written) as usize }; - let copy_result = if HAS_COPY_FILE_RANGE.load(Ordering::Relaxed) { + let copy_result = if has_copy_file_range { let copy_result = unsafe { // We actually don't have to adjust the offsets, // because copy_file_range adjusts the file offset automatically From 3b271eb039d22a4b31caed29a2aac0a9ac604902 Mon Sep 17 00:00:00 2001 From: Nicolas Koch Date: Mon, 28 May 2018 17:19:42 +0200 Subject: [PATCH 8/9] Use FIXME instead of TODO; Move bytes_to_copy calculation inside if branch --- src/libstd/sys/unix/fs.rs | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/libstd/sys/unix/fs.rs b/src/libstd/sys/unix/fs.rs index 6624c48cbe0d..56075c5e8d02 100644 --- a/src/libstd/sys/unix/fs.rs +++ b/src/libstd/sys/unix/fs.rs @@ -821,13 +821,14 @@ pub fn copy(from: &Path, to: &Path) -> io::Result { let has_copy_file_range = HAS_COPY_FILE_RANGE.load(Ordering::Relaxed); let mut written = 0u64; while written < len { - // TODO should ideally use TryFrom - let bytes_to_copy = if len - written > usize::max_value() as u64 { - usize::max_value() - } else { - (len - written) as usize - }; let copy_result = if has_copy_file_range { + // FIXME: should ideally use TryFrom + let bytes_to_copy = if len - written > usize::max_value() as u64 { + usize::max_value() + } else { + (len - written) as usize + }; + let copy_result = unsafe { // We actually don't have to adjust the offsets, // because copy_file_range adjusts the file offset automatically From c7d6a0130b6b76b65982916198e7de2b348f9718 Mon Sep 17 00:00:00 2001 From: Nicolas Koch Date: Tue, 29 May 2018 23:42:42 +0200 Subject: [PATCH 9/9] Fix additional nits: - compute bytes_to_copy more elegantly - add assert that written is 0 in fallback case --- src/libstd/sys/unix/fs.rs | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/src/libstd/sys/unix/fs.rs b/src/libstd/sys/unix/fs.rs index 56075c5e8d02..38f1ac472fa5 100644 --- a/src/libstd/sys/unix/fs.rs +++ b/src/libstd/sys/unix/fs.rs @@ -780,6 +780,7 @@ pub fn copy(from: &Path, to: &Path) -> io::Result { #[cfg(any(target_os = "linux", target_os = "android"))] pub fn copy(from: &Path, to: &Path) -> io::Result { + use cmp; use fs::{File, set_permissions}; use sync::atomic::{AtomicBool, Ordering}; @@ -822,13 +823,7 @@ pub fn copy(from: &Path, to: &Path) -> io::Result { let mut written = 0u64; while written < len { let copy_result = if has_copy_file_range { - // FIXME: should ideally use TryFrom - let bytes_to_copy = if len - written > usize::max_value() as u64 { - usize::max_value() - } else { - (len - written) as usize - }; - + let bytes_to_copy = cmp::min(len - written, usize::max_value() as u64) as usize; let copy_result = unsafe { // We actually don't have to adjust the offsets, // because copy_file_range adjusts the file offset automatically @@ -856,6 +851,7 @@ pub fn copy(from: &Path, to: &Path) -> io::Result { Some(os_err) if os_err == libc::ENOSYS || os_err == libc::EXDEV => { // Either kernel is too old or the files are not mounted on the same fs. // Try again with fallback method + assert_eq!(written, 0); let ret = io::copy(&mut reader, &mut writer)?; set_permissions(to, perm)?; return Ok(ret)