[PATCH] nologin: use sendfile() to submit message to user

[PATCH] nologin: use sendfile() to submit message to user

[Sami Kerola]

A read() write() pair can be replaced with sendfile(), and it should be more
efficient than suffling bytes back and forth user and kernel space.

Signed-off-by: Sami Kerola <kerolasa@cloudflare.com>
---
 login-utils/nologin.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/login-utils/nologin.c b/login-utils/nologin.c
index f38a3aab0..ca4ca4e84 100644
--- a/login-utils/nologin.c
+++ b/login-utils/nologin.c
@@ -3,6 +3,7 @@
  */
 
 #include <stdio.h>
+#include <sys/sendfile.h>
 #include <sys/stat.h>
 #include <sys/types.h>
 #include <fcntl.h>
@@ -97,12 +98,12 @@ int main(int argc, char *argv[])
 	if (c < 0 || !S_ISREG(st.st_mode))
 		goto dflt;
 	else {
-		char buf[BUFSIZ];
-		ssize_t rd;
-
-		while ((rd = read(fd, buf, sizeof(buf))) > 0)
-			ignore_result( write(STDOUT_FILENO, buf, rd) );
+		int stdout_fd;
 
+		stdout_fd = fileno(stdout);
+		if (stdout_fd < 0)
+			goto dflt;
+		ignore_result( sendfile(stdout_fd, fd, NULL, st.st_size) );
 		close(fd);
 		return EXIT_FAILURE;
 	}
-- 
2.28.0

Re: [PATCH] nologin: use sendfile() to submit message to user

[Karel Zak]

On Tue, Sep 01, 2020 at 08:01:31PM +0100, Sami Kerola wrote:
> A read() write() pair can be replaced with sendfile(), and it should be more
> efficient than suffling bytes back and forth user and kernel space.

Good idea, but not sure how portable is it to another OS. Maybe add
a fallback to include/fileutils.h or mark nologin by "if LINUX" in the
Makefile.

    Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

Re: [PATCH] nologin: use sendfile() to submit message to user

[Christoph Hellwig]

On Tue, Sep 01, 2020 at 08:01:31PM +0100, Sami Kerola wrote:
> A read() write() pair can be replaced with sendfile(), and it should be more
> efficient than suffling bytes back and forth user and kernel space.

What kinds of fds are this?  If this involves things like a tty
sendfile will probably stop working in Linux 5.10, as the kernel
fallback is pretty horrible and not exactly more efficient.  Sendfile
also hasn't always been supported on all kinds of files, so you'll still
always need a fallback.

Re: [PATCH] nologin: use sendfile() to submit message to user

[Sami Kerola]

On Wed, 2 Sep 2020 at 15:17, Christoph Hellwig <hch@infradead.org> wrote:
> On Tue, Sep 01, 2020 at 08:01:31PM +0100, Sami Kerola wrote:
> > A read() write() pair can be replaced with sendfile(), and it should be more
> > efficient than suffling bytes back and forth user and kernel space.
>
> What kinds of fds are this?  If this involves things like a tty
> sendfile will probably stop working in Linux 5.10, as the kernel
> fallback is pretty horrible and not exactly more efficient.  Sendfile
> also hasn't always been supported on all kinds of files, so you'll still
> always need a fallback.

Thank you for a review Christoph. Considering fallback to read() && write()
is  always required I do not think there is any advantage to call sendfile().
Karel, please consider this change falling short in simplifying or improving
anything and therefore rejected.

-- 
Sami Kerola
http://www.iki.fi/kerolasa/