<p>laforge <strong>submitted</strong> this change.</p><p><a href="https://gerrit.osmocom.org/c/simtrace2/+/16574">View Change</a></p><div style="white-space:pre-wrap">Approvals:
laforge: Looks good to me, approved
Jenkins Builder: Verified
</div><pre style="font-family: monospace,monospace; white-space: pre-wrap;">usb_buf: count number of elements in queue<br><br>This is in preparation for limiting the maximum queue length<br><br>Change-Id: I7cb184d7a1ccb519010a2f3e3295cc3a5fbf8052<br>Related: OS#4251<br>---<br>M firmware/libcommon/include/usb_buf.h<br>M firmware/libcommon/source/card_emu.c<br>M firmware/libcommon/source/host_communication.c<br>M firmware/libcommon/source/usb_buf.c<br>M firmware/libosmocore/include/osmocom/core/msgb.h<br>M firmware/test/card_emu_tests.c<br>6 files changed, 58 insertions(+), 13 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/firmware/libcommon/include/usb_buf.h b/firmware/libcommon/include/usb_buf.h</span><br><span>index bd6947b..3a4eda2 100644</span><br><span>--- a/firmware/libcommon/include/usb_buf.h</span><br><span>+++ b/firmware/libcommon/include/usb_buf.h</span><br><span>@@ -29,6 +29,8 @@</span><br><span> volatile uint32_t in_progress;</span><br><span> /* Tx queue (IN) / Rx queue (OUT) */</span><br><span> struct llist_head queue;</span><br><span style="color: hsl(120, 100%, 40%);">+ /* current length of queue */</span><br><span style="color: hsl(120, 100%, 40%);">+ unsigned int queue_len;</span><br><span> };</span><br><span> </span><br><span> struct msgb *usb_buf_alloc(uint8_t ep);</span><br><span>diff --git a/firmware/libcommon/source/card_emu.c b/firmware/libcommon/source/card_emu.c</span><br><span>index 923226a..f95f5d1 100644</span><br><span>--- a/firmware/libcommon/source/card_emu.c</span><br><span>+++ b/firmware/libcommon/source/card_emu.c</span><br><span>@@ -260,18 +260,18 @@</span><br><span> while (!msg) {</span><br><span> msg = usb_buf_alloc(ep); // try to allocate some memory</span><br><span> if (!msg) { // allocation failed, we might be out of memory</span><br><span style="color: hsl(0, 100%, 40%);">- struct llist_head *queue = usb_get_queue(ep);</span><br><span style="color: hsl(0, 100%, 40%);">- if (!queue) {</span><br><span style="color: hsl(120, 100%, 40%);">+ struct usb_buffered_ep *bep = usb_get_buf_ep(ep);</span><br><span style="color: hsl(120, 100%, 40%);">+ if (!bep) {</span><br><span> TRACE_ERROR("ep %u: %s queue does not exist\n\r",</span><br><span> ep, __func__);</span><br><span> return NULL;</span><br><span> }</span><br><span style="color: hsl(0, 100%, 40%);">- if (llist_empty(queue)) {</span><br><span style="color: hsl(120, 100%, 40%);">+ if (llist_empty(&bep->queue)) {</span><br><span> TRACE_ERROR("ep %u: %s EOMEM (queue already empty)\n\r",</span><br><span> ep, __func__);</span><br><span> return NULL;</span><br><span> }</span><br><span style="color: hsl(0, 100%, 40%);">- msg = msgb_dequeue(queue);</span><br><span style="color: hsl(120, 100%, 40%);">+ msg = msgb_dequeue_count(&bep->queue, &bep->queue_len);</span><br><span> if (!msg) {</span><br><span> TRACE_ERROR("ep %u: %s no msg in non-empty queue\n\r",</span><br><span> ep, __func__);</span><br><span>diff --git a/firmware/libcommon/source/host_communication.c b/firmware/libcommon/source/host_communication.c</span><br><span>index 1e15ada..0e496c5 100644</span><br><span>--- a/firmware/libcommon/source/host_communication.c</span><br><span>+++ b/firmware/libcommon/source/host_communication.c</span><br><span>@@ -76,7 +76,7 @@</span><br><span> </span><br><span> bep->in_progress++;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- msg = msgb_dequeue(&bep->queue);</span><br><span style="color: hsl(120, 100%, 40%);">+ msg = msgb_dequeue_count(&bep->queue, &bep->queue_len);</span><br><span> </span><br><span> local_irq_restore(x);</span><br><span> </span><br><span>@@ -180,7 +180,7 @@</span><br><span> }</span><br><span> </span><br><span> /* free all queued msgbs */</span><br><span style="color: hsl(0, 100%, 40%);">- while ((msg = msgb_dequeue(&bep->queue))) {</span><br><span style="color: hsl(120, 100%, 40%);">+ while ((msg = msgb_dequeue_count(&bep->queue, &bep->queue_len))) {</span><br><span> usb_buf_free(msg);</span><br><span> ret++;</span><br><span> }</span><br><span>diff --git a/firmware/libcommon/source/usb_buf.c b/firmware/libcommon/source/usb_buf.c</span><br><span>index 418569e..8ad5a0e 100644</span><br><span>--- a/firmware/libcommon/source/usb_buf.c</span><br><span>+++ b/firmware/libcommon/source/usb_buf.c</span><br><span>@@ -78,7 +78,7 @@</span><br><span> </span><br><span> /* no need for irqsafe operation, as the usb_tx_queue is</span><br><span> * processed only by the main loop context */</span><br><span style="color: hsl(0, 100%, 40%);">- msgb_enqueue(&ep->queue, msg);</span><br><span style="color: hsl(120, 100%, 40%);">+ msgb_enqueue_count(&ep->queue, msg, &ep->queue_len);</span><br><span> return 0;</span><br><span> }</span><br><span> </span><br><span>diff --git a/firmware/libosmocore/include/osmocom/core/msgb.h b/firmware/libosmocore/include/osmocom/core/msgb.h</span><br><span>index afb887c..7c4420d 100644</span><br><span>--- a/firmware/libosmocore/include/osmocom/core/msgb.h</span><br><span>+++ b/firmware/libosmocore/include/osmocom/core/msgb.h</span><br><span>@@ -80,6 +80,49 @@</span><br><span> extern struct msgb *msgb_copy(const struct msgb *msg, const char *name);</span><br><span> static int msgb_test_invariant(const struct msgb *msg) __attribute__((pure));</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+/*! Free all msgbs from a queue built with msgb_enqueue().</span><br><span style="color: hsl(120, 100%, 40%);">+ * \param[in] queue list head of a msgb queue.</span><br><span style="color: hsl(120, 100%, 40%);">+ */</span><br><span style="color: hsl(120, 100%, 40%);">+static inline void msgb_queue_free(struct llist_head *queue)</span><br><span style="color: hsl(120, 100%, 40%);">+{</span><br><span style="color: hsl(120, 100%, 40%);">+ struct msgb *msg;</span><br><span style="color: hsl(120, 100%, 40%);">+ while ((msg = msgb_dequeue(queue))) msgb_free(msg);</span><br><span style="color: hsl(120, 100%, 40%);">+}</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+/*! Enqueue message buffer to tail of a queue and increment queue size counter</span><br><span style="color: hsl(120, 100%, 40%);">+ * \param[in] queue linked list header of queue</span><br><span style="color: hsl(120, 100%, 40%);">+ * \param[in] msg message buffer to be added to the queue</span><br><span style="color: hsl(120, 100%, 40%);">+ * \param[in] count pointer to variable holding size of the queue</span><br><span style="color: hsl(120, 100%, 40%);">+ *</span><br><span style="color: hsl(120, 100%, 40%);">+ * The function will append the specified message buffer \a msg to the queue</span><br><span style="color: hsl(120, 100%, 40%);">+ * implemented by \ref llist_head \a queue using function \ref msgb_enqueue_count,</span><br><span style="color: hsl(120, 100%, 40%);">+ * then increment \a count</span><br><span style="color: hsl(120, 100%, 40%);">+ */</span><br><span style="color: hsl(120, 100%, 40%);">+static inline void msgb_enqueue_count(struct llist_head *queue, struct msgb *msg,</span><br><span style="color: hsl(120, 100%, 40%);">+ unsigned int *count)</span><br><span style="color: hsl(120, 100%, 40%);">+{</span><br><span style="color: hsl(120, 100%, 40%);">+ msgb_enqueue(queue, msg);</span><br><span style="color: hsl(120, 100%, 40%);">+ (*count)++;</span><br><span style="color: hsl(120, 100%, 40%);">+}</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+/*! Dequeue message buffer from head of queue and decrement queue size counter</span><br><span style="color: hsl(120, 100%, 40%);">+ * \param[in] queue linked list header of queue</span><br><span style="color: hsl(120, 100%, 40%);">+ * \param[in] count pointer to variable holding size of the queue</span><br><span style="color: hsl(120, 100%, 40%);">+ * \returns message buffer (if any) or NULL if queue empty</span><br><span style="color: hsl(120, 100%, 40%);">+ *</span><br><span style="color: hsl(120, 100%, 40%);">+ * The function will remove the first message buffer from the queue</span><br><span style="color: hsl(120, 100%, 40%);">+ * implemented by \ref llist_head \a queue using function \ref msgb_enqueue_count,</span><br><span style="color: hsl(120, 100%, 40%);">+ * and decrement \a count, all if queue is not empty.</span><br><span style="color: hsl(120, 100%, 40%);">+ */</span><br><span style="color: hsl(120, 100%, 40%);">+static inline struct msgb *msgb_dequeue_count(struct llist_head *queue,</span><br><span style="color: hsl(120, 100%, 40%);">+ unsigned int *count)</span><br><span style="color: hsl(120, 100%, 40%);">+{</span><br><span style="color: hsl(120, 100%, 40%);">+ struct msgb *msg = msgb_dequeue(queue);</span><br><span style="color: hsl(120, 100%, 40%);">+ if (msg)</span><br><span style="color: hsl(120, 100%, 40%);">+ (*count)--;</span><br><span style="color: hsl(120, 100%, 40%);">+ return msg;</span><br><span style="color: hsl(120, 100%, 40%);">+}</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> #ifdef MSGB_DEBUG</span><br><span> #include <osmocom/core/panic.h></span><br><span> #define MSGB_ABORT(msg, fmt, args ...) do { \</span><br><span>diff --git a/firmware/test/card_emu_tests.c b/firmware/test/card_emu_tests.c</span><br><span>index a5ba62e..2a1d682 100644</span><br><span>--- a/firmware/test/card_emu_tests.c</span><br><span>+++ b/firmware/test/card_emu_tests.c</span><br><span>@@ -177,14 +177,14 @@</span><br><span> </span><br><span> static void get_and_verify_rctx(uint8_t ep, const uint8_t *data, unsigned int len)</span><br><span> {</span><br><span style="color: hsl(0, 100%, 40%);">- struct llist_head *queue = usb_get_queue(ep);</span><br><span style="color: hsl(120, 100%, 40%);">+ struct usb_buffered_ep *bep = usb_get_buf_ep(ep);</span><br><span> struct msgb *msg;</span><br><span> struct cardemu_usb_msg_tx_data *td;</span><br><span> struct cardemu_usb_msg_rx_data *rd;</span><br><span> struct simtrace_msg_hdr *mh;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- assert(queue);</span><br><span style="color: hsl(0, 100%, 40%);">- msg = msgb_dequeue(queue);</span><br><span style="color: hsl(120, 100%, 40%);">+ assert(bep);</span><br><span style="color: hsl(120, 100%, 40%);">+ msg = msgb_dequeue_count(&bep->queue, &bep->queue_len);</span><br><span> assert(msg);</span><br><span> dump_rctx(msg);</span><br><span> assert(msg->l1h);</span><br><span>@@ -214,13 +214,13 @@</span><br><span> </span><br><span> static void get_and_verify_rctx_pps(const uint8_t *data, unsigned int len)</span><br><span> {</span><br><span style="color: hsl(0, 100%, 40%);">- struct llist_head *queue = usb_get_queue(PHONE_DATAIN);</span><br><span style="color: hsl(120, 100%, 40%);">+ struct usb_buffered_ep *bep = usb_get_buf_ep(PHONE_DATAIN);</span><br><span> struct msgb *msg;</span><br><span> struct simtrace_msg_hdr *mh;</span><br><span> struct cardemu_usb_msg_pts_info *ptsi;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- assert(queue);</span><br><span style="color: hsl(0, 100%, 40%);">- msg = msgb_dequeue(queue);</span><br><span style="color: hsl(120, 100%, 40%);">+ assert(bep);</span><br><span style="color: hsl(120, 100%, 40%);">+ msg = msgb_dequeue_count(&bep->queue, &bep->queue_len);</span><br><span> assert(msg);</span><br><span> dump_rctx(msg);</span><br><span> assert(msg->l1h);</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.osmocom.org/c/simtrace2/+/16574">change 16574</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.osmocom.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.osmocom.org/c/simtrace2/+/16574"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: simtrace2 </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-Change-Id: I7cb184d7a1ccb519010a2f3e3295cc3a5fbf8052 </div>
<div style="display:none"> Gerrit-Change-Number: 16574 </div>
<div style="display:none"> Gerrit-PatchSet: 3 </div>
<div style="display:none"> Gerrit-Owner: laforge <laforge@osmocom.org> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins Builder </div>
<div style="display:none"> Gerrit-Reviewer: laforge <laforge@osmocom.org> </div>
<div style="display:none"> Gerrit-MessageType: merged </div>