smbd: High CPU usage

Jeremy Allison jra at samba.org
Wed Jul 18 23:16:46 UTC 2018


On Wed, Jul 18, 2018 at 09:20:10PM +0300, Vadim Lazovskiy via samba-technical wrote:
> I would not agree.
> sendfile is great. It makes you able to serve content directly from disk to
> network without unnessesery copying to userspace. It saves you lots
> syscalls and context switching.
> As for our workload profile (1-2Gbit per server) it really saves CPU time.
> 
> If someone of samba developers treat this as bug and eager to fix it I
> could provide extra information and do some debugging.

Here is a full patchset that should handle all platforms
for which we support sendfile(). Can you try it out and confirm it
fixes your issue ?

I logged:

BUG: https://bugzilla.samba.org/show_bug.cgi?id=13537

to track this.

Jeremy.
-------------- next part --------------
From f0ed4f0930673ee044f187085e8972b8be104ebd Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Wed, 18 Jul 2018 13:32:49 -0700
Subject: [PATCH 1/5] s3: smbd: Fix Linux sendfile() for SMB2. Ensure we don't
 spin on EAGAIN.

For SMB2 the socket is set non-blocking. Ensure sendfile()
calls complete by saving the socket state, setting it blocking,
doing the sendfile until completion and then restoring the socket
state.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=13537

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 source3/lib/sendfile.c | 54 ++++++++++++++++++++++++++++++++++++------
 1 file changed, 47 insertions(+), 7 deletions(-)

diff --git a/source3/lib/sendfile.c b/source3/lib/sendfile.c
index 3d457bd6f13..a578a66e7de 100644
--- a/source3/lib/sendfile.c
+++ b/source3/lib/sendfile.c
@@ -24,6 +24,7 @@
  */
 
 #include "includes.h"
+#include "system/filesys.h"
 
 #if defined(LINUX_SENDFILE_API)
 
@@ -36,8 +37,23 @@
 ssize_t sys_sendfile(int tofd, int fromfd, const DATA_BLOB *header, off_t offset, size_t count)
 {
 	size_t total=0;
-	ssize_t ret;
+	ssize_t ret = -1;
 	size_t hdr_len = 0;
+	int saved_errno = 0;
+	int old_flags = 0;
+
+	/*
+	 * Sendfile must complete before we can
+	 * send any other outgoing data on the socket.
+	 * Ensure socket is in blocking mode.
+	 * For SMB2 by default the socket is in non-blocking
+	 * mode.
+	 */
+	old_flags = fcntl(tofd, F_GETFL, 0);
+	ret = set_blocking(tofd, true);
+	if (ret == -1) {
+		goto out;
+	}
 
 	/*
 	 * Send the header first.
@@ -48,8 +64,9 @@ ssize_t sys_sendfile(int tofd, int fromfd, const DATA_BLOB *header, off_t offset
 		hdr_len = header->length;
 		while (total < hdr_len) {
 			ret = sys_send(tofd, header->data + total,hdr_len - total, MSG_MORE);
-			if (ret == -1)
-				return -1;
+			if (ret == -1) {
+				goto out;
+			}
 			total += ret;
 		}
 	}
@@ -59,7 +76,7 @@ ssize_t sys_sendfile(int tofd, int fromfd, const DATA_BLOB *header, off_t offset
 		ssize_t nwritten;
 		do {
 			nwritten = sendfile(tofd, fromfd, &offset, total);
-		} while (nwritten == -1 && (errno == EINTR || errno == EAGAIN || errno == EWOULDBLOCK));
+		} while (nwritten == -1 && errno == EINTR);
 		if (nwritten == -1) {
 			if (errno == ENOSYS || errno == EINVAL) {
 				/* Ok - we're in a world of pain here. We just sent
@@ -72,17 +89,40 @@ ssize_t sys_sendfile(int tofd, int fromfd, const DATA_BLOB *header, off_t offset
 				 */
 				errno = EINTR; /* Normally we can never return this. */
 			}
-			return -1;
+			ret = -1;
+			goto out;
 		}
 		if (nwritten == 0) {
 			/*
 			 * EOF, return a short read
 			 */
-			return hdr_len + (count - total);
+			ret = hdr_len + (count - total);
+			goto out;
 		}
 		total -= nwritten;
 	}
-	return count + hdr_len;
+
+	ret = count + hdr_len;
+
+  out:
+
+	if (ret == -1) {
+		saved_errno = errno;
+	}
+
+	{
+		/* Restore the blocking state of the socket. */
+		int err = fcntl(tofd, F_SETFL, old_flags);
+		if (err == -1) {
+			return -1;
+		}
+	}
+
+	if (ret == -1) {
+		errno = saved_errno;
+	}
+
+	return ret;
 }
 
 #elif defined(SOLARIS_SENDFILE_API)
