Merge pull request #15327 from maribu/mutex_cleanup_splitout

core/mutex: Cleanup

core/include/mutex.h

@@ -105,7 +105,10 @@
 #include <stddef.h>
 #include <stdint.h>
 
+#include "irq.h"
+#include "kernel_defines.h"
 #include "list.h"
+#include "thread.h"
 
 #ifdef __cplusplus
 extern "C" {
@@ -145,10 +148,10 @@ typedef struct {
  */
 
 /**
- * @brief Initializes a mutex object.
+ * @brief   Initializes a mutex object.
  * @details For initialization of variables use MUTEX_INIT instead.
  *          Only use the function call for dynamically allocated mutexes.
- * @param[out] mutex    pre-allocated mutex structure, must not be NULL.
+ * @param[out]      mutex   pre-allocated mutex structure, must not be NULL.
  */
 static inline void mutex_init(mutex_t *mutex)
 {
@@ -156,59 +159,61 @@ static inline void mutex_init(mutex_t *mutex)
 }
 
 /**
- * @brief Lock a mutex, blocking or non-blocking.
+ * @brief   Tries to get a mutex, non-blocking.
  *
- * @details For commit purposes you should probably use mutex_trylock() and
- *          mutex_lock() instead.
+ * @param[in,out]   mutex   Mutex object to lock.
  *
- * @param[in] mutex         Mutex object to lock. Has to be initialized first.
- *                          Must not be NULL.
- * @param[in] blocking      if true, block until mutex is available.
+ * @retval  1               if mutex was unlocked, now it is locked.
+ * @retval  0               if the mutex was locked.
  *
- * @return 1 if mutex was unlocked, now it is locked.
- * @return 0 if the mutex was locked.
- */
-int _mutex_lock(mutex_t *mutex, volatile uint8_t *blocking);
-
-/**
- * @brief Tries to get a mutex, non-blocking.
- *
- * @param[in] mutex Mutex object to lock. Has to be initialized first. Must not
- *                  be NULL.
- *
- * @return 1 if mutex was unlocked, now it is locked.
- * @return 0 if the mutex was locked.
+ * @pre     @p mutex is not `NULL`
+ * @pre     Mutex at @p mutex has been initialized
+ * @pre     Must be called in thread context
  */
 static inline int mutex_trylock(mutex_t *mutex)
 {
-    volatile uint8_t blocking = 0;
-
-    return _mutex_lock(mutex, &blocking);
+    unsigned irq_state = irq_disable();
+    int retval = 0;
+    if (mutex->queue.next == NULL) {
+        mutex->queue.next = MUTEX_LOCKED;
+        retval = 1;
+    };
+    irq_restore(irq_state);
+    return retval;
 }
 
 /**
- * @brief Locks a mutex, blocking.
+ * @brief   Locks a mutex, blocking.
  *
- * @param[in] mutex Mutex object to lock. Has to be initialized first. Must not be NULL.
+ * @param[in,out]   mutex   Mutex object to lock.
+ *
+ * @retval  0               The mutex was locked by the caller
+ *
+ * @pre     @p mutex is not `NULL`
+ * @pre     Mutex at @p mutex has been initialized
+ * @pre     Must be called in thread context
+ *
+ * @post    The mutex @p is locked and held by the calling thread.
  */
-static inline void mutex_lock(mutex_t *mutex)
-{
-    volatile uint8_t blocking = 1;
-
-    _mutex_lock(mutex, &blocking);
-}
+int mutex_lock(mutex_t *mutex);
 
 /**
- * @brief Unlocks the mutex.
+ * @brief   Unlocks the mutex.
  *
- * @param[in] mutex Mutex object to unlock, must not be NULL.
+ * @param[in,out]   mutex   Mutex object to unlock.
+ *
+ * @pre     @p mutex is not `NULL`
+ * @note    It is safe to unlock a mutex held by a different thread.
+ * @note    It is safe to call this function from IRQ context.
  */
 void mutex_unlock(mutex_t *mutex);
 
 /**
- * @brief Unlocks the mutex and sends the current thread to sleep
+ * @brief   Unlocks the mutex and sends the current thread to sleep
  *
- * @param[in] mutex Mutex object to unlock, must not be NULL.
+ * @param[in,out]   mutex   Mutex object to unlock.
+ * @pre     @p mutex is not `NULL`
+ * @pre     Must be called in thread context.
  */
 void mutex_unlock_and_sleep(mutex_t *mutex);
 

core/mutex.c

@@ -20,8 +20,9 @@
  * @}
  */
 
-#include <stdio.h>
+#include <errno.h>
 #include <inttypes.h>
+#include <stdio.h>
 
 #include "mutex.h"
 #include "thread.h"
@@ -32,42 +33,38 @@
 #define ENABLE_DEBUG 0
 #include "debug.h"
 
-int _mutex_lock(mutex_t *mutex, volatile uint8_t *blocking)
+int mutex_lock(mutex_t *mutex)
 {
-    unsigned irqstate = irq_disable();
+    unsigned irq_state = irq_disable();
 
     DEBUG("PID[%" PRIkernel_pid "]: Mutex in use.\n", thread_getpid());
 
     if (mutex->queue.next == NULL) {
         /* mutex is unlocked. */
         mutex->queue.next = MUTEX_LOCKED;
-        DEBUG("PID[%" PRIkernel_pid "]: mutex_wait early out.\n",
+        DEBUG("PID[%" PRIkernel_pid "]: mutex_wait_and_lock early out.\n",
               thread_getpid());
-        irq_restore(irqstate);
-        return 1;
-    }
-    else if (*blocking) {
-        thread_t *me = thread_get_active();
-        DEBUG("PID[%" PRIkernel_pid "]: Adding node to mutex queue: prio: %"
-              PRIu32 "\n", thread_getpid(), (uint32_t)me->priority);
-        sched_set_status(me, STATUS_MUTEX_BLOCKED);
-        if (mutex->queue.next == MUTEX_LOCKED) {
-            mutex->queue.next = (list_node_t *)&me->rq_entry;
-            mutex->queue.next->next = NULL;
-        }
-        else {
-            thread_add_to_list(&mutex->queue, me);
-        }
-        irq_restore(irqstate);
-        thread_yield_higher();
-        /* We were woken up by scheduler. Waker removed us from queue.
-         * We have the mutex now. */
-        return 1;
-    }
-    else {
-        irq_restore(irqstate);
+        irq_restore(irq_state);
         return 0;
     }
+
+    thread_t *me = thread_get_active();
+    DEBUG("PID[%" PRIkernel_pid "]: Adding node to mutex queue: prio: %"
+          PRIu32 "\n", thread_getpid(), (uint32_t)me->priority);
+    sched_set_status(me, STATUS_MUTEX_BLOCKED);
+    if (mutex->queue.next == MUTEX_LOCKED) {
+        mutex->queue.next = (list_node_t *)&me->rq_entry;
+        mutex->queue.next->next = NULL;
+    }
+    else {
+        thread_add_to_list(&mutex->queue, me);
+    }
+
+
+    irq_restore(irq_state);
+    thread_yield_higher();
+    /* We were woken up by scheduler. Waker removed us from queue. */
+    return 0;
 }
 
 void mutex_unlock(mutex_t *mutex)