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

scheduling: functions that might reschedule assume interrupts enabled #20941

Open
wants to merge 5 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
9 changes: 6 additions & 3 deletions core/cond.c
Original file line number Diff line number Diff line change
Expand Up @@ -34,13 +34,16 @@ void cond_init(cond_t *cond)

void cond_wait(cond_t *cond, mutex_t *mutex)
{
unsigned irqstate = irq_disable();
assert(irq_is_enabled());
thread_t *me = thread_get_active();

mutex_unlock(mutex);
irq_disable();
sched_set_status(me, STATUS_COND_BLOCKED);
thread_add_to_list(&cond->queue, me);
irq_restore(irqstate);
irq_enable();
/* if a higher prio thread blocks on the mutex, mutex_unlock() will
* reschedule, so this has to happen after enabling interrupts */
mutex_unlock(mutex);
thread_yield_higher();

/*
Expand Down
3 changes: 3 additions & 0 deletions core/include/mbox.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@

#include "list.h"
#include "cib.h"
#include "irq.h"
#include "msg.h"

#ifdef __cplusplus
Expand Down Expand Up @@ -110,6 +111,7 @@ int _mbox_get(mbox_t *mbox, msg_t *msg, int blocking);
*/
static inline void mbox_put(mbox_t *mbox, msg_t *msg)
{
assume(irq_is_in() || irq_is_enabled());
_mbox_put(mbox, msg, BLOCKING);
}

Expand Down Expand Up @@ -140,6 +142,7 @@ static inline int mbox_try_put(mbox_t *mbox, msg_t *msg)
*/
static inline void mbox_get(mbox_t *mbox, msg_t *msg)
{
assume(irq_is_in() || irq_is_enabled());
_mbox_get(mbox, msg, BLOCKING);
}

Expand Down
2 changes: 0 additions & 2 deletions core/include/sched.h
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@
#define STATUS_NOT_FOUND ((thread_status_t)~0) /**< Describes an illegal thread status */
/** @} */
/**
* @def SCHED_PRIO_LEVELS

Check warning on line 190 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 @@ -184,7 +184,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/thread.h b/core/include/thread.h index b2c2063d4f..ed09705aea 100644
* @brief The number of thread priority levels
*/
#ifndef SCHED_PRIO_LEVELS
Expand Down Expand Up @@ -260,8 +260,6 @@
/**
* @brief Change the priority of the given thread
*
* @note This functions expects interrupts to be disabled when called!
*
* @pre (thread != NULL)
* @pre (priority < SCHED_PRIO_LEVELS)
*
Expand Down
22 changes: 21 additions & 1 deletion core/include/thread.h
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@

#include "clist.h"
#include "cib.h"
#include "irq.h"
#include "msg.h"
#include "sched.h"
#include "thread_config.h"
Expand Down Expand Up @@ -340,6 +341,11 @@
}
#endif

/**
* @brief Arch-specific implementation of @ref thread_yield_higher()
*/
THREAD_MAYBE_INLINE void thread_yield_higher_arch(void);

/**
* @brief Lets current thread yield in favor of a higher prioritized thread.
*
Expand All @@ -352,7 +358,21 @@
*
* @see thread_yield()
*/
THREAD_MAYBE_INLINE void thread_yield_higher(void);
#ifdef DEBUG_THREAD_YIELD_HIGHER
/* A failed assertion will reveal where this was called at the expense of
* increased binary size. */
#define thread_yield_higher() do { \
assume(irq_is_in() || irq_is_enabled()); \
thread_yield_higher_arch(); \
} while (0)

#else

Check warning on line 369 in core/include/thread.h

View workflow job for this annotation

GitHub Actions / static-tests

Uncrustify proposes the following patch: --- a/core/include/thread.h +++ b/core/include/thread.h @@ -362,8 +362,8 @@ THREAD_MAYBE_INLINE void thread_yield_higher_arch(void); /* A failed assertion will reveal where this was called at the expense of * increased binary size. */ #define thread_yield_higher() do { \ - assume(irq_is_in() || irq_is_enabled()); \ - thread_yield_higher_arch(); \ + assume(irq_is_in() || irq_is_enabled()); \ + thread_yield_higher_arch(); \ } while (0) #else diff --git a/core/thread.c b/core/thread.c index c3c5aa29a2..ee0850bf7c 100644
static inline void thread_yield_higher(void)
{
assume(irq_is_in() || irq_is_enabled());
thread_yield_higher_arch();
}
#endif