-- 
2.18.0.203.gfac676dfb9-goog


From abc681420b88a2d795adc44808c7e52eb2775cf6 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Wed, 18 Jul 2018 15:29:37 -0700
Subject: [PATCH 2/5] s3: smbd: Fix Solaris sendfile() for SMB2. Ensure we
 don't spin on EAGAIN.

For SMB2 the socket is set non-blocking. Ensure sendfile()
calls complete by saving the socket state, setting it blocking,
doing the sendfile until completion and then restoring the socket
state.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=13537

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 source3/lib/sendfile.c | 52 +++++++++++++++++++++++++++++++++++++-----
 1 file changed, 46 insertions(+), 6 deletions(-)

diff --git a/source3/lib/sendfile.c b/source3/lib/sendfile.c
index a578a66e7de..6c323213830 100644
--- a/source3/lib/sendfile.c
+++ b/source3/lib/sendfile.c
@@ -139,6 +139,9 @@ ssize_t sys_sendfile(int tofd, int fromfd, const DATA_BLOB *header, off_t offset
 	size_t total, xferred;
 	struct sendfilevec vec[2];
 	ssize_t hdr_len = 0;
+	int saved_errno = 0;
+	int old_flags = 0;
+	ssize_t ret = -1;
 
 	if (header) {
 		sfvcnt = 2;
@@ -164,6 +167,19 @@ ssize_t sys_sendfile(int tofd, int fromfd, const DATA_BLOB *header, off_t offset
 
 	total = count + hdr_len;
 
+	/*
+	 * Sendfile must complete before we can
+	 * send any other outgoing data on the socket.
+	 * Ensure socket is in blocking mode.
+	 * For SMB2 by default the socket is in non-blocking
+	 * mode.
+	 */
+	old_flags = fcntl(tofd, F_GETFL, 0);
+	ret = set_blocking(tofd, true);
+	if (ret == -1) {
+		goto out;
+	}
+
 	while (total) {
 		ssize_t nwritten;
 
@@ -175,17 +191,21 @@ ssize_t sys_sendfile(int tofd, int fromfd, const DATA_BLOB *header, off_t offset
 		xferred = 0;
 
 			nwritten = sendfilev(tofd, vec, sfvcnt, &xferred);
-		if  (nwritten == -1 && (errno == EINTR || errno == EAGAIN || errno == EWOULDBLOCK)) {
+		if  (nwritten == -1 && errno == EINTR) {
 			if (xferred == 0)
 				continue; /* Nothing written yet. */
 			else
 				nwritten = xferred;
 		}
 
-		if (nwritten == -1)
-			return -1;
-		if (nwritten == 0)
-			return -1; /* I think we're at EOF here... */
+		if (nwritten == -1) {
+			ret = -1;
+			goto out;
+		}
+		if (nwritten == 0) {
+			ret = -1;
+			goto out; /* I think we're at EOF here... */
+		}
 
 		/*
 		 * If this was a short (signal interrupted) write we may need
@@ -207,7 +227,27 @@ ssize_t sys_sendfile(int tofd, int fromfd, const DATA_BLOB *header, off_t offset
 		}
 		total -= nwritten;
 	}
-	return count + hdr_len;
+	ret = count + hdr_len;
+
+  out:
+
+	if (ret == -1) {
+		saved_errno = errno;
+	}
+
+	{
+		/* Restore the blocking state of the socket. */
+		int err = fcntl(tofd, F_SETFL, old_flags);
+		if (err == -1) {
+			return -1;
+		}
+	}
+
+	if (ret == -1) {
+		errno = saved_errno;
+	}
+
+	return ret;
 }
 
 #elif defined(HPUX_SENDFILE_API)
-- 
2.18.0.203.gfac676dfb9-goog


From 0068f7d136da89d96d50dced5eda8738c28e2938 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Wed, 18 Jul 2018 15:36:47 -0700
Subject: [PATCH 3/5] s3: smbd: Fix HPUX sendfile() for SMB2. Ensure we don't
 spin on EAGAIN.

For SMB2 the socket is set non-blocking. Ensure sendfile()
calls complete by saving the socket state, setting it blocking,
doing the sendfile until completion and then restoring the socket
state.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=13537

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 source3/lib/sendfile.c | 53 +++++++++++++++++++++++++++++++++++++-----
 1 file changed, 47 insertions(+), 6 deletions(-)

diff --git a/source3/lib/sendfile.c b/source3/lib/sendfile.c
index 6c323213830..63f50d1946f 100644
--- a/source3/lib/sendfile.c
+++ b/source3/lib/sendfile.c
@@ -260,6 +260,9 @@ ssize_t sys_sendfile(int tofd, int fromfd, const DATA_BLOB *header, off_t offset
 	size_t total=0;
 	struct iovec hdtrl[2];
 	size_t hdr_len = 0;
+	int saved_errno = 0;
+	int old_flags = 0;
+	ssize_t ret = -1;
 
 	if (header) {
 		/* Set up the header/trailer iovec. */
@@ -273,6 +276,20 @@ ssize_t sys_sendfile(int tofd, int fromfd, const DATA_BLOB *header, off_t offset
 	hdtrl[1].iov_len = 0;
 
 	total = count;
+
+	/*
+	 * Sendfile must complete before we can
+	 * send any other outgoing data on the socket.
+	 * Ensure socket is in blocking mode.
+	 * For SMB2 by default the socket is in non-blocking
+	 * mode.
+	 */
+	old_flags = fcntl(tofd, F_GETFL, 0);
+	ret = set_blocking(tofd, true);
+	if (ret == -1) {
+		goto out;
+	}
+
 	while (total + hdtrl[0].iov_len) {
 		ssize_t nwritten;
 
@@ -285,11 +302,15 @@ ssize_t sys_sendfile(int tofd, int fromfd, const DATA_BLOB *header, off_t offset
 
 		do {
 			nwritten = sendfile(tofd, fromfd, offset, total, &hdtrl[0], 0);
-		} while (nwritten == -1 && (errno == EINTR || errno == EAGAIN || errno == EWOULDBLOCK));
-		if (nwritten == -1)
-			return -1;
-		if (nwritten == 0)
-			return -1; /* I think we're at EOF here... */
+		} while (nwritten == -1 && errno == EINTR);
+		if (nwritten == -1) {
+			ret = -1;
+			goto out;
+		}
+		if (nwritten == 0) {
+			ret = -1; /* I think we're at EOF here... */
+			goto out;
+		}
 
 		/*
 		 * If this was a short (signal interrupted) write we may need
@@ -313,7 +334,27 @@ ssize_t sys_sendfile(int tofd, int fromfd, const DATA_BLOB *header, off_t offset
 		total -= nwritten;
 		offset += nwritten;
 	}
-	return count + hdr_len;
+	ret = count + hdr_len;
+
+  out:
+
+	if (ret == -1) {
+		saved_errno = errno;
+	}
+
+	{
+		/* Restore the blocking state of the socket. */
+		int err = fcntl(tofd, F_SETFL, old_flags);
+		if (err == -1) {
+			return -1;
+		}
+	}
+
+	if (ret == -1) {
+		errno = saved_errno;
+	}
+
+	return ret;
 }
 
 #elif defined(FREEBSD_SENDFILE_API) || defined(DARWIN_SENDFILE_API)
-- 
2.18.0.203.gfac676dfb9-goog


From 8df7360c2198098a2cb757910974110e33e4d4cf Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Wed, 18 Jul 2018 15:44:34 -0700
Subject: [PATCH 4/5] s3: smbd: Fix FreeBSD sendfile() for SMB2. Ensure we
 don't spin on EAGAIN.

For SMB2 the socket is set non-blocking. Ensure sendfile()
calls complete by saving the socket state, setting it blocking,
doing the sendfile until completion and then restoring the socket
state.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=13537

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 source3/lib/sendfile.c | 44 ++++++++++++++++++++++++++++++++++++++----
 1 file changed, 40 insertions(+), 4 deletions(-)

diff --git a/source3/lib/sendfile.c b/source3/lib/sendfile.c
index 63f50d1946f..575428deb15 100644
--- a/source3/lib/sendfile.c
+++ b/source3/lib/sendfile.c
@@ -368,9 +368,11 @@ ssize_t sys_sendfile(int tofd, int fromfd,
 {
 	struct sf_hdtr	sf_header = {0};
 	struct iovec	io_header = {0};
+	int saved_errno = 0;
+	int old_flags = 0;
 
 	off_t	nwritten;
-	int	ret;
+	ssize_t	ret = -1;
 
 	if (header) {
 		sf_header.headers = &io_header;
@@ -381,6 +383,19 @@ ssize_t sys_sendfile(int tofd, int fromfd,
 		sf_header.trl_cnt = 0;
 	}
 
+	/*
+	 * Sendfile must complete before we can
+	 * send any other outgoing data on the socket.
+	 * Ensure socket is in blocking mode.
+	 * For SMB2 by default the socket is in non-blocking
+	 * mode.
+	 */
+	old_flags = fcntl(tofd, F_GETFL, 0);
+	ret = set_blocking(tofd, true);
+	if (ret == -1) {
+		goto out;
+	}
+
 	while (count != 0) {
 
 		nwritten = count;
@@ -391,9 +406,10 @@ ssize_t sys_sendfile(int tofd, int fromfd,
 #else
 		ret = sendfile(fromfd, tofd, offset, count, &sf_header, &nwritten, 0);
 #endif
-		if (ret == -1 && errno != EINTR && errno != EAGAIN && errno != EWOULDBLOCK) {
+		if (ret == -1 && errno != EINTR) {
 			/* Send failed, we are toast. */
-			return -1;
+			ret = -1;
+			goto out;
 		}
 
 		if (nwritten == 0) {
@@ -420,7 +436,27 @@ ssize_t sys_sendfile(int tofd, int fromfd,
 		count -= nwritten;
 	}
 
-	return nwritten;
+	ret = nwritten;
+
+  out:
+
+	if (ret == -1) {
+		saved_errno = errno;
+	}
+
+	{
+		/* Restore the blocking state of the socket. */
+		int err = fcntl(tofd, F_SETFL, old_flags);
+		if (err == -1) {
+			return -1;
+		}
+	}
+
+	if (ret == -1) {
+		errno = saved_errno;
+	}
+
+	return ret;
 }
 
 #elif defined(AIX_SENDFILE_API)
-- 
2.18.0.203.gfac676dfb9-goog


From 019c677b42184d5f45931bdb549b22aad25ee2e9 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Wed, 18 Jul 2018 15:49:29 -0700
Subject: [PATCH 5/5] s3: smbd: Fix AIX sendfile() for SMB2. Ensure we don't
 spin on EAGAIN.

For SMB2 the socket is set non-blocking. Ensure sendfile()
calls complete by saving the socket state, setting it blocking,
doing the sendfile until completion and then restoring the socket
state.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=13537

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 source3/lib/sendfile.c | 45 +++++++++++++++++++++++++++++++++++++-----
 1 file changed, 40 insertions(+), 5 deletions(-)

diff --git a/source3/lib/sendfile.c b/source3/lib/sendfile.c
index 575428deb15..a28102b5bf9 100644
--- a/source3/lib/sendfile.c
+++ b/source3/lib/sendfile.c
@@ -469,6 +469,9 @@ ssize_t sys_sendfile(int tofd, int fromfd,
 ssize_t sys_sendfile(int tofd, int fromfd, const DATA_BLOB *header, off_t offset, size_t count)
 {
 	struct sf_parms hdtrl;
+	int saved_errno = 0;
+	int old_flags = 0;
+	ssize_t ret = -1;
 
 	/* Set up the header/trailer struct params. */
 	if (header) {
@@ -485,9 +488,20 @@ ssize_t sys_sendfile(int tofd, int fromfd, const DATA_BLOB *header, off_t offset
 	hdtrl.file_offset = offset;
 	hdtrl.file_bytes = count;
 
-	while ( hdtrl.file_bytes + hdtrl.header_length ) {
-		ssize_t ret;
+	/*
+	 * Sendfile must complete before we can
+	 * send any other outgoing data on the socket.
+	 * Ensure socket is in blocking mode.
+	 * For SMB2 by default the socket is in non-blocking
+	 * mode.
+	 */
+	old_flags = fcntl(tofd, F_GETFL, 0);
+	ret = set_blocking(tofd, true);
+	if (ret == -1) {
+		goto out;
+	}
 
+	while ( hdtrl.file_bytes + hdtrl.header_length ) {
 		/*
 		 Return Value
 
@@ -505,12 +519,33 @@ ssize_t sys_sendfile(int tofd, int fromfd, const DATA_BLOB *header, off_t offset
 		*/
 		do {
 			ret = send_file(&tofd, &hdtrl, 0);
-		} while ((ret == 1) || (ret == -1 && (errno == EINTR || errno == EAGAIN || errno == EWOULDBLOCK)));
-		if ( ret == -1 )
+		} while ((ret == 1) || (ret == -1 && errno == EINTR));
+		if ( ret == -1 ) {
+			goto out;
+		}
+	}
+
+	ret = count + header->length;
+
+  out:
+
+	if (ret == -1) {
+		saved_errno = errno;
+	}
+
+	{
+		/* Restore the blocking state of the socket. */
+		int err = fcntl(tofd, F_SETFL, old_flags);
+		if (err == -1) {
 			return -1;
+		}
 	}
 
-	return count + header->length;
+	if (ret == -1) {
+		errno = saved_errno;
+	}
+
+	return ret;
 }
 /* END AIX SEND_FILE */
 
-- 
2.18.0.203.gfac676dfb9-goog



More information about the samba-technical mailing list