kaspar/RIOT
1
0
Fork 0
mirror of https://github.com/RIOT-OS/RIOT.git synced 2025-12-27 15:31:17 +01:00
Code Releases Activity

Merge pull request #17040 from maribu/tests/thread_priority_inversion

Browse Source 
core: implement core_mutex_priority_inheritance
This commit is contained in:
benpicco 2022-08-05 18:40:31 +02:00 committed by GitHub
commit ba33ab5174
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 292 additions and 123 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
core/Kconfig
View File

@ -39,6 +39,9 @@ config MODULE_CORE_MSG_BUS
help
Messaging Bus API for inter process message broadcast.
config MODULE_CORE_MUTEX_PRIORITY_INHERITANCE
bool "Use priority inheritance to mitigate priority inversion for mutexes"
config MODULE_CORE_PANIC
bool "Kernel crash handling module"
default y

42
core/include/mutex.h
View File

@ -12,6 +12,12 @@
* @ingroup core_sync
* @brief Mutex for thread synchronization
*
* @warning By default, no mitigation against priority inversion is
* employed. If your application is subject to priority inversion
* and cannot tolerate the additional delay this can cause, use
* module `core_mutex_priority_inheritance` to employ
* priority inheritance as mitigation.
*
* Mutex Implementation Basics
* ===========================
*
@ -127,6 +133,24 @@ typedef struct {
* @internal
*/
list_node_t queue;
#if defined(DOXYGEN) || defined(MODULE_CORE_MUTEX_PRIORITY_INHERITANCE)
/**
* @brief The current owner of the mutex or `NULL`
* @note Only available if module core_mutex_priority_inheritance
* is used.
*
* If either the mutex is not locked or the mutex is not locked by a thread
* (e.g. because it is used to synchronize a thread with an ISR completion),
* this will have the value of `NULL`.
*/
kernel_pid_t owner;
/**
* @brief Original priority of the owner
* @note Only available if module core_mutex_priority_inheritance
* is used.
*/
uint8_t owner_original_priority;
#endif
} mutex_t;
/**
@ -141,16 +165,21 @@ typedef struct {
uint8_t cancelled; /**< Flag whether the mutex has been cancelled */
} mutex_cancel_t;
#ifndef __cplusplus
/**
* @brief Static initializer for mutex_t.
* @details This initializer is preferable to mutex_init().
*/
#define MUTEX_INIT { { NULL } }
# define MUTEX_INIT { .queue = { .next = NULL } }
/**
* @brief Static initializer for mutex_t with a locked mutex
*/
#define MUTEX_INIT_LOCKED { { MUTEX_LOCKED } }
# define MUTEX_INIT_LOCKED { .queue = { .next = MUTEX_LOCKED } }
#else
# define MUTEX_INIT {}
# define MUTEX_INIT_LOCKED { { MUTEX_LOCKED } }
#endif /* __cplusplus */
/**
* @cond INTERNAL
@ -231,6 +260,15 @@ static inline int mutex_trylock(mutex_t *mutex)
if (mutex->queue.next == NULL) {
mutex->queue.next = MUTEX_LOCKED;
#ifdef MODULE_CORE_MUTEX_PRIORITY_INHERITANCE
mutex->owner = KERNEL_PID_UNDEF;
thread_t *t = thread_get_active();
/* in case mutex_trylock() is not called from thread context */
if (t) {
mutex->owner = t->pid;
mutex->owner_original_priority = t->priority;
}
#endif
retval = 1;
}
irq_restore(irq_state);

31
core/mutex.c
View File