/**
* @brief Puts the current thread into zombie state.
Expand Down
10 changes: 5 additions & 5 deletions core/mbox.c
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ static void _wake_waiter(thread_t *thread, unsigned irqstate)
sched_switch(process_priority);
}

static void _wait(list_node_t *wait_list, unsigned irqstate)
static void _wait(list_node_t *wait_list)
{
DEBUG("mbox: Thread %" PRIkernel_pid " _wait(): going blocked.\n",
thread_getpid());
Expand All @@ -50,7 +50,7 @@ static void _wait(list_node_t *wait_list, unsigned irqstate)

sched_set_status(me, STATUS_MBOX_BLOCKED);
thread_add_to_list(wait_list, me);
irq_restore(irqstate);
irq_enable();
thread_yield();

DEBUG("mbox: Thread %" PRIkernel_pid " _wait(): woke up.\n",
Expand All @@ -75,8 +75,8 @@ int _mbox_put(mbox_t *mbox, msg_t *msg, int blocking)
else {
while (cib_full(&mbox->cib)) {
if (blocking) {
_wait(&mbox->writers, irqstate);
irqstate = irq_disable();
_wait(&mbox->writers);
irq_disable();
}
else {
irq_restore(irqstate);
Expand Down Expand Up @@ -116,7 +116,7 @@ int _mbox_get(mbox_t *mbox, msg_t *msg, int blocking)
}
else if (blocking) {
thread_get_active()->wait_data = msg;
_wait(&mbox->readers, irqstate);
_wait(&mbox->readers);
/* sender has copied message */
return 1;
}
Expand Down
5 changes: 3 additions & 2 deletions core/sched.c
Original file line number Diff line number Diff line change
Expand Up @@ -334,21 +334,22 @@ void sched_register_cb(void (*callback)(kernel_pid_t, kernel_pid_t))

void sched_change_priority(thread_t *thread, uint8_t priority)
{
assume(irq_is_in() || irq_is_enabled());
assert(thread && (priority < SCHED_PRIO_LEVELS));

if (thread->priority == priority) {
return;
}

unsigned irq_state = irq_disable();
irq_disable();

if (thread_is_active(thread)) {
_runqueue_pop(thread);
_runqueue_push(thread, priority);
}
thread->priority = priority;

irq_restore(irq_state);
irq_enable();

thread_t *active = thread_get_active();

Expand Down
10 changes: 6 additions & 4 deletions core/thread.c
Original file line number Diff line number Diff line change
Expand Up @@ -111,10 +111,11 @@
return;
}

unsigned state = irq_disable();
assert(irq_is_enabled());
irq_disable();

sched_set_status(thread_get_active(), STATUS_SLEEPING);
irq_restore(state);
irq_enable();
thread_yield_higher();
}

Expand Down Expand Up @@ -149,13 +150,14 @@

void thread_yield(void)
{
unsigned old_state = irq_disable();
assume(irq_is_in() || irq_is_enabled());
irq_disable();
thread_t *me = thread_get_active();

if (me->status >= STATUS_ON_RUNQUEUE) {
sched_runq_advance(me->priority);
}
irq_restore(old_state);
irq_enable();

thread_yield_higher();
}
Expand Down Expand Up @@ -261,7 +263,7 @@
stacksize = (char *) thread->tls - stack;

_init_tls(thread->tls);
#endif

Check warning on line 266 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 @@ -259,8 +259,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

#if defined(DEVELHELP) || defined(SCHED_TEST_STACK) \
|| defined(MODULE_TEST_UTILS_PRINT_STACK_USAGE)
Expand Down
25 changes: 14 additions & 11 deletions core/thread_flags.c
Original file line number Diff line number Diff line change
Expand Up @@ -70,15 +70,15 @@ static thread_flags_t _thread_flags_clear_atomic(thread_t *thread,
}

static void _thread_flags_wait(thread_flags_t mask, thread_t *thread,
unsigned threadstate, unsigned irqstate)
unsigned threadstate)
{
DEBUG(
"_thread_flags_wait: me->flags=0x%08x me->mask=0x%08x. going blocked.\n",
(unsigned)thread->flags, (unsigned)mask);

thread->wait_data = (void *)(uintptr_t)mask;
sched_set_status(thread, threadstate);
irq_restore(irqstate);
irq_enable();
thread_yield_higher();
}

Expand All @@ -94,14 +94,15 @@ thread_flags_t thread_flags_clear(thread_flags_t mask)

static void _thread_flags_wait_any(thread_flags_t mask)
{
assume(irq_is_in() || irq_is_enabled());
thread_t *me = thread_get_active();
unsigned state = irq_disable();
irq_disable();

if (!(me->flags & mask)) {
_thread_flags_wait(mask, me, STATUS_FLAG_BLOCKED_ANY, state);
_thread_flags_wait(mask, me, STATUS_FLAG_BLOCKED_ANY);
}
else {
irq_restore(state);
irq_enable();
}
}

Expand All @@ -126,34 +127,36 @@ thread_flags_t thread_flags_wait_one(thread_flags_t mask)

