[LEDE-DEV] [PATCH 2/3] ubusd: make `tx_queue` backlog dynamic

Alexandru Ardelean ardeleanalex at gmail.com
Wed Jun 7 04:09:30 PDT 2017


It's not very often that the tx_queue is used,
to store backlog messages to send to a client.

And for most cases, 32 backlog messages seems to be enough.
In fact, for most cases, I've seen ~1 entry in the queue
being used every now-n-then.

The issue is more visible/present with the `ubus list` command.
I've traced the code to ubusd_handle_lookup() in:

```
    if (!attr[UBUS_ATTR_OBJPATH]) {
        avl_for_each_element(&path, obj, path)
            ubusd_send_obj(cl, ub, obj);
        return 0;
    }
```
The code-path eventually leads to `ubus_msg_send()`.
It seems that once the first element is queued, then
the condition check for `(!cl->tx_queue[cl->txq_cur])`
will queue all messages iterated in the above snippet,
without trying any writes.

This can be solved, either by making the queue dynamic
and allow it to expand above the current fixed limit (1).

Or, by forcing/allowing writes during the tx_queue-ing (2).

This patch implements (1).

Signed-off-by: Alexandru Ardelean <ardeleanalex at gmail.com>
---
 ubusd.c       | 18 +++++++++---------
 ubusd.h       |  6 +++---
 ubusd_proto.c |  1 +
 3 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/ubusd.c b/ubusd.c
index f060b38..8fdb85b 100644
--- a/ubusd.c
+++ b/ubusd.c
@@ -59,6 +59,7 @@ struct ubus_msg_buf *ubus_msg_new(void *data, int len, bool shared)
 		return NULL;
 
 	ub->fd = -1;
+	INIT_LIST_HEAD(&ub->list);
 
 	if (shared) {
 		ub->refcount = ~0;
@@ -138,11 +139,9 @@ static int ubus_msg_writev(int fd, struct ubus_msg_buf *ub, int offset)
 
 static void ubus_msg_enqueue(struct ubus_client *cl, struct ubus_msg_buf *ub)
 {
-	if (cl->tx_queue[cl->txq_tail])
-		return;
-
-	cl->tx_queue[cl->txq_tail] = ubus_msg_ref(ub);
-	cl->txq_tail = (cl->txq_tail + 1) % ARRAY_SIZE(cl->tx_queue);
+	ub = ubus_msg_ref(ub);
+	if (ub)
+		list_add_tail(&ub->list, &cl->tx_queue);
 }
 
 /* takes the msgbuf reference */
@@ -153,7 +152,7 @@ void ubus_msg_send(struct ubus_client *cl, struct ubus_msg_buf *ub, bool free)
 	if (ub->hdr.type != UBUS_MSG_MONITOR)
 		ubusd_monitor_message(cl, ub, true);
 
-	if (!cl->tx_queue[cl->txq_cur]) {
+	if (list_empty(&cl->tx_queue)) {
 		written = ubus_msg_writev(cl->sock.fd, ub, 0);
 
 		if (written < 0)
@@ -176,7 +175,9 @@ out:
 
 static struct ubus_msg_buf *ubus_msg_head(struct ubus_client *cl)
 {
-	return cl->tx_queue[cl->txq_cur];
+	if (list_empty(&cl->tx_queue))
+		return NULL;
+	return list_first_entry(&cl->tx_queue, struct ubus_msg_buf, list);
 }
 
 static void ubus_msg_dequeue(struct ubus_client *cl)
@@ -186,10 +187,9 @@ static void ubus_msg_dequeue(struct ubus_client *cl)
 	if (!ub)
 		return;
 
+	list_del(&ub->list);
 	ubus_msg_free(ub);
 	cl->txq_ofs = 0;
-	cl->tx_queue[cl->txq_cur] = NULL;
-	cl->txq_cur = (cl->txq_cur + 1) % ARRAY_SIZE(cl->tx_queue);
 }
 
 static void handle_client_disconnect(struct ubus_client *cl)
diff --git a/ubusd.h b/ubusd.h
index 5031ed4..6748c65 100644
--- a/ubusd.h
+++ b/ubusd.h
@@ -23,12 +23,12 @@
 #include "ubusmsg.h"
 #include "ubusd_acl.h"
 
-#define UBUSD_CLIENT_BACKLOG	32
 #define UBUS_OBJ_HASH_BITS	4
 
 extern struct blob_buf b;
 
 struct ubus_msg_buf {
+	struct list_head list;
 	uint32_t refcount; /* ~0: uses external data buffer */
 	struct ubus_msghdr hdr;
 	struct blob_attr *data;
@@ -47,8 +47,8 @@ struct ubus_client {
 
 	struct list_head objects;
 
-	struct ubus_msg_buf *tx_queue[UBUSD_CLIENT_BACKLOG];
-	unsigned int txq_cur, txq_tail, txq_ofs;
+	unsigned int txq_ofs;
+	struct list_head tx_queue;
 
 	struct ubus_msg_buf *pending_msg;
 	int pending_msg_offset;
diff --git a/ubusd_proto.c b/ubusd_proto.c
index 72da7a7..631047b 100644
--- a/ubusd_proto.c
+++ b/ubusd_proto.c
@@ -480,6 +480,7 @@ struct ubus_client *ubusd_proto_new_client(int fd, uloop_fd_handler cb)
 		goto free;
 
 	INIT_LIST_HEAD(&cl->objects);
+	INIT_LIST_HEAD(&cl->tx_queue);
 	cl->sock.fd = fd;
 	cl->sock.cb = cb;
 	cl->pending_msg_fd = -1;
-- 
2.7.4




More information about the Lede-dev mailing list