From patchwork Wed Jun 7 11:09:30 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Alexandru Ardelean X-Patchwork-Id: 772366 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from bombadil.infradead.org (bombadil.infradead.org [65.50.211.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3wjQqc32dVz9sDG for ; Wed, 7 Jun 2017 21:11:56 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="O72dTI9w"; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="KltsmLST"; dkim-atps=neutral DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender: Content-Transfer-Encoding:Content-Type:MIME-Version:Cc:List-Subscribe: List-Help:List-Post:List-Archive:List-Unsubscribe:List-Id:Subject:References: In-Reply-To:Message-Id:Date:To:From:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=yEjHV4ESqHFeNZn8r3mzBBpF9Ixp7By+ydOIYWqhEUk=; b=O72dTI9wAehYtK 6geBxbcPHpMxpO41+5ssj5GNjmBUvQQzFEyhF4A14f1GJgz2elnApq5baI4hNQ7+U0ZfejdRiG/7B OINFxnLp2kPxYDUMLr7NGWzy8tUpp+IR3sqKUALHURdqUSZrG4SPEpzrTibPvQRnjTjncvpxmfyeq 5msQjd6/8Tl2d68NNcXX9yykfmZu385HIAfNyVCf1i3ehIQ+VC7IoF/h2aLKn6hmJykQEqkzZCCou Q4RcQ5NsIzKXsv5VbhsH/O07XzWe35hfCB+W2mEzKZyZil3hjmrW3FTzKkbeF1BCu+oInshWeGr/B h1Jj30ILrNeetAyWBj2w==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.87 #1 (Red Hat Linux)) id 1dIYsC-0004AY-RF; Wed, 07 Jun 2017 11:11:36 +0000 Received: from mail-wr0-x241.google.com ([2a00:1450:400c:c0c::241]) by bombadil.infradead.org with esmtps (Exim 4.87 #1 (Red Hat Linux)) id 1dIYrQ-0003yz-O8 for lede-dev@lists.infradead.org; Wed, 07 Jun 2017 11:10:50 +0000 Received: by mail-wr0-x241.google.com with SMTP id e23so920775wre.3 for ; Wed, 07 Jun 2017 04:10:28 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references; bh=bmjHmSGUEsVdbGx+dM7WPODTBgcL5zXjTuQATXoqZfg=; b=KltsmLSTEJrO03m/O6K9Kc6J3VE+ycbnBjSUT4N9m6OAb9/5hANHXd8MwtkwOoBcCu gvvWoWftyCW5FvBsiQrCmCLElU/Nz7nLJZoofepfXsa8y0JxmLMKGnJbWziMST+9ANML 1D4a3IoJTsIvapJZgoP6EHVQ1PQwtfgekk2J/x2ocEczlkr9Y9c5FvjTvbBygn+1Qnb6 EUqGFwxIsrjU4O7Ssh7kpc0pqtq6+6O8QIDVUVxwCpM3PbkjRdoPEXlyS5el+BbVD5mh d30iuOGsBXG+jlmSe6y4xQylrLj7y+yGpIPci6yd1zxCo88HYuGnNfmfheTUQEBOUdg5 D5CQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=bmjHmSGUEsVdbGx+dM7WPODTBgcL5zXjTuQATXoqZfg=; b=PITPb/z6UbsiBamWKsFb18Q/LnOhut3VGrb/unMdpt1jB/L066iJMD2EuZZUZisQx2 M2kjY+fgSDSBU93rE9OMAERdIbQ8GkhUsaNQLSmEBXKVJ9AR7H7j4x0N2t8S4tq0ISlm 4wzuZ58eR8qotntfyZXvEVWdsMoBXMyRQgfRdMbVTzK+wS3n5jQInAppvusd0HrjKcUK v/1ps+6+0lLIb6pViuIdDGyHPtaTYJQLzy6S3ByJh4TLlkPn2Ymmbc01hAXB+/fjgvkH 59fWbKiLJzDTx3jRxD2bfz9pWz8aCJlgADTtZ4Q+QOChQFQkFWlwFDvUOshkDzgHXDeb bg1A== X-Gm-Message-State: AODbwcAmpU5BweyW07klevyb7cuaFKoL8O99hHuSbg/27yzTRew2qwNl T9bDE8+/SeTGT11O X-Received: by 10.223.178.149 with SMTP id g21mr19957084wrd.126.1496833826363; Wed, 07 Jun 2017 04:10:26 -0700 (PDT) Received: from localhost.localdomain ([5.2.198.78]) by smtp.googlemail.com with ESMTPSA id d4sm1232222wra.65.2017.06.07.04.10.24 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Wed, 07 Jun 2017 04:10:25 -0700 (PDT) From: Alexandru Ardelean To: lede-dev@lists.infradead.org Date: Wed, 7 Jun 2017 14:09:30 +0300 Message-Id: <1496833771-32723-2-git-send-email-ardeleanalex@gmail.com> X-Mailer: git-send-email 2.7.4 In-Reply-To: <1496833771-32723-1-git-send-email-ardeleanalex@gmail.com> References: <1496833771-32723-1-git-send-email-ardeleanalex@gmail.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20170607_041048_937197_0E1E7554 X-CRM114-Status: GOOD ( 16.31 ) X-Spam-Score: -2.0 (--) X-Spam-Report: SpamAssassin version 3.4.1 on bombadil.infradead.org summary: Content analysis details: (-2.0 points) pts rule name description ---- ---------------------- -------------------------------------------------- -0.0 RCVD_IN_DNSWL_NONE RBL: Sender listed at http://www.dnswl.org/, no trust [2a00:1450:400c:c0c:0:0:0:241 listed in] [list.dnswl.org] 0.0 FREEMAIL_FROM Sender email is commonly abused enduser mail provider (ardeleanalex[at]gmail.com) -0.0 SPF_PASS SPF: sender matches SPF record -1.9 BAYES_00 BODY: Bayes spam probability is 0 to 1% [score: 0.0000] -0.1 DKIM_VALID Message has at least one valid DKIM or DK signature -0.1 DKIM_VALID_AU Message has a valid DKIM or DK signature from author's domain 0.1 DKIM_SIGNED Message has a DKIM or DK signature, not necessarily valid Subject: [LEDE-DEV] [PATCH 2/3] ubusd: make `tx_queue` backlog dynamic X-BeenThere: lede-dev@lists.infradead.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Alexandru Ardelean , nbd@nbd.name MIME-Version: 1.0 Sender: "Lede-dev" Errors-To: lede-dev-bounces+incoming=patchwork.ozlabs.org@lists.infradead.org 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 --- 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;