[Toybox] [PATCH] More use of sendfile.

Rob Landley rob at landley.net
Mon Mar 16 05:56:07 PDT 2020


On 3/15/20 12:44 AM, enh via Toybox wrote:
> 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

Hang on, I need a beverage.

The patch touches 8 files. Right....

> weaker than the usual guarantee, silently ignoring a variety of errors.
> It also made me realize that xsendfile() wasn't actually utilizing
> sendfile(2).

Has it be 7 years already?

  https://lkml.org/lkml/2013/9/17/25

Hmmm, 2.6.33:

  http://man7.org/linux/man-pages/man2/sendfile.2.html

Yes it has. Ok.

> 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.

The idea was that httpd shouldn't exit if one transaction fails, cp shouldn't
exit if one file it's copying fails... These days I've added a WARN_ONLY flag to
the xopen() flag set, but that doesn't help for things that don't take open flags.

> 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).

It's the middle of the night so I should not be reviewing this, but a quick
first pass:

I tend to do wrappers that eliminate unnecessary arguments, but you have exactly
one user of each codepath. Sigh.

Long english error message, "sent %lld (of %lld) bytes" is less of a lift for
non-english speakers?

Moving the size = down later accomplishes what?

Sigh, this patch is completely unreadable. (This is why I want brahm cohen's
algorithm for toys/pending.diff.c)

> All the cat, head, and tar tests pass (the cat test needed a minor
> modification now the error message is slightly different,

This is why I don't test error messages. (I wonder what Elie De Brauwer is up to
these days? Google says working at "Dermascan"...)

> 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).

Lemme look at this again when I'm more awake. It does multiple things...

Rob



More information about the Toybox mailing list