thread_flags_t thread_flags_wait_all(thread_flags_t mask)
{
unsigned state = irq_disable();
assume(irq_is_in() || irq_is_enabled());
irq_disable();
thread_t *me = thread_get_active();

if (!((me->flags & mask) == mask)) {
DEBUG(
"thread_flags_wait_all(): pid %" PRIkernel_pid " waiting for %08x\n",
thread_getpid(), (unsigned)mask);
_thread_flags_wait(mask, me, STATUS_FLAG_BLOCKED_ALL, state);
_thread_flags_wait(mask, me, STATUS_FLAG_BLOCKED_ALL);
}
else {
irq_restore(state);
irq_enable();
}

return _thread_flags_clear_atomic(me, mask);
}

void thread_flags_set(thread_t *thread, thread_flags_t mask)
{
assume(irq_is_in() || irq_is_enabled());
DEBUG("thread_flags_set(): setting 0x%08x for pid %" PRIkernel_pid "\n",
mask, thread->pid);
unsigned state = irq_disable();
irq_disable();

thread->flags |= mask;
if (_thread_flags_wake(thread)) {
irq_restore(state);
irq_enable();
thread_yield_higher();
}
else {
irq_restore(state);
irq_enable();
}
}
2 changes: 1 addition & 1 deletion cpu/arm7_common/include/thread_arch.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ extern "C" {

#ifndef DOXYGEN /* Doxygen is in core/include/thread.h */

static inline __attribute__((always_inline)) void thread_yield_higher(void)
static inline __attribute__((always_inline)) void thread_yield_higher_arch(void)
{
if (irq_is_in()) {
sched_context_switch_request = 1;
Expand Down
2 changes: 1 addition & 1 deletion cpu/avr8_common/thread_arch.c
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,7 @@ void NORETURN avr8_enter_thread_mode(void)
UNREACHABLE();
}

void thread_yield_higher(void)
void thread_yield_higher_arch(void)
{
if (!IS_USED(MODULE_CORE_THREAD)) {
return;
Expand Down
2 changes: 1 addition & 1 deletion cpu/cortexm_common/include/thread_arch.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ extern "C" {

#ifndef DOXYGEN /* Doxygen is in core/include/thread.h */

static inline __attribute__((always_inline)) void thread_yield_higher(void)
static inline __attribute__((always_inline)) void thread_yield_higher_arch(void)
{
/* trigger the PENDSV interrupt to run scheduler and schedule new thread if
* applicable */
Expand Down
2 changes: 1 addition & 1 deletion cpu/msp430/cpu.c
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
* execution at the call site using reti.
*
*/
void thread_yield_higher(void)
void thread_yield_higher_arch(void)
{
if (irq_is_in()) {
sched_context_switch_request = 1;
Expand Down
2 changes: 1 addition & 1 deletion cpu/native/native_cpu.c
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@
sched_run();
}

DEBUG("isr_cpu_switch_context_exit: calling setcontext(%" PRIkernel_pid ")\n\n", thread_getpid());

Check warning on line 174 in cpu/native/native_cpu.c

View workflow job for this annotation

GitHub Actions / static-tests

line is longer than 100 characters
/* Use intermediate cast to uintptr_t to silence -Wcast-align.
* stacks are manually word aligned in thread_static_init() */
ctx = (ucontext_t *)(uintptr_t)(thread_get_active()->sp);
Expand Down Expand Up @@ -241,7 +241,7 @@
}
}

void thread_yield_higher(void)
void thread_yield_higher_arch(void)
{
sched_context_switch_request = 1;

Expand Down Expand Up @@ -274,7 +274,7 @@
end_context.uc_stack.ss_flags = 0;
makecontext(&end_context, sched_task_exit, 0);
(void)VALGRIND_STACK_REGISTER(end_context.uc_stack.ss_sp,
(char *)end_context.uc_stack.ss_sp + end_context.uc_stack.ss_size);

Check warning on line 277 in cpu/native/native_cpu.c

View workflow job for this annotation

GitHub Actions / static-tests

line is longer than 100 characters
VALGRIND_DEBUG("VALGRIND_STACK_REGISTER(%p, %p)\n",
(void*)end_context.uc_stack.ss_sp,
(void*)((char *)end_context.uc_stack.ss_sp + end_context.uc_stack.ss_size));
Expand Down
2 changes: 1 addition & 1 deletion cpu/riscv_common/include/thread_arch.h
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ static inline void _ecall_dispatch(uint32_t num, void *ctx)
);
}

static inline __attribute__((always_inline)) void thread_yield_higher(void)
static inline __attribute__((always_inline)) void thread_yield_higher_arch(void)
{
if (irq_is_in()) {
sched_context_switch_request = 1;
Expand Down
Loading