Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

core/sync: add wait queues #21123

Open
wants to merge 19 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions core/include/sched.h
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,7 @@
STATUS_FLAG_BLOCKED_ALL, /**< waiting for all flags in flag_mask */
STATUS_MBOX_BLOCKED, /**< waiting for get/put on mbox */
STATUS_COND_BLOCKED, /**< waiting for a condition variable */
STATUS_WQ_BLOCKED, /**< waiting in a wait queue */
STATUS_RUNNING, /**< currently running */
STATUS_PENDING, /**< waiting to be scheduled to run */
STATUS_NUMOF /**< number of supported thread states */
Expand All @@ -187,7 +188,7 @@
#define STATUS_NOT_FOUND ((thread_status_t)~0) /**< Describes an illegal thread status */
/** @} */
/**
* @def SCHED_PRIO_LEVELS

Check warning on line 191 in core/include/sched.h

View workflow job for this annotation

GitHub Actions / static-tests

Uncrustify proposes the following patch: --- a/core/include/sched.h +++ b/core/include/sched.h @@ -185,7 +185,7 @@ typedef enum { */ #define STATUS_ON_RUNQUEUE STATUS_RUNNING /**< to check if on run queue: `st >= STATUS_ON_RUNQUEUE` */ -#define STATUS_NOT_FOUND ((thread_status_t)~0) /**< Describes an illegal thread status */ +#define STATUS_NOT_FOUND ((thread_status_t) ~0) /**< Describes an illegal thread status */ /** @} */ /** * @def SCHED_PRIO_LEVELS diff --git a/core/include/wait_queue.h b/core/include/wait_queue.h index be2188bc1c..06a1bbcf9b 100644
* @brief The number of thread priority levels
*/
#ifndef SCHED_PRIO_LEVELS
Expand Down
242 changes: 242 additions & 0 deletions core/include/wait_queue.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,242 @@
/*
* Copyright (C) 2025 Mihai Renea <[email protected]>
*
* This file is subject to the terms and conditions of the GNU Lesser
* General Public License v2.1. See the file LICENSE in the top level
* directory for more details.
*/

/**
* @defgroup core_sync_wait_queue Wait Queue
* @ingroup core_sync
* @brief Linux-like wait queue for condition signaling
*
* Wait queues enable lock-free, IRQ-safe condition signaling. This
* implementation is inspired from the Linux Kernel.
*
* Wait queues have similar semantics to condition variables, but don't require
* setting the condition + signaling to be atomic, hence no mutex is needed. In
* turn, one may safely call queue_wake() from an ISR. Note, while cond_signal()
* and cond_broadcast() are safe to call from an ISR context too, doing so will
* very probably cause a race condition elsewhere. Consider the following
* scenario using condition variables:
*
* ```
* static uint64_t measurement;
* mutex_t cond_lock = MUTEX_INIT;
* cond_t cond = COND_INIT;
*
* void measurement_irq(void)
* {
* measurement = measure();
* cond_broadcast(&cond);
* }
*
* void wait_for_critical_value(void)
* {
* mutex_lock(&cond_lock);
* while (atomic_load_u64(&measurement) < THRESHOLD) {
* cond_wait(&cond, cond_lock);
* }
* mutex_unlock(&cond_lock);
* }
* ```
*
* Note, the mutex is there only for the cond_wait() API call, as we're not
* allowed to call mutex_lock() inside the ISR. This alone is a hint that
* something isn't right and indeed, the following sequence of events is
* possible:
* 1. thread sees measurement < THRESHOLD, and is about to call cond_wait()
* 2. ISR fires, sets measurement = THRESHOLD, and signals the condition
* 3. thread calls cond_wait() and goes to sleep, possibly forever
*
* Using a wait queue, we can do this:
*
* ```
* static uint64_t measurement;
* wait_queue_t wq = WAIT_QUEUE_INIT;
*
* void measurement_irq(void)
* {
* measurement = measure();
* queue_wake(&wq);
* }
*
* void wait_for_critical_value(void)
* {
* QUEUE_WAIT(&wq, atomic_load_u64(&measurement) >= THRESHOLD);
* }
* ```
*
* This is free of the race condition above because QUEUE_WAIT() is a macro
* that checks the condition AFTER queueing the current thread to be waken up.
*
* When to use?
*
* QUEUE_WAIT() is a macro and might come with some additional code size cost
* due to inlining. If you're not synchronizing with an ISR and care very much
* about code size then go for condition variables, otherwise there is no
* reason not to use the wait queue.
*
* Can't I just use a mutex?
*
* You can abuse a mutex by locking it in a loop in the thread context and
* unlocking from the ISR context. But this will only work for a single waiter
* and makes improper use of the mutex semantics.
*
* @{
*
* @file wait_queue.h
* @brief Linux-like wait queue for condition signaling
*
* @author Mihai Renea <[email protected]>
*/

#ifndef WAIT_QUEUE_H
#define WAIT_QUEUE_H

#include "sched.h"