@ -66,6 +66,17 @@ static inline __attribute__((always_inline)) void _block(mutex_t *mutex,
thread_add_to_list(&mutex->queue, me);
}
#ifdef MODULE_CORE_MUTEX_PRIORITY_INHERITANCE
thread_t *owner = thread_get(mutex->owner);
if ((owner) && (owner->priority > me->priority)) {
DEBUG("PID[%" PRIkernel_pid "] prio of %" PRIkernel_pid
": %u --> %u\n",
thread_getpid(), mutex->owner,
(unsigned)owner->priority, (unsigned)me->priority);
sched_change_priority(owner, me->priority);
}
#endif
irq_restore(irq_state);
thread_yield_higher();
/* We were woken up by scheduler. Waker removed us from queue. */
@ -80,6 +91,11 @@ void mutex_lock(mutex_t *mutex)
if (mutex->queue.next == NULL) {
/* mutex is unlocked. */
mutex->queue.next = MUTEX_LOCKED;
#ifdef MODULE_CORE_MUTEX_PRIORITY_INHERITANCE
thread_t *me = thread_get_active();
mutex->owner = me->pid;
mutex->owner_original_priority = me->priority;
#endif
DEBUG("PID[%" PRIkernel_pid "] mutex_lock(): early out.\n",
thread_getpid());
irq_restore(irq_state);
@ -108,6 +124,11 @@ int mutex_lock_cancelable(mutex_cancel_t *mc)
if (mutex->queue.next == NULL) {
/* mutex is unlocked. */
mutex->queue.next = MUTEX_LOCKED;
#ifdef MODULE_CORE_MUTEX_PRIORITY_INHERITANCE
thread_t *me = thread_get_active();
mutex->owner = me->pid;
mutex->owner_original_priority = me->priority;
#endif
DEBUG("PID[%" PRIkernel_pid "] mutex_lock_cancelable() early out.\n",
thread_getpid());
irq_restore(irq_state);
@ -136,6 +157,16 @@ void mutex_unlock(mutex_t *mutex)
return;
}
#ifdef MODULE_CORE_MUTEX_PRIORITY_INHERITANCE
thread_t *owner = thread_get(mutex->owner);
if ((owner) && (owner->priority != mutex->owner_original_priority)) {
DEBUG("PID[%" PRIkernel_pid "] prio %u --> %u\n",
owner->pid,
(unsigned)owner->priority, (unsigned)owner->priority);
sched_change_priority(owner, mutex->owner_original_priority);
}
#endif
if (mutex->queue.next == MUTEX_LOCKED) {
mutex->queue.next = NULL;
/* the mutex was locked and no thread was waiting for it */

2
sys/cpp11-compat/include/riot/mutex.hpp
View File

@ -47,7 +47,7 @@ public:
*/
using native_handle_type = mutex_t*;
inline constexpr mutex() noexcept : m_mtx{{0}} {}
inline constexpr mutex() noexcept : m_mtx{} {}
~mutex();
/**

13
tests/thread_priority_inversion/Makefile
View File

@ -1,6 +1,17 @@
include ../Makefile.tests_common
USEMODULE += fmt
USEMODULE += core_mutex_priority_inheritance
USEMODULE += xtimer
ifneq ($(RIOT_CI_BUILD),1)
# For human beings add a busy delay to the mid priority task to make the problem more approachable
FANCY ?= 1
else
# Skip the fancy delay for the CI to not waste precious CI time
FANCY ?= 0
endif
include $(RIOTBASE)/Makefile.include
CFLAGS += -DFANCY=$(FANCY)
CFLAGS += -DTHREAD_STACKSIZE_MAIN=THREAD_STACKSIZE_SMALL

12
tests/thread_priority_inversion/Makefile.ci
View File

@ -1,12 +0,0 @@
BOARD_INSUFFICIENT_MEMORY := \
arduino-duemilanove \
arduino-leonardo \
arduino-nano \
arduino-uno \
atmega328p \
atmega328p-xplained-mini \
nucleo-f031k6 \
nucleo-l011k4 \
samd10-xmini \
stm32f030f4-demo \
#

91
tests/thread_priority_inversion/README.md
View File

@ -1,45 +1,58 @@
# thread_priority_inversion test application
Priority Inversion
==================
This application uses three threads for demonstrating the
priority inversion problem. In theory, the highest priority thread (**t_high**)
should be scheduled periodically and produce some output:
```
2017-07-17 17:00:29,337 - INFO # t_high: got resource.
...
2017-07-17 17:00:30,343 - INFO # t_high: freeing resource...
```
During this phase of 1s, **t_high** lockes the mutex **res_mtx** which
represents a shared resource. After unlocking the mutex, **t_high** waits 1s
before restaring its cycle again.
Scenario: A low priority thread holds a shared resource when a mid priority and a high
priority thread become runnable. The high priority thread needs exclusive access to the
resource the low priority one is holding and blocks. This mid priority thread does not
compete with the other for resources but takes a long to complete. The scheduler must
prefer the low priority thread over the mid priority thread while it is holding the
resource the high priority thread is waiting for - otherwise the high priority thread
effectively waits for the mid priority thread, which is an inversion of priority.
Output On Failure
-----------------
A low priority thread **t_low** is doing the same. Since both threads sharing
the same resource **res_mtx**, they have to wait for each other until the mutex
is unlocked. On startup, **t_low** starts immediately, while **t_high** waits
0.5s before so that **t_low** allocates the resource first. Together, the output
looks like this:
```
2017-07-17 17:00:28,339 - INFO # t_low: allocating resource...
2017-07-17 17:00:28,339 - INFO # t_low: got resource.
2017-07-17 17:00:28,340 - INFO # t_high: allocating resource...
2017-07-17 17:00:29,337 - INFO # t_low: freeing resource...
2017-07-17 17:00:29,337 - INFO # t_high: got resource.
2017-07-17 17:00:29,338 - INFO # t_low: freed resource.
2017-07-17 17:00:30,343 - INFO # t_high: freeing resource...
2017-07-17 17:00:30,344 - INFO # t_high: freed resource.
2017-07-17 17:00:30,345 - INFO # t_low: allocating resource...
2017-07-17 17:00:30,346 - INFO # t_low: got resource.
...
main(): This is RIOT! (Version: ...)
low priority thread has started
low priority thread started to work on its task
high priority thread has started
mid priority thread has started
mid priority thread started to work on its task
... this ...
... takes ...
... bloody ...
... ages ...
... to ...
... complete ...
mid priority thread is done
low priority thread is done
high priority thread started to work on its task
high priority thread is done
==> Priority inversion occurred
TEST FAILED
```
After 3s, a third thread with medium priority (**t_mid**) is started. This
thread does not touch **res_mtx**, but it runs an infinite loop without leaving
some CPU time to lower priority tasks. This prevents **t_low** from freeing the
resource and thus, **t_high** from running (**Priority Inversion**). In this
situation, the test program output stops with the following lines:
```
2017-07-17 17:00:31,335 - INFO # t_mid: doing some stupid stuff...
2017-07-17 17:00:31,340 - INFO # t_high: allocating resource...
```
If the scheduler contains a mechanism for handling this problem, the program
should continue with output from **t_high**.
Output On Success
-----------------
```
main(): This is RIOT! (Version: ...)
low priority thread has started
low priority thread started to work on its task
high priority thread has started
low priority thread is done
high priority thread started to work on its task
high priority thread is done
mid priority thread has started
mid priority thread started to work on its task
... this ...
... takes ...
... bloody ...
... ages ...
... to ...
... complete ...
mid priority thread is done
TEST PASSED
```

200
tests/thread_priority_inversion/main.c
View File

@ -1,5 +1,6 @@
/*
* Copyright (C) 2017 Technische Universität Berlin
* 2021 Otto-von-Guericke-Universität Magdeburg
*
* 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
@ -14,103 +15,166 @@
* @brief Thread test application for priority inversion problem
*
* @author Thomas Geithner <thomas.geithner@dai-labor.de>
* @author Marian Buschsieweke <marian.buschsieweke@ovgu.de>
*
* @}
*/
#include <stdio.h>
#include <string.h>
#include <stdint.h>
#include "thread.h"
#include "board.h"
#include "fmt.h"
#include "irq.h"
#include "mutex.h"
#include "xtimer.h"
#include "periph_conf.h"
#include "thread.h"
#include "macros/units.h"
mutex_t res_mtx;
#ifndef FANCY
#define FANCY 0
#endif
char stack_high[THREAD_STACKSIZE_DEFAULT];
char stack_mid[THREAD_STACKSIZE_DEFAULT];
char stack_low[THREAD_STACKSIZE_DEFAULT];
/* Fallback in case core clock isn't define - should only be the case on native.
* We don't need a correct value here any, it is only used for the fancy busy delay to make the
* problem more approachable to human beings. */
#ifndef CLOCK_CORECLOCK
#define CLOCK_CORECLOCK GHZ(1)
#endif
void *t_low_handler(void *arg)
static mutex_t mtx_res = MUTEX_INIT;
static mutex_t mtx_start_low = MUTEX_INIT_LOCKED;
static mutex_t mtx_start_mid = MUTEX_INIT_LOCKED;
static mutex_t mtx_start_high = MUTEX_INIT_LOCKED;
static char stack_high[THREAD_STACKSIZE_SMALL];
static char stack_mid[THREAD_STACKSIZE_SMALL];
static char stack_low[THREAD_STACKSIZE_SMALL];
static char run_order[16] = "";
static size_t run_order_pos = 0;
static void busy_delay(void)
{
(void) arg;
/* About 1 second if assuming one loop iteration takes ~20 CPU cycles. Actual duration doesn't
* matter at all, but this delay has to be busy (not yielding during wait). */
for (volatile uint32_t i = 0; i < CLOCK_CORECLOCK / 20; i++) { }
}
/* starting working loop immediately */
while (1) {
puts("t_low: allocating resource...");
mutex_lock(&res_mtx);
puts("t_low: got resource.");
xtimer_sleep(1);
static void record_thread_started(const char *priority)
{
print_str(priority);
print_str(" priority thread has started\n");
}
static void record_thread_working(const char *priority)
{
print_str(priority);
print_str(" priority thread started to work on its task\n");
/* make recording of running thread atomic by disabling IRQs. We could have used a mutex here
* as well, but for something so short just disabling IRQs is more sensible */
unsigned irq_state = irq_disable();
run_order[run_order_pos++] = priority[0];
irq_restore(irq_state);
}
static void record_thread_done(const char *priority)
{
print_str(priority);
print_str(" priority thread is done\n");
}
static void *low_handler(void *arg)
{
(void)arg;
mutex_lock(&mtx_start_low);
record_thread_started("low");
mutex_lock(&mtx_res);
record_thread_working("low");
/* launch mid and high priority thread now */
mutex_unlock(&mtx_start_high);
mutex_unlock(&mtx_start_mid);
record_thread_done("low");
mutex_unlock(&mtx_res);
puts("t_low: freeing resource...");
mutex_unlock(&res_mtx);
puts("t_low: freed resource.");
xtimer_sleep(1);
}
return NULL;
}
void *t_mid_handler(void *arg)
static void *mid_handler(void *arg)
{
(void) arg;
(void)arg;
mutex_lock(&mtx_start_mid);
record_thread_started("mid");
/* starting working loop after 3s */
xtimer_sleep(3);
puts("t_mid: doing some stupid stuff...");
while (1) {
thread_yield_higher();
record_thread_working("mid");
if (FANCY) {
busy_delay();
print_str(" ... this ...\n");
busy_delay();
print_str(" ... takes ...\n");
busy_delay();
print_str(" ... bloody ...\n");
busy_delay();
print_str(" ... ages ...\n");
busy_delay();
print_str(" ... to ...\n");
busy_delay();
print_str(" ... complete ...\n");
}
}
void *t_high_handler(void *arg)
{
(void) arg;
/* starting working loop after 500 ms */
xtimer_msleep(500U);
while (1) {
puts("t_high: allocating resource...");
mutex_lock(&res_mtx);
puts("t_high: got resource.");
xtimer_sleep(1);
puts("t_high: freeing resource...");
mutex_unlock(&res_mtx);
puts("t_high: freed resource.");
xtimer_sleep(1);
}
record_thread_done("mid");
return NULL;
}
kernel_pid_t pid_low;
kernel_pid_t pid_mid;
kernel_pid_t pid_high;
static void *high_handler(void *arg)
{
(void)arg;
mutex_lock(&mtx_start_high);
record_thread_started("high");
mutex_lock(&mtx_res);
record_thread_working("high");
record_thread_done("high");
mutex_unlock(&mtx_res);
return NULL;
}
int main(void)
{
mutex_init(&res_mtx);
puts("This is a scheduling test for Priority Inversion");
thread_create(stack_low, sizeof(stack_low),
THREAD_PRIORITY_MAIN - 1, THREAD_CREATE_STACKTEST,
low_handler, NULL, "low");
pid_low = thread_create(stack_low, sizeof(stack_low),
THREAD_PRIORITY_MAIN - 1,
THREAD_CREATE_STACKTEST,
t_low_handler, NULL,
"t_low");
thread_create(stack_mid, sizeof(stack_mid),
THREAD_PRIORITY_MAIN - 2, THREAD_CREATE_STACKTEST,
mid_handler, NULL, "mid");
pid_mid = thread_create(stack_mid, sizeof(stack_mid),
THREAD_PRIORITY_MAIN - 2,
THREAD_CREATE_STACKTEST,
t_mid_handler, NULL,
"t_mid");
thread_create(stack_high, sizeof(stack_high),
THREAD_PRIORITY_MAIN - 3, THREAD_CREATE_STACKTEST,
high_handler, NULL, "high");
pid_high = thread_create(stack_high, sizeof(stack_high),
THREAD_PRIORITY_MAIN - 3,
THREAD_CREATE_STACKTEST,
t_high_handler, NULL,
"t_high");
/* Start low priority thread first, which will start high and mid priority ones after the
* shared resource is obtained */
mutex_unlock(&mtx_start_low);
thread_sleep();
if (strcmp("lhm", run_order) == 0) {
print_str("TEST PASSED\n");
return 0;
}
else if (strcmp("lmh", run_order) == 0) {
print_str("==> Priority inversion occurred\n");
}
else {
/* This should occur neither with nor without priority inversion! */
print_str("BUG: \"");
print_str(run_order);
print_str("\"\n");
}
print_str("TEST FAILED\n");
return 0;
}

21
tests/thread_priority_inversion/tests/01-run.py Executable file
View File

@ -0,0 +1,21 @@
#!/usr/bin/env python3
# Copyright (C) 2021 Otto-von-Guericke-Universität Magdeburg
#
# 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.
# @author Marian Buschsieweke <marian.buschsieweke@ovgu.de>
import sys
from testrunner import run
def testfunc(child):
child.expect(r"TEST ([A-Z]+)\r\n")
assert child.match.group(1) == "PASSED"
if __name__ == "__main__":
sys.exit(run(testfunc))