Skip to content

Commit

Permalink
net: buf: Disallow blocking allocation in syswq
Browse files Browse the repository at this point in the history
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 b08b1c2)

Original-Signed-off-by: Jonathan Rico <jonathan.rico@nordicsemi.no>
GitOrigin-RevId: b08b1c2
Change-Id: Icb45e148317eb510473ddcdfba62126dec5e56e4
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/zephyr/+/5482305
Tested-by: Dawid Niedźwiecki <dawidn@google.com>
Commit-Queue: Dawid Niedźwiecki <dawidn@google.com>
Reviewed-by: Dawid Niedźwiecki <dawidn@google.com>
Tested-by: ChromeOS Prod (Robot) <chromeos-ci-prod@chromeos-bot.iam.gserviceaccount.com>
  • Loading branch information
jori-nordic authored and Chromeos LUCI committed Apr 24, 2024
1 parent 7f8a292 commit 53b0108
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 12 deletions.
39 changes: 27 additions & 12 deletions include/zephyr/net/buf.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/
Expand Down Expand Up @@ -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.
*/
Expand All @@ -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.
*/
Expand Down
6 changes: 6 additions & 0 deletions subsys/net/buf.c
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down

0 comments on commit 53b0108

Please sign in to comment.