[ctdb-common] Small improvements to sock_io's memory handling

[Swen Schillig]

On Mon, 2018-01-29 at 15:11 +1100, Amitay Isaacs wrote:
> Hi Swen,
> 
> On Thu, Jan 25, 2018 at 9:13 PM, Swen Schillig via samba-technical
> <samba-technical at lists.samba.org> wrote:
> > Hi
> > 
> > attached is the outcome of the discussed improvements to sock_io.
> > Please review.
> > 
> 
> Can you move the #defines in the last patch next to the definition of
> sock_queue structure?  Also, the comment in sock_queue_setup.
> That should make code much more readable.
Ok

Here we go.

Cheers Swen.
-------------- next part --------------
From f5572f92379e3a49b61cc68e30350805a62d838a Mon Sep 17 00:00:00 2001
From: Swen Schillig <swen at vnet.ibm.com>
Date: Mon, 8 Jan 2018 14:10:40 +0100
Subject: [PATCH 1/3] ctdb-common: Return if packet size is zero

Prevent further processing of sock_queue_process
if the received packet size is zero.

Signed-off-by: Swen Schillig <swen at vnet.ibm.com>
---
 ctdb/common/sock_io.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/ctdb/common/sock_io.c b/ctdb/common/sock_io.c
index 3f7138f0b7f..8a0b9859d01 100644
--- a/ctdb/common/sock_io.c
+++ b/ctdb/common/sock_io.c
@@ -231,6 +231,7 @@ static void sock_queue_process(struct sock_queue *queue)
 	if (pkt_size == 0) {
 		D_ERR("Invalid packet of length 0\n");
 		queue->callback(NULL, 0, queue->private_data);
+		return;
 	}
 
 	if ((queue->end - queue->begin) < pkt_size) {
-- 
2.14.3


From 7e37949980f9be361c1f3499927536603797b042 Mon Sep 17 00:00:00 2001
From: Swen Schillig <swen at vnet.ibm.com>
Date: Mon, 8 Jan 2018 14:13:46 +0100
Subject: [PATCH 2/3] ctdb-common: Remove sock_queue_destructor

The sock_queue_destructor is not needed.
The performed tasks will be performed automatically.

Signed-off-by: Swen Schillig <swen at vnet.ibm.com>
---
 ctdb/common/sock_io.c | 11 -----------
 1 file changed, 11 deletions(-)

diff --git a/ctdb/common/sock_io.c b/ctdb/common/sock_io.c
index 8a0b9859d01..ef7cfeb2ea1 100644
--- a/ctdb/common/sock_io.c
+++ b/ctdb/common/sock_io.c
@@ -95,7 +95,6 @@ struct sock_queue {
 };
 
 static bool sock_queue_set_fd(struct sock_queue *queue, int fd);
-static int sock_queue_destructor(struct sock_queue *queue);
 static void sock_queue_handler(struct tevent_context *ev,
 			       struct tevent_fd *fde, uint16_t flags,
 			       void *private_data);
@@ -138,8 +137,6 @@ struct sock_queue *sock_queue_setup(TALLOC_CTX *mem_ctx,
 		return NULL;
 	}
 
-	talloc_set_destructor(queue, sock_queue_destructor);
-
 	return queue;
 }
 
@@ -168,14 +165,6 @@ static bool sock_queue_set_fd(struct sock_queue *queue, int fd)
 	return true;
 }
 
-static int sock_queue_destructor(struct sock_queue *queue)
-{
-	TALLOC_FREE(queue->fde);
-	queue->fd = -1;
-
-	return 0;
-}
-
 static void sock_queue_handler(struct tevent_context *ev,
 			       struct tevent_fd *fde, uint16_t flags,
 			       void *private_data)
-- 
2.14.3


From e23e1bde866a9418a4771a6026cd09ccf827f9fc Mon Sep 17 00:00:00 2001
From: Swen Schillig <swen at vnet.ibm.com>
Date: Mon, 8 Jan 2018 14:55:31 +0100
Subject: [PATCH 3/3] ctdb-common: Optimize sock_queue's memory managament

Make use of talloc pools for the sock_queue's memory requirements.

Signed-off-by: Swen Schillig <swen at vnet.ibm.com>
---
 ctdb/common/sock_io.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/ctdb/common/sock_io.c b/ctdb/common/sock_io.c
index ef7cfeb2ea1..51341ce023e 100644
--- a/ctdb/common/sock_io.c
+++ b/ctdb/common/sock_io.c
@@ -94,6 +94,17 @@ struct sock_queue {
 	size_t buflen, begin, end;
 };
 
+/*
+ * The reserved talloc headers, SOCK_QUEUE_OBJ_COUNT,
+ * and the pre-allocated pool-memory SOCK_QUEUE_POOL_SIZE,
+ * are used for the sub-objects queue->im, queue->queue, queue->fde
+ * and queue->buf.
+ * If the memory allocating sub-objects of struct sock_queue change,
+ * those values need to be adjusted.
+ */
+#define SOCK_QUEUE_OBJ_COUNT 4
+#define SOCK_QUEUE_POOL_SIZE 2048
+
 static bool sock_queue_set_fd(struct sock_queue *queue, int fd);
 static void sock_queue_handler(struct tevent_context *ev,
 			       struct tevent_fd *fde, uint16_t flags,
@@ -111,10 +122,12 @@ struct sock_queue *sock_queue_setup(TALLOC_CTX *mem_ctx,
 {
 	struct sock_queue *queue;
 
-	queue = talloc_zero(mem_ctx, struct sock_queue);
+	queue = talloc_pooled_object(mem_ctx, struct sock_queue,
+				     SOCK_QUEUE_OBJ_COUNT, SOCK_QUEUE_POOL_SIZE);
 	if (queue == NULL) {
 		return NULL;
 	}
+	memset(queue, 0, sizeof(struct sock_queue));
 
 	queue->ev = ev;
 	queue->callback = callback;
-- 
2.14.3