From 964b9aafadf564178a7b9e72e1355cf6437451f0 Mon Sep 17 00:00:00 2001 From: Aleksander Wasaznik Date: Fri, 12 Sep 2025 17:25:39 +0200 Subject: [PATCH 1/5] Bluetooth: Host: Add bt_taskq workqueue for quick non-blocking tasks Add a new workqueue bt_taskq specifically designed for quick non-blocking work items in the Bluetooth subsystem. This workqueue is always available and does not depend on any Kconfig option. Signed-off-by: Aleksander Wasaznik --- subsys/bluetooth/host/CMakeLists.txt | 1 + subsys/bluetooth/host/Kconfig | 14 ++++++++++ subsys/bluetooth/host/bt_taskq.c | 34 +++++++++++++++++++++++ subsys/bluetooth/host/bt_taskq.h | 40 ++++++++++++++++++++++++++++ subsys/bluetooth/host/hci_core.c | 2 ++ 5 files changed, 91 insertions(+) create mode 100644 subsys/bluetooth/host/bt_taskq.c create mode 100644 subsys/bluetooth/host/bt_taskq.h diff --git a/subsys/bluetooth/host/CMakeLists.txt b/subsys/bluetooth/host/CMakeLists.txt index 0911c22c8f866..ca035871386f0 100644 --- a/subsys/bluetooth/host/CMakeLists.txt +++ b/subsys/bluetooth/host/CMakeLists.txt @@ -11,6 +11,7 @@ zephyr_library_sources_ifdef(CONFIG_BT_MONITOR monitor.c) zephyr_library_sources_ifdef(CONFIG_BT_SETTINGS settings.c) zephyr_library_sources_ifdef(CONFIG_BT_HOST_CCM aes_ccm.c) zephyr_library_sources_ifdef(CONFIG_BT_LONG_WQ long_wq.c) +zephyr_library_sources(bt_taskq.c) if(CONFIG_BT_HCI_HOST) zephyr_library_sources( diff --git a/subsys/bluetooth/host/Kconfig b/subsys/bluetooth/host/Kconfig index 72deb4343dc45..9faf1dfa29cd7 100644 --- a/subsys/bluetooth/host/Kconfig +++ b/subsys/bluetooth/host/Kconfig @@ -35,6 +35,20 @@ config BT_LONG_WQ_INIT_PRIO endif # BT_LONG_WQ +config BT_TASKQ_STACK_SIZE_WITH_PROMPT + bool "Override BT taskq thread stack size" + +config BT_TASKQ_STACK_SIZE + int + prompt "BT taskq thread stack size" if BT_HCI_TX_STACK_SIZE_WITH_PROMPT + default 1024 + +config BT_TASKQ_THREAD_PRIO + int + # -1 is the least urgent cooperative priority. + # tx_processor() needs a cooperative thread for now. + default -1 + config BT_HCI_HOST # Hidden option to make the conditions more intuitive bool diff --git a/subsys/bluetooth/host/bt_taskq.c b/subsys/bluetooth/host/bt_taskq.c new file mode 100644 index 0000000000000..75f890607527c --- /dev/null +++ b/subsys/bluetooth/host/bt_taskq.c @@ -0,0 +1,34 @@ +/* bt_taskq.c - Workqueue for quick non-blocking Bluetooth tasks */ + +/* + * Copyright (c) 2025 Nordic Semiconductor ASA + * + * SPDX-License-Identifier: Apache-2.0 + */ + +#include +#include +#include +#include + +static K_THREAD_STACK_DEFINE(bt_taskq_stack, CONFIG_BT_TASKQ_STACK_SIZE); +struct k_work_q bt_taskq; + +static int bt_taskq_init(void) +{ + const struct k_work_queue_config cfg = { + .name = "bt_taskq", + /* Enable CONFIG_WORKQUEUE_WORK_TIMEOUT to detect tasks that take too long. */ + .work_timeout_ms = 10, + }; + + k_work_queue_start(&bt_taskq, bt_taskq_stack, K_THREAD_STACK_SIZEOF(bt_taskq_stack), + CONFIG_BT_TASKQ_THREAD_PRIO, &cfg); + + return 0; +} + +/* The init priority is set to POST_KERNEL 999, the last level + * before APPLICATION. + */ +SYS_INIT(bt_taskq_init, POST_KERNEL, 999); diff --git a/subsys/bluetooth/host/bt_taskq.h b/subsys/bluetooth/host/bt_taskq.h new file mode 100644 index 0000000000000..472443535589e --- /dev/null +++ b/subsys/bluetooth/host/bt_taskq.h @@ -0,0 +1,40 @@ +/* bt_taskq.h - Workqueue for quick non-blocking Bluetooth tasks */ + +/* + * Copyright (c) 2025 Nordic Semiconductor ASA + * + * SPDX-License-Identifier: Apache-2.0 + */ + +#include + +/** + * @brief Bluetooth task workqueue + * + * bt_taskq is a workqueue intended for quick non-blocking work + * items ("tasks") in the Bluetooth subsystem. This workqueue + * must always exist and is not controlled by any Kconfig + * option. + * + * Blocking means "waiting for something while running". A + * task is NOT allowed to block. If a task need to "wait", it + * should instead return immediately and schedule itself to run + * later. + * + * Work items submitted to this queue should be: + * - Quick to execute (non-blocking). + * - Not perform long-running operations. + * - Not block. + * + * @warning Non-blocking violation pitfalls: + * - net_buf_unref() on a foreign buffer could have a blocking + * destroy callback + * - Any user-defined callback might be blocking + * - Avoid any operations that could sleep or block the thread + * + * Use bt_long_wq for long-running or potentially blocking + * operations instead. + * + * Available in APPLICATION initialization level and later. + */ +extern struct k_work_q bt_taskq; diff --git a/subsys/bluetooth/host/hci_core.c b/subsys/bluetooth/host/hci_core.c index 33306596b2ce7..1b09530e03478 100644 --- a/subsys/bluetooth/host/hci_core.c +++ b/subsys/bluetooth/host/hci_core.c @@ -282,6 +282,8 @@ void bt_send_one_host_num_completed_packets(uint16_t handle) BT_ASSERT_MSG(err == 0, "Unable to send Host NCP (err %d)", err); } +#include "bt_taskq.h" + #if defined(CONFIG_BT_TESTING) __weak void bt_testing_trace_event_acl_pool_destroy(struct net_buf *buf) { From 4b57af49173ff906157a2f9734febd78ee501ecd Mon Sep 17 00:00:00 2001 From: Aleksander Wasaznik Date: Tue, 23 Sep 2025 13:55:33 +0200 Subject: [PATCH 2/5] Bluetooth: Host: Move tx_processor to bt_taskq The Bluetooth Host itself performs operations on the system workqueue which are blocking waiting for the tx_processor to run. This results in deadlocks. TX processor is supposed to be non-blocking, so this change puts it on the new bt_taskq. This effectively gives tx_processor its own thread, like the BT TX thread that used to exist. But, this time this thread is intended to be shared with any other non-blocking Bluetooth Host tasks. Signed-off-by: Aleksander Wasaznik --- subsys/bluetooth/host/bt_taskq.c | 4 +--- subsys/bluetooth/host/hci_core.c | 5 ++--- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/subsys/bluetooth/host/bt_taskq.c b/subsys/bluetooth/host/bt_taskq.c index 75f890607527c..b2651de013d8f 100644 --- a/subsys/bluetooth/host/bt_taskq.c +++ b/subsys/bluetooth/host/bt_taskq.c @@ -16,10 +16,8 @@ struct k_work_q bt_taskq; static int bt_taskq_init(void) { - const struct k_work_queue_config cfg = { + struct k_work_queue_config cfg = { .name = "bt_taskq", - /* Enable CONFIG_WORKQUEUE_WORK_TIMEOUT to detect tasks that take too long. */ - .work_timeout_ms = 10, }; k_work_queue_start(&bt_taskq, bt_taskq_stack, K_THREAD_STACK_SIZEOF(bt_taskq_stack), diff --git a/subsys/bluetooth/host/hci_core.c b/subsys/bluetooth/host/hci_core.c index 1b09530e03478..1875691f771aa 100644 --- a/subsys/bluetooth/host/hci_core.c +++ b/subsys/bluetooth/host/hci_core.c @@ -48,6 +48,7 @@ #include "addr_internal.h" #include "adv.h" +#include "bt_taskq.h" #include "common/hci_common_internal.h" #include "common/bt_str.h" #include "common/rpa.h" @@ -282,8 +283,6 @@ void bt_send_one_host_num_completed_packets(uint16_t handle) BT_ASSERT_MSG(err == 0, "Unable to send Host NCP (err %d)", err); } -#include "bt_taskq.h" - #if defined(CONFIG_BT_TESTING) __weak void bt_testing_trace_event_acl_pool_destroy(struct net_buf *buf) { @@ -5038,5 +5037,5 @@ static K_WORK_DEFINE(tx_work, tx_processor); void bt_tx_irq_raise(void) { LOG_DBG("kick TX"); - k_work_submit(&tx_work); + k_work_submit_to_queue(&bt_taskq, &tx_work); } From 66e7332bc7223ee2ad5bab63e75f869bc8008bc7 Mon Sep 17 00:00:00 2001 From: Aleksander Wasaznik Date: Tue, 23 Sep 2025 13:56:57 +0200 Subject: [PATCH 3/5] Bluetooth: Host: Remove syswq workaround in bt_hci_cmd_send_sync() Now that tx_processor is in bt_taskq, we don't need special handling for when it runs on the syswq. Signed-off-by: Aleksander Wasaznik --- subsys/bluetooth/host/hci_core.c | 29 ----------------------------- 1 file changed, 29 deletions(-) diff --git a/subsys/bluetooth/host/hci_core.c b/subsys/bluetooth/host/hci_core.c index 1875691f771aa..f081ce881dd05 100644 --- a/subsys/bluetooth/host/hci_core.c +++ b/subsys/bluetooth/host/hci_core.c @@ -471,35 +471,6 @@ int bt_hci_cmd_send_sync(uint16_t opcode, struct net_buf *buf, return err; } - /* TODO: disallow sending sync commands from syswq altogether */ - - /* Since the commands are now processed in the syswq, we cannot suspend - * and wait. We have to send the command from the current context. - */ - if (k_current_get() == &k_sys_work_q.thread) { - /* drain the command queue until we get to send the command of interest. */ - struct net_buf *cmd = NULL; - - do { - cmd = k_fifo_peek_head(&bt_dev.cmd_tx_queue); - LOG_DBG("process cmd %p want %p", cmd, buf); - - /* Wait for a response from the Bluetooth Controller. - * The Controller may fail to respond if: - * - It was never programmed or connected. - * - There was a fatal error. - * - * See the `BT_HCI_OP_` macros in hci_types.h or - * Core_v5.4, Vol 4, Part E, Section 5.4.1 and Section 7 - * to map the opcode to the HCI command documentation. - * Example: 0x0c03 represents HCI_Reset command. - */ - __maybe_unused bool success = process_pending_cmd(HCI_CMD_TIMEOUT); - - BT_ASSERT_MSG(success, "command opcode 0x%04x timeout", opcode); - } while (buf != cmd); - } - /* Now that we have sent the command, suspend until the LL replies */ err = k_sem_take(&sync_sem, HCI_CMD_TIMEOUT); BT_ASSERT_MSG(err == 0, From 47bcc4478365ffc084fe95fa3ae0dc00ff26d706 Mon Sep 17 00:00:00 2001 From: Aleksander Wasaznik Date: Thu, 9 Oct 2025 15:19:53 +0200 Subject: [PATCH 4/5] Bluetooth: Samples: peripheral_hr: Optimize stack minimal sizes Reduce stack sizes for peripheral_hr sample to minimal sizes. Signed-off-by: Aleksander Wasaznik --- samples/bluetooth/peripheral_hr/prj_minimal.conf | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/samples/bluetooth/peripheral_hr/prj_minimal.conf b/samples/bluetooth/peripheral_hr/prj_minimal.conf index d4d368101fa54..21a7faa4f3de1 100644 --- a/samples/bluetooth/peripheral_hr/prj_minimal.conf +++ b/samples/bluetooth/peripheral_hr/prj_minimal.conf @@ -72,10 +72,10 @@ CONFIG_BUILTIN_STACK_GUARD=n CONFIG_BT_RX_STACK_SIZE=1024 CONFIG_BT_HCI_TX_STACK_SIZE_WITH_PROMPT=y CONFIG_BT_HCI_TX_STACK_SIZE=640 -CONFIG_SYSTEM_WORKQUEUE_STACK_SIZE=1100 +CONFIG_SYSTEM_WORKQUEUE_STACK_SIZE=200 CONFIG_BT_LONG_WQ_STACK_SIZE=1100 -CONFIG_IDLE_STACK_SIZE=128 -CONFIG_MAIN_STACK_SIZE=640 +CONFIG_IDLE_STACK_SIZE=64 +CONFIG_MAIN_STACK_SIZE=500 CONFIG_ISR_STACK_SIZE=1024 # Disable features not needed From 4c2bd4974c1f6837db0237ced02607c37491a920 Mon Sep 17 00:00:00 2001 From: Aleksander Wasaznik Date: Thu, 9 Oct 2025 16:30:50 +0200 Subject: [PATCH 5/5] Bluetooth: Host: Don't call user callback from TX thread We invoked user callbacks from a net_buf_unref() inside the HCI driver. This is not OK now that tx_processor is a non-blocking task on bt_taskq. Instead defer the net_buf destruction to the system workqueue. This was detected because the following test was failing: tests/bsim/bluetooth/ll/throughput/tests_scripts/gatt_write.sh Signed-off-by: Aleksander Wasaznik --- subsys/bluetooth/host/att.c | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/subsys/bluetooth/host/att.c b/subsys/bluetooth/host/att.c index 66833d1cfcc53..c43c0e16cd64d 100644 --- a/subsys/bluetooth/host/att.c +++ b/subsys/bluetooth/host/att.c @@ -248,8 +248,13 @@ const char *bt_att_err_to_str(uint8_t att_err) } #endif /* CONFIG_BT_ATT_ERR_TO_STR */ -static void att_tx_destroy(struct net_buf *buf) +static void att_tx_destroy_work_handler(struct k_work *work); +static K_WORK_DEFINE(att_tx_destroy_work, att_tx_destroy_work_handler); +static sys_slist_t tx_destroy_queue; + +static void att_tx_destroy_work_handler(struct k_work *work) { + struct net_buf *buf = net_buf_slist_get(&tx_destroy_queue); struct bt_att_tx_meta_data *p_meta = att_get_tx_meta_data(buf); struct bt_att_tx_meta_data meta; @@ -278,6 +283,16 @@ static void att_tx_destroy(struct net_buf *buf) if (meta.opcode != 0) { att_on_sent_cb(&meta); } + + if (!sys_slist_is_empty(&tx_destroy_queue)) { + k_work_submit(&att_tx_destroy_work); + } +} + +static void att_tx_destroy(struct net_buf *buf) +{ + net_buf_slist_put(&tx_destroy_queue, buf); + k_work_submit(&att_tx_destroy_work); } NET_BUF_POOL_DEFINE(att_pool, CONFIG_BT_ATT_TX_COUNT,