From 3c7ad3c9ca5a7d664fe02087fae02e7039ae9287 Mon Sep 17 00:00:00 2001 From: Marek Pieta Date: Tue, 1 Oct 2024 09:06:05 +0200 Subject: [PATCH 1/2] bluetooth: conn: Use a separate workqueue for connection TX notify Use a separate workqueue instead of system workqueue for connection TX notify processing. This makes Bluetooth stack more independent from the system workqueue. Signed-off-by: Marek Pieta --- subsys/bluetooth/host/Kconfig | 29 ++++++++ subsys/bluetooth/host/conn.c | 103 +++++++++++++++++--------- subsys/bluetooth/host/conn_internal.h | 2 + subsys/bluetooth/host/hci_core.c | 2 +- 4 files changed, 99 insertions(+), 37 deletions(-) diff --git a/subsys/bluetooth/host/Kconfig b/subsys/bluetooth/host/Kconfig index 05214f224b133..3124b35b3df01 100644 --- a/subsys/bluetooth/host/Kconfig +++ b/subsys/bluetooth/host/Kconfig @@ -129,6 +129,35 @@ config BT_DRIVER_RX_HIGH_PRIO int default 6 +config BT_CONN_TX_NOTIFY_WQ + bool "Use a separate workqueue for connection TX notify processing [EXPERIMENTAL]" + depends on BT_CONN_TX + select EXPERIMENTAL + help + Use a separate workqueue instead of system workqueue for + bt_conn_tx_notify processing. The option can be used to make Bluetooth + stack more independent from the system workqueue. + +if BT_CONN_TX_NOTIFY_WQ + +config BT_CONN_TX_NOTIFY_WQ_STACK_SIZE + int "Stack size of workqueue for connection TX notify processing" + default SYSTEM_WORKQUEUE_STACK_SIZE + +config BT_CONN_TX_NOTIFY_WQ_PRIO + int "Cooperative priority of workqueue for connection TX notify processing" + default 8 + +config BT_CONN_TX_NOTIFY_WQ_INIT_PRIORITY + int "Init priority of workqueue for connection TX notify processing" + default 50 + help + The connection TX notify processing workqueue is initialized during + system initialization (at POST_KERNEL level). The Kconfig option + controls the initialization priority within level. + +endif # BT_CONN_TX_NOTIFY_WQ + menu "Bluetooth Host" if BT_HCI_HOST diff --git a/subsys/bluetooth/host/conn.c b/subsys/bluetooth/host/conn.c index c5cee7b8464b3..742801683e3d5 100644 --- a/subsys/bluetooth/host/conn.c +++ b/subsys/bluetooth/host/conn.c @@ -52,6 +52,11 @@ LOG_MODULE_REGISTER(bt_conn); K_FIFO_DEFINE(free_tx); +#if defined(CONFIG_BT_CONN_TX_NOTIFY_WQ) +static struct k_work_q conn_tx_workq; +static K_KERNEL_STACK_DEFINE(conn_tx_workq_thread_stack, CONFIG_BT_CONN_TX_NOTIFY_WQ_STACK_SIZE); +#endif /* CONFIG_BT_CONN_TX_NOTIFY_WQ */ + static void tx_free(struct bt_conn_tx *tx); static void conn_tx_destroy(struct bt_conn *conn, struct bt_conn_tx *tx) @@ -254,12 +259,21 @@ static void tx_free(struct bt_conn_tx *tx) } #if defined(CONFIG_BT_CONN_TX) -static void tx_notify(struct bt_conn *conn) +static struct k_work_q *tx_notify_workqueue_get(void) { - __ASSERT_NO_MSG(k_current_get() == - k_work_queue_thread_get(&k_sys_work_q)); +#if defined(CONFIG_BT_CONN_TX_NOTIFY_WQ) + return &conn_tx_workq; +#else + return &k_sys_work_q; +#endif /* CONFIG_BT_CONN_TX_NOTIFY_WQ */ +} - LOG_DBG("conn %p", conn); +static void tx_notify_process(struct bt_conn *conn) +{ + /* TX notify processing is done only from a single thread. */ + __ASSERT_NO_MSG(k_current_get() == k_work_queue_thread_get(tx_notify_workqueue_get())); + + LOG_DBG("conn %p", (void *)conn); while (1) { struct bt_conn_tx *tx = NULL; @@ -300,7 +314,30 @@ static void tx_notify(struct bt_conn *conn) bt_tx_irq_raise(); } } -#endif /* CONFIG_BT_CONN_TX */ +#endif /* CONFIG_BT_CONN_TX */ + +void bt_conn_tx_notify(struct bt_conn *conn, bool wait_for_completion) +{ +#if defined(CONFIG_BT_CONN_TX) + /* Ensure that function is called only from a single context. */ + if (k_current_get() == k_work_queue_thread_get(tx_notify_workqueue_get())) { + tx_notify_process(conn); + } else { + struct k_work_sync sync; + int err; + + err = k_work_submit_to_queue(tx_notify_workqueue_get(), &conn->tx_complete_work); + __ASSERT(err >= 0, "couldn't submit (err %d)", err); + + if (wait_for_completion) { + (void)k_work_flush(&conn->tx_complete_work, &sync); + } + } +#else + ARG_UNUSED(conn); + ARG_UNUSED(wait_for_completion); +#endif /* CONFIG_BT_CONN_TX */ +} struct bt_conn *bt_conn_new(struct bt_conn *conns, size_t size) { @@ -439,38 +476,15 @@ static void bt_acl_recv(struct bt_conn *conn, struct net_buf *buf, uint8_t flags bt_l2cap_recv(conn, buf, true); } -static void wait_for_tx_work(struct bt_conn *conn) -{ -#if defined(CONFIG_BT_CONN_TX) - LOG_DBG("conn %p", conn); - - if (IS_ENABLED(CONFIG_BT_RECV_WORKQ_SYS) || - k_current_get() == k_work_queue_thread_get(&k_sys_work_q)) { - tx_notify(conn); - } else { - struct k_work_sync sync; - int err; - - err = k_work_submit(&conn->tx_complete_work); - __ASSERT(err >= 0, "couldn't submit (err %d)", err); - - k_work_flush(&conn->tx_complete_work, &sync); - } - LOG_DBG("done"); -#else - ARG_UNUSED(conn); -#endif /* CONFIG_BT_CONN_TX */ -} - void bt_conn_recv(struct bt_conn *conn, struct net_buf *buf, uint8_t flags) { /* Make sure we notify any pending TX callbacks before processing * new data for this connection. * * Always do so from the same context for sanity. In this case that will - * be the system workqueue. + * be either a dedicated Bluetooth connection TX workqueue or system workqueue. */ - wait_for_tx_work(conn); + bt_conn_tx_notify(conn, true); LOG_DBG("handle %u len %u flags %02x", conn->handle, buf->len, flags); @@ -1240,7 +1254,7 @@ void bt_conn_set_state(struct bt_conn *conn, bt_conn_state_t state) */ switch (old_state) { case BT_CONN_DISCONNECT_COMPLETE: - wait_for_tx_work(conn); + bt_conn_tx_notify(conn, true); bt_conn_reset_rx_state(conn); @@ -1631,12 +1645,9 @@ struct net_buf *bt_conn_create_pdu_timeout(struct net_buf_pool *pool, #if defined(CONFIG_BT_CONN_TX) static void tx_complete_work(struct k_work *work) { - struct bt_conn *conn = CONTAINER_OF(work, struct bt_conn, - tx_complete_work); + struct bt_conn *conn = CONTAINER_OF(work, struct bt_conn, tx_complete_work); - LOG_DBG("conn %p", conn); - - tx_notify(conn); + tx_notify_process(conn); } #endif /* CONFIG_BT_CONN_TX */ @@ -4163,3 +4174,23 @@ void bt_hci_le_df_cte_req_failed(struct net_buf *buf) #endif /* CONFIG_BT_DF_CONNECTION_CTE_REQ */ #endif /* CONFIG_BT_CONN */ + +#if defined(CONFIG_BT_CONN_TX_NOTIFY_WQ) +static int bt_conn_tx_workq_init(void) +{ + const struct k_work_queue_config cfg = { + .name = "BT CONN TX WQ", + .no_yield = false, + .essential = false, + }; + + k_work_queue_init(&conn_tx_workq); + k_work_queue_start(&conn_tx_workq, conn_tx_workq_thread_stack, + K_THREAD_STACK_SIZEOF(conn_tx_workq_thread_stack), + K_PRIO_COOP(CONFIG_BT_CONN_TX_NOTIFY_WQ_PRIO), &cfg); + + return 0; +} + +SYS_INIT(bt_conn_tx_workq_init, POST_KERNEL, CONFIG_BT_CONN_TX_NOTIFY_WQ_INIT_PRIORITY); +#endif /* CONFIG_BT_CONN_TX_NOTIFY_WQ */ diff --git a/subsys/bluetooth/host/conn_internal.h b/subsys/bluetooth/host/conn_internal.h index a818a85fb7fe9..4d689f0087802 100644 --- a/subsys/bluetooth/host/conn_internal.h +++ b/subsys/bluetooth/host/conn_internal.h @@ -361,6 +361,8 @@ static inline void *closure_data(void *storage) return ((struct closure *)storage)->data; } +void bt_conn_tx_notify(struct bt_conn *conn, bool wait_for_completion); + void bt_conn_reset_rx_state(struct bt_conn *conn); /* Process incoming data for a connection */ diff --git a/subsys/bluetooth/host/hci_core.c b/subsys/bluetooth/host/hci_core.c index 0f203831bf543..14b48ed0e797a 100644 --- a/subsys/bluetooth/host/hci_core.c +++ b/subsys/bluetooth/host/hci_core.c @@ -605,7 +605,7 @@ static void hci_num_completed_packets(struct net_buf *buf) atomic_dec(&conn->in_ll); /* TX context free + callback happens in there */ - k_work_submit(&conn->tx_complete_work); + bt_conn_tx_notify(conn, false); } bt_conn_unref(conn); From 26b7104d234bddaa521f289c5d9bd791c2c323bb Mon Sep 17 00:00:00 2001 From: Marek Pieta Date: Fri, 11 Oct 2024 10:24:49 +0200 Subject: [PATCH 2/2] doc: releases: Add note for CONFIG_BT_CONN_TX_NOTIFY_WQ Change adds a release note informing about the newly introduced Kconfig option for Bluetooth stack. Signed-off-by: Marek Pieta --- doc/releases/release-notes-4.0.rst | 3 +++ 1 file changed, 3 insertions(+) diff --git a/doc/releases/release-notes-4.0.rst b/doc/releases/release-notes-4.0.rst index e468827a5addd..b470f5c4f2c36 100644 --- a/doc/releases/release-notes-4.0.rst +++ b/doc/releases/release-notes-4.0.rst @@ -107,6 +107,9 @@ Bluetooth * Added API :c:func:`bt_gatt_get_uatt_mtu` to get current Unenhanced ATT MTU of a given connection (experimental). + * Added :kconfig:option:`CONFIG_BT_CONN_TX_NOTIFY_WQ`. + The option allows using a separate workqueue for connection TX notify processing + (:c:func:`bt_conn_tx_notify`) to make Bluetooth stack more independent from the system workqueue. * The host now disconnects from the peer upon ATT timeout.