refactor(zkernel): back k_mutex with FreeRTOS recursive mutex#55
Open
swoisz wants to merge 1 commit into
Open
Conversation
FreeRTOS recursive mutexes already provide both re-entrancy and priority inheritance: a blocking xSemaphoreTakeRecursive routes through xQueueSemaphoreTake, which priority-inherits for any mutex-type queue (gated only on uxQueueType == queueQUEUE_IS_MUTEX, set for recursive and non-recursive alike). The prior non-recursive-mutex + manual owner/count implementation was built on the false premise that recursive mutexes lack PI. Collapse k_mutex to a thin wrapper over the recursive primitive and drop the manual re-entrancy state. Public API (init/lock/unlock), error codes, and ISR handling are unchanged. Also removes the CONFIG_ZKERNEL_MUTEX_DEBUG lock-ordering / hold-time instrumentation and the ordered-mutex API, which existed only to feed off that manual state and were not yet justified -- safer to ride the battle-tested FreeRTOS primitive. Verified: linux host build clean, 224 tests pass including all 8 mutex tests and the 3 priority-inheritance tests. BREAKING CHANGE: removes k_mutex_init_ordered, K_MUTEX_DEFINE_ORDERED, CONFIG_ZKERNEL_MUTEX_DEBUG, and CONFIG_ZKERNEL_MUTEX_HOLD_WARNING_MS (Boreas extensions, not Zephyr API; shipped in v0.1.0). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
k_mutexwas implemented as a non-recursive FreeRTOS mutex plus manualowner/countre-entrancy tracking, on the belief that FreeRTOS recursive mutexes lack priority inheritance. That premise is false.Both mutex types route a blocking take through
xQueueSemaphoreTake, which priority-inherits for any mutex-type queue — gated only onuxQueueType == queueQUEUE_IS_MUTEX, set for recursive and non-recursive alike byprvInitialiseMutex. Verified against ESP-IDF's FreeRTOS-Kernel (configUSE_RECURSIVE_MUTEXES=1):xQueueTakeMutexRecursive(queue.c) falls through toxQueueSemaphoreTakewhen it blocksxQueueSemaphoreTakecallsxTaskPriorityInherit(...)So the recursive mutex already gives us re-entrancy + PI in one primitive. The manual layer bought only the debug instrumentation, which isn't yet justified — safer to ride the battle-tested FreeRTOS primitive.
What
k_mutexto a thin wrapper overxSemaphoreCreateRecursiveMutexStatic/TakeRecursive/GiveRecursive. Public API, error codes (-EBUSY/-EAGAIN/-EPERM/-EWOULDBLOCK), and ISR handling unchanged.owner/countstate and the debug/ordered extension (k_mutex_init_ordered,K_MUTEX_DEFINE_ORDERED,CONFIG_ZKERNEL_MUTEX_DEBUG,CONFIG_ZKERNEL_MUTEX_HOLD_WARNING_MS) — no in-repo callers.k_mutex.c,kernel.h,README.md, anddevlog/review/k_mutex.md; document the ISR (-EWOULDBLOCK) and-EPERM-breadth divergences inline.k_mutex.c: 109 → 55 lines.Removes
k_mutex_init_ordered,K_MUTEX_DEFINE_ORDERED,CONFIG_ZKERNEL_MUTEX_DEBUG,CONFIG_ZKERNEL_MUTEX_HOLD_WARNING_MS— Boreas extensions (not Zephyr API) that shipped in v0.1.0. Wants a minor bump (0.2.0), not a patch. The two submodule consumers don't use these.Verification
clang-format --dry-run -Werrorclean on changed files-Werror)224 Tests 0 Failures, including all 8 mutex tests and the 3 PI tests (priority_boost,priority_restore,pi_prevents_inversion)Reviews run
/boreas-review: 1 blocker (stale header banner), fixed in-branch. Zephyr conformance improved./code-review xhigh: zero correctness findings.🤖 Generated with Claude Code