From c4e61f8a2686a520c2c6c1b4e70e43a2784a23c4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=A5vard=20Reierstad?= Date: Fri, 21 Feb 2025 09:52:54 +0100 Subject: [PATCH 1/3] Revert "[nrf noup] lib: net_buf: buf: Revert alloc DBG to WRN change" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit 6c9b2f2f417cad2a64de64abb93cca93d78a1269. Signed-off-by: Håvard Reierstad --- lib/net_buf/buf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/net_buf/buf.c b/lib/net_buf/buf.c index a2f504ac06be..6d409430f086 100644 --- a/lib/net_buf/buf.c +++ b/lib/net_buf/buf.c @@ -273,7 +273,7 @@ struct net_buf *net_buf_alloc_len(struct net_buf_pool *pool, size_t size, if (!K_TIMEOUT_EQ(timeout, K_NO_WAIT) && k_current_get() == k_work_queue_thread_get(&k_sys_work_q)) { - LOG_DBG("Timeout discarded. No blocking in syswq"); + LOG_WRN("Timeout discarded. No blocking in syswq"); timeout = K_NO_WAIT; } From c2056bb78920b8f0b648ae31f771e0d7c374b0a2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=A5vard=20Reierstad?= Date: Fri, 21 Feb 2025 09:52:54 +0100 Subject: [PATCH 2/3] [nrf fromtree] net: buf: revert disallowing blocking in syswq MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reverts the change made in d4d53010f00cadbb4f89c6d41391937646fc1740 (The changes was moved to another file in a restructuring) The commit incorrectly assumed that no blocking would be allowed in the syswq. This has caused issues observed among others in #77241 and #80167 Signed-off-by: Håvard Reierstad (cherry picked from commit e13e893dfcf5754b002421ad9af83eab09f0c559) Signed-off-by: Håvard Reierstad --- include/zephyr/net_buf.h | 39 ++++++++++++--------------------------- lib/net_buf/buf.c | 6 ------ 2 files changed, 12 insertions(+), 33 deletions(-) diff --git a/include/zephyr/net_buf.h b/include/zephyr/net_buf.h index 02c041e9dcae..62f6299cd30b 100644 --- a/include/zephyr/net_buf.h +++ b/include/zephyr/net_buf.h @@ -1325,19 +1325,14 @@ int net_buf_id(const struct net_buf *buf); /** * @brief Allocate a new fixed buffer from a pool. * - * @note Some types of data allocators do not support - * blocking (such as the HEAP type). In this case it's still possible - * for net_buf_alloc() to fail (return NULL) even if it was given - * K_FOREVER. - * - * @note The timeout value will be overridden to K_NO_WAIT if called from the - * system workqueue. - * * @param pool Which pool to allocate the buffer from. * @param timeout Affects the action taken should the pool be empty. * If K_NO_WAIT, then return immediately. If K_FOREVER, then * wait as long as necessary. Otherwise, wait until the specified - * timeout. + * timeout. Note that some types of data allocators do not support + * blocking (such as the HEAP type). In this case it's still possible + * for net_buf_alloc() to fail (return NULL) even if it was given + * K_FOREVER. * * @return New buffer or NULL if out of buffers. */ @@ -1365,20 +1360,15 @@ static inline struct net_buf * __must_check net_buf_alloc(struct net_buf_pool *p /** * @brief Allocate a new variable length buffer from a pool. * - * @note Some types of data allocators do not support - * blocking (such as the HEAP type). In this case it's still possible - * for net_buf_alloc() to fail (return NULL) even if it was given - * K_FOREVER. - * - * @note The timeout value will be overridden to K_NO_WAIT if called from the - * system workqueue. - * * @param pool Which pool to allocate the buffer from. * @param size Amount of data the buffer must be able to fit. * @param timeout Affects the action taken should the pool be empty. * If K_NO_WAIT, then return immediately. If K_FOREVER, then * wait as long as necessary. Otherwise, wait until the specified - * timeout. + * timeout. Note that some types of data allocators do not support + * blocking (such as the HEAP type). In this case it's still possible + * for net_buf_alloc() to fail (return NULL) even if it was given + * K_FOREVER. * * @return New buffer or NULL if out of buffers. */ @@ -1402,21 +1392,16 @@ struct net_buf * __must_check net_buf_alloc_len(struct net_buf_pool *pool, * Allocate a new buffer from a pool, where the data pointer comes from the * user and not from the pool. * - * @note Some types of data allocators do not support - * blocking (such as the HEAP type). In this case it's still possible - * for net_buf_alloc() to fail (return NULL) even if it was given - * K_FOREVER. - * - * @note The timeout value will be overridden to K_NO_WAIT if called from the - * system workqueue. - * * @param pool Which pool to allocate the buffer from. * @param data External data pointer * @param size Amount of data the pointed data buffer if able to fit. * @param timeout Affects the action taken should the pool be empty. * If K_NO_WAIT, then return immediately. If K_FOREVER, then * wait as long as necessary. Otherwise, wait until the specified - * timeout. + * timeout. Note that some types of data allocators do not support + * blocking (such as the HEAP type). In this case it's still possible + * for net_buf_alloc() to fail (return NULL) even if it was given + * K_FOREVER. * * @return New buffer or NULL if out of buffers. */ diff --git a/lib/net_buf/buf.c b/lib/net_buf/buf.c index 6d409430f086..c8f42e6f510a 100644 --- a/lib/net_buf/buf.c +++ b/lib/net_buf/buf.c @@ -271,12 +271,6 @@ struct net_buf *net_buf_alloc_len(struct net_buf_pool *pool, size_t size, k_spin_unlock(&pool->lock, key); - if (!K_TIMEOUT_EQ(timeout, K_NO_WAIT) && - k_current_get() == k_work_queue_thread_get(&k_sys_work_q)) { - LOG_WRN("Timeout discarded. No blocking in syswq"); - timeout = K_NO_WAIT; - } - #if defined(CONFIG_NET_BUF_LOG) && (CONFIG_NET_BUF_LOG_LEVEL >= LOG_LEVEL_WRN) if (K_TIMEOUT_EQ(timeout, K_FOREVER)) { uint32_t ref = k_uptime_get_32(); From 56953e32267a83ce4cc7a577172ab39283ac79d3 Mon Sep 17 00:00:00 2001 From: Ludvig Jordet Date: Fri, 21 Feb 2025 09:52:54 +0100 Subject: [PATCH 3/3] [nrf fromlist] Bluetooth: Mesh: Update Kconfig help about Mesh on System WQ This updates the help text for the Kconfig option BT_MESH_WORKQ_SYS to take into account the change made in PR #84282 which causes the host to no longer return -ENOBUFS. Since the mesh will now instead block the work queue, a note has been added about a potential consequence of this. Upstream PR #: 86022 Signed-off-by: Ludvig Jordet --- subsys/bluetooth/mesh/Kconfig | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/subsys/bluetooth/mesh/Kconfig b/subsys/bluetooth/mesh/Kconfig index 8feef62d9473..367744d91a2f 100644 --- a/subsys/bluetooth/mesh/Kconfig +++ b/subsys/bluetooth/mesh/Kconfig @@ -130,10 +130,12 @@ config BT_MESH_WORKQ_SYS refer to BT_MESH_ADV_STACK_SIZE for the recommended minimum. When this option is enabled and the mesh tries to send a message, - and the host ran out the HCI command buffers controlled by - CONFIG_BT_BUF_CMD_TX_COUNT, the host returns -ENOBUFS immediately - and the mesh drops the message transmission. To mitigate this - issue, make sure to have sufficient number of HCI command buffers. + and the host ran out of the HCI command buffers controlled by + CONFIG_BT_BUF_CMD_TX_COUNT, the system work queue will be blocked + until an HCI command buffer has been freed. This may cause a deadlock + where the host cannot use the system workqueue to free the buffer + that the mesh is waiting for. To mitigate this issue, make sure to + have a sufficient number of HCI command buffers. When this option is enabled, the latency of sending mesh messages will be affected by other users on the system work queue, resulting in reduced reliability for sending mesh messages.