#ifdef __cplusplus
extern "C" {
#endif

/**
* @brief Forward declaration for @ref wait_queue_entry_t
*/
typedef struct wait_queue_entry wait_queue_entry_t;

/**
* @cond INTERNAL
* @brief Wait queue entry
*
* Always allocated on the stack. We can't use the linked list node in thread_t
* because while evaluating the condition expression the thread may:
* - queue on some other wait queue
* - block on something else, e.g. a mutex
*/
struct wait_queue_entry {
wait_queue_entry_t *next; /**< linked list head */
thread_t *thread; /**< thread blocking on the queue */
};
/**
* @endcond

Check warning on line 123 in core/include/wait_queue.h

View workflow job for this annotation

GitHub Actions / static-tests

Uncrustify proposes the following patch: --- a/core/include/wait_queue.h +++ b/core/include/wait_queue.h @@ -116,8 +116,8 @@ typedef struct wait_queue_entry wait_queue_entry_t; * - block on something else, e.g. a mutex */ struct wait_queue_entry { - wait_queue_entry_t *next; /**< linked list head */ - thread_t *thread; /**< thread blocking on the queue */ + wait_queue_entry_t *next; /**< linked list head */ + thread_t *thread; /**< thread blocking on the queue */ }; /** * @endcond diff --git a/core/thread.c b/core/thread.c index 0c7387b926..860f7c7cee 100644
*/

/**
* @brief Wait queue struct
*/
typedef struct {
wait_queue_entry_t *list; /**< next thread, has lower or equal priority */
} wait_queue_t;

/**
* @cond INTERNAL
* @brief List terminator for the wait queue
*/
#define WAIT_QUEUE_TAIL ((void *)(-1))
/**
* @endcond
*/

/**
* @brief Init a wait queue
*/
#define WAIT_QUEUE_INIT { .list = WAIT_QUEUE_TAIL }

/**
* @brief Enable @ref QUEUE_WAIT() early exit optimization if the condition
* evaluates true.
*
* This optimization is turned off to facilitate the testing of some edge cases.
* There's no good reason to disable this other than to save a few bytes.
*/
#ifndef CONFIG_QUEUE_WAIT_EARLY_EXIT
# define CONFIG_QUEUE_WAIT_EARLY_EXIT 1
#endif

/**
* @cond INTERNAL
*/
#if CONFIG_QUEUE_WAIT_EARLY_EXIT
# define BREAK_IF_TRUE(cond) \
if (cond) { \
break; \
}
#else
# define BREAK_IF_TRUE(cond) (void)(0)
#endif

/* For internal use within the @ref QUEUE_WAIT() macro only. Not the cleanest
* decomposition, but we want to keep the macro tight. */
void _prepare_to_wait(wait_queue_t *wq, wait_queue_entry_t *entry);
void _maybe_yield_and_enqueue(wait_queue_t *wq, wait_queue_entry_t *entry);
void _wait_dequeue(wait_queue_t *wq, wait_queue_entry_t *entry);

/* For internal use only in queue_wake() and queue_wake_exclusive(). */
void _queue_wake_common(wait_queue_t *wq, bool all);
/**
* @endcond
*/

/**
* @brief Wait for a condition to become true.
*
* Will not return for as long as the condition expression @p cond evaluates to
* false.
*
* @note @p cond may get evaluated multiple times.
*
* @note The interrupt state at the moment of calling this macro will be
* restored before executing the condition expression and before
* returning, but interrupts MAY get enabled if the condition evaluates
* false, as the thread MAY have to go to sleep.
*
* @warning @p cond is NOT executed atomically. If that is a requirement, you
* can:
* - lock within the expression, e.g. lock a mutex, disable interrupts.
* - call this with interrupts disabled. Interrupts will be kept
* disabled during the condition evaluation.
*
* @param[in] wq wait queue to wait on
* @param[in] cond condition expression to be evaluated
*/
#define QUEUE_WAIT(wq, cond) \
do { \
BREAK_IF_TRUE(cond); \
\
wait_queue_entry_t me; \
_prepare_to_wait(wq, &me); \
while (!(cond)) { \
_maybe_yield_and_enqueue(wq, &me); \
} \
_wait_dequeue(wq, &me); \
} while (0)


Check warning on line 216 in core/include/wait_queue.h

View workflow job for this annotation

GitHub Actions / static-tests

too many consecutive empty lines
/**
* @brief Wake one thread queued on the wait queue.
*
* @param wq wait queue to signal
*/
static inline void queue_wake_exclusive(wait_queue_t *wq)
{
_queue_wake_common(wq, false);
}

/**
* @brief Wake all threads queued on the wait queue.
*
* @param wq wait queue to signal
*/
static inline void queue_wake(wait_queue_t *wq)
{
_queue_wake_common(wq, true);
}

#ifdef __cplusplus
}
#endif

#endif /* WAIT_QUEUE_H */
/** @} */
1 change: 1 addition & 0 deletions core/thread.c
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,7 @@
stacksize = (char *) thread->tls - stack;

_init_tls(thread->tls);
#endif

Check warning on line 264 in core/thread.c

View workflow job for this annotation

GitHub Actions / static-tests

Uncrustify proposes the following patch: --- a/core/thread.c +++ b/core/thread.c @@ -257,8 +257,8 @@ kernel_pid_t thread_create(char *stack, int stacksize, uint8_t priority, * Make sure the TLS area is aligned as required and that the * resulting stack will also be aligned as required */ - thread->tls = (void *) ((uintptr_t) tls & ~ (TLS_ALIGN - 1)); - stacksize = (char *) thread->tls - stack; + thread->tls = (void *)((uintptr_t)tls & ~(TLS_ALIGN - 1)); + stacksize = (char *)thread->tls - stack; _init_tls(thread->tls); #endif diff --git a/core/wq.c b/core/wq.c index 6c90ec14bc..50e5a81c4b 100644

#if defined(DEVELHELP) || defined(SCHED_TEST_STACK) \
|| defined(MODULE_TEST_UTILS_PRINT_STACK_USAGE)
Expand Down Expand Up @@ -364,6 +364,7 @@
[STATUS_FLAG_BLOCKED_ALL] = "bl allfl",
[STATUS_MBOX_BLOCKED] = "bl mbox",
[STATUS_COND_BLOCKED] = "bl cond",
[STATUS_WQ_BLOCKED] = "bl wq",
[STATUS_RUNNING] = "running",
[STATUS_PENDING] = "pending",
};
Expand Down
Loading
Loading