From: Lennart Poettering Date: Tue, 12 May 2026 14:13:36 +0000 (+0200) Subject: copy: retire splice use() for copying files on disk X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=94e05aca0d1f8f37a4656947f6bf0aa77594364c;p=thirdparty%2Fsystemd.git 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 93d163adda6..1e01da0c94f 100644 --- 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 d5788c7d77f..5b68bb9a5ab 100644 --- 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;