tests: add test for priority inversion using overlapping mutexes

core/Makefile

@@ -1,7 +1,8 @@
 # exclude submodule sources from *.c wildcard source selection
-SRC := $(filter-out mbox.c msg.c thread_flags.c,$(wildcard *.c))
+SRC := $(filter-out mbox.c msg.c thread_flags.c priority_inheritance.c,$(wildcard *.c))
 
 # enable submodules
 SUBMODULES := 1
+SUBMODULES_NOFORCE := 1
 
 include $(RIOTBASE)/Makefile.base

core/include/mutex.h

@@ -1,6 +1,6 @@
 /*
  * Copyright (C) 2015 Kaspar Schleiser <kaspar@schleiser.de>
- *               2013, 2014 Freie Universität Berlin
+ *               2013-2017 Freie Universität Berlin
  *
  * 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
@@ -17,6 +17,7 @@
  * @brief       RIOT synchronization API
  *
  * @author      Kaspar Schleiser <kaspar@schleiser.de>
+ * @author      Hauke Petersen <hauke.petersen@fu-berlin.de>
  */
 
 #ifndef MUTEX_H
@@ -26,6 +27,11 @@
 
 #include "list.h"
 
+#ifdef MODULE_CORE_PRIORITY_INHERITANCE
+#include "thread.h"
+#include "kernel_types.h"
+#endif
+
 #ifdef __cplusplus
  extern "C" {
 #endif
@@ -40,18 +46,30 @@ typedef struct {
      * @internal
      */
     list_node_t queue;
+#ifdef MODULE_CORE_PRIORITY_INHERITANCE
+    kernel_pid_t owner;
+    uint8_t prio_before;
+#endif
 } mutex_t;
 
 /**
  * @brief Static initializer for mutex_t.
  * @details This initializer is preferable to mutex_init().
  */
+#ifdef MODULE_CORE_PRIORITY_INHERITANCE
+#define MUTEX_INIT { { NULL }, KERNEL_PID_UNDEF, THREAD_PRIORITY_UNDEF }
+#else
 #define MUTEX_INIT { { NULL } }
+#endif
 
 /**
  * @brief Static initializer for mutex_t with a locked mutex
  */
+#ifdef MODULE_CORE_PRIORITY_INHERITANCE
+#define MUTEX_INIT_LOCKED { { MUTEX_LOCKED }, KERNEL_PID_UNDEF, THREAD_PRIORITY_UNDEF }
+#else
 #define MUTEX_INIT_LOCKED { { MUTEX_LOCKED } }
+#endif
 
 /**
  * @cond INTERNAL

core/include/sched.h

@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2014 Freie Universität Berlin
+ * Copyright (C) 2014-2017 Freie Universität Berlin
  *
  * 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
@@ -75,6 +75,7 @@
  * @brief       Scheduler API definition
  *
  * @author      Kaspar Schleiser <kaspar@schleiser.de>
+ * @author      Hauke Petersen <hauke.petersen@fu-berlin.de>
  */
 
 #ifndef SCHED_H
@@ -174,6 +175,21 @@ extern clist_node_t sched_runqueues[SCHED_PRIO_LEVELS];
  */
 NORETURN void sched_task_exit(void);
 
+#ifdef MODULE_CORE_PRIORITY_INHERITANCE
+/**
+ * @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)
+ *
+ * @param[in,out] thread    target thread
+ * @param[in]     priority  new priority to assign to @p thread
+ */
+void sched_change_priority(thread_t *thread, uint8_t priority);
+#endif
+
 #ifdef MODULE_SCHEDSTATISTICS
 /**
  *  Scheduler statistics

core/include/thread.h

@@ -119,6 +119,8 @@
 #ifndef THREAD_H
 #define THREAD_H
 
+#include <stdint.h>
+
 #include "clist.h"
 #include "cib.h"
 #include "msg.h"
@@ -172,7 +174,6 @@ struct _thread {
     char *sp;                       /**< thread's stack pointer         */
     uint8_t status;                 /**< thread's status                */
     uint8_t priority;               /**< thread's priority              */
-
     kernel_pid_t pid;               /**< thread's process id            */
 
 #ifdef MODULE_CORE_THREAD_FLAGS
@@ -259,6 +260,12 @@ struct _thread {
 #define THREAD_STACKSIZE_MINIMUM  (sizeof(thread_t))
 #endif
 
+/**
+ * @def THREAD_PRIORITY_UNDEF
+ * @brief Value for explicitly marking a priority undefined
+ */
+#define THREAD_PRIORITY_UNDEF           (UINT8_MAX)
+
 /**
  * @def THREAD_PRIORITY_MIN
  * @brief Least priority a thread can have

core/msg.c

@@ -208,16 +208,33 @@ int msg_send_int(msg_t *m, kernel_pid_t target_pid)
 int msg_send_receive(msg_t *m, msg_t *reply, kernel_pid_t target_pid)
 {
     assert(sched_active_pid != target_pid);
+
     unsigned state = irq_disable();
     thread_t *me = (thread_t*) sched_threads[sched_active_pid];
     sched_set_status(me, STATUS_REPLY_BLOCKED);
     me->wait_data = (void*) reply;
 
+#ifdef MODULE_CORE_PRIORITY_INHERITANCE
+    thread_t *recv_thread = (thread_t*) sched_threads[target_pid];
+    uint8_t prio_backup = recv_thread->priority;
+    /* When priority inheritance is enabled, we lend the receiver thread our
+     * current priority if lower */
+    if (me->priority < recv_thread->priority) {
+        sched_change_priority(recv_thread, me->priority);
+    }
+#endif
+
     /* we re-use (abuse) reply for sending, because wait_data might be
      * overwritten if the target is not in RECEIVE_BLOCKED */
     *reply = *m;
     /* msg_send blocks until reply received */
+#ifdef MODULE_CORE_PRIORITY_INHERITANCE
+    int res = _msg_send(reply, target_pid, true, state);
+    sched_change_priority(recv_thread, prio_backup);
+    return res;
+#else
     return _msg_send(reply, target_pid, true, state);
+#endif
 }
 
 int msg_reply(msg_t *m, msg_t *reply)

core/mutex.c

@@ -1,6 +1,6 @@
 /*
  * Copyright (C) 2015 Kaspar Schleiser <kaspar@schleiser.de>
- *               2013 Freie Universität Berlin
+ *               2013-2017 Freie Universität Berlin
  *
  * 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
@@ -16,6 +16,7 @@
  *
  * @author      Kaspar Schleiser <kaspar@schleiser.de>
  * @author      Joakim Nohlgård <joakim.nohlgard@eistec.se>
+ * @author      Hauke Petersen <hauke.petersen@fu-berlin.de>
  *
  * @}
  */
@@ -34,6 +35,7 @@
 
 int _mutex_lock(mutex_t *mutex, int blocking)
 {
+    thread_t *me = (thread_t*)sched_active_thread;
     unsigned irqstate = irq_disable();
 
     DEBUG("PID[%" PRIkernel_pid "]: Mutex in use.\n", sched_active_pid);
@@ -43,11 +45,15 @@ int _mutex_lock(mutex_t *mutex, int blocking)
         mutex->queue.next = MUTEX_LOCKED;
         DEBUG("PID[%" PRIkernel_pid "]: mutex_wait early out.\n",
               sched_active_pid);
+
+#ifdef MODULE_CORE_PRIORITY_INHERITANCE
+        mutex->owner = me->pid;
+        mutex->prio_before = THREAD_PRIORITY_UNDEF;
+#endif
         irq_restore(irqstate);
         return 1;
     }
     else if (blocking) {
-        thread_t *me = (thread_t*)sched_active_thread;
         DEBUG("PID[%" PRIkernel_pid "]: Adding node to mutex queue: prio: %"
               PRIu32 "\n", sched_active_pid, (uint32_t)me->priority);
         sched_set_status(me, STATUS_MUTEX_BLOCKED);
@@ -58,6 +64,19 @@ int _mutex_lock(mutex_t *mutex, int blocking)
         else {
             thread_add_to_list(&mutex->queue, me);
         }
+
+#ifdef MODULE_CORE_PRIORITY_INHERITANCE
+        thread_t *owner = (thread_t *)thread_get(mutex->owner);
+        /* if my priority is higher than the priority of the current mutex
+         * owner, I lend my priority to the current owner */
+        if (me->priority < owner->priority) {
+            DEBUG("PID[%" PRIkernel_pid "]: changing priority from %i -> %i\n",
+                  owner->pid, owner->priority, me->priority);
+            mutex->prio_before = owner->priority;
+            sched_change_priority(owner, me->priority);
+        }
+#endif
+
         irq_restore(irqstate);
         thread_yield_higher();
         /* We were woken up by scheduler. Waker removed us from queue.
@@ -83,6 +102,16 @@ void mutex_unlock(mutex_t *mutex)
         return;
     }
 
+
+#ifdef MODULE_CORE_PRIORITY_INHERITANCE
+    thread_t *owner = (thread_t *)thread_get(mutex->owner);
+    /* restore the priority of the releasing thread */
+    if (mutex->prio_before != THREAD_PRIORITY_UNDEF) {
+        sched_change_priority(owner, mutex->prio_before);
+        mutex->prio_before = THREAD_PRIORITY_UNDEF;
+    }
+#endif
+
     if (mutex->queue.next == MUTEX_LOCKED) {
         mutex->queue.next = NULL;
         /* the mutex was locked and no thread was waiting for it */
@@ -91,9 +120,13 @@ void mutex_unlock(mutex_t *mutex)
     }
 
     list_node_t *next = list_remove_head(&mutex->queue);
-
     thread_t *process = container_of((clist_node_t*)next, thread_t, rq_entry);
 
+#ifdef MODULE_CORE_PRIORITY_INHERITANCE
+    /* set context for next mutex owner */
+    mutex->owner = process->pid;
+#endif
+
     DEBUG("mutex_unlock: waking up waiting thread %" PRIkernel_pid "\n",
           process->pid);
     sched_set_status(process, STATUS_PENDING);

core/sched.c

@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2014 Freie Universität Berlin
+ * Copyright (C) 2014-2017 Freie Universität Berlin
  *
  * 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
@@ -15,6 +15,7 @@
  *
  * @author      Kaspar Schleiser <kaspar@schleiser.de>
  * @author      René Kijewski <rene.kijewski@fu-berlin.de>
+ * @author      Hauke Petersen <hauke.petersen@fu-berlin.de>
  *
  * @}
  */
@@ -79,6 +80,18 @@ static void (*sched_cb) (uint32_t timestamp, uint32_t value) = NULL;
 schedstat sched_pidlist[KERNEL_PID_LAST + 1];
 #endif
 
+static inline void runqueue_push(thread_t *thread, uint8_t priority) {
+    clist_rpush(&sched_runqueues[priority], &(thread->rq_entry));
+    runqueue_bitcache |= 1 << priority;
+}
+
+static inline void runqueue_pop(thread_t *thread) {
+    clist_lpop(&sched_runqueues[thread->priority]);
+    if (!sched_runqueues[thread->priority].next) {
+        runqueue_bitcache &= ~(1 << thread->priority);
+    }
+}
+
 int __attribute__((used)) sched_run(void)
 {
     sched_context_switch_request = 0;
@@ -164,19 +177,14 @@ void sched_set_status(thread_t *process, unsigned int status)
         if (!(process->status >= STATUS_ON_RUNQUEUE)) {
             DEBUG("sched_set_status: adding thread %" PRIkernel_pid " to runqueue %" PRIu16 ".\n",
                   process->pid, process->priority);
-            clist_rpush(&sched_runqueues[process->priority], &(process->rq_entry));
-            runqueue_bitcache |= 1 << process->priority;
+            runqueue_push(process, process->priority);
         }
     }
     else {
         if (process->status >= STATUS_ON_RUNQUEUE) {
             DEBUG("sched_set_status: removing thread %" PRIkernel_pid " to runqueue %" PRIu16 ".\n",
                   process->pid, process->priority);
-            clist_lpop(&sched_runqueues[process->priority]);
-
-            if (!sched_runqueues[process->priority].next) {
-                runqueue_bitcache &= ~(1 << process->priority);
-            }
+            runqueue_pop(process);
         }
     }
 
@@ -221,3 +229,21 @@ NORETURN void sched_task_exit(void)
     sched_active_thread = NULL;
     cpu_switch_context_exit();
 }
