[PATCH] smbd: Use mutex instead of fcntl lock for echohandler coordination

Christof Schmitt cs at samba.org
Wed Jul 2 14:49:38 MDT 2014


>From be394a99160cc9921731e50dba9c0e8af96796a4 Mon Sep 17 00:00:00 2001
From: Christof Schmitt <cs at samba.org>
Date: Fri, 27 Jun 2014 13:05:59 -0700
Subject: [PATCH] smbd: Use mutex instead of fcntl lock for echohandler coordination

This is based on a patch from Volker. When the system supports roboust
mutexes, they will be used for the coordiations between worker and
echohandler process. This avoids another aspect of the fcntl scalibility
issue when handling many client connections. When mutexes are not
available, the code falls back to the fcntl lock.

Signed-off-by: Christof Schmitt <cs at samba.org>
---
 source3/smbd/globals.h |    9 +++-
 source3/smbd/msdfs.c   |    1 +
 source3/smbd/process.c |  141 +++++++++++++++++++++++++++++++++++++++---------
 3 files changed, 124 insertions(+), 27 deletions(-)

diff --git a/source3/smbd/globals.h b/source3/smbd/globals.h
index d9ca5e3..b7f4ae9 100644
--- a/source3/smbd/globals.h
+++ b/source3/smbd/globals.h
@@ -694,10 +694,17 @@ struct smbd_server_connection {
 		struct tevent_fd *fde;
 
 		struct {
+
 			/*
-			 * fd for the fcntl lock mutexing access to our sock
+			 * fd for the fcntl lock and process shared
+			 * robust mutex to coordinate access to the
+			 * client socket. When the system supports
+			 * process shared robust mutexes, those are
+			 * used. If not, then the fcntl lock will be
+			 * used.
 			 */
 			int socket_lock_fd;
+			pthread_mutex_t *socket_mutex;
 
 			/*
 			 * fd for the trusted pipe from
diff --git a/source3/smbd/msdfs.c b/source3/smbd/msdfs.c
index 97849d2..2569941 100644
--- a/source3/smbd/msdfs.c
+++ b/source3/smbd/msdfs.c
@@ -249,6 +249,7 @@ static NTSTATUS create_conn_struct_as_root(TALLOC_CTX *ctx,
 	sconn->sock = -1;
 	sconn->smb1.echo_handler.trusted_fd = -1;
 	sconn->smb1.echo_handler.socket_lock_fd = -1;
+	sconn->smb1.echo_handler.socket_mutex = NULL;
 
 	conn = conn_new(sconn);
 	if (conn == NULL) {
diff --git a/source3/smbd/process.c b/source3/smbd/process.c
index b1d522d..35d1885 100644
--- a/source3/smbd/process.c
+++ b/source3/smbd/process.c
@@ -40,6 +40,7 @@
 #include "../libcli/security/security_token.h"
 #include "lib/id_cache.h"
 #include "serverid.h"
+#include "pthread.h"
 
 /* Internal message queue for deferred opens. */
 struct pending_message_list {
@@ -63,9 +64,8 @@ static bool smb_splice_chain(uint8_t **poutbuf, const uint8_t *andx_buf);
 
 static bool smbd_lock_socket_internal(struct smbd_server_connection *sconn)
 {
-	bool ok;
-
-	if (sconn->smb1.echo_handler.socket_lock_fd == -1) {
+	if (sconn->smb1.echo_handler.socket_lock_fd == -1 &&
+	    sconn->smb1.echo_handler.socket_mutex == NULL) {
 		return true;
 	}
 
@@ -77,15 +77,34 @@ static bool smbd_lock_socket_internal(struct smbd_server_connection *sconn)
 
 	DEBUG(10,("pid[%d] wait for socket lock\n", (int)getpid()));
 
-	do {
-		ok = fcntl_lock(
-			sconn->smb1.echo_handler.socket_lock_fd,
-			F_SETLKW, 0, 0, F_WRLCK);
-	} while (!ok && (errno == EINTR));
+	if (sconn->smb1.echo_handler.socket_mutex != NULL) {
+		int ret = EINTR;
 
-	if (!ok) {
-		DEBUG(1, ("fcntl_lock failed: %s\n", strerror(errno)));
-		return false;
+		while (ret == EINTR) {
+			ret = pthread_mutex_lock(
+				sconn->smb1.echo_handler.socket_mutex);
+			if (ret == 0) {
+				break;
+			}
+		}
+		if (ret != 0) {
+			DEBUG(1, ("pthread_mutex_lock failed: %s\n",
+				  strerror(ret)));
+			return false;
+		}
+	} else {
+		bool ok;
+
+		do {
+			ok = fcntl_lock(
+				sconn->smb1.echo_handler.socket_lock_fd,
+				F_SETLKW, 0, 0, F_WRLCK);
+		} while (!ok && (errno == EINTR));
+
+		if (!ok) {
+			DEBUG(1, ("fcntl_lock failed: %s\n", strerror(errno)));
+			return false;
+		}
 	}
 
 	DEBUG(10,("pid[%d] got socket lock\n", (int)getpid()));
@@ -102,9 +121,8 @@ void smbd_lock_socket(struct smbd_server_connection *sconn)
 
 static bool smbd_unlock_socket_internal(struct smbd_server_connection *sconn)
 {
-	bool ok;
-
-	if (sconn->smb1.echo_handler.socket_lock_fd == -1) {
+	if (sconn->smb1.echo_handler.socket_lock_fd == -1 &&
+	    sconn->smb1.echo_handler.socket_mutex == NULL) {
 		return true;
 	}
 
@@ -114,15 +132,34 @@ static bool smbd_unlock_socket_internal(struct smbd_server_connection *sconn)
 		return true;
 	}
 
-	do {
-		ok = fcntl_lock(
-			sconn->smb1.echo_handler.socket_lock_fd,
-			F_SETLKW, 0, 0, F_UNLCK);
-	} while (!ok && (errno == EINTR));
+	if (sconn->smb1.echo_handler.socket_mutex != NULL) {
+		int ret = EINTR;
 
-	if (!ok) {
-		DEBUG(1, ("fcntl_lock failed: %s\n", strerror(errno)));
-		return false;
+		while (ret == EINTR) {
+			ret = pthread_mutex_unlock(
+				sconn->smb1.echo_handler.socket_mutex);
+			if (ret == 0) {
+				break;
+			}
+		}
+		if (ret != 0) {
+			DEBUG(1, ("pthread_mutex_unlock failed: %s\n",
+				  strerror(ret)));
+			return false;
+		}
+	} else {
+		bool ok;
+
+		do {
+			ok = fcntl_lock(
+				sconn->smb1.echo_handler.socket_lock_fd,
+				F_SETLKW, 0, 0, F_UNLCK);
+		} while (!ok && (errno == EINTR));
+
+		if (!ok) {
+			DEBUG(1, ("fcntl_lock failed: %s\n", strerror(errno)));
+			return false;
+		}
 	}
 
 	DEBUG(10,("pid[%d] unlocked socket\n", (int)getpid()));
@@ -3141,10 +3178,56 @@ bool fork_echo_handler(struct smbd_server_connection *sconn)
 		DEBUG(1, ("pipe() failed: %s\n", strerror(errno)));
 		return false;
 	}
-	sconn->smb1.echo_handler.socket_lock_fd = create_unlink_tmp(lp_lock_directory());
-	if (sconn->smb1.echo_handler.socket_lock_fd == -1) {
-		DEBUG(1, ("Could not create lock fd: %s\n", strerror(errno)));
-		goto fail;
+
+	if (tdb_runtime_check_for_robust_mutexes()) {
+		pthread_mutexattr_t a;
+
+		sconn->smb1.echo_handler.socket_mutex =
+			(pthread_mutex_t *)anonymous_shared_allocate(
+				sizeof(pthread_mutex_t));
+		if (sconn->smb1.echo_handler.socket_mutex == NULL) {
+			DEBUG(1, ("Could not create mutex shared memory: %s\n",
+				  strerror(errno)));
+			goto fail;
+		}
+
+		res = pthread_mutexattr_init(&a);
+		if (res != 0) {
+			DEBUG(1, ("pthread_mutexattr_init failed: %s\n",
+				  strerror(res)));
+			goto fail;
+		}
+		res = pthread_mutexattr_setpshared(&a, PTHREAD_PROCESS_SHARED);
+		if (res != 0) {
+			DEBUG(1, ("pthread_mutexattr_setpshared failed: %s\n",
+				  strerror(res)));
+			pthread_mutexattr_destroy(&a);
+			goto fail;
+		}
+		res = pthread_mutexattr_setrobust_np(
+			&a, PTHREAD_MUTEX_ROBUST_NP);
+		if (res != 0) {
+			DEBUG(1, ("pthread_mutexattr_setrobust_np failed: "
+				  "%s\n", strerror(res)));
+			pthread_mutexattr_destroy(&a);
+			goto fail;
+		}
+		res = pthread_mutex_init(
+			sconn->smb1.echo_handler.socket_mutex, &a);
+		pthread_mutexattr_destroy(&a);
+		if (res != 0) {
+			DEBUG(1, ("pthread_mutex_init failed: %s\n",
+				  strerror(res)));
+			goto fail;
+		}
+	} else {
+		sconn->smb1.echo_handler.socket_lock_fd =
+			create_unlink_tmp(lp_lock_directory());
+		if (sconn->smb1.echo_handler.socket_lock_fd == -1) {
+			DEBUG(1, ("Could not create lock fd: %s\n",
+				  strerror(errno)));
+			goto fail;
+		}
 	}
 
 	child = fork();
@@ -3199,8 +3282,13 @@ fail:
 	if (sconn->smb1.echo_handler.socket_lock_fd != -1) {
 		close(sconn->smb1.echo_handler.socket_lock_fd);
 	}
+	if (sconn->smb1.echo_handler.socket_mutex != NULL) {
+		anonymous_shared_free(sconn->smb1.echo_handler.socket_mutex);
+	}
 	sconn->smb1.echo_handler.trusted_fd = -1;
 	sconn->smb1.echo_handler.socket_lock_fd = -1;
+	sconn->smb1.echo_handler.socket_mutex = NULL;
+
 	return false;
 }
 
@@ -3419,6 +3507,7 @@ void smbd_process(struct tevent_context *ev_ctx,
 	sconn->sock = sock_fd;
 	sconn->smb1.echo_handler.trusted_fd = -1;
 	sconn->smb1.echo_handler.socket_lock_fd = -1;
+	sconn->smb1.echo_handler.socket_mutex = NULL;
 
 	if (!interactive) {
 		smbd_setup_sig_term_handler(sconn);
-- 
1.7.1



More information about the samba-technical mailing list