[PATCH] Cache messaging dgm connections

Jeremy Allison jra at samba.org
Sat Sep 17 08:35:14 UTC 2016


On Sat, Sep 17, 2016 at 01:06:47AM -0700, Jeremy Allison wrote:
> On Fri, Sep 16, 2016 at 11:49:32PM -0700, Jeremy Allison wrote:
> > On Sat, Sep 17, 2016 at 07:54:28AM +0200, Ralph Böhme wrote:
> > > 
> > > I'm now running autobuilds with it on sn-devel.
> > 
> > Yeah, the reinit after fork makes it hard to follow.
> > 
> > Here's a new version with pid's attached to the ev
> > context debugs.
> > 
> > I think I've also found another issue we might have
> > missed before.
> > 
> > In poll_funcs_context_slot_find() the logic for
> > looking for a free slot (ctx == NULL) is tied
> > up with the logic looking for an existing match
> > (ctx->ev == ev).
> > 
> > I think that needs to be split so we look for
> > an existing match, and only if not found do
> > we look for a free slot, and if no free slot
> > then we create one.
> > 
> > The last patch in this series has the fix
> > for this.
> 
> Ah, that last patch should look like the
> attached (missed checking for ctx != NULL
> before testing ctx->ev == ev).
> 
> Retrying a private autobuild now.

So here is the full patchset I'm testing with.

I have a good feeling about this one (when
running with the canary debug patch on top
of the last patch that fixes the logic for
looking for a free context slot I saw much
fewer EV CONTEXT debug messages)....

I'll leave this running overnight and check
up in the morning.
-------------- next part --------------
>From 40ce3214466b8f0cbc6a31901f6c7f8063dc0a47 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Thu, 15 Sep 2016 14:19:27 +0200
Subject: [PATCH 1/4] lib/poll_funcs: free timers in
 poll_funcs_state_destructor()

Signed-off-by: Ralph Boehme <slow at samba.org>
Reviewed-by: Jeremy Allison <jra at samba.org>
---
 lib/poll_funcs/poll_funcs_tevent.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/lib/poll_funcs/poll_funcs_tevent.c b/lib/poll_funcs/poll_funcs_tevent.c
index 3059ebc..3d79b75 100644
--- a/lib/poll_funcs/poll_funcs_tevent.c
+++ b/lib/poll_funcs/poll_funcs_tevent.c
@@ -474,6 +474,7 @@ struct poll_funcs *poll_funcs_init_tevent(TALLOC_CTX *mem_ctx)
 static int poll_funcs_state_destructor(struct poll_funcs_state *state)
 {
 	size_t num_watches = talloc_array_length(state->watches);
+	size_t num_timeouts = talloc_array_length(state->timeouts);
 	size_t i;
 	/*
 	 * Make sure the watches are cleared before the contexts. The watches
@@ -482,6 +483,9 @@ static int poll_funcs_state_destructor(struct poll_funcs_state *state)
 	for (i=0; i<num_watches; i++) {
 		TALLOC_FREE(state->watches[i]);
 	}
+	for (i=0; i<num_timeouts; i++) {
+		TALLOC_FREE(state->timeouts[i]);
+	}
 	return 0;
 }
 
-- 
2.7.4


>From 1c80a02a350dbcb78c84b0ba2a4cae9111f165b7 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Fri, 16 Sep 2016 17:55:56 +0200
Subject: [PATCH 2/4] lib/poll_funcs: free contexts in
 poll_funcs_state_destructor()

This ensures the destructors get called in the proper order.

Signed-off-by: Ralph Boehme <slow at samba.org>
Reviewed-by: Jeremy Allison <jra at samba.org>
---
 lib/poll_funcs/poll_funcs_tevent.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/lib/poll_funcs/poll_funcs_tevent.c b/lib/poll_funcs/poll_funcs_tevent.c
index 3d79b75..233e911 100644
--- a/lib/poll_funcs/poll_funcs_tevent.c
+++ b/lib/poll_funcs/poll_funcs_tevent.c
@@ -475,6 +475,7 @@ static int poll_funcs_state_destructor(struct poll_funcs_state *state)
 {
 	size_t num_watches = talloc_array_length(state->watches);
 	size_t num_timeouts = talloc_array_length(state->timeouts);
+	size_t num_contexts = talloc_array_length(state->contexts);
 	size_t i;
 	/*
 	 * Make sure the watches are cleared before the contexts. The watches
@@ -486,6 +487,9 @@ static int poll_funcs_state_destructor(struct poll_funcs_state *state)
 	for (i=0; i<num_timeouts; i++) {
 		TALLOC_FREE(state->timeouts[i]);
 	}
+	for (i=0; i<num_contexts; i++) {
+		TALLOC_FREE(state->contexts[i]);
+	}
 	return 0;
 }
 
-- 
2.7.4


>From 53254ec922db99e1433e7c581ab42f08615e8db8 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Thu, 15 Sep 2016 14:19:51 +0200
Subject: [PATCH 3/4] s4/messaging: let the imessaging ctx destructor free
 msg_dgm_ref

Signed-off-by: Ralph Boehme <slow at samba.org>
Reviewed-by: Jeremy Allison <jra at samba.org>
---
 source4/lib/messaging/messaging.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/source4/lib/messaging/messaging.c b/source4/lib/messaging/messaging.c
index ea50627..d0beef6 100644
--- a/source4/lib/messaging/messaging.c
+++ b/source4/lib/messaging/messaging.c
@@ -304,6 +304,7 @@ static struct imessaging_context *msg_ctxs;
 static int imessaging_context_destructor(struct imessaging_context *msg)
 {
 	DLIST_REMOVE(msg_ctxs, msg);
+	TALLOC_FREE(msg->msg_dgm_ref);
 	return 0;
 }
 
-- 
2.7.4


>From a3d33320ea3ee273b0a811bb1dd26908411288b5 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Fri, 16 Sep 2016 23:37:20 -0700
Subject: [PATCH 4/4] test: look for a match first, before a free slot.

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 lib/poll_funcs/poll_funcs_tevent.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/lib/poll_funcs/poll_funcs_tevent.c b/lib/poll_funcs/poll_funcs_tevent.c
index 233e911..4dd09a2 100644
--- a/lib/poll_funcs/poll_funcs_tevent.c
+++ b/lib/poll_funcs/poll_funcs_tevent.c
@@ -504,10 +504,21 @@ static bool poll_funcs_context_slot_find(struct poll_funcs_state *state,
 	size_t num_contexts = talloc_array_length(state->contexts);
 	size_t i;
 
+	/* Look for an existing match first. */
 	for (i=0; i<num_contexts; i++) {
 		struct poll_funcs_tevent_context *ctx = state->contexts[i];
 
-		if ((ctx == NULL) || (ctx->ev == ev)) {
+		if (ctx != NULL && ctx->ev == ev) {
+			*slot = i;
+			return true;
+		}
+	}
+
+	/* Now look for a free slot. */
+	for (i=0; i<num_contexts; i++) {
+		struct poll_funcs_tevent_context *ctx = state->contexts[i];
+
+		if (ctx == NULL) {
 			*slot = i;
 			return true;
 		}
-- 
2.7.4



More information about the samba-technical mailing list