+
+#ifdef MODULE_CORE_PRIORITY_INHERITANCE
+void sched_change_priority(thread_t *thread, uint8_t priority)
+{
+    assert(thread && (priority < SCHED_PRIO_LEVELS));
+
+    if (thread->priority == priority) {
+        return;
+    }
+
+    if (thread->status >= STATUS_ON_RUNQUEUE) {
+        runqueue_pop(thread);
+        runqueue_push(thread, priority);
+    }
+
+    thread->priority = priority;
+}
+#endif

tests/msg_priority_inversion/Makefile

@@ -0,0 +1,10 @@
+APPLICATION = msg_priority_inversion
+include ../Makefile.tests_common
+
+BOARD_INSUFFICIENT_MEMORY := nucleo32-f031 nucleo32-f042 nucleo32-l031 nucleo-f030 \
+                             nucleo-l053 stm32f0discovery weio
+
+USEMODULE += core_priority_inheritance
+USEMODULE += xtimer
+
+include $(RIOTBASE)/Makefile.include

tests/msg_priority_inversion/README.md

@@ -0,0 +1,44 @@
+NOTE
+====
+THIS TEST WILL FAIL WHEN RUNNING RIOT USING THE DEFAULT CORE/KERNEL CONFIGURATION!
+
+You need to select the `core_priority_inheritance` module (using
+`USEMODULE += core_priority_inheritance`) to see this test succeeding.
+
+Expected result
+===============
+When successful, you will see the 3 configured threads printing 6 different
+events in a defined order. If the test passes, the output should look like this:
+
+```
+main(): This is RIOT! (Version: xxx)
+Test for showing priority inversion when using msg_send_receive
+
+If this tests succeeds, you should see 6 events appearing in order.
+The expected output should look like this:
+Event  1:      t3 - waiting for incoming message
+Event  2:      t2 - starting infinite loop, potentially starving others
+Event  3:      t1 - sending msg to t3 (msg_send_receive)
+Event  4:      t3 - received message
+Event  5:      t3 - sending reply
+Event  6:      t1 - received reply
+
+TEST OUTPUT:
+Event  1:      t3 - waiting for incoming message
+Event  2:      t2 - starting infinite loop, potentially starving others
+Event  3:      t1 - sending msg to t3 (msg_send_receive)
+Event  4:      t3 - received message
+Event  5:      t3 - sending reply
+Event  6:      t1 - received reply
+
+   *** result: SUCCESS ***
+```
+
+Background
+==========
+Priority inversion is a known problem in real-time systems, leading in certain
+constellations to lower priority threads indirectly blocking higher priority
+threads. This test constructs a simple scenario, where a high priority thread
+(`t1`) is trying to `send_receive` a message to a low priority thread (`t3`),
+but never gets a reply as a medium priority thread (`t2`) is never letting `t3`
+to be scheduled.