[PATCH] Remove "multicasting" inside messages.c
Jeremy Allison
jra at samba.org
Wed Jun 21 00:39:03 UTC 2017
On Tue, Jun 20, 2017 at 10:10:33PM +0200, Volker Lendecke via samba-technical wrote:
> Hi!
>
> There's more to come. g_lock_ping_pong (like ctdb's ping_pong on
> g_lock) is a fun test to run. It exposes quite a few problems in our
> messaging system.
>
> Until then, review appreciated!
W00t! Looking forward to reviewing g_lock_ping_pong !
Until then, RB+. Pushed.
> 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
> From 2cbe1ccd9177116dda52dc014b3cc192a5d63a13 Mon Sep 17 00:00:00 2001
> From: Volker Lendecke <vl at samba.org>
> Date: Sat, 17 Jun 2017 08:48:16 +0200
> Subject: [PATCH] messaging: Deliver messages only once
>
> This survived an autobuild, so no subsystem strictly needs this anymore. In
> particular the notify subsystem has been rewritten.
>
> Why this patch? It removes some complexity from core code, and it reduces the
> potential memory overconsumption: Right now I'm working on a g_lock_ping_pong
> test. This test does a lot of messaging_filtered_read_send calls in a tight
> loop on a nested event context. With the current code we let the
> messaging_filtered_read code consume the message that arrives, but it also
> posts it for consumption by the main event context attached to the messaging
> context with its "classic" callback. This test never comes back to the main
> event context, so it accumulates more and more self-posted messages. That's
> just unnecessary, given that due to the successful autobuild nothing but the
> read1 test makes use of the "multicasting" of messages.
>
> Signed-off-by: Volker Lendecke <vl at samba.org>
> ---
> source3/lib/messages.c | 24 ++++++++++--------------
> source3/torture/test_messaging_read.c | 6 +++---
> 2 files changed, 13 insertions(+), 17 deletions(-)
>
> diff --git a/source3/lib/messages.c b/source3/lib/messages.c
> index a38f40e..b0edb30 100644
> --- a/source3/lib/messages.c
> +++ b/source3/lib/messages.c
> @@ -1015,7 +1015,7 @@ static bool messaging_append_new_waiters(struct messaging_context *msg_ctx)
> return true;
> }
>
> -static void messaging_dispatch_classic(struct messaging_context *msg_ctx,
> +static bool messaging_dispatch_classic(struct messaging_context *msg_ctx,
> struct messaging_rec *rec)
> {
> struct messaging_callback *cb, *next;
> @@ -1041,13 +1041,10 @@ static void messaging_dispatch_classic(struct messaging_context *msg_ctx,
> cb->fn(msg_ctx, cb->private_data, rec->msg_type,
> rec->src, &rec->buf);
>
> - /*
> - * we continue looking for matching messages after finding
> - * one. This matters for subsystems like the internal notify
> - * code which register more than one handler for the same
> - * message type
> - */
> + return true;
> }
> +
> + return false;
> }
>
> /*
> @@ -1058,9 +1055,13 @@ static void messaging_dispatch_rec(struct messaging_context *msg_ctx,
> struct messaging_rec *rec)
> {
> size_t i;
> + bool consumed;
>
> if (ev == msg_ctx->event_ctx) {
> - messaging_dispatch_classic(msg_ctx, rec);
> + consumed = messaging_dispatch_classic(msg_ctx, rec);
> + if (consumed) {
> + return;
> + }
> }
>
> if (!messaging_append_new_waiters(msg_ctx)) {
> @@ -1102,12 +1103,7 @@ static void messaging_dispatch_rec(struct messaging_context *msg_ctx,
> if ((ev == state->ev) &&
> state->filter(rec, state->private_data)) {
> messaging_filtered_read_done(req, rec);
> -
> - /*
> - * Only the first one gets the fd-array
> - */
> - rec->num_fds = 0;
> - rec->fds = NULL;
> + return;
> }
>
> i += 1;
> diff --git a/source3/torture/test_messaging_read.c b/source3/torture/test_messaging_read.c
> index 43b4367..d3e4079 100644
> --- a/source3/torture/test_messaging_read.c
> +++ b/source3/torture/test_messaging_read.c
> @@ -122,7 +122,7 @@ bool run_messaging_read1(int dummy)
> goto fail;
> }
>
> - for (i=0; i<3; i++) {
> + for (i=0; i<2; i++) {
> if (tevent_loop_once(ev) != 0) {
> fprintf(stderr, "tevent_loop_once failed\n");
> goto fail;
> @@ -131,8 +131,8 @@ bool run_messaging_read1(int dummy)
>
> printf("%u/%u\n", count1, count2);
>
> - if ((count1 != 1) || (count2 != 1)){
> - fprintf(stderr, "Got %u/%u msgs, expected 1 each\n",
> + if ((count1 != 1) || (count2 != 0)) {
> + fprintf(stderr, "Got %u/%u msgs, expected 1/0\n",
> count1, count2);
> goto fail;
> }
> --
> 2.1.4
>
More information about the samba-technical
mailing list