Bluetooth: host: add dedicated WQ for long-running tasks

Send long-running tasks to a dedicated low-priority workqueue.

This shouldn't increase memory usage since by doing this, we get rid of the
ECC processing thread.

This should fix issues like #43811, since the system workqueue runs at a
cooperative priority, and the new dedicated one runs at a pre-emptible
priority.

Fixes #43811

Signed-off-by: Jonathan Rico <[email protected]>

Author: jori-nordic

subsys/bluetooth/host/CMakeLists.txt

@@ -12,6 +12,7 @@ zephyr_library_sources_ifdef(CONFIG_BT_RFCOMM           rfcomm.c)
 zephyr_library_sources_ifdef(CONFIG_BT_TESTING          testing.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_ifdef(
   CONFIG_BT_BREDR

subsys/bluetooth/host/Kconfig

@@ -4,6 +4,26 @@
 # Copyright (c) 2015-2016 Intel Corporation
 # SPDX-License-Identifier: Apache-2.0
 
+config BT_LONG_WQ
+	bool "Dedicated workqueue for long-running tasks."
+	default y if BT_GATT_CACHING
+	help
+	  Adds an API for a workqueue dedicated to long-running tasks.
+
+if BT_LONG_WQ
+config BT_LONG_WQ_STACK_SIZE
+	# Hidden: Long workqueue stack size. Should be derived from system
+	# requirements.
+	int
+	default BT_HCI_ECC_STACK_SIZE if BT_TINYCRYPT_ECC
+	default 1024
+
+config BT_LONG_WQ_PRIO
+	int "Long workqueue priority. Should be pre-emptible."
+	default 10
+	range 0 NUM_PREEMPT_PRIORITIES
+endif # BT_LONG_WQ
+
 config BT_HCI_HOST
 	# Hidden option to make the conditions more intuitive
 	bool
@@ -700,6 +720,7 @@ config BT_TINYCRYPT_ECC
 	bool "Emulate ECDH in the Host using TinyCrypt library"
 	select TINYCRYPT
 	select TINYCRYPT_ECC_DH
+	select BT_LONG_WQ
 	depends on BT_ECC && (BT_HCI_RAW || BT_HCI_HOST)
 	default y if BT_CTLR && !BT_CTLR_ECDH
 	help

subsys/bluetooth/host/gatt.c

@@ -44,6 +44,7 @@
 #include "smp.h"
 #include "settings.h"
 #include "gatt_internal.h"
+#include "long_wq.h"
 
 #define SC_TIMEOUT	K_MSEC(10)
 #define CCC_STORE_DELAY	K_SECONDS(1)
@@ -1307,7 +1308,11 @@ void bt_gatt_init(void)
 	/* Submit work to Generate initial hash as there could be static
 	 * services already in the database.
 	 */
-	k_work_schedule(&db_hash.work, DB_HASH_TIMEOUT);
+	if (IS_ENABLED(CONFIG_BT_LONG_WQ)) {
+		bt_long_wq_schedule(&db_hash.work, DB_HASH_TIMEOUT);
+	} else {
+		k_work_schedule(&db_hash.work, DB_HASH_TIMEOUT);
+	}
 #endif /* CONFIG_BT_GATT_CACHING */
 
 #if defined(CONFIG_BT_GATT_SERVICE_CHANGED)
@@ -1377,7 +1382,12 @@ static void db_changed(void)
 	int i;
 
 	atomic_clear_bit(gatt_sc.flags, DB_HASH_VALID);
-	k_work_reschedule(&db_hash.work, DB_HASH_TIMEOUT);
+
+	if (IS_ENABLED(CONFIG_BT_LONG_WQ)) {
+		bt_long_wq_reschedule(&db_hash.work, DB_HASH_TIMEOUT);
+	} else {
+		k_work_reschedule(&db_hash.work, DB_HASH_TIMEOUT);
+	}
 
 	for (i = 0; i < ARRAY_SIZE(cf_cfg); i++) {
 		struct gatt_cf_cfg *cfg = &cf_cfg[i];
@@ -5586,9 +5596,12 @@ static int db_hash_commit(void)
 	/* Reschedule work to calculate and compare against the Hash value
 	 * loaded from flash.
 	 */
-	k_work_reschedule(&db_hash.work, K_NO_WAIT);
+	if (IS_ENABLED(CONFIG_BT_LONG_WQ)) {
+		return bt_long_wq_reschedule(&db_hash.work, K_NO_WAIT);
+	} else {
+		return k_work_reschedule(&db_hash.work, K_NO_WAIT);
+	}
 
-	return 0;
 }
 
 SETTINGS_STATIC_HANDLER_DEFINE(bt_hash, "bt/hash", NULL, db_hash_set,

subsys/bluetooth/host/hci_core.c

@@ -3676,10 +3676,6 @@ int bt_enable(bt_ready_cb_t cb)
 	k_thread_name_set(&bt_workq.thread, "BT RX");
 #endif
 
-	if (IS_ENABLED(CONFIG_BT_TINYCRYPT_ECC)) {
-		bt_hci_ecc_init();
-	}
-
 	err = bt_dev.drv->open();
 	if (err) {
 		BT_ERR("HCI driver open failed (%d)", err);
@@ -3737,10 +3733,6 @@ int bt_disable(void)
 	k_thread_abort(&bt_workq.thread);
 #endif
 
-	if (IS_ENABLED(CONFIG_BT_TINYCRYPT_ECC)) {
-		bt_hci_ecc_deinit();
-	}
-
 	bt_monitor_send(BT_MONITOR_CLOSE_INDEX, NULL, 0);
 
 	/* Clear BT_DEV_ENABLE here to prevent early bt_enable() calls */

subsys/bluetooth/host/hci_ecc.c

@@ -36,9 +36,10 @@
 #else
 #include "hci_core.h"
 #endif
+#include "long_wq.h"
 
-static struct k_thread ecc_thread_data;
-static K_KERNEL_STACK_DEFINE(ecc_thread_stack, CONFIG_BT_HCI_ECC_STACK_SIZE);
+static void ecc_process(struct k_work *work);
+K_WORK_DEFINE(ecc_work, ecc_process);
 
 /* based on Core Specification 4.2 Vol 3. Part H 2.3.5.6.1 */
 static const uint8_t debug_private_key_be[BT_PRIV_KEY_LEN] = {
@@ -60,8 +61,6 @@ enum {
 
 static ATOMIC_DEFINE(flags, NUM_FLAGS);
 
-static K_SEM_DEFINE(cmd_sem, 0, 1);
-
 static struct {
 	uint8_t private_key_be[BT_PRIV_KEY_LEN];
 
@@ -208,18 +207,14 @@ static void emulate_le_generate_dhkey(void)
 	bt_recv(buf);
 }
 
-static void ecc_thread(void *p1, void *p2, void *p3)
+static void ecc_process(struct k_work *work)
 {
-	while (true) {
-		k_sem_take(&cmd_sem, K_FOREVER);
-
-		if (atomic_test_bit(flags, PENDING_PUB_KEY)) {
-			emulate_le_p256_public_key_cmd();
-		} else if (atomic_test_bit(flags, PENDING_DHKEY)) {
-			emulate_le_generate_dhkey();
-		} else {
-			__ASSERT(0, "Unhandled ECC command");
-		}
+	if (atomic_test_bit(flags, PENDING_PUB_KEY)) {
+		emulate_le_p256_public_key_cmd();
+	} else if (atomic_test_bit(flags, PENDING_DHKEY)) {
+		emulate_le_generate_dhkey();
+	} else {
+		__ASSERT(0, "Unhandled ECC command");
 	}
 }
 
@@ -261,7 +256,7 @@ static uint8_t le_gen_dhkey(uint8_t *key, uint8_t key_type)
 	atomic_set_bit_to(flags, USE_DEBUG_KEY,
 			  key_type == BT_HCI_LE_KEY_TYPE_DEBUG);
 
-	k_sem_give(&cmd_sem);
+	bt_long_wq_submit(&ecc_work);
 
 	return BT_HCI_ERR_SUCCESS;
 }
@@ -301,7 +296,7 @@ static void le_p256_pub_key(struct net_buf *buf)
 	} else if (atomic_test_and_set_bit(flags, PENDING_PUB_KEY)) {
 		status = BT_HCI_ERR_CMD_DISALLOWED;
 	} else {
-		k_sem_give(&cmd_sem);
+		bt_long_wq_submit(&ecc_work);
 		status = BT_HCI_ERR_SUCCESS;
 	}
 
@@ -351,16 +346,3 @@ int default_CSPRNG(uint8_t *dst, unsigned int len)
 {
 	return !bt_rand(dst, len);
 }
-
-void bt_hci_ecc_init(void)
-{
-	k_thread_create(&ecc_thread_data, ecc_thread_stack,
-			K_KERNEL_STACK_SIZEOF(ecc_thread_stack), ecc_thread,
-			NULL, NULL, NULL, K_PRIO_PREEMPT(10), 0, K_NO_WAIT);
-	k_thread_name_set(&ecc_thread_data, "BT ECC");
-}
-
-void bt_hci_ecc_deinit(void)
-{
-	k_thread_abort(&ecc_thread_data);
-}

subsys/bluetooth/host/hci_ecc.h

@@ -6,7 +6,5 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
-void bt_hci_ecc_init(void);
-void bt_hci_ecc_deinit(void);
 int bt_hci_ecc_send(struct net_buf *buf);
 void bt_hci_ecc_supported_commands(uint8_t *supported_commands);

subsys/bluetooth/host/hci_raw.c

@@ -372,10 +372,6 @@ int bt_enable_raw(struct k_fifo *rx_queue)
 		return -ENODEV;
 	}
 
-	if (IS_ENABLED(CONFIG_BT_TINYCRYPT_ECC)) {
-		bt_hci_ecc_init();
-	}
-
 	err = drv->open();
 	if (err) {
 		BT_ERR("HCI driver open failed (%d)", err);

subsys/bluetooth/host/long_wq.c

@@ -0,0 +1,44 @@
+/* long_work.c - Workqueue intended for long-running operations. */
+
+/*
+ * Copyright (c) 2022 Nordic Semiconductor ASA
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ */
+#include <zephyr/kernel.h>
+#include <zephyr/init.h>
+
+K_THREAD_STACK_DEFINE(bt_lw_stack_area, CONFIG_BT_LONG_WQ_STACK_SIZE);
+static struct k_work_q bt_long_wq;
+
+int bt_long_wq_schedule(struct k_work_delayable *dwork, k_timeout_t timeout)
+{
+	return k_work_schedule_for_queue(&bt_long_wq, dwork, timeout);
+}
+
+int bt_long_wq_reschedule(struct k_work_delayable *dwork, k_timeout_t timeout)
+{
+	return k_work_reschedule_for_queue(&bt_long_wq, dwork, timeout);
+}
+
+int bt_long_wq_submit(struct k_work *work)
+{
+	return k_work_submit_to_queue(&bt_long_wq, work);
+}
+
+static int long_wq_init(const struct device *d)
+{
+	ARG_UNUSED(d);
+
+	const struct k_work_queue_config cfg = {.name = "BT_LW_WQ"};
+
+	k_work_queue_init(&bt_long_wq);
+
+	k_work_queue_start(&bt_long_wq, bt_lw_stack_area,
+			   K_THREAD_STACK_SIZEOF(bt_lw_stack_area),
+			   CONFIG_BT_LONG_WQ_PRIO, &cfg);
+
+	return 0;
+}
+
+SYS_INIT(long_wq_init, POST_KERNEL, CONFIG_KERNEL_INIT_PRIORITY_DEFAULT);

subsys/bluetooth/host/long_wq.h

@@ -0,0 +1,11 @@
+/* long_wq.h - Workqueue API intended for long-running operations. */
+
+/*
+ * Copyright (c) 2022 Nordic Semiconductor ASA
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+int bt_long_wq_schedule(struct k_work_delayable *dwork, k_timeout_t timeout);
+int bt_long_wq_reschedule(struct k_work_delayable *dwork, k_timeout_t timeout);
+int bt_long_wq_submit(struct k_work *dwork);