[libcxx-commits] [libcxx] [libc++] use copy_file_range for fs::copy (PR #109211)

Louis Dionne via libcxx-commits libcxx-commits at lists.llvm.org
Thu Dec 5 13:05:05 PST 2024


Jannik =?utf-8?q?Glückert?= <jannik.glueckert at gmail.com>,Louis Dionne
 <ldionne.2 at gmail.com>,Louis Dionne <ldionne.2 at gmail.com>,Louis Dionne
 <ldionne.2 at gmail.com>
Message-ID:
In-Reply-To: <llvm.org/llvm/llvm-project/pull/109211 at github.com>


https://github.com/ldionne updated https://github.com/llvm/llvm-project/pull/109211

>From 0a4372f657e7e21640cfb9fdbec87e4b125bd3dc Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jannik=20Gl=C3=BCckert?= <jannik.glueckert at gmail.com>
Date: Wed, 18 Sep 2024 21:45:08 +0200
Subject: [PATCH 1/5] [libc++] use copy_file_range for fs::copy_file
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

opportunistically use copy_file_range (Linux, FreeBSD) where possible.
This allows for fast copies via reflinks,
and server side copies for network filesystems.
Fall back to sendfile if not supported.

Signed-off-by: Jannik Glückert <jannik.glueckert at gmail.com>
---
 libcxx/src/filesystem/operations.cpp | 75 +++++++++++++++++++++++++++-
 1 file changed, 74 insertions(+), 1 deletion(-)

diff --git a/libcxx/src/filesystem/operations.cpp b/libcxx/src/filesystem/operations.cpp
index d771f200973528..d26802e214bfac 100644
--- a/libcxx/src/filesystem/operations.cpp
+++ b/libcxx/src/filesystem/operations.cpp
@@ -32,11 +32,16 @@
 #  include <dirent.h>
 #  include <sys/stat.h>
 #  include <sys/statvfs.h>
+#  include <sys/types.h>
 #  include <unistd.h>
 #endif
 #include <fcntl.h> /* values for fchmodat */
 #include <time.h>
 
+// since Linux 4.5 and FreeBSD 13
+#if defined(__linux__) || defined(__FreeBSD__)
+#  define _LIBCPP_FILESYSTEM_USE_COPY_FILE_RANGE
+#endif
 #if __has_include(<sys/sendfile.h>)
 #  include <sys/sendfile.h>
 #  define _LIBCPP_FILESYSTEM_USE_SENDFILE
@@ -178,8 +183,36 @@ void __copy(const path& from, const path& to, copy_options options, error_code*
 namespace detail {
 namespace {
 
+#if defined(_LIBCPP_FILESYSTEM_USE_COPY_FILE_RANGE)
+bool copy_file_impl_copy_file_range(FileDescriptor& read_fd, FileDescriptor& write_fd, error_code& ec) {
+  size_t count = read_fd.get_stat().st_size;
+  // a zero-length file is either empty, or not copyable by this syscall
+  // return early to avoid the syscall cost
+  if (count == 0) {
+    ec = {EINVAL, generic_category()};
+    return false;
+  }
+  // do not modify the fd positions as copy_file_impl_sendfile may be called after a partial copy
+  off_t off_in  = 0;
+  off_t off_out = 0;
+  do {
+    ssize_t res;
+
+    if ((res = ::copy_file_range(read_fd.fd, &off_in, write_fd.fd, &off_out, count, 0)) == -1) {
+      ec = capture_errno();
+      return false;
+    }
+    count -= res;
+  } while (count > 0);
+
+  ec.clear();
+
+  return true;
+}
+#endif
+
 #if defined(_LIBCPP_FILESYSTEM_USE_SENDFILE)
-bool copy_file_impl(FileDescriptor& read_fd, FileDescriptor& write_fd, error_code& ec) {
+bool copy_file_impl_sendfile(FileDescriptor& read_fd, FileDescriptor& write_fd, error_code& ec) {
   size_t count = read_fd.get_stat().st_size;
   do {
     ssize_t res;
@@ -194,6 +227,46 @@ bool copy_file_impl(FileDescriptor& read_fd, FileDescriptor& write_fd, error_cod
 
   return true;
 }
+#endif
+
+#if defined(__linux__) || defined(__FreeBSD__)
+bool copy_file_impl(FileDescriptor& read_fd, FileDescriptor& write_fd, error_code& ec) {
+#  if defined(_LIBCPP_FILESYSTEM_USE_COPY_FILE_RANGE)
+  if (copy_file_impl_copy_file_range(read_fd, write_fd, ec)) {
+    return true;
+  }
+  // EINVAL: src and dst are the same file (this is not cheaply
+  // detectable from userspace)
+  // EINVAL: copy_file_range is unsupported for this file type by the
+  // underlying filesystem
+  // ENOTSUP: undocumented, can arise with old kernels and NFS
+  // EOPNOTSUPP: filesystem does not implement copy_file_range
+  // ETXTBSY: src or dst is an active swapfile (nonsensical, but allowed
+  // with normal copying)
+  // EXDEV: src and dst are on different filesystems that do not support
+  // cross-fs copy_file_range
+  // ENOENT: undocumented, can arise with CIFS
+  // ENOSYS: unsupported by kernel or blocked by seccomp
+  if (ec.value() != EINVAL && ec.value() != ENOTSUP && ec.value() != EOPNOTSUPP && ec.value() != ETXTBSY &&
+      ec.value() != EXDEV && ec.value() != ENOENT && ec.value() != ENOSYS) {
+    return false;
+  }
+  ec.clear();
+#  endif
+
+#  if defined(_LIBCPP_FILESYSTEM_USE_SENDFILE)
+  if (copy_file_impl_sendfile(read_fd, write_fd, ec)) {
+    return true;
+  }
+  // EINVAL: unsupported file type
+  if (ec.value() != EINVAL) {
+    return false;
+  }
+  ec.clear();
+#  endif
+  ec = {EINVAL, generic_category()};
+  return false;
+}
 #elif defined(_LIBCPP_FILESYSTEM_USE_COPYFILE)
 bool copy_file_impl(FileDescriptor& read_fd, FileDescriptor& write_fd, error_code& ec) {
   struct CopyFileState {

>From bdf6774b20cbf1d00042cf0e3507c3b675b6510e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jannik=20Gl=C3=BCckert?= <jannik.glueckert at gmail.com>
Date: Fri, 20 Sep 2024 17:06:08 +0200
Subject: [PATCH 2/5] [libc++] support special linux files in fs::copy_file
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Virtual linux filesystems such as /proc and /sys
may contain files that appear as zero length,
but do contain data.
These require a traditional userspace read + write loop.

Signed-off-by: Jannik Glückert <jannik.glueckert at gmail.com>
---
 libcxx/src/filesystem/operations.cpp          | 87 +++++++++++--------
 .../fs.op.copy_file/copy_file_procfs.pass.cpp | 52 +++++++++++
 2 files changed, 105 insertions(+), 34 deletions(-)
 create mode 100644 libcxx/test/std/input.output/filesystems/fs.op.funcs/fs.op.copy_file/copy_file_procfs.pass.cpp

diff --git a/libcxx/src/filesystem/operations.cpp b/libcxx/src/filesystem/operations.cpp
index d26802e214bfac..b9e1921a7d810a 100644
--- a/libcxx/src/filesystem/operations.cpp
+++ b/libcxx/src/filesystem/operations.cpp
@@ -49,10 +49,17 @@
 #  include <copyfile.h>
 #  define _LIBCPP_FILESYSTEM_USE_COPYFILE
 #else
-#  include <fstream>
 #  define _LIBCPP_FILESYSTEM_USE_FSTREAM
 #endif
 
+// sendfile and copy_file_range need to fall back
+// to the fstream implementation for special files
+#if defined(_LIBCPP_FILESYSTEM_USE_SENDFILE) || defined(_LIBCPP_FILESYSTEM_USE_COPY_FILE_RANGE) ||                     \
+    defined(_LIBCPP_FILESYSTEM_USE_FSTREAM)
+#  include <fstream>
+#  define _LIBCPP_FILESYSTEM_NEED_FSTREAM
+#endif
+
 #if defined(__ELF__) && defined(_LIBCPP_LINK_RT_LIB)
 #  pragma comment(lib, "rt")
 #endif
@@ -183,6 +190,42 @@ void __copy(const path& from, const path& to, copy_options options, error_code*
 namespace detail {
 namespace {
 
+#if defined(_LIBCPP_FILESYSTEM_NEED_FSTREAM)
+bool copy_file_impl_fstream(FileDescriptor& read_fd, FileDescriptor& write_fd, error_code& ec) {
+  ifstream in;
+  in.__open(read_fd.fd, ios::binary);
+  if (!in.is_open()) {
+    // This assumes that __open didn't reset the error code.
+    ec = capture_errno();
+    return false;
+  }
+  read_fd.fd = -1;
+  ofstream out;
+  out.__open(write_fd.fd, ios::binary);
+  if (!out.is_open()) {
+    ec = capture_errno();
+    return false;
+  }
+  write_fd.fd = -1;
+
+  if (in.good() && out.good()) {
+    using InIt  = istreambuf_iterator<char>;
+    using OutIt = ostreambuf_iterator<char>;
+    InIt bin(in);
+    InIt ein;
+    OutIt bout(out);
+    copy(bin, ein, bout);
+  }
+  if (out.fail() || in.fail()) {
+    ec = make_error_code(errc::io_error);
+    return false;
+  }
+
+  ec.clear();
+  return true;
+}
+#endif
+
 #if defined(_LIBCPP_FILESYSTEM_USE_COPY_FILE_RANGE)
 bool copy_file_impl_copy_file_range(FileDescriptor& read_fd, FileDescriptor& write_fd, error_code& ec) {
   size_t count = read_fd.get_stat().st_size;
@@ -214,6 +257,12 @@ bool copy_file_impl_copy_file_range(FileDescriptor& read_fd, FileDescriptor& wri
 #if defined(_LIBCPP_FILESYSTEM_USE_SENDFILE)
 bool copy_file_impl_sendfile(FileDescriptor& read_fd, FileDescriptor& write_fd, error_code& ec) {
   size_t count = read_fd.get_stat().st_size;
+  // a zero-length file is either empty, or not copyable by this syscall
+  // return early to avoid the syscall cost
+  if (count == 0) {
+    ec = {EINVAL, generic_category()};
+    return false;
+  }
   do {
     ssize_t res;
     if ((res = ::sendfile(write_fd.fd, read_fd.fd, nullptr, count)) == -1) {
@@ -264,8 +313,8 @@ bool copy_file_impl(FileDescriptor& read_fd, FileDescriptor& write_fd, error_cod
   }
   ec.clear();
 #  endif
-  ec = {EINVAL, generic_category()};
-  return false;
+
+  return copy_file_impl_fstream(read_fd, write_fd, ec);
 }
 #elif defined(_LIBCPP_FILESYSTEM_USE_COPYFILE)
 bool copy_file_impl(FileDescriptor& read_fd, FileDescriptor& write_fd, error_code& ec) {
@@ -290,37 +339,7 @@ bool copy_file_impl(FileDescriptor& read_fd, FileDescriptor& write_fd, error_cod
 }
 #elif defined(_LIBCPP_FILESYSTEM_USE_FSTREAM)
 bool copy_file_impl(FileDescriptor& read_fd, FileDescriptor& write_fd, error_code& ec) {
-  ifstream in;
-  in.__open(read_fd.fd, ios::binary);
-  if (!in.is_open()) {
-    // This assumes that __open didn't reset the error code.
-    ec = capture_errno();
-    return false;
-  }
-  read_fd.fd = -1;
-  ofstream out;
-  out.__open(write_fd.fd, ios::binary);
-  if (!out.is_open()) {
-    ec = capture_errno();
-    return false;
-  }
-  write_fd.fd = -1;
-
-  if (in.good() && out.good()) {
-    using InIt  = istreambuf_iterator<char>;
-    using OutIt = ostreambuf_iterator<char>;
-    InIt bin(in);
-    InIt ein;
-    OutIt bout(out);
-    copy(bin, ein, bout);
-  }
-  if (out.fail() || in.fail()) {
-    ec = make_error_code(errc::io_error);
-    return false;
-  }
-
-  ec.clear();
-  return true;
+  return copy_file_impl_fstream(read_fd, write_fd, ec);
 }
 #else
 #  error "Unknown implementation for copy_file_impl"
diff --git a/libcxx/test/std/input.output/filesystems/fs.op.funcs/fs.op.copy_file/copy_file_procfs.pass.cpp b/libcxx/test/std/input.output/filesystems/fs.op.funcs/fs.op.copy_file/copy_file_procfs.pass.cpp
new file mode 100644
index 00000000000000..ebe4c651ae2576
--- /dev/null
+++ b/libcxx/test/std/input.output/filesystems/fs.op.funcs/fs.op.copy_file/copy_file_procfs.pass.cpp
@@ -0,0 +1,52 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// UNSUPPORTED: c++03, c++11, c++14
+// REQUIRES: linux
+// UNSUPPORTED: no-filesystem
+// UNSUPPORTED: availability-filesystem-missing
+
+// <filesystem>
+
+// bool copy_file(const path& from, const path& to);
+// bool copy_file(const path& from, const path& to, error_code& ec) noexcept;
+// bool copy_file(const path& from, const path& to, copy_options options);
+// bool copy_file(const path& from, const path& to, copy_options options,
+//           error_code& ec) noexcept;
+
+#include <cassert>
+#include <filesystem>
+#include <system_error>
+
+#include "test_macros.h"
+#include "filesystem_test_helper.h"
+
+namespace fs = std::filesystem;
+
+// Linux has various virtual filesystems such as /proc and /sys
+// where files may have no length (st_size == 0), but still contain data.
+// This is because the to-be-read data is usually generated ad-hoc by the reading syscall
+// These files can not be copied with kernel-side copies like copy_file_range or sendfile,
+// and must instead be copied via a traditional userspace read + write loop.
+int main(int, char** argv) {
+  const fs::path procfile{"/proc/self/comm"};
+  assert(file_size(procfile) == 0);
+
+  scoped_test_env env;
+  std::error_code ec = GetTestEC();
+
+  const fs::path dest = env.make_env_path("dest");
+
+  assert(copy_file(procfile, dest, ec));
+  assert(!ec);
+
+  // /proc/self/comm contains the filename of the executable, plus a null terminator
+  assert(file_size(dest) == fs::path(argv[0]).filename().string().size() + 1);
+
+  return 0;
+}

>From 8f314b977c7a5a986f68a97e9b29b7725770460d Mon Sep 17 00:00:00 2001
From: Louis Dionne <ldionne.2 at gmail.com>
Date: Thu, 5 Dec 2024 13:50:53 -0500
Subject: [PATCH 3/5] Minor reorganization per review comments

---
 libcxx/src/filesystem/operations.cpp | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/libcxx/src/filesystem/operations.cpp b/libcxx/src/filesystem/operations.cpp
index b9e1921a7d810a..ecfd964a95d938 100644
--- a/libcxx/src/filesystem/operations.cpp
+++ b/libcxx/src/filesystem/operations.cpp
@@ -278,9 +278,8 @@ bool copy_file_impl_sendfile(FileDescriptor& read_fd, FileDescriptor& write_fd,
 }
 #endif
 
-#if defined(__linux__) || defined(__FreeBSD__)
+#if defined(_LIBCPP_FILESYSTEM_USE_COPY_FILE_RANGE)
 bool copy_file_impl(FileDescriptor& read_fd, FileDescriptor& write_fd, error_code& ec) {
-#  if defined(_LIBCPP_FILESYSTEM_USE_COPY_FILE_RANGE)
   if (copy_file_impl_copy_file_range(read_fd, write_fd, ec)) {
     return true;
   }
@@ -301,9 +300,10 @@ bool copy_file_impl(FileDescriptor& read_fd, FileDescriptor& write_fd, error_cod
     return false;
   }
   ec.clear();
-#  endif
-
-#  if defined(_LIBCPP_FILESYSTEM_USE_SENDFILE)
+  return copy_file_impl_fstream(read_fd, write_fd, ec);
+}
+#elif defined(_LIBCPP_FILESYSTEM_USE_SENDFILE)
+bool copy_file_impl(FileDescriptor& read_fd, FileDescriptor& write_fd, error_code& ec) {
   if (copy_file_impl_sendfile(read_fd, write_fd, ec)) {
     return true;
   }
@@ -312,8 +312,6 @@ bool copy_file_impl(FileDescriptor& read_fd, FileDescriptor& write_fd, error_cod
     return false;
   }
   ec.clear();
-#  endif
-
   return copy_file_impl_fstream(read_fd, write_fd, ec);
 }
 #elif defined(_LIBCPP_FILESYSTEM_USE_COPYFILE)

>From e0d8bc0c952ec2da7bcf8b5ecc388a841f013f9c Mon Sep 17 00:00:00 2001
From: Louis Dionne <ldionne.2 at gmail.com>
Date: Thu, 5 Dec 2024 15:33:43 -0500
Subject: [PATCH 4/5] Make _LIBCPP_FILESYSTEM_USE_COPY_FILE_RANGE and
 _LIBCPP_FILESYSTEM_USE_SENDFILE disjoint

---
 libcxx/src/filesystem/operations.cpp | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/libcxx/src/filesystem/operations.cpp b/libcxx/src/filesystem/operations.cpp
index ecfd964a95d938..f2d4953078ea38 100644
--- a/libcxx/src/filesystem/operations.cpp
+++ b/libcxx/src/filesystem/operations.cpp
@@ -41,8 +41,7 @@
 // since Linux 4.5 and FreeBSD 13
 #if defined(__linux__) || defined(__FreeBSD__)
 #  define _LIBCPP_FILESYSTEM_USE_COPY_FILE_RANGE
-#endif
-#if __has_include(<sys/sendfile.h>)
+#elif __has_include(<sys/sendfile.h>)
 #  include <sys/sendfile.h>
 #  define _LIBCPP_FILESYSTEM_USE_SENDFILE
 #elif defined(__APPLE__) || __has_include(<copyfile.h>)

>From 74b1b09df1f6f1dfad71cb1d845eaa493879c6c9 Mon Sep 17 00:00:00 2001
From: Louis Dionne <ldionne.2 at gmail.com>
Date: Thu, 5 Dec 2024 16:04:52 -0500
Subject: [PATCH 5/5] Fix incorrect semantics

---
 libcxx/src/filesystem/operations.cpp | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/libcxx/src/filesystem/operations.cpp b/libcxx/src/filesystem/operations.cpp
index f2d4953078ea38..c6a358e883d6fd 100644
--- a/libcxx/src/filesystem/operations.cpp
+++ b/libcxx/src/filesystem/operations.cpp
@@ -41,7 +41,8 @@
 // since Linux 4.5 and FreeBSD 13
 #if defined(__linux__) || defined(__FreeBSD__)
 #  define _LIBCPP_FILESYSTEM_USE_COPY_FILE_RANGE
-#elif __has_include(<sys/sendfile.h>)
+#endif
+#if __has_include(<sys/sendfile.h>)
 #  include <sys/sendfile.h>
 #  define _LIBCPP_FILESYSTEM_USE_SENDFILE
 #elif defined(__APPLE__) || __has_include(<copyfile.h>)
@@ -277,8 +278,11 @@ bool copy_file_impl_sendfile(FileDescriptor& read_fd, FileDescriptor& write_fd,
 }
 #endif
 
-#if defined(_LIBCPP_FILESYSTEM_USE_COPY_FILE_RANGE)
+#if defined(_LIBCPP_FILESYSTEM_USE_COPY_FILE_RANGE) || defined(_LIBCPP_FILESYSTEM_USE_SENDFILE)
+// If we have copy_file_range or sendfile, try both in succession (if available).
+// If both fail, fall back to using fstream.
 bool copy_file_impl(FileDescriptor& read_fd, FileDescriptor& write_fd, error_code& ec) {
+#  if defined(_LIBCPP_FILESYSTEM_USE_COPY_FILE_RANGE)
   if (copy_file_impl_copy_file_range(read_fd, write_fd, ec)) {
     return true;
   }
@@ -299,10 +303,9 @@ bool copy_file_impl(FileDescriptor& read_fd, FileDescriptor& write_fd, error_cod
     return false;
   }
   ec.clear();
-  return copy_file_impl_fstream(read_fd, write_fd, ec);
-}
-#elif defined(_LIBCPP_FILESYSTEM_USE_SENDFILE)
-bool copy_file_impl(FileDescriptor& read_fd, FileDescriptor& write_fd, error_code& ec) {
+#  endif
+
+#  if defined(_LIBCPP_FILESYSTEM_USE_SENDFILE)
   if (copy_file_impl_sendfile(read_fd, write_fd, ec)) {
     return true;
   }
@@ -311,6 +314,8 @@ bool copy_file_impl(FileDescriptor& read_fd, FileDescriptor& write_fd, error_cod
     return false;
   }
   ec.clear();
+#  endif
+
   return copy_file_impl_fstream(read_fd, write_fd, ec);
 }
 #elif defined(_LIBCPP_FILESYSTEM_USE_COPYFILE)



More information about the libcxx-commits mailing list