[PATCH] Trim down remove_child_pid()

Volker Lendecke Volker.Lendecke at SerNet.DE
Mon Nov 9 15:14:38 UTC 2015


Hi!

This is the one where part of my latest patch stream comes
from: We need to trim down remove_child_pid() to its barely
necessary minimum. In particular, we need to avoid that the
parent smbd has to talk to ctdbd, if possible at all.

We've had the case where the smb listener was stuck for
ages, because ctdb was busy. We can't allow this to happen.

The attached patchset implements a helper process that does
all the dirty work at process exit that remove_child_pid
did.

In theory, we should also remove the 20-second thingy, but
that does not really hurt and would be something for another
day.

Comments?

Volker

-- 
SerNet GmbH, Bahnhofsallee 1b, 37081 Göttingen
phone: +49-551-370000-0, fax: +49-551-370000-9
AG Göttingen, HRB 2816, GF: Dr. Johannes Loxen
http://www.sernet.de, mailto:kontakt at sernet.de

Besuchen Sie uns vom 10.-11.11.15 auf der ISSE!
Information Security Solutions Europe Conference
Hotel Palace Berlin, 20%-Rabattcode: "ISSE15SP"

Meet us at Information Security Conference ISSE!
November 10th - 11th 2015 in Hotel Palace Berlin
For 20% discount take voucher code:  "ISSE15SP"
-------------- next part --------------
From 2ef02a7ccdbe22d323923da268f7e09d72d4bae4 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Mon, 2 Nov 2015 12:47:13 +0100
Subject: [PATCH 1/7] smbd: Implement a cleanup daemon

We do way too much stuff in the parent smbd in remove_child_pid(). In
particular accessing ctdbd is not a good idea when ctdbd is stuck in something.
We've had a case where smbd exited itself with "ctdb timeout" being set to 60
seconds. ctdb was just stuck doing recoveries, and the parent smbd was sitting
in serverid_exists trying to retrieve a record for a child that had exited. Not
good.

This daemon sits there as parent->cleanupd and receives MSG_SMB_NOTIFY_CLEANUP
messages that hold the serverid and exit status of a former child. The next
commits will step by step empty remove_child_pid in the parent and move the
tasks to the helper.

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source3/smbd/server.c        | 132 +++++++++++++++++++++++++++++++++++++++++++
 source3/smbd/smbd_cleanupd.c | 106 ++++++++++++++++++++++++++++++++++
 source3/smbd/smbd_cleanupd.h |  33 +++++++++++
 source3/wscript_build        |   2 +-
 4 files changed, 272 insertions(+), 1 deletion(-)
 create mode 100644 source3/smbd/smbd_cleanupd.c
 create mode 100644 source3/smbd/smbd_cleanupd.h

diff --git a/source3/smbd/server.c b/source3/smbd/server.c
index de0bf24..8d8f18a 100644
--- a/source3/smbd/server.c
+++ b/source3/smbd/server.c
@@ -49,6 +49,8 @@
 #include "scavenger.h"
 #include "locking/leases_db.h"
 #include "smbd/notifyd/notifyd.h"
+#include "smbd/smbd_cleanupd.h"
+#include "lib/util/sys_rw.h"
 
 #ifdef CLUSTER_SUPPORT
 #include "ctdb_protocol.h"
@@ -70,6 +72,8 @@ struct smbd_parent_context {
 	struct smbd_child_pid *children;
 	size_t num_children;
 
+	struct server_id cleanupd;
+
 	struct tevent_timer *cleanup_te;
 };
 
@@ -409,6 +413,118 @@ static bool smbd_notifyd_init(struct messaging_context *msg, bool interactive)
 	return tevent_req_poll(req, ev);
 }
 
