From ccbc94cacc60ad0db51508fded417d77faa871e5 Mon Sep 17 00:00:00 2001 From: Zhou Xiao Date: Tue, 18 Aug 2026 19:20:12 +0800 Subject: [PATCH 1/2] refactor(ble_log): decouple runtime submission ownership Move transport ownership from the runtime to the LBM: submit and recycle now hand the peripheral-owned flag explicitly instead of the runtime reaching back into LBM buffers. The submit path never blocks producers - a transport that cannot be queued is recycled immediately so its data survives for the next flush. Cross-context ownership accesses go through explicit atomic helpers: release-store on recycle pairs with acquire-loads in the flush paths, and the inflight high-water mark stays a relaxed CAS-max (a plain volatile update races the runtime hook's statistics reads). --- .../bt/common/ble_log/src/ble_log_lbm.c | 44 +++++++++++++------ components/bt/common/ble_log/src/ble_log_rt.c | 20 ++------- .../src/internal_include/ble_log_prph.h | 2 +- .../ble_log/src/internal_include/ble_log_rt.h | 4 +- .../src/internal_include/ble_log_util.h | 7 ++- 5 files changed, 43 insertions(+), 34 deletions(-) diff --git a/components/bt/common/ble_log/src/ble_log_lbm.c b/components/bt/common/ble_log/src/ble_log_lbm.c index ed5f3805ea0..1329bb20de5 100644 --- a/components/bt/common/ble_log/src/ble_log_lbm.c +++ b/components/bt/common/ble_log/src/ble_log_lbm.c @@ -39,6 +39,7 @@ BLE_LOG_STATIC bool ble_log_lbm_acquire_trans(size_t log_len, ble_log_lbm_t **out_lbm, ble_log_prph_trans_t ***out_trans); BLE_LOG_STATIC void ble_log_lbm_release(ble_log_lbm_t *lbm); +BLE_LOG_STATIC void ble_log_lbm_submit_trans(ble_log_prph_trans_t **trans); BLE_LOG_STATIC ble_log_prph_trans_t **ble_log_lbm_get_trans(ble_log_lbm_t *lbm, size_t log_len); BLE_LOG_STATIC bool ble_log_lbm_flush_all_trans(void); @@ -125,6 +126,23 @@ void ble_log_lbm_release(ble_log_lbm_t *lbm) } } +BLE_LOG_IRAM_ATTR BLE_LOG_STATIC +void ble_log_lbm_submit_trans(ble_log_prph_trans_t **trans) +{ + ble_log_prph_trans_t *submitted = *trans; + BLE_LOG_ATOMIC_STORE_RELAXED(submitted->prph_owned, true); + + ble_log_lbm_t *lbm = (ble_log_lbm_t *)submitted->owner; + uint32_t inflight = __atomic_add_fetch(&lbm->trans_inflight, 1, __ATOMIC_RELAXED); + uint32_t peak = __atomic_load_n(&lbm->trans_inflight_peak, __ATOMIC_RELAXED); + while (inflight > peak && + !__atomic_compare_exchange_n(&lbm->trans_inflight_peak, &peak, inflight, + false, __ATOMIC_RELAXED, __ATOMIC_RELAXED)) { + } + + ble_log_rt_submit_trans(submitted); +} + BLE_LOG_STATIC bool ble_log_lbm_flush_all_trans(void) { ble_log_lbm_t *lbm; @@ -138,9 +156,8 @@ BLE_LOG_STATIC bool ble_log_lbm_flush_all_trans(void) int trans_idx = lbm->trans_idx; for (int j = 0; j < BLE_LOG_TRANS_BUF_CNT; j++) { trans = &(lbm->trans[trans_idx]); - if (!__atomic_load_n(&(*trans)->prph_owned, __ATOMIC_ACQUIRE) && - (*trans)->pos) { - ble_log_rt_queue_trans(trans); + if (!BLE_LOG_ATOMIC_LOAD_ACQUIRE((*trans)->prph_owned) && (*trans)->pos) { + ble_log_lbm_submit_trans(trans); } trans_idx = (trans_idx + 1) & (BLE_LOG_TRANS_BUF_CNT - 1); } @@ -153,7 +170,7 @@ BLE_LOG_STATIC bool ble_log_lbm_flush_all_trans(void) lbm = &(lbm_ctx->lbm_pool[i]); for (int j = 0; j < BLE_LOG_TRANS_BUF_CNT; j++) { trans = &(lbm->trans[j]); - in_progress |= __atomic_load_n(&(*trans)->prph_owned, __ATOMIC_ACQUIRE); + in_progress |= BLE_LOG_ATOMIC_LOAD_ACQUIRE((*trans)->prph_owned); } } if (in_progress) { @@ -230,7 +247,7 @@ void ble_log_lbm_write_trans(ble_log_prph_trans_t **trans, ble_log_src_t src_cod /* Queue trans if full */ if (BLE_LOG_TRANS_FREE_SPACE((*trans)) <= BLE_LOG_FRAME_OVERHEAD) { - ble_log_rt_queue_trans(trans); + ble_log_lbm_submit_trans(trans); } } @@ -257,7 +274,7 @@ void ble_log_lbm_stream_seal(ble_log_prph_trans_t **trans, ble_log_src_t src_cod ble_log_stat_mgr_update(src_code, payload_len, false); - ble_log_rt_queue_trans(trans); + ble_log_lbm_submit_trans(trans); } #endif /* BLE_LOG_UART_REDIR_ENABLED */ @@ -319,7 +336,7 @@ BLE_LOG_IRAM_ATTR void ble_log_lbm_recycle_trans(ble_log_prph_trans_t *trans) { ble_log_lbm_t *lbm = (ble_log_lbm_t *)trans->owner; __atomic_fetch_sub(&lbm->trans_inflight, 1, __ATOMIC_RELAXED); - __atomic_store_n(&trans->prph_owned, false, __ATOMIC_RELEASE); + BLE_LOG_ATOMIC_STORE_RELEASE(trans->prph_owned, false); } bool ble_log_lbm_init(void) @@ -388,7 +405,6 @@ bool ble_log_lbm_init(void) } /* Initialization done */ - lbm_ref_count = 0; lbm_inited = true; lbm_enabled = false; return true; @@ -448,7 +464,7 @@ ble_log_prph_trans_t **ble_log_lbm_get_trans(ble_log_lbm_t *lbm, size_t log_len) ble_log_prph_trans_t **trans; for (int i = 0; i < BLE_LOG_TRANS_BUF_CNT; i++) { trans = &(lbm->trans[lbm->trans_idx]); - if (!__atomic_load_n(&(*trans)->prph_owned, __ATOMIC_ACQUIRE)) { + if (!BLE_LOG_ATOMIC_LOAD_ACQUIRE((*trans)->prph_owned)) { /* Return if there's enough free space in current transport */ if (BLE_LOG_TRANS_FREE_SPACE((*trans)) >= (log_len + BLE_LOG_FRAME_OVERHEAD)) { return trans; @@ -456,7 +472,7 @@ ble_log_prph_trans_t **ble_log_lbm_get_trans(ble_log_lbm_t *lbm, size_t log_len) /* Queue transport if there's insufficient free space */ if ((*trans)->pos) { - ble_log_rt_queue_trans(trans); + ble_log_lbm_submit_trans(trans); } } @@ -560,8 +576,8 @@ void ble_log_lbm_stream_flush(ble_log_lbm_t *lbm, ble_log_src_t src_code) int trans_idx = lbm->trans_idx; for (int i = 0; i < BLE_LOG_TRANS_BUF_CNT; i++) { ble_log_prph_trans_t **trans = &(lbm->trans[trans_idx]); - if (!__atomic_load_n(&(*trans)->prph_owned, __ATOMIC_ACQUIRE) && - (*trans)->pos > BLE_LOG_FRAME_HEAD_LEN) { + if (!BLE_LOG_ATOMIC_LOAD_ACQUIRE((*trans)->prph_owned) && + (*trans)->pos > BLE_LOG_FRAME_HEAD_LEN) { ble_log_lbm_stream_seal(trans, src_code); } trans_idx = (trans_idx + 1) & (BLE_LOG_TRANS_BUF_CNT - 1); @@ -575,8 +591,8 @@ BLE_LOG_STATIC void ble_log_emit_buf_util(ble_log_lbm_t *lbm, uint8_t lbm_id) .int_src_code = BLE_LOG_INT_SRC_BUF_UTIL, .lbm_id = lbm_id, .trans_cnt = BLE_LOG_TRANS_BUF_CNT, - .inflight_peak = (uint8_t)__atomic_load_n( - &lbm->trans_inflight_peak, __ATOMIC_RELAXED), + .inflight_peak = (uint8_t)__atomic_load_n(&lbm->trans_inflight_peak, + __ATOMIC_RELAXED), }; ble_log_write_hex(BLE_LOG_SRC_INTERNAL, (const uint8_t *)&util, sizeof(ble_log_buf_util_t)); diff --git a/components/bt/common/ble_log/src/ble_log_rt.c b/components/bt/common/ble_log/src/ble_log_rt.c index 3c4e21505eb..9f1e6cf8c6d 100644 --- a/components/bt/common/ble_log/src/ble_log_rt.c +++ b/components/bt/common/ble_log/src/ble_log_rt.c @@ -174,33 +174,21 @@ void ble_log_rt_deinit(void) } } -BLE_LOG_IRAM_ATTR void ble_log_rt_queue_trans(ble_log_prph_trans_t **trans) +BLE_LOG_IRAM_ATTR void ble_log_rt_submit_trans(ble_log_prph_trans_t *trans) { - __atomic_store_n(&(*trans)->prph_owned, true, __ATOMIC_RELAXED); - - ble_log_lbm_t *lbm = (ble_log_lbm_t *)(*trans)->owner; - uint32_t inflight = __atomic_add_fetch(&lbm->trans_inflight, 1, __ATOMIC_RELAXED); - uint32_t peak = __atomic_load_n(&lbm->trans_inflight_peak, __ATOMIC_RELAXED); - while (inflight > peak) { - if (__atomic_compare_exchange_n(&lbm->trans_inflight_peak, &peak, inflight, - true, __ATOMIC_RELAXED, __ATOMIC_RELAXED)) { - break; - } - } - if (BLE_LOG_IN_ISR()) { BaseType_t woken = pdFALSE; /* Queue depth == total transport buffer count; queue-full is impossible * for a valid transport, so the return value is not checked. */ - xQueueSendFromISR(rt_queue_handle, trans, &woken); + xQueueSendFromISR(rt_queue_handle, &trans, &woken); portYIELD_FROM_ISR(woken); } else if (xTaskGetSchedulerState() == taskSCHEDULER_SUSPENDED) { /* Non-blocking send to avoid configASSERT when scheduler is suspended * (e.g., during light sleep transitions). Queue-full is impossible; * see comment above. */ - xQueueSend(rt_queue_handle, trans, 0); + xQueueSend(rt_queue_handle, &trans, 0); } else { - xQueueSend(rt_queue_handle, trans, portMAX_DELAY); + xQueueSend(rt_queue_handle, &trans, portMAX_DELAY); } } diff --git a/components/bt/common/ble_log/src/internal_include/ble_log_prph.h b/components/bt/common/ble_log/src/internal_include/ble_log_prph.h index a466df91795..e9986c6efb7 100644 --- a/components/bt/common/ble_log/src/internal_include/ble_log_prph.h +++ b/components/bt/common/ble_log/src/internal_include/ble_log_prph.h @@ -15,7 +15,7 @@ /* TYPEDEF */ typedef struct { - bool prph_owned; + volatile uint32_t prph_owned; uint8_t *buf; uint16_t size; uint16_t pos; diff --git a/components/bt/common/ble_log/src/internal_include/ble_log_rt.h b/components/bt/common/ble_log/src/internal_include/ble_log_rt.h index 7becb1202b8..13b6aa43658 100644 --- a/components/bt/common/ble_log/src/internal_include/ble_log_rt.h +++ b/components/bt/common/ble_log/src/internal_include/ble_log_rt.h @@ -30,8 +30,8 @@ #endif /* CONFIG_BLE_LOG_TS_ENABLED */ /* INTERFACE */ -bool ble_log_rt_init(); +bool ble_log_rt_init(void); void ble_log_rt_deinit(void); -void ble_log_rt_queue_trans(ble_log_prph_trans_t **trans); +void ble_log_rt_submit_trans(ble_log_prph_trans_t *trans); #endif /* __BLE_LOG_RT_H__ */ diff --git a/components/bt/common/ble_log/src/internal_include/ble_log_util.h b/components/bt/common/ble_log/src/internal_include/ble_log_util.h index e1335dda5f5..3c0a2712410 100644 --- a/components/bt/common/ble_log/src/internal_include/ble_log_util.h +++ b/components/bt/common/ble_log/src/internal_include/ble_log_util.h @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2025 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2025-2026 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -25,6 +25,11 @@ #endif /* !UNIT_TEST */ /* MACRO */ +#define BLE_LOG_ATOMIC_LOAD_ACQUIRE(VAR) __atomic_load_n(&(VAR), __ATOMIC_ACQUIRE) +#define BLE_LOG_ATOMIC_LOAD_RELAXED(VAR) __atomic_load_n(&(VAR), __ATOMIC_RELAXED) +#define BLE_LOG_ATOMIC_STORE_RELEASE(VAR, VALUE) __atomic_store_n(&(VAR), (VALUE), __ATOMIC_RELEASE) +#define BLE_LOG_ATOMIC_STORE_RELAXED(VAR, VALUE) __atomic_store_n(&(VAR), (VALUE), __ATOMIC_RELAXED) + /* Unit test */ #ifndef UNIT_TEST From 68ece6c8b6f6e33e5f7c314be6ac44f4a70a106f Mon Sep 17 00:00:00 2001 From: Zhou Xiao Date: Wed, 19 Aug 2026 23:17:46 +0800 Subject: [PATCH 2/2] refactor(ble_log): close the lifecycle gate before teardown Introduce a seq_cst closing gate shared by the runtime and the LBM: submitters increment the reference count before checking the inited flag, and deinit closes the gate and waits for the count to drain before deleting tasks, timers, queues, or buffers. A producer either observes shutdown or its reference is visible to the wait, which makes ble_log_deinit safe while write APIs are still active. Submitters no longer block on the queue while holding a reference: a timeout-0 send that cannot queue recycles the transport so its data survives for the next flush. Extract ble_log_ref_count_try_acquire/wait into the utility layer and gate every LBM writer through ble_log_lbm_ref_acquire. ble_log_deinit now closes the LBM gate first (ble_log_lbm_close) instead of clearing the enable flag. --- components/bt/common/ble_log/src/ble_log.c | 16 ++- .../bt/common/ble_log/src/ble_log_lbm.c | 121 +++++++++--------- components/bt/common/ble_log/src/ble_log_rt.c | 64 +++++---- .../bt/common/ble_log/src/ble_log_util.c | 32 +++++ .../src/internal_include/ble_log_lbm.h | 2 +- .../src/internal_include/ble_log_util.h | 16 +++ 6 files changed, 156 insertions(+), 95 deletions(-) diff --git a/components/bt/common/ble_log/src/ble_log.c b/components/bt/common/ble_log/src/ble_log.c index 83ac2fcbd40..6d5b18692c1 100644 --- a/components/bt/common/ble_log/src/ble_log.c +++ b/components/bt/common/ble_log/src/ble_log.c @@ -93,23 +93,27 @@ void ble_log_deinit(void) ESP_LOGW(TAG, "Unregister shutdown handler failed, ret = 0x%x", ret); } } - ble_log_enable(false); ble_log_inited = false; + ble_log_lbm_close(); /* CRITICAL — Deinit ordering rationale: * - * 1. Runtime task must be stopped FIRST to prevent it from sending + * 1. The LBM writer gate is closed before submodule teardown. Writers + * already inside the gate keep a reference until they finish; later + * writers are rejected. + * + * 2. Runtime task must be stopped FIRST to prevent it from sending * transports to an already-destroyed peripheral driver. The queue * is drained and pending transports are discarded. * - * 2. Peripheral interface is deinitialized SECOND. It waits for any + * 3. Peripheral interface is deinitialized SECOND. It waits for any * in-flight DMA operations (started before the task was killed) to * complete, then destroys the driver. This is safe because no new * DMA operations can be started (the task is already dead). * - * 3. LBM is deinitialized LAST. At this point all DMA has completed - * (ensured by step 2) and all queued transports have been drained - * (ensured by step 1), so freeing the buffers is safe. */ + * 4. LBM is deinitialized LAST. At this point all DMA has completed + * (ensured by step 3) and all queued transports have been drained + * (ensured by step 2), so freeing the buffers is safe. */ ble_log_rt_deinit(); ble_log_prph_deinit(); ble_log_lbm_deinit(); diff --git a/components/bt/common/ble_log/src/ble_log_lbm.c b/components/bt/common/ble_log/src/ble_log_lbm.c index 1329bb20de5..a15c5f4782b 100644 --- a/components/bt/common/ble_log/src/ble_log_lbm.c +++ b/components/bt/common/ble_log/src/ble_log_lbm.c @@ -28,8 +28,8 @@ #define BLE_LOG_LBM_WAIT_TIMEOUT_MS (1000) BLE_LOG_STATIC BLE_LOG_DRAM_ATTR volatile uint32_t lbm_ref_count = 0; -BLE_LOG_STATIC BLE_LOG_DRAM_ATTR bool lbm_inited = false; -BLE_LOG_STATIC BLE_LOG_DRAM_ATTR bool lbm_enabled = false; +BLE_LOG_STATIC BLE_LOG_DRAM_ATTR uint32_t lbm_inited = 0; +BLE_LOG_STATIC BLE_LOG_DRAM_ATTR uint32_t lbm_enabled = 0; BLE_LOG_STATIC volatile bool flush_in_progress = false; BLE_LOG_STATIC BLE_LOG_DRAM_ATTR ble_log_lbm_ctx_t *lbm_ctx = NULL; BLE_LOG_STATIC BLE_LOG_DRAM_ATTR ble_log_stat_mgr_t *stat_mgr_ctx[BLE_LOG_SRC_MAX] = {0}; @@ -59,6 +59,19 @@ BLE_LOG_STATIC void ble_log_stat_mgr_update(ble_log_src_t src_code, uint32_t len /* ------------------------- */ /* PRIVATE INTERFACE */ /* ------------------------- */ +BLE_LOG_IRAM_ATTR BLE_LOG_STATIC +bool ble_log_lbm_ref_acquire(bool require_enabled) +{ + if (!ble_log_ref_count_try_acquire(&lbm_ref_count, &lbm_inited)) { + return false; + } + if (require_enabled && !BLE_LOG_ATOMIC_LOAD_ACQUIRE(lbm_enabled)) { + BLE_LOG_REF_COUNT_RELEASE(&lbm_ref_count); + return false; + } + return true; +} + BLE_LOG_IRAM_ATTR BLE_LOG_STATIC bool ble_log_lbm_acquire_trans(size_t log_len, ble_log_lbm_t **out_lbm, ble_log_prph_trans_t ***out_trans) @@ -319,7 +332,7 @@ bool ble_log_write_hex_core(ble_log_src_t src_code, const uint8_t *addr, size_t return true; failed: - if (lbm_inited) { + if (BLE_LOG_ATOMIC_LOAD_RELAXED(lbm_inited)) { ble_log_stat_mgr_update(src_code, payload_len, true); } return false; @@ -342,7 +355,7 @@ BLE_LOG_IRAM_ATTR void ble_log_lbm_recycle_trans(ble_log_prph_trans_t *trans) bool ble_log_lbm_init(void) { /* Avoid double init */ - if (lbm_inited) { + if (BLE_LOG_ATOMIC_LOAD_ACQUIRE(lbm_inited)) { return true; } @@ -405,8 +418,8 @@ bool ble_log_lbm_init(void) } /* Initialization done */ - lbm_inited = true; - lbm_enabled = false; + BLE_LOG_ATOMIC_STORE_RELAXED(lbm_enabled, false); + BLE_LOG_ATOMIC_STORE_RELEASE(lbm_inited, true); return true; exit: @@ -414,22 +427,22 @@ exit: return false; } +void ble_log_lbm_close(void) +{ + BLE_LOG_ATOMIC_STORE_SEQ_CST(lbm_inited, false); + BLE_LOG_ATOMIC_STORE_RELEASE(lbm_enabled, false); +} + void ble_log_lbm_deinit(void) { - /* Set inited flag to false to prevent new references */ - lbm_inited = false; - lbm_enabled = false; + /* Close before waiting: every LBM entry increments the reference count + * before checking this seq_cst gate. */ + ble_log_lbm_close(); /* Disable module and wait for all references to be released */ - TickType_t start_tick = xTaskGetTickCount(); - while (__atomic_load_n(&lbm_ref_count, __ATOMIC_ACQUIRE) > 0) { - if ((xTaskGetTickCount() - start_tick) >= - pdMS_TO_TICKS(BLE_LOG_LBM_WAIT_TIMEOUT_MS)) { - ESP_LOGE(TAG, "Timed out waiting for BLE Log references during deinit"); - } - BLE_LOG_ASSERT((xTaskGetTickCount() - start_tick) < - pdMS_TO_TICKS(BLE_LOG_LBM_WAIT_TIMEOUT_MS)); - vTaskDelay(1); + while (!ble_log_ref_count_wait(&lbm_ref_count, 0)) { + ESP_LOGE(TAG, "Timed out waiting for BLE Log references during deinit"); + BLE_LOG_ASSERT(false); } /* Release statistic manager context */ @@ -486,9 +499,8 @@ ble_log_prph_trans_t **ble_log_lbm_get_trans(ble_log_lbm_t *lbm, size_t log_len) void ble_log_write_enh_stat(void) { - BLE_LOG_REF_COUNT_ACQUIRE(&lbm_ref_count); - if (!lbm_enabled) { - goto deref; + if (!ble_log_lbm_ref_acquire(true)) { + return; } /* Snapshot all sources under one critical section so the set of @@ -510,7 +522,6 @@ void ble_log_write_enh_stat(void) ble_log_write_hex(BLE_LOG_SRC_INTERNAL, (const uint8_t *)&snapshots[i], sizeof(ble_log_enh_stat_t)); } -deref: BLE_LOG_REF_COUNT_RELEASE(&lbm_ref_count); } @@ -600,9 +611,8 @@ BLE_LOG_STATIC void ble_log_emit_buf_util(ble_log_lbm_t *lbm, uint8_t lbm_id) void ble_log_write_buf_util(void) { - BLE_LOG_REF_COUNT_ACQUIRE(&lbm_ref_count); - if (!lbm_enabled) { - goto deref; + if (!ble_log_lbm_ref_acquire(true)) { + return; } ble_log_emit_buf_util(&lbm_ctx->spin_task, @@ -634,15 +644,13 @@ void ble_log_write_buf_util(void) } #endif -deref: BLE_LOG_REF_COUNT_RELEASE(&lbm_ref_count); } void ble_log_write_final_stat(void) { - BLE_LOG_REF_COUNT_ACQUIRE(&lbm_ref_count); - if (!lbm_inited) { - goto deref; + if (!ble_log_lbm_ref_acquire(false)) { + return; } ble_log_final_stat_t final_stat; @@ -661,7 +669,6 @@ void ble_log_write_final_stat(void) ble_log_write_internal((const uint8_t *)&final_stat, sizeof(final_stat)); -deref: BLE_LOG_REF_COUNT_RELEASE(&lbm_ref_count); } @@ -670,10 +677,10 @@ deref: /* ------------------------ */ bool ble_log_enable(bool enable) { - if (!lbm_inited) { + if (!BLE_LOG_ATOMIC_LOAD_ACQUIRE(lbm_inited)) { return false; } - lbm_enabled = enable; + BLE_LOG_ATOMIC_STORE_RELEASE(lbm_enabled, enable); return true; } @@ -686,9 +693,8 @@ void ble_log_flush(void) return; } - BLE_LOG_REF_COUNT_ACQUIRE(&lbm_ref_count); - if (!lbm_inited) { - goto deref; + if (!ble_log_lbm_ref_acquire(false)) { + goto clear; } ble_log_write_enh_stat(); ble_log_write_buf_util(); @@ -701,17 +707,12 @@ void ble_log_flush(void) ble_log_write_hex(BLE_LOG_SRC_INTERNAL, (const uint8_t *)&ble_log_info, sizeof(ble_log_info_t)); /* Disable module and wait for all other references to release */ - bool lbm_enabled_copy = lbm_enabled; + bool lbm_enabled_copy = BLE_LOG_ATOMIC_LOAD_ACQUIRE(lbm_enabled); - lbm_enabled = false; - TickType_t start_tick = xTaskGetTickCount(); - while (__atomic_load_n(&lbm_ref_count, __ATOMIC_ACQUIRE) > 1) { - if ((xTaskGetTickCount() - start_tick) >= - pdMS_TO_TICKS(BLE_LOG_LBM_WAIT_TIMEOUT_MS)) { - ESP_LOGE(TAG, "Timed out waiting for BLE Log writers"); - goto fail; - } - vTaskDelay(1); + BLE_LOG_ATOMIC_STORE_RELEASE(lbm_enabled, false); + if (!ble_log_ref_count_wait(&lbm_ref_count, 1)) { + ESP_LOGE(TAG, "Timed out waiting for BLE Log writers"); + goto fail; } if (!ble_log_lbm_flush_all_trans()) { @@ -727,9 +728,9 @@ void ble_log_flush(void) fail: /* Resume enable status after a completed or failed flush. */ - lbm_enabled = lbm_enabled_copy; -deref: + BLE_LOG_ATOMIC_STORE_RELEASE(lbm_enabled, lbm_enabled_copy); BLE_LOG_REF_COUNT_RELEASE(&lbm_ref_count); +clear: __atomic_clear(&flush_in_progress, __ATOMIC_RELEASE); } @@ -738,13 +739,11 @@ bool ble_log_write_hex(ble_log_src_t src_code, const uint8_t *addr, size_t len) { bool ret = false; - BLE_LOG_REF_COUNT_ACQUIRE(&lbm_ref_count); - if (!lbm_enabled) { - goto exit; + if (!ble_log_lbm_ref_acquire(true)) { + return false; } ret = ble_log_write_hex_core(src_code, addr, len); -exit: BLE_LOG_REF_COUNT_RELEASE(&lbm_ref_count); return ret; } @@ -754,13 +753,11 @@ bool ble_log_write_internal(const uint8_t *addr, size_t len) { bool ret = false; - BLE_LOG_REF_COUNT_ACQUIRE(&lbm_ref_count); - if (!lbm_inited) { - goto exit; + if (!ble_log_lbm_ref_acquire(false)) { + return false; } ret = ble_log_write_hex_core(BLE_LOG_SRC_INTERNAL, addr, len); -exit: BLE_LOG_REF_COUNT_RELEASE(&lbm_ref_count); return ret; } @@ -770,9 +767,8 @@ BLE_LOG_IRAM_ATTR void ble_log_write_hex_ll(uint32_t len, const uint8_t *addr, uint32_t len_append, const uint8_t *addr_append, uint32_t flag) { - BLE_LOG_REF_COUNT_ACQUIRE(&lbm_ref_count); - if (!lbm_enabled) { - goto exit; + if (!ble_log_lbm_ref_acquire(true)) { + return; } /* Source code shall be determined before LBM determination */ @@ -830,10 +826,9 @@ void ble_log_write_hex_ll(uint32_t len, const uint8_t *addr, return; failed: - if (lbm_inited) { + if (BLE_LOG_ATOMIC_LOAD_RELAXED(lbm_inited)) { ble_log_stat_mgr_update(src_code, payload_len, true); } -exit: BLE_LOG_REF_COUNT_RELEASE(&lbm_ref_count); return; } @@ -841,9 +836,8 @@ exit: void ble_log_dump_to_console(void) { - BLE_LOG_REF_COUNT_ACQUIRE(&lbm_ref_count); - if (!lbm_inited) { - goto deref; + if (!ble_log_lbm_ref_acquire(false)) { + return; } int trans_idx; @@ -870,7 +864,6 @@ void ble_log_dump_to_console(void) BLE_LOG_CONSOLE("\n:BLE_LOG_DUMP_END]\n\n"); BLE_LOG_EXIT_CRITICAL(); -deref: BLE_LOG_REF_COUNT_RELEASE(&lbm_ref_count); return; } diff --git a/components/bt/common/ble_log/src/ble_log_rt.c b/components/bt/common/ble_log/src/ble_log_rt.c index 9f1e6cf8c6d..afec0b1e126 100644 --- a/components/bt/common/ble_log/src/ble_log_rt.c +++ b/components/bt/common/ble_log/src/ble_log_rt.c @@ -13,12 +13,18 @@ #include "ble_log_rt.h" #include "ble_log_lbm.h" +#include "esp_log.h" + +/* MACRO */ +#define TAG "ble_log_rt" + /* VARIABLE */ -BLE_LOG_STATIC BLE_LOG_DRAM_ATTR bool rt_inited = false; +BLE_LOG_STATIC BLE_LOG_DRAM_ATTR uint32_t rt_inited = 0; +BLE_LOG_STATIC BLE_LOG_DRAM_ATTR volatile uint32_t rt_ref_count = 0; BLE_LOG_STATIC TaskHandle_t rt_task_handle = NULL; BLE_LOG_STATIC BLE_LOG_DRAM_ATTR QueueHandle_t rt_queue_handle = NULL; #if CONFIG_BLE_LOG_TS_ENABLED -BLE_LOG_STATIC BLE_LOG_DRAM_ATTR bool rt_ts_enabled = false; +BLE_LOG_STATIC BLE_LOG_DRAM_ATTR uint32_t rt_ts_enabled = 0; #if CONFIG_BLE_LOG_TS_TRIGGER_ESP_TIMER BLE_LOG_STATIC esp_timer_handle_t rt_ts_timer = NULL; #endif /* CONFIG_BLE_LOG_TS_TRIGGER_ESP_TIMER */ @@ -81,7 +87,8 @@ BLE_LOG_IRAM_ATTR BLE_LOG_STATIC void ble_log_rt_ts_trigger(void *arg) { (void)arg; - if (!rt_inited || !rt_ts_enabled) { + if (!BLE_LOG_ATOMIC_LOAD_ACQUIRE(rt_inited) || + !BLE_LOG_ATOMIC_LOAD_ACQUIRE(rt_ts_enabled)) { return; } ble_log_ts_info_t *ts_info = NULL; @@ -95,7 +102,7 @@ BLE_LOG_STATIC void ble_log_rt_ts_trigger(void *arg) /* INTERFACE */ bool ble_log_rt_init(void) { - if (rt_inited) { + if (BLE_LOG_ATOMIC_LOAD_ACQUIRE(rt_inited)) { return true; } @@ -113,7 +120,7 @@ bool ble_log_rt_init(void) } #if CONFIG_BLE_LOG_TS_ENABLED - rt_ts_enabled = false; + BLE_LOG_ATOMIC_STORE_RELAXED(rt_ts_enabled, false); #if CONFIG_BLE_LOG_TS_TRIGGER_ESP_TIMER /* Initialize ESP Timer Trigger */ esp_timer_create_args_t ts_timer_args = { @@ -133,7 +140,7 @@ bool ble_log_rt_init(void) #endif /* CONFIG_BLE_LOG_TS_TRIGGER_ESP_TIMER */ #endif /* CONFIG_BLE_LOG_TS_ENABLED */ - rt_inited = true; + BLE_LOG_ATOMIC_STORE_RELEASE(rt_inited, true); return true; exit: @@ -143,12 +150,19 @@ exit: void ble_log_rt_deinit(void) { - rt_inited = false; + /* Closing gate: seq_cst on both sides (see also submit) so a submitter + * either sees rt_inited == false and bails, or its reference is visible + * to the ref-count wait before the task and queue are torn down. */ + BLE_LOG_ATOMIC_STORE_SEQ_CST(rt_inited, false); + while (!ble_log_ref_count_wait(&rt_ref_count, 0)) { + ESP_LOGE(TAG, "Timed out waiting for BLE Log runtime references"); + BLE_LOG_ASSERT(false); + } #if CONFIG_BLE_LOG_TS_ENABLED - rt_ts_enabled = false; + BLE_LOG_ATOMIC_STORE_RELEASE(rt_ts_enabled, false); #if CONFIG_BLE_LOG_TS_TRIGGER_ESP_TIMER if (rt_ts_timer) { - esp_timer_stop(rt_ts_timer); + esp_timer_stop_blocking(rt_ts_timer, portMAX_DELAY); esp_timer_delete(rt_ts_timer); rt_ts_timer = NULL; } @@ -176,29 +190,31 @@ void ble_log_rt_deinit(void) BLE_LOG_IRAM_ATTR void ble_log_rt_submit_trans(ble_log_prph_trans_t *trans) { - if (BLE_LOG_IN_ISR()) { - BaseType_t woken = pdFALSE; - /* Queue depth == total transport buffer count; queue-full is impossible - * for a valid transport, so the return value is not checked. */ - xQueueSendFromISR(rt_queue_handle, &trans, &woken); - portYIELD_FROM_ISR(woken); - } else if (xTaskGetSchedulerState() == taskSCHEDULER_SUSPENDED) { - /* Non-blocking send to avoid configASSERT when scheduler is suspended - * (e.g., during light sleep transitions). Queue-full is impossible; - * see comment above. */ - xQueueSend(rt_queue_handle, &trans, 0); - } else { - xQueueSend(rt_queue_handle, &trans, portMAX_DELAY); + if (!ble_log_ref_count_try_acquire(&rt_ref_count, &rt_inited)) { + ble_log_lbm_recycle_trans(trans); + return; } + /* Queue depth == total transport buffer count, so a timeout-0 send cannot + * fail for a valid transport; recycling on failure keeps submitters from + * blocking while they hold a lifetime reference. */ + BaseType_t queued = BLE_LOG_IN_ISR() + ? xQueueSendFromISR(rt_queue_handle, &trans, NULL) + : xQueueSend(rt_queue_handle, &trans, 0); + if (queued != pdTRUE) { + BLE_LOG_REF_COUNT_RELEASE(&rt_ref_count); + ble_log_lbm_recycle_trans(trans); + return; + } + BLE_LOG_REF_COUNT_RELEASE(&rt_ref_count); } #if CONFIG_BLE_LOG_TS_ENABLED bool ble_log_sync_enable(bool enable) { - if (!rt_inited) { + if (!BLE_LOG_ATOMIC_LOAD_ACQUIRE(rt_inited)) { return false; } - rt_ts_enabled = enable; + BLE_LOG_ATOMIC_STORE_RELEASE(rt_ts_enabled, enable); ble_log_ts_reset(enable); return true; } diff --git a/components/bt/common/ble_log/src/ble_log_util.c b/components/bt/common/ble_log/src/ble_log_util.c index 51034dbdeae..8c84c389aa9 100644 --- a/components/bt/common/ble_log/src/ble_log_util.c +++ b/components/bt/common/ble_log/src/ble_log_util.c @@ -11,6 +11,11 @@ /* INCLUDE */ #include "ble_log_util.h" +#include "freertos/task.h" + +/* MACRO */ +#define BLE_LOG_REF_COUNT_WAIT_TIMEOUT_MS (1000) + /* VARIABLE */ #ifndef UNIT_TEST BLE_LOG_DRAM_ATTR portMUX_TYPE ble_log_spin_lock = portMUX_INITIALIZER_UNLOCKED; @@ -68,3 +73,30 @@ uint32_t ble_log_fast_checksum(const uint8_t *data, size_t len) /* Step 6: Rotate the final result */ return ror32(checksum, start_offset_shift); } + +BLE_LOG_IRAM_ATTR +bool ble_log_ref_count_try_acquire(volatile uint32_t *ref_count, + const uint32_t *inited) +{ + /* The seq_cst increment/check pairs with deinit's seq_cst gate close + * before it waits for the reference count. */ + BLE_LOG_REF_COUNT_ACQUIRE_SEQ_CST(ref_count); + if (BLE_LOG_ATOMIC_LOAD_SEQ_CST(*inited)) { + return true; + } + BLE_LOG_REF_COUNT_RELEASE(ref_count); + return false; +} + +bool ble_log_ref_count_wait(volatile uint32_t *ref_count, uint32_t max_ref_count) +{ + TickType_t start_tick = xTaskGetTickCount(); + while (BLE_LOG_ATOMIC_LOAD_SEQ_CST(*ref_count) > max_ref_count) { + if ((xTaskGetTickCount() - start_tick) >= + pdMS_TO_TICKS(BLE_LOG_REF_COUNT_WAIT_TIMEOUT_MS)) { + return false; + } + vTaskDelay(1); + } + return true; +} diff --git a/components/bt/common/ble_log/src/internal_include/ble_log_lbm.h b/components/bt/common/ble_log/src/internal_include/ble_log_lbm.h index 9a2778281ae..b9f2b9e32d9 100644 --- a/components/bt/common/ble_log/src/internal_include/ble_log_lbm.h +++ b/components/bt/common/ble_log/src/internal_include/ble_log_lbm.h @@ -255,8 +255,8 @@ _Static_assert(BLE_LOG_TRANS_BUF_CNT <= 255, /* Internal Interfaces */ /* --------------------------- */ bool ble_log_lbm_init(void); +void ble_log_lbm_close(void); void ble_log_lbm_deinit(void); -void ble_log_lbm_enable(bool enable); void ble_log_lbm_recycle_trans(ble_log_prph_trans_t *trans); bool ble_log_write_internal(const uint8_t *addr, size_t len); void ble_log_write_final_stat(void); diff --git a/components/bt/common/ble_log/src/internal_include/ble_log_util.h b/components/bt/common/ble_log/src/internal_include/ble_log_util.h index 3c0a2712410..274ea1adbd9 100644 --- a/components/bt/common/ble_log/src/internal_include/ble_log_util.h +++ b/components/bt/common/ble_log/src/internal_include/ble_log_util.h @@ -36,6 +36,12 @@ /* Reference counting macros */ #define BLE_LOG_REF_COUNT_ACQUIRE(VAR) __atomic_fetch_add(VAR, 1, __ATOMIC_ACQUIRE) #define BLE_LOG_REF_COUNT_RELEASE(VAR) __atomic_fetch_sub(VAR, 1, __ATOMIC_RELEASE) +/* Closing gate: pairs an inited-flag store with a reference-count load (and + * vice versa) at seq_cst so deinit and a submitter cannot both observe the + * pre-transition values on SMP (store-buffer / Dekker pattern). */ +#define BLE_LOG_ATOMIC_LOAD_SEQ_CST(VAR) __atomic_load_n(&(VAR), __ATOMIC_SEQ_CST) +#define BLE_LOG_ATOMIC_STORE_SEQ_CST(VAR, VALUE) __atomic_store_n(&(VAR), (VALUE), __ATOMIC_SEQ_CST) +#define BLE_LOG_REF_COUNT_ACQUIRE_SEQ_CST(VAR) __atomic_fetch_add(VAR, 1, __ATOMIC_SEQ_CST) /* Specifier */ #define BLE_LOG_STATIC static @@ -95,6 +101,9 @@ void ble_log_cas_release(volatile bool *cas_lock) /* Reference counting macros */ #define BLE_LOG_REF_COUNT_ACQUIRE(VAR) (*VAR)++ #define BLE_LOG_REF_COUNT_RELEASE(VAR) (*VAR)-- +#define BLE_LOG_ATOMIC_LOAD_SEQ_CST(VAR) BLE_LOG_ATOMIC_LOAD_ACQUIRE(VAR) +#define BLE_LOG_ATOMIC_STORE_SEQ_CST(VAR, VALUE) BLE_LOG_ATOMIC_STORE_RELAXED(VAR, VALUE) +#define BLE_LOG_REF_COUNT_ACQUIRE_SEQ_CST(VAR) BLE_LOG_REF_COUNT_ACQUIRE(VAR) /* Specifier*/ #define BLE_LOG_STATIC @@ -167,4 +176,11 @@ typedef struct { /* INTERFACE */ uint32_t ble_log_fast_checksum(const uint8_t *data, size_t len); +/* Acquire a lifetime reference only while the closing gate remains open. */ +bool ble_log_ref_count_try_acquire(volatile uint32_t *ref_count, + const uint32_t *inited); + +/* Task-context wait; returns false if the count stays above max for one second. */ +bool ble_log_ref_count_wait(volatile uint32_t *ref_count, uint32_t max_ref_count); + #endif /* __BLE_LOG_UTIL_H__ */