[PATCH] Cache messaging dgm connections
Jeremy Allison
jra at samba.org
Sat Sep 17 06:49:32 UTC 2016
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.
-------------- next part --------------
>From 421b90c32909ed7495599eedf45248629c303ee1 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/5] 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 273a1cd9c0675966880861f35868de61639c74e9 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/5] 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 c7536d3219ee395824271808833ce4af0e0021e5 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/5] 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 1ba4685989abb0ccd10cdf0c5aebf42356a3d73d Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Fri, 16 Sep 2016 22:02:03 -0700
Subject: [PATCH 4/5] WIP. Add canary destructor to track ev_ctx free.
Signed-off-by: Jeremy Allison <jra at samba.org>
---
lib/poll_funcs/poll_funcs_tevent.c | 32 ++++++++++++++++++++++++++++++++
lib/poll_funcs/poll_funcs_tevent.h | 1 +
source3/lib/messages.c | 10 ++++++++++
source4/lib/messaging/messaging.c | 8 ++++++++
4 files changed, 51 insertions(+)
diff --git a/lib/poll_funcs/poll_funcs_tevent.c b/lib/poll_funcs/poll_funcs_tevent.c
index 233e911..e84e18f 100644
--- a/lib/poll_funcs/poll_funcs_tevent.c
+++ b/lib/poll_funcs/poll_funcs_tevent.c
@@ -20,6 +20,7 @@
#include "tevent.h"
#include "system/select.h"
#include "lib/util/dlinklist.h"
+#include "system/filesys.h"
/*
* A poll_watch is asked for by the engine using this library via
@@ -84,6 +85,23 @@ struct poll_funcs_tevent_handle {
struct poll_funcs_tevent_context *ctx;
};
+static void (*debug_fn_ptr)(const char *str);
+
+void poll_funcs_tevent_set_debug_fn(void (*fn)(const char *str))
+{
+ debug_fn_ptr = fn;
+}
+
+static int canary_destructor(char *str)
+{
+ char *dbg_str = talloc_asprintf(NULL,
+ "CANARY DESTROYING EV_CTX %p PID %u\n",
+ talloc_parent(str),
+ (unsigned int)getpid());
+ (*debug_fn_ptr)(dbg_str);
+ talloc_free(dbg_str);
+ return 0;
+}
static uint16_t poll_events_to_tevent(short events)
{
uint16_t ret = 0;
@@ -538,6 +556,7 @@ static struct poll_funcs_tevent_context *poll_funcs_tevent_context_new(
size_t num_watches = talloc_array_length(state->watches);
size_t num_timeouts = talloc_array_length(state->timeouts);
size_t i;
+ char *canary = NULL;
ctx = talloc(mem_ctx, struct poll_funcs_tevent_context);
if (ctx == NULL) {
@@ -588,6 +607,19 @@ static struct poll_funcs_tevent_context *poll_funcs_tevent_context_new(
}
talloc_set_destructor(ctx, poll_funcs_tevent_context_destructor);
+
+ canary = talloc_memdup(ev, "canary", 7);
+ talloc_set_destructor(canary, canary_destructor);
+
+ {
+ char *dbg_str = talloc_asprintf(NULL,
+ "CANARY ADDING EV_CTX %p PID %u\n",
+ ev,
+ (unsigned int)getpid());
+ (*debug_fn_ptr)(dbg_str);
+ talloc_free(dbg_str);
+ }
+
return ctx;
fail:
TALLOC_FREE(ctx);
diff --git a/lib/poll_funcs/poll_funcs_tevent.h b/lib/poll_funcs/poll_funcs_tevent.h
index 8b2964c..0d2d293 100644
--- a/lib/poll_funcs/poll_funcs_tevent.h
+++ b/lib/poll_funcs/poll_funcs_tevent.h
@@ -35,4 +35,5 @@ struct poll_funcs *poll_funcs_init_tevent(TALLOC_CTX *mem_ctx);
void *poll_funcs_tevent_register(TALLOC_CTX *mem_ctx, struct poll_funcs *f,
struct tevent_context *ev);
+void poll_funcs_tevent_set_debug_fn(void (*fn)(const char *str));
#endif
diff --git a/source3/lib/messages.c b/source3/lib/messages.c
index 3ed6dfe..1ca0f01 100644
--- a/source3/lib/messages.c
+++ b/source3/lib/messages.c
@@ -56,6 +56,7 @@
#include "lib/util/server_id_db.h"
#include "lib/messages_dgm_ref.h"
#include "lib/messages_util.h"
+#include "lib/poll_funcs/poll_funcs_tevent.h"
struct messaging_callback {
struct messaging_callback *prev, *next;
@@ -183,6 +184,11 @@ static const char *private_path(const char *name)
return talloc_asprintf(talloc_tos(), "%s/%s", lp_private_dir(), name);
}
+static void canary_debug(const char *str)
+{
+ DEBUG(0,("%s", str));
+}
+
struct messaging_context *messaging_init(TALLOC_CTX *mem_ctx,
struct tevent_context *ev)
{
@@ -234,6 +240,8 @@ struct messaging_context *messaging_init(TALLOC_CTX *mem_ctx,
return NULL;
}
+ poll_funcs_tevent_set_debug_fn(canary_debug);
+
ctx->msg_dgm_ref = messaging_dgm_ref(
ctx, ctx->event_ctx, &ctx->id.unique_id,
priv_path, lck_path, messaging_recv_cb, ctx, &ret);
@@ -307,6 +315,8 @@ NTSTATUS messaging_reinit(struct messaging_context *msg_ctx)
return NT_STATUS_NO_MEMORY;
}
+ poll_funcs_tevent_set_debug_fn(canary_debug);
+
msg_ctx->msg_dgm_ref = messaging_dgm_ref(
msg_ctx, msg_ctx->event_ctx, &msg_ctx->id.unique_id,
private_path("msg.sock"), lck_path,
diff --git a/source4/lib/messaging/messaging.c b/source4/lib/messaging/messaging.c
index d0beef6..62b0198 100644
--- a/source4/lib/messaging/messaging.c
+++ b/source4/lib/messaging/messaging.c
@@ -38,6 +38,7 @@
#include "../source3/lib/messages_dgm_ref.h"
#include "../source3/lib/messages_util.h"
#include <tdb.h>
+#include "lib/poll_funcs/poll_funcs_tevent.h"
/* change the message version with any incompatible changes in the protocol */
#define IMESSAGING_VERSION 1
@@ -324,6 +325,11 @@ void imessaging_dgm_unref_all(void)
}
}
+static void canary_debug(const char *str)
+{
+ DEBUG(0,("%s", str));
+}
+
/*
create the listening socket and setup the dispatcher
*/
@@ -374,6 +380,8 @@ struct imessaging_context *imessaging_init(TALLOC_CTX *mem_ctx,
goto fail;
}
+ poll_funcs_tevent_set_debug_fn(canary_debug);
+
msg->msg_dgm_ref = messaging_dgm_ref(
msg, ev, &server_id.unique_id, msg->sock_dir, msg->lock_dir,
imessaging_dgm_recv, msg, &ret);
--
2.7.4
>From 718d9ede1121e91c184f29db05508486ed4808e2 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 5/5] 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 e84e18f..44fda0f 100644
--- a/lib/poll_funcs/poll_funcs_tevent.c
+++ b/lib/poll_funcs/poll_funcs_tevent.c
@@ -522,10 +522,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->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