+static void cleanupd_stopped(struct tevent_req *req);
+
+static bool cleanupd_init(struct messaging_context *msg, bool interactive,
+			  struct server_id *ppid)
+{
+	struct tevent_context *ev = messaging_tevent_context(msg);
+	struct server_id parent_id = messaging_server_id(msg);
+	struct tevent_req *req;
+	pid_t pid;
+	NTSTATUS status;
+	ssize_t rwret;
+	int ret;
+	bool ok;
+	char c;
+	int up_pipe[2];
+
+	if (interactive) {
+		req = smbd_cleanupd_send(msg, ev, msg, parent_id.pid);
+		*ppid = messaging_server_id(msg);
+		return (req != NULL);
+	}
+
+	ret = pipe(up_pipe);
+	if (ret == -1) {
+		DBG_WARNING("pipe failed: %s\n", strerror(errno));
+		return false;
+	}
+
+	pid = fork();
+	if (pid == -1) {
+		DBG_WARNING("fork failed: %s\n", strerror(errno));
+		close(up_pipe[0]);
+		close(up_pipe[1]);
+		return false;
+	}
+
+	if (pid != 0) {
+
+		close(up_pipe[1]);
+		rwret = sys_read(up_pipe[0], &c, 1);
+		close(up_pipe[0]);
+
+		if (rwret == -1) {
+			DBG_WARNING("sys_read failed: %s\n", strerror(errno));
+			return false;
+		}
+		if (rwret == 0) {
+			DBG_WARNING("cleanupd could not start\n");
+			return false;
+		}
+		if (c != 0) {
+			DBG_WARNING("cleanupd returned %d\n", (int)c);
+			return false;
+		}
+
+		DBG_DEBUG("Started cleanupd pid=%d\n", (int)pid);
+
+		*ppid = pid_to_procid(pid);
+		return true;
+	}
+
+	close(up_pipe[0]);
+
+	status = reinit_after_fork(msg, ev, true, "cleanupd");
+	if (!NT_STATUS_IS_OK(status)) {
+		DBG_WARNING("reinit_after_fork failed: %s\n",
+			    nt_errstr(status));
+		c = 1;
+		sys_write(up_pipe[1], &c, 1);
+
+		exit(1);
+	}
+
+	req = smbd_cleanupd_send(msg, ev, msg, parent_id.pid);
+	if (req == NULL) {
+		DBG_WARNING("smbd_cleanupd_send failed\n");
+		c = 2;
+		sys_write(up_pipe[1], &c, 1);
+
+		exit(1);
+	}
+
+	tevent_req_set_callback(req, cleanupd_stopped, msg);
+
+	c = 0;
+	rwret = sys_write(up_pipe[1], &c, 1);
+	close(up_pipe[1]);
+
+	if (rwret == -1) {
+		DBG_WARNING("sys_write failed: %s\n", strerror(errno));
+		exit(1);
+	}
+	if (rwret != 1) {
+		DBG_WARNING("sys_write could not write result\n");
+		exit(1);
+	}
+
+	ok = tevent_req_poll(req, ev);
+	if (!ok) {
+		DBG_WARNING("tevent_req_poll returned %s\n", strerror(errno));
+	}
+	exit(0);
+}
+
+static void cleanupd_stopped(struct tevent_req *req)
+{
+	NTSTATUS status;
+
+	status = smbd_cleanupd_recv(req);
+	DBG_WARNING("cleanupd stopped: %s\n", nt_errstr(status));
+}
+
 /*
   at most every smbd:cleanuptime seconds (default 20), we scan the BRL
   and locking database for entries to cleanup. As a side effect this
@@ -444,10 +560,22 @@ static void remove_child_pid(struct smbd_parent_context *parent,
 {
 	struct smbd_child_pid *child;
 	struct server_id child_id;
+	struct iovec iov[2];
+	NTSTATUS status;
 	int ret;
 
 	child_id = pid_to_procid(pid);
 
+	iov[0] = (struct iovec) { .iov_base = (uint8_t *)&pid,
+				  .iov_len = sizeof(pid) };
+	iov[1] = (struct iovec) { .iov_base = (uint8_t *)&unclean_shutdown,
+				  .iov_len = sizeof(bool) };
+
+	status = messaging_send_iov(parent->msg_ctx, parent->cleanupd,
+				    MSG_SMB_NOTIFY_CLEANUP,
+				    iov, ARRAY_SIZE(iov), NULL, 0);
+	DEBUG(10, ("messaging_send_iov returned %s\n", nt_errstr(status)));
+
 	ret = messaging_cleanup(parent->msg_ctx, pid);
 
 	if ((ret != 0) && (ret != ENOENT)) {
@@ -1480,6 +1608,10 @@ extern void build_options(bool screen);
 		exit_daemon("Samba cannot init notification", EACCES);
 	}
 
+	if (!cleanupd_init(msg_ctx, interactive, &parent->cleanupd)) {
+		exit_daemon("Samba cannot init the cleanupd", EACCES);
+	}
+
 	if (!messaging_parent_dgm_cleanup_init(msg_ctx)) {
 		exit(1);
 	}
diff --git a/source3/smbd/smbd_cleanupd.c b/source3/smbd/smbd_cleanupd.c
new file mode 100644
index 0000000..f11a0a4
--- /dev/null
+++ b/source3/smbd/smbd_cleanupd.c
@@ -0,0 +1,106 @@
+/*
+ * Unix SMB/CIFS implementation.
+ *
+ * Copyright (C) Volker Lendecke 2015
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "replace.h"
+#include "smbd_cleanupd.h"
+#include "lib/util/tevent_ntstatus.h"
+#include "lib/util/debug.h"
+
+struct smbd_cleanupd_state {
+	pid_t parent_pid;
+};
+
+static void smbd_cleanupd_shutdown(struct messaging_context *msg,
+				   void *private_data, uint32_t msg_type,
+				   struct server_id server_id,
+				   DATA_BLOB *data);
+static void smbd_cleanupd_process_exited(struct messaging_context *msg,
+					 void *private_data, uint32_t msg_type,
+					 struct server_id server_id,
+					 DATA_BLOB *data);
+
+struct tevent_req *smbd_cleanupd_send(TALLOC_CTX *mem_ctx,
+				      struct tevent_context *ev,
+				      struct messaging_context *msg,
+				      pid_t parent_pid)
+{
+	struct tevent_req *req;
+	struct smbd_cleanupd_state *state;
+	NTSTATUS status;
+
+	req = tevent_req_create(mem_ctx, &state, struct smbd_cleanupd_state);
+	if (req == NULL) {
+		return NULL;
+	}
+	state->parent_pid = parent_pid;
+
+	status = messaging_register(msg, req, MSG_SHUTDOWN,
+				    smbd_cleanupd_shutdown);
+	if (tevent_req_nterror(req, status)) {
+		return tevent_req_post(req, ev);
+	}
+
+	status = messaging_register(msg, req, MSG_SMB_NOTIFY_CLEANUP,
+				    smbd_cleanupd_process_exited);
+	if (tevent_req_nterror(req, status)) {
+		return tevent_req_post(req, ev);
+	}
+
+	return req;
+}
+
+static void smbd_cleanupd_shutdown(struct messaging_context *msg,
+				   void *private_data, uint32_t msg_type,
+				   struct server_id server_id,
+				   DATA_BLOB *data)
+{
+	struct tevent_req *req = talloc_get_type_abort(
+		private_data, struct tevent_req);
+	tevent_req_done(req);
+}
+
+static void smbd_cleanupd_process_exited(struct messaging_context *msg,
+					 void *private_data, uint32_t msg_type,
+					 struct server_id server_id,
+					 DATA_BLOB *data)
+{
+	struct tevent_req *req = talloc_get_type_abort(
+		private_data, struct tevent_req);
+	struct smbd_cleanupd_state *state = tevent_req_data(
+		req, struct smbd_cleanupd_state);
+	pid_t pid;
+	bool unclean_shutdown;
+
+	if (data->length != (sizeof(pid) + sizeof(unclean_shutdown))) {
+		DBG_WARNING("Got invalid length: %zu\n", data->length);
+		return;
+	}
+
+	memcpy(&pid, data->data, sizeof(pid));
+	memcpy(&unclean_shutdown, data->data + sizeof(pid),
+	       sizeof(unclean_shutdown));
+
+	DBG_DEBUG("%d exited %sclean\n", (int)pid,
+		  unclean_shutdown ? "un" : "");
+}
+
+NTSTATUS smbd_cleanupd_recv(struct tevent_req *req)
+{
+	return tevent_req_simple_recv_ntstatus(req);
+}
diff --git a/source3/smbd/smbd_cleanupd.h b/source3/smbd/smbd_cleanupd.h
new file mode 100644
index 0000000..6e5d87f
--- /dev/null
+++ b/source3/smbd/smbd_cleanupd.h
@@ -0,0 +1,33 @@
+/*
+ * Unix SMB/CIFS implementation.
+ *
+ * Copyright (C) Volker Lendecke 2014
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef __SMBD_CLEANUPD_H__
+#define __SMBD_CLEANUPD_H__
+
+#include "replace.h"
+#include <tevent.h>
+#include "messages.h"
+
+struct tevent_req *smbd_cleanupd_send(TALLOC_CTX *mem_ctx,
+				      struct tevent_context *ev,
+				      struct messaging_context *msg,
+				      pid_t parent_pid);
+NTSTATUS smbd_cleanupd_recv(struct tevent_req *req);
+
+#endif
diff --git a/source3/wscript_build b/source3/wscript_build
index 4c6390e..b5b5ea0 100755
--- a/source3/wscript_build
+++ b/source3/wscript_build
@@ -858,7 +858,7 @@ bld.SAMBA3_SUBSYSTEM('LIBLSA',
 ########################## BINARIES #################################
 
 bld.SAMBA3_BINARY('smbd/smbd',
-                 source='smbd/server.c',
+                 source='smbd/server.c smbd/smbd_cleanupd.c',
                  deps='smbd_base EPMD LSASD FSSD MDSSD',
                  install_path='${SBINDIR}')
 
-- 
1.9.1


From 63da88e53b449dc451f7f9fccc485ac15ebbc9b2 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Fri, 6 Nov 2015 14:55:35 +0100
Subject: [PATCH 2/7] smbprofile: Add dst pid to smbprofile_cleanup

The consolidation will soon be done by a separate process. We need to
avoid the getpid() call in smbprofile_cleanup().

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source3/include/smbprofile.h | 4 ++--
 source3/profile/profile.c    | 4 ++--
 source3/smbd/server.c        | 2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/source3/include/smbprofile.h b/source3/include/smbprofile.h
index 76d9d2b..c771fd4 100644
--- a/source3/include/smbprofile.h
+++ b/source3/include/smbprofile.h
@@ -533,7 +533,7 @@ static inline bool smbprofile_dump_pending(void)
 
 void smbprofile_dump(void);
 
-void smbprofile_cleanup(pid_t pid);
+void smbprofile_cleanup(pid_t pid, pid_t dst);
 void smbprofile_stats_accumulate(struct profile_stats *acc,
 				 const struct profile_stats *add);
 void smbprofile_collect(struct profile_stats *stats);
@@ -610,7 +610,7 @@ static inline void smbprofile_dump(void)
 	return;
 }
 
-static inline void smbprofile_cleanup(pid_t pid)
+static inline void smbprofile_cleanup(pid_t pid, pid_t dst)
 {
 	return;
 }
diff --git a/source3/profile/profile.c b/source3/profile/profile.c
index 00cb3e5..1464a42 100644
--- a/source3/profile/profile.c
+++ b/source3/profile/profile.c
@@ -312,7 +312,7 @@ void smbprofile_dump(void)
 	return;
 }
 
-void smbprofile_cleanup(pid_t pid)
+void smbprofile_cleanup(pid_t pid, pid_t dst)
 {
 	TDB_DATA key = { .dptr = (uint8_t *)&pid, .dsize = sizeof(pid) };
 	struct profile_stats s = {};
@@ -336,7 +336,7 @@ void smbprofile_cleanup(pid_t pid)
 	tdb_delete(smbprofile_state.internal.db->tdb, key);
 	tdb_chainunlock(smbprofile_state.internal.db->tdb, key);
 
-	pid = getpid();
+	pid = dst;
 	ret = tdb_chainlock(smbprofile_state.internal.db->tdb, key);
 	if (ret != 0) {
 		return;
diff --git a/source3/smbd/server.c b/source3/smbd/server.c
index 8d8f18a..9e71226 100644
--- a/source3/smbd/server.c
+++ b/source3/smbd/server.c
@@ -583,7 +583,7 @@ static void remove_child_pid(struct smbd_parent_context *parent,
 			   __func__, strerror(ret)));
 	}
 
-	smbprofile_cleanup(pid);
+	smbprofile_cleanup(pid, getpid());
 
 	for (child = parent->children; child != NULL; child = child->next) {
 		if (child->pid == pid) {
-- 
1.9.1


From d05fa63898f61fd7e4fa1195fd6499b1def1bc8c Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Fri, 6 Nov 2015 15:21:59 +0100
Subject: [PATCH 3/7] smbd: Move smbprofile_cleanup() to the cleanupd

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source3/smbd/server.c        | 2 --
 source3/smbd/smbd_cleanupd.c | 3 +++
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/source3/smbd/server.c b/source3/smbd/server.c
index 9e71226..ca79772 100644
--- a/source3/smbd/server.c
+++ b/source3/smbd/server.c
@@ -583,8 +583,6 @@ static void remove_child_pid(struct smbd_parent_context *parent,
 			   __func__, strerror(ret)));
 	}
 
-	smbprofile_cleanup(pid, getpid());
-
 	for (child = parent->children; child != NULL; child = child->next) {
 		if (child->pid == pid) {
 			struct smbd_child_pid *tmp = child;
diff --git a/source3/smbd/smbd_cleanupd.c b/source3/smbd/smbd_cleanupd.c
index f11a0a4..2f3d19f 100644
--- a/source3/smbd/smbd_cleanupd.c
+++ b/source3/smbd/smbd_cleanupd.c
@@ -21,6 +21,7 @@
 #include "smbd_cleanupd.h"
 #include "lib/util/tevent_ntstatus.h"
 #include "lib/util/debug.h"
+#include "smbprofile.h"
 
 struct smbd_cleanupd_state {
 	pid_t parent_pid;
@@ -98,6 +99,8 @@ static void smbd_cleanupd_process_exited(struct messaging_context *msg,
 
 	DBG_DEBUG("%d exited %sclean\n", (int)pid,
 		  unclean_shutdown ? "un" : "");
+
+	smbprofile_cleanup(pid, state->parent_pid);
 }
 
 NTSTATUS smbd_cleanupd_recv(struct tevent_req *req)
-- 
1.9.1


From 1c85cf51b91f2ff1fe4668cdca3195551e48b2ec Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Fri, 6 Nov 2015 15:32:46 +0100
Subject: [PATCH 4/7] smbd: Move messaging_cleanup() to the cleanupd

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source3/smbd/server.c        | 8 --------
 source3/smbd/smbd_cleanupd.c | 7 +++++++
 2 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/source3/smbd/server.c b/source3/smbd/server.c
index ca79772..fa184cc 100644
--- a/source3/smbd/server.c
+++ b/source3/smbd/server.c
@@ -562,7 +562,6 @@ static void remove_child_pid(struct smbd_parent_context *parent,
 	struct server_id child_id;
 	struct iovec iov[2];
 	NTSTATUS status;
-	int ret;
 
 	child_id = pid_to_procid(pid);
 
@@ -576,13 +575,6 @@ static void remove_child_pid(struct smbd_parent_context *parent,
 				    iov, ARRAY_SIZE(iov), NULL, 0);
 	DEBUG(10, ("messaging_send_iov returned %s\n", nt_errstr(status)));
 
-	ret = messaging_cleanup(parent->msg_ctx, pid);
-
-	if ((ret != 0) && (ret != ENOENT)) {
-		DEBUG(10, ("%s: messaging_cleanup returned %s\n",
-			   __func__, strerror(ret)));
-	}
-
 	for (child = parent->children; child != NULL; child = child->next) {
 		if (child->pid == pid) {
 			struct smbd_child_pid *tmp = child;
diff --git a/source3/smbd/smbd_cleanupd.c b/source3/smbd/smbd_cleanupd.c
index 2f3d19f..90a054f 100644
--- a/source3/smbd/smbd_cleanupd.c
+++ b/source3/smbd/smbd_cleanupd.c
@@ -87,6 +87,7 @@ static void smbd_cleanupd_process_exited(struct messaging_context *msg,
 		req, struct smbd_cleanupd_state);
 	pid_t pid;
 	bool unclean_shutdown;
+	int ret;
 
 	if (data->length != (sizeof(pid) + sizeof(unclean_shutdown))) {
 		DBG_WARNING("Got invalid length: %zu\n", data->length);
@@ -101,6 +102,12 @@ static void smbd_cleanupd_process_exited(struct messaging_context *msg,
 		  unclean_shutdown ? "un" : "");
 
 	smbprofile_cleanup(pid, state->parent_pid);
+
+	ret = messaging_cleanup(msg, pid);
+
+	if ((ret != 0) && (ret != ENOENT)) {
+		DBG_DEBUG("messaging_cleanup returned %s\n", strerror(ret));
+	}
 }
 
 NTSTATUS smbd_cleanupd_recv(struct tevent_req *req)
-- 
1.9.1


From 4381d51ce0ff83bee9d1f4e9a7476fbf494aa35e Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Fri, 6 Nov 2015 17:01:02 +0100
Subject: [PATCH 5/7] smbd: Move serverid_deregister() to the cleanupd

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source3/smbd/server.c        |  8 --------
 source3/smbd/smbd_cleanupd.c | 16 ++++++++++++++++
 2 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/source3/smbd/server.c b/source3/smbd/server.c
index fa184cc..26f1306 100644
--- a/source3/smbd/server.c
+++ b/source3/smbd/server.c
@@ -559,12 +559,9 @@ static void remove_child_pid(struct smbd_parent_context *parent,
 			     bool unclean_shutdown)
 {
 	struct smbd_child_pid *child;
-	struct server_id child_id;
 	struct iovec iov[2];
 	NTSTATUS status;
 
-	child_id = pid_to_procid(pid);
-
 	iov[0] = (struct iovec) { .iov_base = (uint8_t *)&pid,
 				  .iov_len = sizeof(pid) };
 	iov[1] = (struct iovec) { .iov_base = (uint8_t *)&unclean_shutdown,
@@ -609,11 +606,6 @@ static void remove_child_pid(struct smbd_parent_context *parent,
 			DEBUG(1,("Scheduled cleanup of brl and lock database after unclean shutdown\n"));
 		}
 	}
-
-	if (!serverid_deregister(child_id)) {
-		DEBUG(1, ("Could not remove pid %d from serverid.tdb\n",
-			  (int)pid));
-	}
 }
 
 /****************************************************************************
diff --git a/source3/smbd/smbd_cleanupd.c b/source3/smbd/smbd_cleanupd.c
index 90a054f..e931a28 100644
--- a/source3/smbd/smbd_cleanupd.c
+++ b/source3/smbd/smbd_cleanupd.c
@@ -19,9 +19,11 @@
 
 #include "replace.h"
 #include "smbd_cleanupd.h"
+#include "lib/util_procid.h"
 #include "lib/util/tevent_ntstatus.h"
 #include "lib/util/debug.h"
 #include "smbprofile.h"
+#include "serverid.h"
 
 struct smbd_cleanupd_state {
 	pid_t parent_pid;
@@ -86,6 +88,7 @@ static void smbd_cleanupd_process_exited(struct messaging_context *msg,
 	struct smbd_cleanupd_state *state = tevent_req_data(
 		req, struct smbd_cleanupd_state);
 	pid_t pid;
+	struct server_id child_id;
 	bool unclean_shutdown;
 	int ret;
 
@@ -101,6 +104,14 @@ static void smbd_cleanupd_process_exited(struct messaging_context *msg,
 	DBG_DEBUG("%d exited %sclean\n", (int)pid,
 		  unclean_shutdown ? "un" : "");
 
+	/*
+	 * Get child_id before messaging_cleanup which wipes the
+	 * unique_id. Not that it really matters here for functionality (the
+	 * child should have properly cleaned up :-)) though, but it looks
+	 * nicer.
+	 */
+	child_id = pid_to_procid(pid);
+
 	smbprofile_cleanup(pid, state->parent_pid);
 
 	ret = messaging_cleanup(msg, pid);
