copy: retire splice use() for copying files on disk

Apparently splice() is quite problematic, hence just don't anymore. It's
also unnecessary these days since either copy_file_range() or sendfile()
nowadays typically work, the splice() fallback doesn't give us much
anymore.

(At least I am not aware of a combo of fds where splice() would work
where neither cfr nor sf would work).

This leaves one use of splice() in place, in
src/shared/socket-forward.c. We should probably kill that too, but
that'd require some reworking to use sendfile() I guess, and I am too
lazy for that right now. Moreover, in contrast to the other uses it's
probably even safe, since it uses an intermediary pipe always. But what
do I know...

Fixes: #29044

diff --git a/src/import/export-tar.c b/src/import/export-tar.c
index 93d163adda63d34f83b2c14fb21ef3daa6f7005a..1e01da0c94faf61351404705ede02c3d7d920516 100644 (file)
--- a/src/import/export-tar.c
+++ b/src/import/export-tar.c
@@ -59,7 +59,6 @@ typedef struct TarExport {
         RateLimit progress_ratelimit;
 
         bool eof;
-        bool tried_splice;
 } TarExport;
 
 TarExport *tar_export_unref(TarExport *e) {
@@ -189,27 +188,6 @@ static int tar_export_process(TarExport *e) {
 
         assert(e);
 
-        if (!e->tried_splice && compressor_type(e->compress) == COMPRESSION_NONE) {
-
-                l = splice(e->tar_fd, NULL, e->output_fd, NULL, COMPRESS_PIPE_BUFFER_SIZE, 0);
-                if (l < 0) {
-                        if (errno == EAGAIN)
-                                return 0;
-
-                        e->tried_splice = true;
-                } else if (l == 0) {
-                        r = tar_export_finish(e);
-                        goto finish;
-                } else {
-                        e->written_uncompressed += l;
-                        e->written_compressed += l;
-
-                        tar_export_report_progress(e);
-
-                        return 0;
-                }
-        }
-
         while (e->buffer_size <= 0) {
                 uint8_t input[COMPRESS_PIPE_BUFFER_SIZE];
 

diff --git a/src/shared/copy.c b/src/shared/copy.c
index d5788c7d77fb5f55ed2789ff61a772842f44f2f6..5b68bb9a5ab97a1128b1c9fbb2c9e14da1ec91a5 100644 (file)
--- a/src/shared/copy.c
+++ b/src/shared/copy.c
@@ -71,31 +71,6 @@ static ssize_t try_copy_file_range(
         return r;
 }
 
-enum {
-        FD_IS_NO_PIPE,
-        FD_IS_BLOCKING_PIPE,
-        FD_IS_NONBLOCKING_PIPE,
-};
-
-static int fd_is_nonblock_pipe(int fd) {
-        struct stat st;
-        int flags;
-
-        /* Checks whether the specified file descriptor refers to a pipe, and if so if O_NONBLOCK is set. */
-
-        if (fstat(fd, &st) < 0)
-                return -errno;
-
-        if (!S_ISFIFO(st.st_mode))
-                return FD_IS_NO_PIPE;
-
-        flags = fcntl(fd, F_GETFL);
-        if (flags < 0)
-                return -errno;
-
-        return FLAGS_SET(flags, O_NONBLOCK) ? FD_IS_NONBLOCKING_PIPE : FD_IS_BLOCKING_PIPE;
-}
-
 static int look_for_signals(CopyFlags copy_flags) {
         int r;
 
@@ -164,9 +139,9 @@ int copy_bytes_full(
                 void *userdata) {
 
         _cleanup_close_ int fdf_opened = -EBADF, fdt_opened = -EBADF;
-        bool try_cfr = true, try_sendfile = true, try_splice = true;
+        bool try_cfr = true, try_sendfile = true;
         uint64_t copied_total = 0;
-        int r, nonblock_pipe = -1;
+        int r;
 
         assert(fdf >= 0);
         assert(fdt >= 0);
@@ -366,7 +341,7 @@ int copy_bytes_full(
                         }
                 }
 
-                /* First try copy_file_range(), unless we already tried */
+                /* First, try copy_file_range(), unless we already tried */
                 if (try_cfr) {
                         n = try_copy_file_range(fdf, NULL, fdt, NULL, m, 0u);
                         if (n < 0) {
@@ -387,13 +362,13 @@ int copy_bytes_full(
                                  * back to simple read()s in case we encounter empty files.
                                  *
                                  * See: https://lwn.net/Articles/846403/ */
-                                try_cfr = try_sendfile = try_splice = false;
+                                try_cfr = try_sendfile = false;
                         } else
                                 /* Success! */
                                 goto next;
                 }
 
-                /* First try sendfile(), unless we already tried */
+                /* Second, try sendfile(), unless we already tried */
                 if (try_sendfile) {
                         n = sendfile(fdt, fdf, NULL, m);
                         if (n < 0) {
@@ -407,76 +382,13 @@ int copy_bytes_full(
                                 if (copied_total > 0)
                                         break;
 
-                                try_sendfile = try_splice = false; /* same logic as above for copy_file_range() */
-                        } else
-                                /* Success! */
-                                goto next;
-                }
-
-                /* Then try splice, unless we already tried. */
-                if (try_splice) {
-
-                        /* splice()'s asynchronous I/O support is a bit weird. When it encounters a pipe file
-                         * descriptor, then it will ignore its O_NONBLOCK flag and instead only honour the
-                         * SPLICE_F_NONBLOCK flag specified in its flag parameter. Let's hide this behaviour
-                         * here, and check if either of the specified fds are a pipe, and if so, let's pass
-                         * the flag automatically, depending on O_NONBLOCK being set.
-                         *
-                         * Here's a twist though: when we use it to move data between two pipes of which one
-                         * has O_NONBLOCK set and the other has not, then we have no individual control over
-                         * O_NONBLOCK behaviour. Hence in that case we can't use splice() and still guarantee
-                         * systematic O_NONBLOCK behaviour, hence don't. */
-
-                        if (nonblock_pipe < 0) {
-                                int a, b;
-
-                                /* Check if either of these fds is a pipe, and if so non-blocking or not */
-                                a = fd_is_nonblock_pipe(fdf);
-                                if (a < 0)
-                                        return a;
-
-                                b = fd_is_nonblock_pipe(fdt);
-                                if (b < 0)
-                                        return b;
-
-                                if ((a == FD_IS_NO_PIPE && b == FD_IS_NO_PIPE) ||
-                                    (a == FD_IS_BLOCKING_PIPE && b == FD_IS_NONBLOCKING_PIPE) ||
-                                    (a == FD_IS_NONBLOCKING_PIPE && b == FD_IS_BLOCKING_PIPE))
-
-                                        /* splice() only works if one of the fds is a pipe. If neither is,
-                                         * let's skip this step right-away. As mentioned above, if one of the
-                                         * two fds refers to a blocking pipe and the other to a non-blocking
-                                         * pipe, we can't use splice() either, hence don't try either. This
-                                         * hence means we can only use splice() if either only one of the two
-                                         * fds is a pipe, or if both are pipes with the same nonblocking flag
-                                         * setting. */
-
-                                        try_splice = false;
-                                else
-                                        nonblock_pipe = a == FD_IS_NONBLOCKING_PIPE || b == FD_IS_NONBLOCKING_PIPE;
-                        }
-                }
-
-                if (try_splice) {
-                        n = splice(fdf, NULL, fdt, NULL, m, nonblock_pipe ? SPLICE_F_NONBLOCK : 0);
-                        if (n < 0) {
-                                if (!IN_SET(errno, EINVAL, ENOSYS))
-                                        return -errno;
-
-                                try_splice = false;
-                                /* use fallback below */
-                        } else if (n == 0) { /* likely EOF */
-
-                                if (copied_total > 0)
-                                        break;
-
-                                try_splice = false; /* same logic as above for copy_file_range() + sendfile() */
+                                try_sendfile = false; /* same logic as above for copy_file_range() */
                         } else
                                 /* Success! */
                                 goto next;
                 }
 
-                /* As a fallback just copy bits by hand */
+                /* Third, as a fallback just copy bits by hand */
                 {
                         uint8_t buf[MIN(m, COPY_BUFFER_SIZE)], *p = buf;
                         ssize_t z;