feat!: notification-backed k_event (full 32 bits, upstream semantics)#42
Merged
Conversation
Second primitive converted under #40, on the architecture proven by the k_sem conversion: events word and waiter list in the caller-owned struct under a portMUX; blocking on the shared reserved notification index; stack-resident waiter nodes severed under the lock before any return; the consume-the-in-flight-give protocol for the timeout race. A post wakes ALL waiters whose conditions become met: satisfied nodes are unlinked into a local chain in one lock pass and notified after unlock (safe -- woken waiters block in the consume path until the notification lands). This retires the FreeRTOS event-group backend and with it three parity gaps plus a hard ceiling: - Full 32-bit events (EventBits_t reserved the top byte: 24 usable). - k_event_set now REPLACES the tracked set (upstream: setting differs from posting); previously it merged, aliasing k_event_post. - wait/wait_all reset=true zeroes the ENTIRE tracked set BEFORE waiting (previously: cleared only the matched bits, after). - wait_all on timeout returns 0 (previously could return a truthy partial match). New upstream surface: k_event_set_masked, k_event_test, k_event_wait_safe / k_event_wait_all_safe (atomically consume matched bits), all mutators return the previous value of the affected bits, K_EVENT_DEFINE is a true compile-time initializer, and k_event_init returns void (upstream signature; it can no longer fail). The notification index and its config requirement move to zkernel_internal.h (Z_KERNEL_NOTIFY_INDEX), shared by k_sem and k_event -- safe because a task blocks on at most one primitive at a time and every blocking call drains in-flight notifications before returning. BREAKING: k_event_set merge->replace (use k_event_post to accumulate); reset=true semantics as above; k_event_init signature. In-tree tests updated intentionally; the MT multi-setter now uses k_event_post and the consume-on-wake test uses k_event_wait_safe. Tests: 190 -> 196 (set-replaces vs post-merges, previous-value returns, set_masked, wait_safe consumption, wait_all timeout-zero, full-32-bit bits 24/31, K_EVENT_DEFINE static init). Linux suite green x3. Part of #40 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR replaces the FreeRTOS EventGroup-backed k_event implementation with a notification-backed design that owns the event state (32-bit word + waiter list) in the caller’s struct k_event, aligning Boreas behavior with upstream Zephyr semantics and removing the old 24-bit ceiling.
Changes:
- Reimplements
k_eventon top of a caller-owned event word + waiter list guarded by a portMUX lock, using the reserved task-notification index (Z_KERNEL_NOTIFY_INDEX) for blocking/wakeup. - Updates/expands the public API and semantics (
setreplaces vspostmerges,resetclears before waiting,_safewait variants that atomically consume matched bits,wait_alltimeout returns 0, mutators return previous values). - Updates unit and MT tests plus README/kernel.h docs to reflect new semantics and adds regression coverage.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
components/zkernel/src/k_event.c |
New notification-backed k_event implementation (state owned by struct k_event, waiter list + wake-all semantics). |
components/zkernel/include/boreas/zephyr/kernel.h |
Updates struct k_event, K_EVENT_DEFINE, and public API declarations/docs (new _safe waits, set_masked, test, k_event_init signature). |
components/zkernel/src/zkernel_internal.h |
Centralizes the reserved notification index and enforces configTASK_NOTIFICATION_ARRAY_ENTRIES >= 2. |
components/zkernel/src/k_sem.c |
Switches to the shared Z_KERNEL_NOTIFY_INDEX (removes per-file index define). |
test/main/test_k_event.c |
Updates/adds single-thread tests for new semantics (replace vs merge, masked set, previous-value returns, _safe consumption, 32-bit coverage, static define). |
test/main/test_k_event_mt.c |
Updates MT behavior to use post, adds blocking _safe test and a wake-all regression test. |
components/zkernel/README.md |
Documents the notification-backed behavior and upstream semantics deltas. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- The post-unlock chain walk's safety premise is now stated precisely at the source: within the protocol exactly one in-flight notification can exist per blocked waiter (targeting is exclusive under the lock; every return path drains what woke it), so a chained waiter cannot be released before our notify -- the premise IS the notification-index reservation, the same one k_sem_give's post-unlock handle use rests on. - ISR-context posts accumulate the higher-priority-woken flag across the wake chain and yield once after the loop instead of per waiter. Co-Authored-By: Claude Opus 4.8 (1M context) <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
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.
Summary
Part of #40 — the second (and per the design verdict, final planned) primitive converted to the own-the-synchronization-state architecture proven by the k_sem conversion (#41): events word + waiter list in the caller-owned struct under a portMUX, blocking on the shared reserved notification index (now
Z_KERNEL_NOTIFY_INDEXinzkernel_internal.h), stack-resident waiter nodes severed under the lock before any return. A post wakes all waiters whose conditions become met — satisfied nodes are unlinked into a local chain in one lock pass and notified after unlock.This retires the FreeRTOS event-group backend, and with it a hard ceiling plus three parity gaps:
EventBits_treserves the top byte)k_event_setpost)reset=truewait_alltimeoutNew upstream surface:
k_event_set_masked,k_event_test,k_event_wait_safe/k_event_wait_all_safe(atomically consume matched bits), previous-value returns from all mutators, compile-timeK_EVENT_DEFINE,void k_event_init.k_event_setmerge→replace (usek_event_postto accumulate);reset=truesemantics as above;k_event_initreturnsvoid. In-tree tests updated intentionally (the MT multi-setter now usespost; the consume-on-wake test useswait_safe). Same config requirement as #41 (sdkconfig.boreas— no new requirements).Review-caught bug (fixed + regression-tested)
The first implementation cleared
_safewaiters' matched bits mid-walk, starving overlapping waiters that upstream wakes (upstream accumulatesclear_eventsand applies them after its wait-queue walk). Fixed to accumulate-then-apply, with an MT regression test where a_safewaiter enqueued first and a plain waiter must both wake from one post.Tests (190 → 197 linux / 221 S3)
Set-replaces vs post-merges, previous-value returns,
set_masked,wait_safeconsumption,wait_alltimeout-zero, bits 24/31 round-trip (the ceiling lift),K_EVENT_DEFINEstatic init, blockingwait_safefrom a thread, and the wake-all-with-safe-waiter regression.Validation
🤖 Generated with Claude Code