@@ -108,6 +119,11 @@ static void smbd_cleanupd_process_exited(struct messaging_context *msg,
 	if ((ret != 0) && (ret != ENOENT)) {
 		DBG_DEBUG("messaging_cleanup returned %s\n", strerror(ret));
 	}
+
+	if (!serverid_deregister(child_id)) {
+		DEBUG(1, ("Could not remove pid %d from serverid.tdb\n",
+			  (int)pid));
+	}
 }
 
 NTSTATUS smbd_cleanupd_recv(struct tevent_req *req)
-- 
1.9.1


From 412b747e5b8b5ba3d29fd4f2ffbb2c700b0157f8 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Sat, 7 Nov 2015 19:58:28 +0100
Subject: [PATCH 6/7] smbd: Send MSG_SMB_UNLOCK to children

Walking our in-memory child list is cheaper than traversing serverid.tdb,
and all who care are the smbd children

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source3/smbd/server.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/source3/smbd/server.c b/source3/smbd/server.c
index 26f1306..a16b8ef 100644
--- a/source3/smbd/server.c
+++ b/source3/smbd/server.c
@@ -548,7 +548,8 @@ static void cleanup_timeout_fn(struct tevent_context *event_ctx,
 	parent->cleanup_te = NULL;
 
 	DEBUG(1,("Cleaning up brl and lock database after unclean shutdown\n"));
-	message_send_all(parent->msg_ctx, MSG_SMB_UNLOCK, NULL, 0, NULL);
+	messaging_send_to_children(parent->msg_ctx, MSG_SMB_UNLOCK, NULL);
+
 	messaging_send_buf(parent->msg_ctx,
 			   messaging_server_id(parent->msg_ctx),
 			   MSG_SMB_BRL_VALIDATE, NULL, 0);
-- 
1.9.1


From 432d9174f4234e010ebe74633bba7a738c8b6201 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Sat, 7 Nov 2015 20:18:52 +0100
Subject: [PATCH 7/7] smbd: Move brl_validate to the cleanupd

This walks brlock.tdb, which can be time-consuming.

This adds a new includes.h include. It's too much of a pain for me now to
make locking/proto.h clean to include on its own.

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source3/smbd/server.c        | 11 +++++------
 source3/smbd/smbd_cleanupd.c |  9 ++++++++-
 2 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/source3/smbd/server.c b/source3/smbd/server.c
index a16b8ef..350cbab 100644
--- a/source3/smbd/server.c
+++ b/source3/smbd/server.c
@@ -287,8 +287,10 @@ static int smbd_parent_ctdb_reconfigured(
 	 * Someone from the family died, validate our locks
 	 */
 
-	messaging_send_buf(msg_ctx, messaging_server_id(msg_ctx),
-			   MSG_SMB_BRL_VALIDATE, NULL, 0);
+	if (am_parent) {
+		messaging_send_buf(msg_ctx, parent->cleanupd,
+				   MSG_SMB_BRL_VALIDATE, NULL, 0);
+	}
 
 	return 0;
 }
@@ -550,8 +552,7 @@ static void cleanup_timeout_fn(struct tevent_context *event_ctx,
 	DEBUG(1,("Cleaning up brl and lock database after unclean shutdown\n"));
 	messaging_send_to_children(parent->msg_ctx, MSG_SMB_UNLOCK, NULL);
 
-	messaging_send_buf(parent->msg_ctx,
-			   messaging_server_id(parent->msg_ctx),
+	messaging_send_buf(parent->msg_ctx, parent->cleanupd,
 			   MSG_SMB_BRL_VALIDATE, NULL, 0);
 }
 
@@ -1008,8 +1009,6 @@ static bool open_sockets_smbd(struct smbd_parent_context *parent,
 	messaging_register(msg_ctx, NULL, MSG_SMB_STAT_CACHE_DELETE,
 			   smb_stat_cache_delete);
 	messaging_register(msg_ctx, NULL, MSG_DEBUG, smbd_msg_debug);
-	messaging_register(msg_ctx, NULL, MSG_SMB_BRL_VALIDATE,
-			   brl_revalidate);
 	messaging_register(msg_ctx, NULL, MSG_SMB_FORCE_TDIS,
 			   smb_parent_send_to_children);
 	messaging_register(msg_ctx, NULL, MSG_SMB_KILL_CLIENT_IP,
diff --git a/source3/smbd/smbd_cleanupd.c b/source3/smbd/smbd_cleanupd.c
index e931a28..6b423e3 100644
--- a/source3/smbd/smbd_cleanupd.c
+++ b/source3/smbd/smbd_cleanupd.c
@@ -17,13 +17,14 @@
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
-#include "replace.h"
+#include "includes.h"
 #include "smbd_cleanupd.h"
 #include "lib/util_procid.h"
 #include "lib/util/tevent_ntstatus.h"
 #include "lib/util/debug.h"
 #include "smbprofile.h"
 #include "serverid.h"
+#include "locking/proto.h"
 
 struct smbd_cleanupd_state {
 	pid_t parent_pid;
@@ -65,6 +66,12 @@ struct tevent_req *smbd_cleanupd_send(TALLOC_CTX *mem_ctx,
 		return tevent_req_post(req, ev);
 	}
 
+	status = messaging_register(msg, NULL, MSG_SMB_BRL_VALIDATE,
+				    brl_revalidate);
+	if (tevent_req_nterror(req, status)) {
+		return tevent_req_post(req, ev);
+	}
+
 	return req;
 }
 
-- 
1.9.1



More information about the samba-technical mailing list