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 <[email protected]>

Author: MarekPieta

subsys/bluetooth/host/Kconfig

@@ -129,6 +129,26 @@ config BT_DRIVER_RX_HIGH_PRIO
 	int
 	default 6
 
+config BT_CONN_TX_WQ
+	bool "Use a separate workqueue for connection TX notify processing"
+	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 independed 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
+
+endif # BT_CONN_TX_WQ
+
 menu "Bluetooth Host"
 
 if BT_HCI_HOST

subsys/bluetooth/host/conn.c

@@ -52,6 +52,13 @@ LOG_MODULE_REGISTER(bt_conn);
 
 K_FIFO_DEFINE(free_tx);
 
+#if defined(CONFIG_BT_CONN_TX_WQ)
+static bool conn_tx_workq_initialized;
+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)
@@ -253,11 +260,32 @@ static void tx_free(struct bt_conn_tx *tx)
 	k_fifo_put(&free_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 */
+}
+
+void bt_conn_tx_notify(struct bt_conn *conn, bool wait_for_completion)
+{
+#if defined(CONFIG_BT_CONN_TX)
+	if (k_current_get() != k_work_queue_thread_get(tx_notify_workqueue_get())) {
+		/* Ensure that function is called only from a single context. */
+		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);
+		}
+		return;
+	}
 
 	LOG_DBG("conn %p", conn);
 
@@ -299,8 +327,8 @@ static void tx_notify(struct bt_conn *conn)
 		LOG_DBG("raise TX IRQ");
 		bt_tx_irq_raise();
 	}
-}
 #endif	/* CONFIG_BT_CONN_TX */
+}
 
 struct bt_conn *bt_conn_new(struct bt_conn *conns, size_t size)
 {
@@ -439,38 +467,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);
 
@@ -778,6 +783,16 @@ static void conn_destroy(struct bt_conn *conn, void *data)
 void bt_conn_cleanup_all(void)
 {
 	bt_conn_foreach(BT_CONN_TYPE_ALL, conn_destroy, NULL);
+#if defined(CONFIG_BT_CONN_TX_WQ)
+	if (conn_tx_workq_initialized) {
+		int ret = k_work_queue_drain(&conn_tx_workq, true);
+
+		/* No works are expected to be pending at this point. */
+		if (ret) {
+			LOG_ERR("conn_tx_workq drain returned: %d", ret);
+		}
+	}
+#endif /* CONFIG_BT_CONN_TX_WQ */
 }
 
 #if defined(CONFIG_BT_CONN)
@@ -1240,7 +1255,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);
 
@@ -1636,7 +1651,11 @@ static void tx_complete_work(struct k_work *work)
 
 	LOG_DBG("conn %p", conn);
 
-	tx_notify(conn);
+	/* Ensure single execution context. */
+	__ASSERT_NO_MSG(k_current_get() ==
+			k_work_queue_thread_get(tx_notify_workqueue_get()));
+
+	bt_conn_tx_notify(conn, false);
 }
 #endif /* CONFIG_BT_CONN_TX */
 
@@ -4054,6 +4073,27 @@ int bt_conn_init(void)
 		}
 	}
 
+#if defined(CONFIG_BT_CONN_TX_WQ)
+	if (conn_tx_workq_initialized) {
+		err = k_work_queue_unplug(&conn_tx_workq);
+		if (err) {
+			LOG_ERR("conn_tx_workq unplug returned: %d", err);
+		}
+	} else {
+		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);
+		conn_tx_workq_initialized = true;
+	}
+#endif /* CONFIG_BT_CONN_TX_WQ */
+
 	return 0;
 }
 

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 */

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);