Skip to content

Commit

Permalink
bluetooth: conn: Use a separate workqueue for connection TX notify
Browse files Browse the repository at this point in the history
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 <Marek.Pieta@nordicsemi.no>
  • Loading branch information
MarekPieta committed Oct 9, 2024
1 parent 029540a commit 7fcfa90
Show file tree
Hide file tree
Showing 4 changed files with 99 additions and 37 deletions.
29 changes: 29 additions & 0 deletions subsys/bluetooth/host/Kconfig
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,35 @@ config BT_DRIVER_RX_HIGH_PRIO
int
default 6

config BT_CONN_TX_WQ
bool "Use a separate workqueue for connection TX notify processing"
depends on BT_CONN_TX
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_WQ

config BT_CONN_TX_WQ_STACK_SIZE
int "Stack size of workqueue for connection TX notify processing"
default 1200

config BT_CONN_TX_WQ_PRIO
# Hidden option for cooperative connection TX notify workqueue priority
int
default 8

config BT_CONN_TX_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_WQ

menu "Bluetooth Host"

if BT_HCI_HOST
Expand Down
103 changes: 67 additions & 36 deletions subsys/bluetooth/host/conn.c
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,11 @@ LOG_MODULE_REGISTER(bt_conn);

K_FIFO_DEFINE(free_tx);

#if defined(CONFIG_BT_CONN_TX_WQ)
static struct k_work_q conn_tx_workq;
static K_KERNEL_STACK_DEFINE(conn_tx_workq_thread_stack, CONFIG_BT_CONN_TX_WQ_STACK_SIZE);
#endif /* CONFIG_BT_CONN_TX_WQ */

static void tx_free(struct bt_conn_tx *tx);

static void conn_tx_destroy(struct bt_conn *conn, struct bt_conn_tx *tx)
Expand Down Expand Up @@ -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_WQ)
return &conn_tx_workq;
#else
return &k_sys_work_q;
#endif /* CONFIG_BT_CONN_TX_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;
Expand Down Expand Up @@ -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)
{
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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 */

Expand Down Expand Up @@ -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_WQ)
static int bt_conn_tx_workq_init(void)
{
static 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_WQ_PRIO), &cfg);

return 0;
}

SYS_INIT(bt_conn_tx_workq_init, POST_KERNEL, CONFIG_BT_CONN_TX_WQ_INIT_PRIORITY);
#endif /* CONFIG_BT_CONN_TX_WQ */
2 changes: 2 additions & 0 deletions subsys/bluetooth/host/conn_internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand Down
2 changes: 1 addition & 1 deletion subsys/bluetooth/host/hci_core.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down

0 comments on commit 7fcfa90

Please sign in to comment.