Skip to content

fix/update BLE log SPI LBM (IDFGH-17969)#18834

Open
lifeiyu-u wants to merge 1 commit into
espressif:masterfrom
lifeiyu-u:fix/ble-log-spi-lbm
Open

fix/update BLE log SPI LBM (IDFGH-17969)#18834
lifeiyu-u wants to merge 1 commit into
espressif:masterfrom
lifeiyu-u:fix/ble-log-spi-lbm

Conversation

@lifeiyu-u

@lifeiyu-u lifeiyu-u commented Jul 13, 2026

Copy link
Copy Markdown

Description

Related

Testing


Checklist

Before submitting a Pull Request, please ensure the following:

  • 🚨 This PR does not introduce breaking changes.
  • All CI checks (GH Actions) pass.
  • Documentation is updated as needed.
  • Tests are updated or added as necessary.
  • Code is well-commented, especially in complex areas.
  • Git history is clean — commits are squashed to the minimum necessary.

Note

High Risk
Large concurrency and lifecycle changes in ISR/task logging, flush/deinit, and sdkconfig tuning; mis-tuning or races could cause dropped logs, stalls, or use-after-free if shutdown ordering regresses.

Overview
Replaces the old multi-LBM design (separate atomic/spin/Link Layer managers and per-LBM transport sizing) with a single global pool of transport buffers shared by task, ISR, and Link Layer writers.

Kconfig drops per-LBM options (auto-flush, atomic LBM counts, common/LL total memory) and adds pool tuning: BLE_LOG_POOL_TRANS_CNT, ISR reserve count, per-buffer size, optional ISR OPEN-scan cap, and task backpressure timeout.

Allocation path uses per-buffer atomic_lock, FREE/OPEN/SENDING lifecycle, and free/open hint bitmaps with round-robin scan. Task writers can block on a counting semaphore (with timeout) when the pool is full; ISR writers use shared buffers first, then a reserved ISR region, and never block. Recycle is centralized in ble_log_lbm_recycle_trans() from runtime drain and UART/SPI/dummy tx-done or send-failure paths.

Flush/deinit wake blocked tasks, wait on both ref count and waiting_task_count, seal all OPEN buffers, then wait for pool inflight to drain; per-LBM utilization reporting collapses to one pool record. UART port-0 redirection stays on a separate ble_log_redir_t (not pool IDs). ble_log_write_final_stat / ble_log_write_internal and the old LBM acquire/release write path are removed in favor of direct pool acquire + ble_log_pool_write_frame.

Reviewed by Cursor Bugbot for commit 77586fb. Bugbot is set up for automated code reviews on this repo. Configure here.

@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using default effort and found 3 potential issues.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 77586fb. Configure here.


size_t payload_len = len + len_append;

ble_log_prph_trans_t *trans = ble_log_pool_acquire(payload_len, is_isr);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LL task writers can block

High Severity

ble_log_write_hex_ll in task context now goes through ble_log_pool_acquire with backpressure enabled, so the Link Layer or HCI task can block on xSemaphoreTake when the pool is empty. Previously those paths used dedicated non-blocking LBMs and dropped on exhaustion. Blocking the controller task under log pressure can stall BLE scheduling.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 77586fb. Configure here.

ble_log_prph_trans_t *trans = g_pool.trans[id];
while (!ble_log_cas_acquire(&trans->atomic_lock)) {
/* A writer holds the lock only for a single-frame memcpy. */
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Flush lock spin can hang

High Severity

ble_log_flush busy-waits on ble_log_cas_acquire with no yield or delay. If a lower-priority writer holds atomic_lock and flush runs at higher priority, the writer never resumes, the spin never exits, and the system hangs until the watchdog.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 77586fb. Configure here.

goto failed;
else if (src_code == BLE_LOG_SRC_LL_HCI) {
if (len_append > CONFIG_BLE_LOG_LL_HCI_LOG_PAYLOAD_LEN_LIMIT) {
len_append = CONFIG_BLE_LOG_LL_HCI_LOG_PAYLOAD_LEN_LIMIT;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

HCI upstream limit skipped

Medium Severity

When BLE_LOG_LL_HCI_LOG_PAYLOAD_LEN_LIMIT_ENABLED is set, truncation only runs for BLE_LOG_SRC_LL_HCI. Upstream HCI logs (BLE_LOG_SRC_HCI / BLE_LOG_LL_FLAG_HCI_UPSTREAM) no longer get the limit, so large len_append values can exceed the intended cap or fail to fit a transport.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 77586fb. Configure here.

@github-actions github-actions Bot changed the title fix/update BLE log SPI LBM fix/update BLE log SPI LBM (IDFGH-17969) Jul 13, 2026
@espressif-bot espressif-bot added the Status: Opened Issue is new label Jul 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Status: Opened Issue is new

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants