This patch started by removing the CAT_V config option and using FLAG(), and switching to xsendfile() for the no-options fast path.
This pointed out that the 'x' in the xsendfile family was significantly weaker than the usual guarantee, silently ignoring a variety of errors. It also made me realize that xsendfile() wasn't actually utilizing sendfile(2). This patch removes sendfile_len (without the 'x') which only had one user that wasn't checking for failure and looks like it should have used the 'x' form instead. It also changes xsendfile_len() to take a boolean argument to specify whether you want exactly that number of bytes (in tar, say) or up to that many bytes (as in head, which this patch also switches over to xsendfile). All the cat, head, and tar tests pass (the cat test needed a minor modification now the error message is slightly different, but the new version of the test also passes with coreutils cat where the old one didn't). cat and head -c are significantly faster than they were, and are now faster than coreutils. (coreutils doesn't use sendfile(2), it seems, but was faster by virtue of using a 128KiB buffer than the 4KiB toybuf/libbuf). --- lib/lib.h | 3 +- lib/portability.c | 19 +++++++++++++ lib/portability.h | 2 ++ lib/xwrap.c | 71 +++++++++++++++++++++++++++++------------------ tests/cat.test | 4 +-- toys/posix/cat.c | 64 ++++++++++++++++++++---------------------- toys/posix/head.c | 14 ++++++---- toys/posix/tar.c | 3 +- 8 files changed, 108 insertions(+), 72 deletions(-)
From da69907f70f70e6b0885cd1c04c88b22b1dde3ca Mon Sep 17 00:00:00 2001 From: Elliott Hughes <[email protected]> Date: Thu, 12 Mar 2020 18:56:14 -0700 Subject: [PATCH] More use of sendfile. This patch started by removing the CAT_V config option and using FLAG(), and switching to xsendfile() for the no-options fast path. This pointed out that the 'x' in the xsendfile family was significantly weaker than the usual guarantee, silently ignoring a variety of errors. It also made me realize that xsendfile() wasn't actually utilizing sendfile(2). This patch removes sendfile_len (without the 'x') which only had one user that wasn't checking for failure and looks like it should have used the 'x' form instead. It also changes xsendfile_len() to take a boolean argument to specify whether you want exactly that number of bytes (in tar, say) or up to that many bytes (as in head, which this patch also switches over to xsendfile). All the cat, head, and tar tests pass (the cat test needed a minor modification now the error message is slightly different, but the new version of the test also passes with coreutils cat where the old one didn't). cat and head -c are significantly faster than they were, and are now faster than coreutils. (coreutils doesn't use sendfile(2), it seems, but was faster by virtue of using a 128KiB buffer than the 4KiB toybuf/libbuf). --- lib/lib.h | 3 +- lib/portability.c | 19 +++++++++++++ lib/portability.h | 2 ++ lib/xwrap.c | 71 +++++++++++++++++++++++++++++------------------ tests/cat.test | 4 +-- toys/posix/cat.c | 64 ++++++++++++++++++++---------------------- toys/posix/head.c | 14 ++++++---- toys/posix/tar.c | 3 +- 8 files changed, 108 insertions(+), 72 deletions(-) diff --git a/lib/lib.h b/lib/lib.h index 66b39d67..58cb8592 100644 --- a/lib/lib.h +++ b/lib/lib.h @@ -241,8 +241,7 @@ void loopfiles_rw(char **argv, int flags, int permissions, void (*function)(int fd, char *name)); void loopfiles(char **argv, void (*function)(int fd, char *name)); void loopfiles_lines(char **argv, void (*function)(char **pline, long len)); -long long sendfile_len(int in, int out, long long len, long long *consumed); -long long xsendfile_len(int in, int out, long long len); +long long xsendfile_len(int in, int out, long long len, int exact); void xsendfile_pad(int in, int out, long long len); long long xsendfile(int in, int out); int wfchmodat(int rc, char *name, mode_t mode); diff --git a/lib/portability.c b/lib/portability.c index 26b854d6..0feadea9 100644 --- a/lib/portability.c +++ b/lib/portability.c @@ -595,3 +595,22 @@ int get_block_device_size(int fd, unsigned long long* size) return (ioctl(fd, BLKGETSIZE64, size) >= 0); } #endif + +#if defined(__linux__) +#include <sys/sendfile.h> +long long os_sendfile(int in, int out, long long bytes) +{ + unsigned long long count = bytes; + + if (count >= 0x7ffff000) count = 0x7ffff000; + return sendfile(out, in, 0, count); +} +#else +long long os_sendfile(int in, int out, long long bytes) +{ + // Darwin does have a sendfile(2), but it *requires* an offset, and can only + // write to a socket, so it's not very useful to us. + errno = ENOSYS; + return -1; +} +#endif diff --git a/lib/portability.h b/lib/portability.h index acc32fd4..c51a100a 100644 --- a/lib/portability.h +++ b/lib/portability.h @@ -361,3 +361,5 @@ int dev_makedev(int major, int minor); char *fs_type_name(struct statfs *statfs); int get_block_device_size(int fd, unsigned long long *size); + +long long os_sendfile(int in, int out, long long bytes); diff --git a/lib/xwrap.c b/lib/xwrap.c index 2f47ac66..20915341 100644 --- a/lib/xwrap.c +++ b/lib/xwrap.c @@ -813,48 +813,64 @@ void xpidfile(char *name) close(fd); } -// Return bytes copied from in to out. If bytes <0 copy all of in to out. -// If consuemd isn't null, amount read saved there (return is written or error) -long long sendfile_len(int in, int out, long long bytes, long long *consumed) +static long long do_sendfile(int in, int out, long long bytes, + void (*report)(char *, ...)) { long long total = 0, len; + int try_sendfile = 1; - if (consumed) *consumed = 0; - if (in<0) return 0; while (bytes != total) { - len = bytes-total; - if (bytes<0 || len>sizeof(libbuf)) len = sizeof(libbuf); - - len = read(in, libbuf, len); - if (!len && errno==EAGAIN) continue; - if (len<1) break; - if (consumed) *consumed += len; - if (writeall(out, libbuf, len) != len) return -1; + if (try_sendfile) { + len = os_sendfile(in, out, bytes); + if (len == -1) { + if (errno == EINVAL || errno == ENOSYS) { + try_sendfile = 0; + continue; + } + report("sendfile"); + break; + } + } else { + len = bytes-total; + if (bytes<0 || len>sizeof(libbuf)) len = sizeof(libbuf); + + len = read(in, libbuf, len); + if (len == -1) { + if (errno == EAGAIN) continue; + report("sendfile read"); + break; + } + if (len && writeall(out, libbuf, len) != len) { + report("sendfile write"); + break; + } + } + if (!len) break; total += len; } return total; } -// error_exit if we couldn't copy all bytes -long long xsendfile_len(int in, int out, long long bytes) +// Return bytes copied from in to out. +// If bytes == -1, copy all of in to out. +// If exact, perror_exit if we didn't copy exactly that many bytes. +long long xsendfile_len(int in, int out, long long bytes, int exact) { - long long len = sendfile_len(in, out, bytes, 0); - - if (bytes != -1 && bytes != len) { - if (out == 1 && len<0) xexit(); - error_exit("short %s", (len<0) ? "write" : "read"); - } + long long sent = do_sendfile(in, out, bytes, perror_exit); - return len; + if (exact && bytes != -1 && sent != bytes) + perror_exit("not enough data for %lld bytes (only %lld)", bytes, sent); + return sent; } -// warn and pad with zeroes if we couldn't copy all bytes +// Copy len bytes from in to out. +// Read failures emit a warning and pad with zeroes. +// Write failures perror_exit. void xsendfile_pad(int in, int out, long long len) { - len -= xsendfile_len(in, out, len); + len -= do_sendfile(in, out, len, perror_msg); if (len) { - perror_msg("short read"); memset(libbuf, 0, sizeof(libbuf)); while (len) { int i = len>sizeof(libbuf) ? sizeof(libbuf) : len; @@ -865,10 +881,11 @@ void xsendfile_pad(int in, int out, long long len) } } -// copy all of in to out +// Return bytes copied when trying to copy all of in to out. +// perror_exit on any kind of failure. long long xsendfile(int in, int out) { - return xsendfile_len(in, out, -1); + return do_sendfile(in, out, -1, perror_exit); } double xstrtod(char *s) diff --git a/tests/cat.test b/tests/cat.test index 9432d4e8..a0f1e0b7 100755 --- a/tests/cat.test +++ b/tests/cat.test @@ -25,7 +25,7 @@ testing "- file1" \ skipnot [ -e /dev/full ] testing "> /dev/full" \ - "cat - > /dev/full 2>stderr && echo ok; cat stderr; rm stderr" \ - "cat: xwrite: No space left on device\n" "" "zero\n" + "cat - > /dev/full 2>err && echo fail ; grep -q 'No space' err && echo ok" \ + "ok\n" "" "zero\n" rm file1 file2 diff --git a/toys/posix/cat.c b/toys/posix/cat.c index 6daba7a2..0bbbef70 100644 --- a/toys/posix/cat.c +++ b/toys/posix/cat.c @@ -7,29 +7,21 @@ * And "Cat -v considered harmful" at * http://cm.bell-labs.com/cm/cs/doc/84/kp.ps.gz -USE_CAT(NEWTOY(cat, "u"USE_CAT_V("vte"), TOYFLAG_BIN)) +USE_CAT(NEWTOY(cat, "uvte", TOYFLAG_BIN)) USE_CATV(NEWTOY(catv, USE_CATV("vte"), TOYFLAG_USR|TOYFLAG_BIN)) config CAT bool "cat" default y help - usage: cat [-u] [FILE...] + usage: cat [-etuv] [FILE...] Copy (concatenate) files to stdout. If no files listed, copy from stdin. Filename "-" is a synonym for stdin. - -u Copy one byte at a time (slow) - -config CAT_V - bool "cat -etv" - default n - depends on CAT - help - usage: cat [-evt] - -e Mark each newline with $ -t Show tabs as ^I + -u Copy one byte at a time (slow) -v Display nonprinting characters as escape sequences with M-x for high ascii characters (>127), and ^x for other nonprinting chars @@ -53,40 +45,44 @@ config CATV static void do_cat(int fd, char *name) { - int i, len, size=(toys.optflags & FLAG_u) ? 1 : sizeof(toybuf); + int i, len, size; - for(;;) { + if (!toys.optflags) { + xsendfile(fd, 1); + return; + } + + size = FLAG(u) ? 1 : sizeof(toybuf); + for (;;) { len = read(fd, toybuf, size); if (len < 0) { toys.exitval = EXIT_FAILURE; perror_msg_raw(name); } if (len < 1) break; - if ((CFG_CAT_V || CFG_CATV) && (toys.optflags&~FLAG_u)) { - for (i=0; i<len; i++) { - char c=toybuf[i]; + for (i=0; i<len; i++) { + char c=toybuf[i]; - if (c > 126 && (toys.optflags & FLAG_v)) { - if (c > 127) { - printf("M-"); - c -= 128; - } - if (c == 127) { - printf("^?"); - continue; - } + if (c > 126 && FLAG(v)) { + if (c > 127) { + printf("M-"); + c -= 128; } - if (c < 32) { - if (c == 10) { - if (toys.optflags & FLAG_e) xputc('$'); - } else if (toys.optflags & (c==9 ? FLAG_t : FLAG_v)) { - printf("^%c", c+'@'); - continue; - } + if (c == 127) { + printf("^?"); + continue; } - xputc(c); } - } else xwrite(1, toybuf, len); + if (c < ' ') { + if (c == '\n') { + if (FLAG(e)) xputc('$'); + } else if (c == '\t' ? FLAG(t) : FLAG(v)) { + printf("^%c", c+'@'); + continue; + } + } + xputc(c); + } } } diff --git a/toys/posix/head.c b/toys/posix/head.c index d5c0700c..b02241f7 100644 --- a/toys/posix/head.c +++ b/toys/posix/head.c @@ -34,7 +34,7 @@ GLOBALS( static void do_head(int fd, char *name) { - long i, len, lines=TT.n, bytes=TT.c; + long i, len, lines = TT.n; if ((toys.optc > 1 && !FLAG(q)) || FLAG(v)) { // Print an extra newline for all but the first file @@ -42,15 +42,17 @@ static void do_head(int fd, char *name) xprintf("==> %s <==\n", name); } - while (FLAG(c) ? bytes : lines) { + if (FLAG(c)) { + xsendfile_len(fd, 1, TT.c, 0); + return; + } + + while (lines) { len = read(fd, toybuf, sizeof(toybuf)); if (len<0) perror_msg_raw(name); if (len<1) break; - if (bytes) { - i = bytes >= len ? len : bytes; - bytes -= i; - } else for(i=0; i<len;) if (toybuf[i++] == '\n' && !--lines) break; + for(i=0; i<len;) if (toybuf[i++] == '\n' && !--lines) break; xwrite(1, toybuf, i); } diff --git a/toys/posix/tar.c b/toys/posix/tar.c index 87e88f65..fdeac228 100644 --- a/toys/posix/tar.c +++ b/toys/posix/tar.c @@ -455,7 +455,8 @@ static void sendfile_sparse(int fd) if (len+used>TT.hdr.size) error_exit("sparse overflow"); } else len = TT.hdr.size; - len -= sendfile_len(TT.fd, fd, len, &sent); + sent = xsendfile_len(TT.fd, fd, len, 1); + len -= sent; used += sent; if (len) { error: -- 2.25.1.481.gfbce0eb801-goog
_______________________________________________ Toybox mailing list [email protected] http://lists.landley.net/listinfo.cgi/toybox-landley.net
