diff --git a/components/zkernel/README.md b/components/zkernel/README.md index fc8142d..b99900d 100644 --- a/components/zkernel/README.md +++ b/components/zkernel/README.md @@ -146,7 +146,14 @@ k_event_wait_all(&evt, 0x07, false, K_MSEC(100)); // wait for ALL k_event_clear(&evt, BIT(0)); // ISR-safe ``` -**BEHAVIORAL DELTA:** FreeRTOS reserves bits 24-31 for internal use. Only bits 0-23 are available (vs Zephyr's 32 bits). +**Notification-backed** (no FreeRTOS event group): the full 32-bit events +word is available (the old event-group backend reserved bits 24-31). +Upstream semantics throughout: `set` replaces (use `post` to merge), +mutators return the previous value, `reset` zeroes the whole tracked set +before waiting, `wait_all` returns 0 on timeout, and the `_safe` wait +variants atomically consume matched bits. Same blocking architecture and +caveats as `k_sem` (reserved notification index; do not abort a blocked +waiter). ## Timer (`k_timer`) diff --git a/components/zkernel/include/boreas/zephyr/kernel.h b/components/zkernel/include/boreas/zephyr/kernel.h index 6f568ab..b28bbd5 100644 --- a/components/zkernel/include/boreas/zephyr/kernel.h +++ b/components/zkernel/include/boreas/zephyr/kernel.h @@ -285,29 +285,68 @@ uint32_t k_msgq_num_free_get(struct k_msgq *msgq); /* ---------------------------------------------------------------- * Event - * - * BEHAVIORAL DELTA: FreeRTOS EventGroup reserves bits 24-31 for - * internal use. Only bits 0-23 (24 bits) are available, vs - * Zephyr's full 32 bits. * ---------------------------------------------------------------- */ +/* Notification-backed (no FreeRTOS event group): the full 32-bit + * events word and the waiter list live here, guarded by the lock -- + * the old event-group backend capped usable events at 24 bits. See + * the README design principle: when a wait returns, the kernel holds + * no references into this struct. */ struct k_event { - EventGroupHandle_t handle; - StaticEventGroup_t buffer; + uint32_t events; + sys_dlist_t waiters; /* of z_event_waiter, caller-stack resident */ + portMUX_TYPE lock; }; -#define K_EVENT_DEFINE(name) struct k_event name = {0} +/** Statically define a fully-initialized event object (compile-time + * initializer, matching upstream). */ +#define K_EVENT_DEFINE(name) \ + struct k_event name = { \ + .events = 0, \ + .waiters = SYS_DLIST_STATIC_INIT(&name.waiters), \ + .lock = portMUX_INITIALIZER_UNLOCKED, \ + } -int k_event_init(struct k_event *event); +void k_event_init(struct k_event *event); +/** Merge (OR) @p events into the tracked set; all waiters whose + * conditions become met wake. @return the previous value of the + * posted events bits. ISR-safe. */ uint32_t k_event_post(struct k_event *event, uint32_t events); -/** @note In ISR context, returns 0 on failure (timer command queue full) - * rather than the previous event state. FreeRTOS's - * xEventGroupSetBitsFromISR defers to the timer daemon and cannot - * report the prior bits synchronously. */ +/** REPLACE the tracked set with @p events (upstream semantics -- + * setting differs from posting). @return the previous events value. + * ISR-safe. */ uint32_t k_event_set(struct k_event *event, uint32_t events); +/** Overwrite only the bits selected by @p events_mask. @return the + * previous value of the masked bits. ISR-safe. */ +uint32_t k_event_set_masked(struct k_event *event, uint32_t events, uint32_t events_mask); +/** Clear @p events from the tracked set. @return their previous + * value. ISR-safe. */ uint32_t k_event_clear(struct k_event *event, uint32_t events); +/** + * Wait for ANY of @p events. @p reset zeroes the ENTIRE tracked set + * before waiting (upstream semantics). @return the matching events + * (left set -- use the _safe variant to consume them), or 0 on + * timeout. + * + * @note Like k_sem_take: blocking is forbidden in ISRs (K_NO_WAIT is + * fine), and a thread blocked here must not be aborted. + */ uint32_t k_event_wait(struct k_event *event, uint32_t events, bool reset, k_timeout_t timeout); +/** As k_event_wait, but requires ALL of @p events. */ uint32_t k_event_wait_all(struct k_event *event, uint32_t events, bool reset, k_timeout_t timeout); +/** As k_event_wait, but atomically CLEARS the matched events before + * returning (upstream parity). */ +uint32_t k_event_wait_safe(struct k_event *event, uint32_t events, bool reset, k_timeout_t timeout); +/** As k_event_wait_all, but atomically clears the matched events. */ +uint32_t k_event_wait_all_safe(struct k_event *event, uint32_t events, bool reset, + k_timeout_t timeout); + +/** Non-blocking poll: the currently-set events matching + * @p events_mask (not cleared). ISR-safe. */ +static inline uint32_t k_event_test(struct k_event *event, uint32_t events_mask) +{ + return k_event_wait(event, events_mask, false, K_NO_WAIT); +} /* ---------------------------------------------------------------- * Timer (over esp_timer) diff --git a/components/zkernel/src/k_event.c b/components/zkernel/src/k_event.c index e4b69fb..acc3706 100644 --- a/components/zkernel/src/k_event.c +++ b/components/zkernel/src/k_event.c @@ -1,67 +1,267 @@ /* * SPDX-License-Identifier: Apache-2.0 * Copyright 2026 Intercreate + * + * Notification-backed k_event (same architecture as k_sem): the events + * word and waiter list live in the caller-owned struct under its lock; + * blocking rides direct-to-task notifications on the reserved index. + * Nothing of the caller's memory ever enters a kernel list (the + * object-lifetime principle -- zkernel README, #40). Replaces the + * FreeRTOS event-group backend, which capped usable events at 24 bits + * (EventBits_t reserves the top byte) and could not express upstream's + * set-replaces / reset-before-wait / 0-on-timeout semantics. */ #include "zephyr/kernel.h" -#include - #include "esp_attr.h" -#include "esp_log.h" #include "sdkconfig.h" -static const char *TAG = "k_event"; +#include "zkernel_internal.h" + +struct z_event_waiter { + sys_dnode_t node; + TaskHandle_t task; + uint32_t mask; /* events the waiter is interested in */ + bool all; /* require all bits vs any bit */ + bool clear; /* _safe variant: consume matched bits on wake */ + uint32_t matched; /* set by the waker under the lock */ + bool woken; /* a post/set targeted this waiter */ +}; -int k_event_init(struct k_event *event) +static uint32_t z_event_match(uint32_t current, uint32_t mask, bool all) { - event->handle = xEventGroupCreateStatic(&event->buffer); - if (event->handle == NULL) { - ESP_LOGE(TAG, "Failed to create event group"); - return -ENOMEM; + uint32_t hit = current & mask; + + if (all && hit != mask) { + return 0; + } + return hit; +} + +/* Apply `events` under `mask`, wake every waiter whose condition + * becomes met (upstream: all satisfied waiters unpend), and return the + * previous value of the masked events. Satisfied waiters are unlinked + * into a local chain in ONE lock pass and notified after unlock. + * + * Safety of touching the stack-resident nodes post-unlock: a chained + * waiter cannot return from its wait until OUR notification lands. + * Within the protocol, exactly one in-flight notification can exist + * per blocked waiter -- a waiter is targeted by at most one waker + * (unlinking under the lock makes targeting exclusive), and every + * return path drains the notification it was woken by (including the + * timeout-race consume path). So when woken==true, the only + * notification that can release the waiter is ours, sent after we + * finish with its node. This premise is the index reservation itself: + * external code notifying Z_KERNEL_NOTIFY_INDEX is documented- + * forbidden misuse (zkernel_internal.h), under which a stale wake + * could release a chained waiter early -- the same premise + * k_sem_give's post-unlock handle use rests on. */ +static uint32_t K_ISR_SAFE z_event_post_internal(struct k_event *event, uint32_t events, + uint32_t mask) +{ + sys_dlist_t woken_chain; + + sys_dlist_init(&woken_chain); + + z_kernel_lock(&event->lock); + + uint32_t previous = event->events & mask; + + event->events = (event->events & ~mask) | (events & mask); + + sys_dnode_t *n = sys_dlist_peek_head(&event->waiters); + uint32_t clear_accum = 0; + + while (n != NULL) { + sys_dnode_t *next = sys_dlist_peek_next(&event->waiters, n); + struct z_event_waiter *w = CONTAINER_OF(n, struct z_event_waiter, node); + uint32_t hit = z_event_match(event->events, w->mask, w->all); + + if (hit != 0) { + w->matched = hit; + w->woken = true; + if (w->clear) { + /* Accumulate; applied AFTER the walk so every + * waiter is evaluated against the same posted + * state (upstream applies clear_events at the + * end of its wait-queue walk -- clearing + * mid-walk would starve overlapping waiters). */ + clear_accum |= hit; + } + sys_dlist_remove(&w->node); + sys_dlist_append(&woken_chain, &w->node); + } + n = next; + } + + event->events &= ~clear_accum; + + z_kernel_unlock(&event->lock); + + bool in_isr = xPortInIsrContext(); + BaseType_t isr_yield = pdFALSE; + + for (n = sys_dlist_get(&woken_chain); n != NULL; n = sys_dlist_get(&woken_chain)) { + struct z_event_waiter *w = CONTAINER_OF(n, struct z_event_waiter, node); + + if (in_isr) { + /* Accumulate across the chain; yield once below. */ + vTaskNotifyGiveIndexedFromISR(w->task, Z_KERNEL_NOTIFY_INDEX, &isr_yield); + } else { + xTaskNotifyGiveIndexed(w->task, Z_KERNEL_NOTIFY_INDEX); + } } - return 0; + + if (in_isr) { + portYIELD_FROM_ISR(isr_yield); + } + + return previous; +} + +void k_event_init(struct k_event *event) +{ + event->events = 0; + sys_dlist_init(&event->waiters); + event->lock = (portMUX_TYPE)portMUX_INITIALIZER_UNLOCKED; } uint32_t K_ISR_SAFE k_event_post(struct k_event *event, uint32_t events) { - return k_event_set(event, events); + return z_event_post_internal(event, events, events); } uint32_t K_ISR_SAFE k_event_set(struct k_event *event, uint32_t events) { - if (xPortInIsrContext()) { - BaseType_t wake = pdFALSE; - BaseType_t ret = - xEventGroupSetBitsFromISR(event->handle, (EventBits_t)events, &wake); - if (wake) { - portYIELD_FROM_ISR(wake); - } - return (ret == pdPASS) ? events : 0; - } - return (uint32_t)xEventGroupSetBits(event->handle, (EventBits_t)events); + return z_event_post_internal(event, events, UINT32_MAX); +} + +uint32_t K_ISR_SAFE k_event_set_masked(struct k_event *event, uint32_t events, uint32_t events_mask) +{ + return z_event_post_internal(event, events, events_mask); } uint32_t K_ISR_SAFE k_event_clear(struct k_event *event, uint32_t events) { - if (xPortInIsrContext()) { - return (uint32_t)xEventGroupClearBitsFromISR(event->handle, (EventBits_t)events); + return z_event_post_internal(event, 0, events); +} + +static uint32_t z_event_wait_internal(struct k_event *event, uint32_t events, bool all, bool reset, + bool clear, k_timeout_t timeout) +{ + /* The waiter node lives on THIS stack frame; it is unlinked under + * the event lock (by a waker or by our timeout path) before we + * return -- synchronous severance by construction. Identity is + * sampled OUTSIDE the lock and only on the must-block path, via + * the unlock-sample-relock-recheck loop (see k_sem_take: keeps + * the fast paths pre-scheduler-safe and FreeRTOS calls out of + * the critical section). */ + struct z_event_waiter w = {0}; + + for (;;) { + z_kernel_lock(&event->lock); + + if (reset) { + /* Upstream: zero the ENTIRE tracked set before + * waiting -- once, not on every recheck pass. */ + event->events = 0; + reset = false; + } + + uint32_t hit = z_event_match(event->events, events, all); + + if (hit != 0) { + if (clear) { + event->events &= ~hit; + } + z_kernel_unlock(&event->lock); + return hit; + } + + if (k_timeout_is_no_wait(timeout)) { + z_kernel_unlock(&event->lock); + return 0; + } + + if (w.task != NULL) { + break; /* sampled on a previous pass; enqueue under THIS lock */ + } + + z_kernel_unlock(&event->lock); + w.task = xTaskGetCurrentTaskHandle(); + w.mask = events; + w.all = all; + w.clear = clear; + } + + sys_dlist_append(&event->waiters, &w.node); + z_kernel_unlock(&event->lock); + + bool forever = k_timeout_is_forever(timeout); + TickType_t total = forever ? 0 : k_timeout_to_ticks(timeout); + TickType_t start = xTaskGetTickCount(); + + for (;;) { + TickType_t wait = portMAX_DELAY; + + if (!forever) { + /* Unsigned tick diff is wraparound-safe. */ + TickType_t elapsed = xTaskGetTickCount() - start; + + wait = (elapsed >= total) ? 0 : (total - elapsed); + } + + uint32_t got = ulTaskNotifyTakeIndexed(Z_KERNEL_NOTIFY_INDEX, pdTRUE, wait); + + z_kernel_lock(&event->lock); + if (w.woken) { + /* A waker unlinked us. If our wait timed out in the + * same instant (got == 0), its notification is still + * in flight -- consume it before returning so it + * cannot poison a later blocking call on this task. */ + uint32_t matched = w.matched; + + z_kernel_unlock(&event->lock); + if (got == 0) { + (void)ulTaskNotifyTakeIndexed(Z_KERNEL_NOTIFY_INDEX, pdTRUE, + portMAX_DELAY); + } + return matched; + } + + if (got == 0 && !forever) { + /* Timed out, not targeted: unlink ourselves. After + * this unlock no waker can see the node. */ + sys_dlist_remove(&w.node); + z_kernel_unlock(&event->lock); + return 0; + } + + /* Spurious wake (stale notification from code misusing the + * reserved index): loop; the wait recompute above absorbs + * the elapsed time. */ + z_kernel_unlock(&event->lock); } - return (uint32_t)xEventGroupClearBits(event->handle, (EventBits_t)events); } uint32_t k_event_wait(struct k_event *event, uint32_t events, bool reset, k_timeout_t timeout) { - EventBits_t bits = xEventGroupWaitBits(event->handle, (EventBits_t)events, - reset ? pdTRUE : pdFALSE, pdFALSE, /* wait for ANY */ - k_timeout_to_ticks(timeout)); - return (uint32_t)(bits & events); + return z_event_wait_internal(event, events, false, reset, false, timeout); } uint32_t k_event_wait_all(struct k_event *event, uint32_t events, bool reset, k_timeout_t timeout) { - EventBits_t bits = xEventGroupWaitBits(event->handle, (EventBits_t)events, - reset ? pdTRUE : pdFALSE, pdTRUE, /* wait for ALL */ - k_timeout_to_ticks(timeout)); - return (uint32_t)(bits & events); + return z_event_wait_internal(event, events, true, reset, false, timeout); +} + +uint32_t k_event_wait_safe(struct k_event *event, uint32_t events, bool reset, k_timeout_t timeout) +{ + return z_event_wait_internal(event, events, false, reset, true, timeout); +} + +uint32_t k_event_wait_all_safe(struct k_event *event, uint32_t events, bool reset, + k_timeout_t timeout) +{ + return z_event_wait_internal(event, events, true, reset, true, timeout); } diff --git a/components/zkernel/src/k_sem.c b/components/zkernel/src/k_sem.c index d65003a..784118d 100644 --- a/components/zkernel/src/k_sem.c +++ b/components/zkernel/src/k_sem.c @@ -21,13 +21,6 @@ #include "zkernel_internal.h" -#if configTASK_NOTIFICATION_ARRAY_ENTRIES < 2 -#error "Boreas zkernel reserves task-notification index 1 for blocking primitives (index 0 stays free for ESP-IDF internals such as esp_ipc/pthread/eth). Set CONFIG_FREERTOS_TASK_NOTIFICATION_ARRAY_ENTRIES=2 (or higher) in sdkconfig." -#endif - -/* Reserved for zkernel: index 0 is used by ESP-IDF internals. */ -#define Z_SEM_NOTIFY_INDEX 1 - struct z_sem_waiter { sys_dnode_t node; TaskHandle_t task; @@ -127,7 +120,7 @@ int k_sem_take(struct k_sem *sem, k_timeout_t timeout) wait = (elapsed >= total) ? 0 : (total - elapsed); } - uint32_t got = ulTaskNotifyTakeIndexed(Z_SEM_NOTIFY_INDEX, pdTRUE, wait); + uint32_t got = ulTaskNotifyTakeIndexed(Z_KERNEL_NOTIFY_INDEX, pdTRUE, wait); z_kernel_lock(&sem->lock); if (w.woken) { @@ -141,7 +134,7 @@ int k_sem_take(struct k_sem *sem, k_timeout_t timeout) z_kernel_unlock(&sem->lock); if (got == 0) { - (void)ulTaskNotifyTakeIndexed(Z_SEM_NOTIFY_INDEX, pdTRUE, + (void)ulTaskNotifyTakeIndexed(Z_KERNEL_NOTIFY_INDEX, pdTRUE, portMAX_DELAY); } return reset ? -EAGAIN : 0; @@ -185,10 +178,10 @@ void K_ISR_SAFE k_sem_give(struct k_sem *sem) if (xPortInIsrContext()) { BaseType_t woken = pdFALSE; - vTaskNotifyGiveIndexedFromISR(to_wake, Z_SEM_NOTIFY_INDEX, &woken); + vTaskNotifyGiveIndexedFromISR(to_wake, Z_KERNEL_NOTIFY_INDEX, &woken); portYIELD_FROM_ISR(woken); } else { - xTaskNotifyGiveIndexed(to_wake, Z_SEM_NOTIFY_INDEX); + xTaskNotifyGiveIndexed(to_wake, Z_KERNEL_NOTIFY_INDEX); } } } @@ -215,7 +208,7 @@ void k_sem_reset(struct k_sem *sem) if (to_wake == NULL) { return; } - xTaskNotifyGiveIndexed(to_wake, Z_SEM_NOTIFY_INDEX); + xTaskNotifyGiveIndexed(to_wake, Z_KERNEL_NOTIFY_INDEX); } } diff --git a/components/zkernel/src/zkernel_internal.h b/components/zkernel/src/zkernel_internal.h index ea3c9d6..0017ddf 100644 --- a/components/zkernel/src/zkernel_internal.h +++ b/components/zkernel/src/zkernel_internal.h @@ -13,6 +13,17 @@ #include "esp_attr.h" +#if configTASK_NOTIFICATION_ARRAY_ENTRIES < 2 +#error "Boreas zkernel reserves task-notification index 1 for blocking primitives (index 0 stays free for ESP-IDF internals such as esp_ipc/pthread/eth). Add sdkconfig.boreas to SDKCONFIG_DEFAULTS (see the README) or set CONFIG_FREERTOS_TASK_NOTIFICATION_ARRAY_ENTRIES=2 in sdkconfig." +#endif + +/* Reserved for zkernel blocking primitives (k_sem, k_event): index 0 + * is used by ESP-IDF internals. Sharing one index across primitives is + * safe -- a task blocks on at most one primitive at a time, and every + * blocking call drains in-flight notifications before returning (the + * consume-the-in-flight-give protocol). */ +#define Z_KERNEL_NOTIFY_INDEX 1 + /* ISR-aware critical section: ESP-IDF needs different macros for ISR * vs task context. Inlined to avoid runtime overhead in the common * (task) case. */ diff --git a/test/main/test_k_event.c b/test/main/test_k_event.c index 7c1b632..3e582ca 100644 --- a/test/main/test_k_event.c +++ b/test/main/test_k_event.c @@ -62,11 +62,12 @@ static void test_event_post(void) struct k_event evt; k_event_init(&evt); - /* post is an alias for set */ + /* post MERGES (set replaces -- upstream semantics) */ + k_event_set(&evt, EVT_B); k_event_post(&evt, EVT_A | EVT_C); - uint32_t got = k_event_wait(&evt, EVT_C, false, K_NO_WAIT); - TEST_ASSERT_EQUAL(EVT_C, got & EVT_C); + uint32_t got = k_event_wait_all(&evt, EVT_A | EVT_B | EVT_C, false, K_NO_WAIT); + TEST_ASSERT_EQUAL(EVT_A | EVT_B | EVT_C, got); /* B survived the post */ } static void test_event_wait_with_reset(void) @@ -76,16 +77,15 @@ static void test_event_wait_with_reset(void) k_event_set(&evt, EVT_A | EVT_B); - /* Wait with reset=true should clear the matched bits */ + /* Upstream semantics: reset=true zeroes the ENTIRE tracked set + * BEFORE waiting -- so a K_NO_WAIT wait after reset sees nothing, + * and EVT_B is gone too. (The old FreeRTOS backend cleared only + * the matched bits, after the wait.) */ uint32_t got = k_event_wait(&evt, EVT_A, true, K_NO_WAIT); - TEST_ASSERT_EQUAL(EVT_A, got & EVT_A); - - /* EVT_A should be cleared now, EVT_B still set */ - got = k_event_wait(&evt, EVT_A, false, K_NO_WAIT); - TEST_ASSERT_EQUAL(0, got & EVT_A); + TEST_ASSERT_EQUAL(0, got); - got = k_event_wait(&evt, EVT_B, false, K_NO_WAIT); - TEST_ASSERT_EQUAL(EVT_B, got & EVT_B); + got = k_event_wait(&evt, EVT_A | EVT_B, false, K_NO_WAIT); + TEST_ASSERT_EQUAL(0, got); } static void test_event_wait_timeout(void) @@ -111,17 +111,95 @@ static void test_event_set_clear_all(void) TEST_ASSERT_EQUAL(0, got); } -static void test_event_set_overlapping(void) +static void test_event_set_replaces(void) { struct k_event evt; k_event_init(&evt); - /* Set A, then set A|B -- A should still be set */ + /* Upstream: set REPLACES the tracked set (post merges). */ k_event_set(&evt, EVT_A); + k_event_set(&evt, EVT_B); + + TEST_ASSERT_EQUAL(0, k_event_test(&evt, EVT_A)); /* A replaced away */ + TEST_ASSERT_EQUAL(EVT_B, k_event_test(&evt, EVT_B)); + + k_event_post(&evt, EVT_A); /* merge keeps B */ + TEST_ASSERT_EQUAL(EVT_A | EVT_B, k_event_test(&evt, EVT_A | EVT_B)); +} + +static void test_event_previous_value_returns(void) +{ + struct k_event evt; + k_event_init(&evt); + + /* All mutators return the PREVIOUS value of the affected bits. */ + TEST_ASSERT_EQUAL(0, k_event_set(&evt, EVT_A | EVT_B)); + TEST_ASSERT_EQUAL(EVT_A, k_event_post(&evt, EVT_A | EVT_C)); + TEST_ASSERT_EQUAL(EVT_B, k_event_clear(&evt, EVT_B)); + TEST_ASSERT_EQUAL(EVT_A | EVT_C, k_event_set(&evt, 0)); +} + +static void test_event_set_masked(void) +{ + struct k_event evt; + k_event_init(&evt); + k_event_set(&evt, EVT_A | EVT_B); - uint32_t got = k_event_wait_all(&evt, EVT_A | EVT_B, false, K_NO_WAIT); - TEST_ASSERT_EQUAL(EVT_A | EVT_B, got); + /* Overwrite only the B|C lanes: B clears, C sets, A untouched. */ + TEST_ASSERT_EQUAL(EVT_B, k_event_set_masked(&evt, EVT_C, EVT_B | EVT_C)); + TEST_ASSERT_EQUAL(EVT_A | EVT_C, k_event_test(&evt, EVT_A | EVT_B | EVT_C)); +} + +static void test_event_wait_safe_consumes(void) +{ + struct k_event evt; + k_event_init(&evt); + + k_event_set(&evt, EVT_A | EVT_B); + + /* _safe consumes the matched bits atomically; others survive. */ + TEST_ASSERT_EQUAL(EVT_A, k_event_wait_safe(&evt, EVT_A, false, K_NO_WAIT)); + TEST_ASSERT_EQUAL(0, k_event_test(&evt, EVT_A)); + TEST_ASSERT_EQUAL(EVT_B, k_event_test(&evt, EVT_B)); +} + +static void test_event_wait_all_timeout_returns_zero(void) +{ + struct k_event evt; + k_event_init(&evt); + + k_event_set(&evt, EVT_A); /* partial match only */ + + /* Upstream: timeout returns 0 -- NOT the partial bits (the old + * event-group backend leaked a truthy partial mask here). */ + TEST_ASSERT_EQUAL(0, k_event_wait_all(&evt, EVT_A | EVT_B, false, K_MSEC(30))); + TEST_ASSERT_EQUAL(EVT_A, k_event_test(&evt, EVT_A)); /* untouched */ +} + +static void test_event_full_32_bits(void) +{ + struct k_event evt; + k_event_init(&evt); + + /* The FreeRTOS event-group backend reserved bits 24-31; the + * notification-backed implementation tracks all 32. */ + uint32_t high = BIT(24) | BIT(31); + + TEST_ASSERT_EQUAL(0, k_event_set(&evt, high)); + TEST_ASSERT_EQUAL(high, k_event_wait_all(&evt, high, false, K_NO_WAIT)); + TEST_ASSERT_EQUAL(high, k_event_clear(&evt, high)); + TEST_ASSERT_EQUAL(0, k_event_test(&evt, UINT32_MAX)); +} + +K_EVENT_DEFINE(static_evt); + +static void test_event_define_static_init(void) +{ + /* Compile-time initializer: usable without k_event_init. */ + TEST_ASSERT_EQUAL(0, k_event_test(&static_evt, UINT32_MAX)); + k_event_post(&static_evt, EVT_C); + TEST_ASSERT_EQUAL(EVT_C, k_event_test(&static_evt, EVT_C)); } void test_k_event_group(void) @@ -134,5 +212,11 @@ void test_k_event_group(void) RUN_TEST(test_event_wait_with_reset); RUN_TEST(test_event_wait_timeout); RUN_TEST(test_event_set_clear_all); - RUN_TEST(test_event_set_overlapping); + RUN_TEST(test_event_set_replaces); + RUN_TEST(test_event_previous_value_returns); + RUN_TEST(test_event_set_masked); + RUN_TEST(test_event_wait_safe_consumes); + RUN_TEST(test_event_wait_all_timeout_returns_zero); + RUN_TEST(test_event_full_32_bits); + RUN_TEST(test_event_define_static_init); } diff --git a/test/main/test_k_event_mt.c b/test/main/test_k_event_mt.c index 1170616..23f69ee 100644 --- a/test/main/test_k_event_mt.c +++ b/test/main/test_k_event_mt.c @@ -61,10 +61,10 @@ static void event_multi_setter_entry(void *p1, void *p2, void *p3) struct k_event *evt = (struct k_event *)p1; k_msleep(30); - k_event_set(evt, EVT_DONE); + k_event_post(evt, EVT_DONE); /* post MERGES; set would replace */ k_msleep(30); - k_event_set(evt, EVT_GO); + k_event_post(evt, EVT_GO); } static void test_event_wait_all_from_thread(void) @@ -83,10 +83,12 @@ static void test_event_wait_all_from_thread(void) } /* ---------------------------------------------------------------- - * Wait with reset -- verify bits are cleared after wake + * Blocking wait_safe -- matched bits are consumed on wake + * (upstream parity: plain wait leaves them set; reset=true zeroes the + * whole tracked set BEFORE waiting, it does not clear after) * ---------------------------------------------------------------- */ -static void test_event_wait_reset_from_thread(void) +static void test_event_wait_safe_from_thread(void) { k_event_init(&mt_evt); memset(&setter_thread, 0, sizeof(setter_thread)); @@ -94,20 +96,78 @@ static void test_event_wait_reset_from_thread(void) k_thread_create(&setter_thread, setter_stack, sizeof(setter_stack), event_setter_entry, (void *)(uintptr_t)EVT_DONE, NULL, NULL, 5, 0, K_NO_WAIT); - /* Wait with reset=true */ - uint32_t got = k_event_wait(&mt_evt, EVT_DONE, true, K_MSEC(500)); - TEST_ASSERT_EQUAL(EVT_DONE, got & EVT_DONE); + /* Block until the setter posts, consuming the matched bits. */ + uint32_t got = k_event_wait_safe(&mt_evt, EVT_DONE, false, K_MSEC(500)); + TEST_ASSERT_EQUAL(EVT_DONE, got); - /* EVT_DONE should be cleared now */ - got = k_event_wait(&mt_evt, EVT_DONE, false, K_NO_WAIT); - TEST_ASSERT_EQUAL(0, got); + /* Consumed atomically on wake. */ + TEST_ASSERT_EQUAL(0, k_event_test(&mt_evt, EVT_DONE)); k_thread_abort(&setter_thread); } +/* ---------------------------------------------------------------- + * One post wakes ALL satisfied waiters -- including when an earlier + * _safe waiter consumes the matched bits. Regression for the + * clear-mid-walk starvation found in review: clears must be + * accumulated and applied after the waiter walk (upstream order), or + * the second waiter here never wakes. + * ---------------------------------------------------------------- */ + +K_THREAD_STACK_DEFINE(safe_waiter_stack, 2048); +K_THREAD_STACK_DEFINE(plain_waiter_stack, 2048); +static struct k_thread safe_waiter_thread; +static struct k_thread plain_waiter_thread; +static volatile uint32_t safe_waiter_got; +static volatile uint32_t plain_waiter_got; + +static void safe_waiter_entry(void *p1, void *p2, void *p3) +{ + (void)p2; + (void)p3; + safe_waiter_got = k_event_wait_safe((struct k_event *)p1, EVT_GO, false, K_MSEC(500)); +} + +static void plain_waiter_entry(void *p1, void *p2, void *p3) +{ + (void)p2; + (void)p3; + plain_waiter_got = k_event_wait((struct k_event *)p1, EVT_GO, false, K_MSEC(500)); +} + +static void test_event_post_wakes_all_with_safe_waiter(void) +{ + k_event_init(&mt_evt); + memset(&safe_waiter_thread, 0, sizeof(safe_waiter_thread)); + memset(&plain_waiter_thread, 0, sizeof(plain_waiter_thread)); + safe_waiter_got = 0xdead; + plain_waiter_got = 0xdead; + + /* Safe waiter enqueues FIRST so its consume would starve the + * plain waiter under the buggy ordering. */ + k_thread_create(&safe_waiter_thread, safe_waiter_stack, + K_THREAD_STACK_SIZEOF(safe_waiter_stack), safe_waiter_entry, &mt_evt, NULL, + NULL, 5, 0, K_NO_WAIT); + k_msleep(20); + k_thread_create(&plain_waiter_thread, plain_waiter_stack, + K_THREAD_STACK_SIZEOF(plain_waiter_stack), plain_waiter_entry, &mt_evt, + NULL, NULL, 5, 0, K_NO_WAIT); + k_msleep(20); + + k_event_post(&mt_evt, EVT_GO); /* ONE post must wake BOTH */ + + TEST_ASSERT_EQUAL(0, k_thread_join(&safe_waiter_thread, K_SECONDS(2))); + TEST_ASSERT_EQUAL(0, k_thread_join(&plain_waiter_thread, K_SECONDS(2))); + TEST_ASSERT_EQUAL(EVT_GO, safe_waiter_got); + TEST_ASSERT_EQUAL(EVT_GO, plain_waiter_got); + /* The safe waiter consumed the bits after the walk. */ + TEST_ASSERT_EQUAL(0, k_event_test(&mt_evt, EVT_GO)); +} + void test_k_event_mt_group(void) { RUN_TEST(test_event_wait_from_thread); RUN_TEST(test_event_wait_all_from_thread); - RUN_TEST(test_event_wait_reset_from_thread); + RUN_TEST(test_event_wait_safe_from_thread); + RUN_TEST(test_event_post_wakes_all_with_safe_waiter); }