From 53b0108dd0b691a49498e7ebb89ebdc1c99f48be Mon Sep 17 00:00:00 2001 From: Jonathan Rico Date: Fri, 19 Apr 2024 13:09:37 +0200 Subject: [PATCH] net: buf: Disallow blocking allocation in syswq MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Work items on the syswq should always run-to-completion. Override the timeout value to always be K_NO_WAIT. Allocating with K_FOREVER leads to deadlocks if the freeing also happens from the syswq. Non-zero timeouts are also not nice for the other users of the syswq. (cherry picked from commit b08b1c21a069d6154efff1ea719a9c81f53e43f8) Original-Signed-off-by: Jonathan Rico GitOrigin-RevId: b08b1c21a069d6154efff1ea719a9c81f53e43f8 Change-Id: Icb45e148317eb510473ddcdfba62126dec5e56e4 Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/zephyr/+/5482305 Tested-by: Dawid Niedźwiecki Commit-Queue: Dawid Niedźwiecki Reviewed-by: Dawid Niedźwiecki Tested-by: ChromeOS Prod (Robot) --- include/zephyr/net/buf.h | 39 +++++++++++++++++++++++++++------------ subsys/net/buf.c | 6 ++++++ 2 files changed, 33 insertions(+), 12 deletions(-) diff --git a/include/zephyr/net/buf.h b/include/zephyr/net/buf.h index cf7e1279662..3af2782a442 100644 --- a/include/zephyr/net/buf.h +++ b/include/zephyr/net/buf.h @@ -1315,14 +1315,19 @@ int net_buf_id(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. 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. + * timeout. * * @return New buffer or NULL if out of buffers. */ @@ -1350,15 +1355,20 @@ 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. 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. + * timeout. * * @return New buffer or NULL if out of buffers. */ @@ -1382,16 +1392,21 @@ 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. 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. + * timeout. * * @return New buffer or NULL if out of buffers. */ diff --git a/subsys/net/buf.c b/subsys/net/buf.c index 8f65c402806..4ef60e90fd3 100644 --- a/subsys/net/buf.c +++ b/subsys/net/buf.c @@ -271,6 +271,12 @@ 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_DBG("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();