On 06/04/2026 05:52, Collin Funk wrote:
Pádraig Brady <[email protected]> writes:

On 04/04/2026 03:27, Collin Funk wrote:
Attatched a v4 patch.

Since cat is such a fundamental tool, I think it's worth being extra defensive
by trying the read()/write() loop after the splice.
I've heard of cases where some fuse file systems lie about the size in stat,
while read() provides all the data.

Also it seems a bit wasteful to allocate large pipes in kernel mem,
for catting small files.

I think we could handle both these cases with the attached adjustment,
which avoids splice for small regular files, and tries read()/write()
after splice() always.  Note the size threshold was determined with:

   $ truncate -s $((32*1024)) mid  $ hyperfine -N 'src/cat-splice mid' 'src/cat 
mid'

Since we're not doing any more stats() to determine this info,
this should be more memory efficient and safer.

This seems reasonable.

However, there is an issue with your patch. Falling back to read() means
that the user will have to send two EOFs to exit the program. One for
the splice() call and one for the read() call. I forgot about this but
noticed it when tests/tty/tty-eof.pl failed after running 'make check'.

How about this patch which disables the use of splice() when the input
file descriptor is associated with a terminal? It feels a bit safer than
trusting the EOF from splice().

Oh right, EOF from tty is not persistent.
I suppose any device could behave like that theoretically.
I.e., the unconditional fallback read() is not appropriate.
Sorry for the misdirection there.

I'm thinking the attached adjustment is best now.
I.e., keep the small size exclusion for splice(),
but don't do the fallback read() after splice finishes.

I also tweaked NEWS the attached as it was a bit inaccurate wrt pipes.

If you agree with the above feel free to push.

BTW I'm still thinking about yes -z to efficiently generate zeros,
as the splicing from /dev/zero isn't that fast in comparison:

  $ timeout 2 src/cat /dev/zero | pv -r > /dev/null
  [5.50GiB/s]

  $ timeout 2 src/yes | pv -r > /dev/null
  [31.1GiB/s]

cheers,
Padraig
diff --git a/NEWS b/NEWS
index 35c04626f..4460957a8 100644
--- a/NEWS
+++ b/NEWS
@@ -56,8 +56,7 @@ GNU coreutils NEWS                                    -*- outline -*-
 
 ** Improvements
 
-  'cat' now uses zero-copy I/O on Linux when the input or output are pipes,
-  to significantly increase throughput.
+  'cat' now uses zero-copy I/O on Linux when appropriate, to improve throughput.
   E.g., throughput improved 6x from 12.9GiB/s to 81.8GiB/s on a Power10 system.
 
   'df --local' recognises more file system types as remote.
diff --git a/src/cat.c b/src/cat.c
index 02b2acdc8..457a2dcb9 100644
--- a/src/cat.c
+++ b/src/cat.c
@@ -865,25 +865,19 @@ main (int argc, char **argv)
           else
             {
               /* Note 32768 was determined as the limit when splice
-                 starts to have a performance advantage.  It also
-                 excludes zero length files which may not be compatible
-                 with splice in some edge cases.  Also, don't use splice
-                 on terminals where falling back to read() would require
-                 the user to send an EOF twice to exit the program.  */
-              int splice_cat_status = (((usable_st_size (&istat_buf)
-                                         && istat_buf.st_size < 32768)
-                                        || isatty (input_desc))
-                                       ? 0 : splice_cat ());
-              if (splice_cat_status < 0)
+                 starts to have a performance advantage.
+                 It also excludes zero length files which may not
+                 be compatible with splice in some edge cases.  */
+              int splice_cat_status =
+                usable_st_size (&istat_buf) && istat_buf.st_size <= 32768
+                ? 0 : splice_cat ();
+              if (splice_cat_status != 0)
                 {
                   inbuf = NULL;
-                  ok = false;
+                  ok &= 0 < splice_cat_status;
                 }
               else
                 {
-                  /* Note we try simple_cat() even if splice_cat() succeeded,
-                     to handle edge cases where splice finishes but read()
-                     still returns data (seen on some FUSE systems).  */
                   insize = MAX (insize, outsize);
                   inbuf = xalignalloc (page_size, insize);
                   ok &= simple_cat (inbuf, insize);

Reply via email to