From 3c5bf5089ad8465b82e59c57fb1ad8b78b48d971 Mon Sep 17 00:00:00 2001 From: Jonathan Rico Date: Mon, 30 May 2022 16:08:11 +0200 Subject: [PATCH] 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 --- subsys/bluetooth/host/CMakeLists.txt | 1 + subsys/bluetooth/host/Kconfig | 21 +++++++++++++ subsys/bluetooth/host/gatt.c | 21 ++++++++++--- subsys/bluetooth/host/hci_core.c | 8 ----- subsys/bluetooth/host/hci_ecc.c | 42 ++++++++------------------ subsys/bluetooth/host/hci_ecc.h | 2 -- subsys/bluetooth/host/hci_raw.c | 4 --- subsys/bluetooth/host/long_wq.c | 44 ++++++++++++++++++++++++++++ subsys/bluetooth/host/long_wq.h | 11 +++++++ 9 files changed, 106 insertions(+), 48 deletions(-) create mode 100644 subsys/bluetooth/host/long_wq.c create mode 100644 subsys/bluetooth/host/long_wq.h diff --git a/subsys/bluetooth/host/CMakeLists.txt b/subsys/bluetooth/host/CMakeLists.txt index 711bb8a74a1..95461369a69 100644 --- a/subsys/bluetooth/host/CMakeLists.txt +++ b/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 diff --git a/subsys/bluetooth/host/Kconfig b/subsys/bluetooth/host/Kconfig index 6be730e01cb..6af6db78344 100644 --- a/subsys/bluetooth/host/Kconfig +++ b/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 diff --git a/subsys/bluetooth/host/gatt.c b/subsys/bluetooth/host/gatt.c index a5d41a53f29..1d8ced7cf64 100644 --- a/subsys/bluetooth/host/gatt.c +++ b/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, diff --git a/subsys/bluetooth/host/hci_core.c b/subsys/bluetooth/host/hci_core.c index 152122b562e..8d26b87de0c 100644 --- a/subsys/bluetooth/host/hci_core.c +++ b/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 */ diff --git a/subsys/bluetooth/host/hci_ecc.c b/subsys/bluetooth/host/hci_ecc.c index 979d57f356d..45ebb208a06 100644 --- a/subsys/bluetooth/host/hci_ecc.c +++ b/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); -} diff --git a/subsys/bluetooth/host/hci_ecc.h b/subsys/bluetooth/host/hci_ecc.h index 26368260639..e22593c832a 100644 --- a/subsys/bluetooth/host/hci_ecc.h +++ b/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); diff --git a/subsys/bluetooth/host/hci_raw.c b/subsys/bluetooth/host/hci_raw.c index 8ef5b2a8dfc..37f1d3bebe2 100644 --- a/subsys/bluetooth/host/hci_raw.c +++ b/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); diff --git a/subsys/bluetooth/host/long_wq.c b/subsys/bluetooth/host/long_wq.c new file mode 100644 index 00000000000..f857b3d75ca --- /dev/null +++ b/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 +#include + +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); diff --git a/subsys/bluetooth/host/long_wq.h b/subsys/bluetooth/host/long_wq.h new file mode 100644 index 00000000000..714aa0af355 --- /dev/null +++ b/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);