Bluetooth: Host: Run tx_processor on its own thread

samples/bluetooth/peripheral_identity/prj.conf

@@ -6,8 +6,8 @@ CONFIG_BT_PRIVACY=y
 CONFIG_BT_DEVICE_NAME="Zephyr Peripheral"
 CONFIG_BT_GAP_AUTO_UPDATE_CONN_PARAMS=n
 
-CONFIG_BT_MAX_CONN=62
-CONFIG_BT_ID_MAX=62
+CONFIG_BT_MAX_CONN=61
+CONFIG_BT_ID_MAX=61
 
 # CONFIG_BT_SMP=y
 # CONFIG_BT_MAX_PAIRED=62

subsys/bluetooth/host/Kconfig

@@ -132,6 +132,32 @@ config BT_DRIVER_RX_HIGH_PRIO
 	int
 	default 6
 
+config BT_TX_PROCESSOR_THREAD
+	# Hidden unconfigurable option
+	bool
+	# TODO: Remove default n just after Zephyr 4.3 release. This
+	# feature increases the RAM use by 1kB. We want to enable it
+	# at the least disruptive moment in the release cycle.
+	default n
+	default y if !SOC_SERIES_NRF51X
+	# This option is automatically selected for all platforms except nRF51
+	# due to limited RAM on nRF51 devices.
+	#
+	# This thread is used to send pending HCI Commands, ACL and ISO data to
+	# Controller.
+
+if BT_TX_PROCESSOR_THREAD
+
+config BT_TX_PROCESSOR_THREAD_PRIO
+	int
+	default -1
+
+config BT_TX_PROCESSOR_STACK_SIZE
+	int
+	default 1024
+
+endif
+
 config BT_CONN_TX_NOTIFY_WQ
 	bool "Use a separate workqueue for connection TX notify processing [EXPERIMENTAL]"
 	depends on BT_CONN_TX

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,33 @@ 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_to_queue(NULL, work);
+	}
+}
+
+static void att_tx_destroy(struct net_buf *buf)
+{
+	/* We need to invoke att_on_sent_cb, which may block. We
+	 * don't want to block in a net buf destroy callback, so we
+	 * defer to a sensible workqueue.
+	 *
+	 * bt_workq cannot be used because it currently forms a
+	 * deadlock with att_pool: bt_workq -> btt_att_recv ->
+	 * send_err_rsp waits for att pool.
+	 *
+	 * We use the system work queue to preserve earlier
+	 * behavior. The tx_processor used to run on the system work
+	 * queue, and it could end up here: tx_processor ->
+	 * bt_hci_send -> net_buf_unref.
+	 *
+	 * A possible alternative is tx_notify_workqueue_get() since
+	 * this workqueue is processing similar "completion" events.
+	 */
+	net_buf_slist_put(&tx_destroy_queue, buf);
+	k_work_submit(&att_tx_destroy_work);
+	/* Continues in att_tx_destroy_work_handler() */
 }
 
 NET_BUF_POOL_DEFINE(att_pool, CONFIG_BT_ATT_TX_COUNT,

subsys/bluetooth/host/hci_core.c

@@ -472,10 +472,11 @@ int bt_hci_cmd_send_sync(uint16_t opcode, struct net_buf *buf,
 
 	/* 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 the commands are processed in the syswq and we are on the
+	 * syswq, then we cannot suspend and wait. We have to send the
+	 * command from the current context.
 	 */
-	if (k_current_get() == &k_sys_work_q.thread) {
+	if (!IS_ENABLED(CONFIG_BT_TX_PROCESSOR_THREAD) && 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;
 
@@ -5033,8 +5034,34 @@ static void tx_processor(struct k_work *item)
 
 static K_WORK_DEFINE(tx_work, tx_processor);
 
+#if defined(CONFIG_BT_TX_PROCESSOR_THREAD)
+static K_THREAD_STACK_DEFINE(bt_tx_processor_stack, CONFIG_BT_TX_PROCESSOR_STACK_SIZE);
+static struct k_work_q bt_tx_processor_workq;
+
+__maybe_unused static int bt_tx_processor_init(void)
+{
+	struct k_work_queue_config cfg = {};
+
+	if (IS_ENABLED(CONFIG_THREAD_NAME)) {
+		cfg.name = "bt_tx_processor";
+	}
+
+	k_work_queue_start(&bt_tx_processor_workq, bt_tx_processor_stack,
+			   K_THREAD_STACK_SIZEOF(bt_tx_processor_stack),
+			   CONFIG_BT_TX_PROCESSOR_THREAD_PRIO, &cfg);
+
+	return 0;
+}
+
+SYS_INIT(bt_tx_processor_init, POST_KERNEL, 999);
+#endif /* CONFIG_BT_TX_PROCESSOR_THREAD */
+
 void bt_tx_irq_raise(void)
 {
 	LOG_DBG("kick TX");
+#if defined(CONFIG_BT_TX_PROCESSOR_THREAD)
+	k_work_submit_to_queue(&bt_tx_processor_workq, &tx_work);
+#else
 	k_work_submit(&tx_work);
+#endif
 }