From 2d2bb4b3080c11dab1880057c09575c61e4612e4 Mon Sep 17 00:00:00 2001
From: Marian Buschsieweke <marian.buschsieweke@ovgu.de>
Date: Wed, 21 Sep 2022 13:17:22 +0200
Subject: [PATCH] core/mutex: clean up

This restores a pre-existing design decision to implement both
blocking and non-blocking mutex locking with the same code. Those
implementations have been split prior to the introduction of
the `core_mutex_priority_inheritance` module when `mutex_trylock()`
indeed was trivial. This decision didn't age well, so undo it.
---
 core/include/mutex.h | 80 ++++++++++++++++----------------------------
 core/mutex.c         | 17 +++++-----
 2 files changed, 37 insertions(+), 60 deletions(-)

diff --git a/core/include/mutex.h b/core/include/mutex.h
index 0633f5777f..ac05572c55 100644
--- a/core/include/mutex.h
+++ b/core/include/mutex.h
@@ -110,15 +110,12 @@
 
 #include <stddef.h>
 #include <stdint.h>
+#include <stdbool.h>
 
 #include "kernel_defines.h"
 #include "list.h"
 #include "thread.h"
 
-#ifndef __cplusplus
-#include "irq.h"
-#endif
-
 #ifdef __cplusplus
 extern "C" {
 #endif
@@ -153,6 +150,27 @@ typedef struct {
 #endif
 } mutex_t;
 
+/**
+ * @brief   Internal function implementing @ref mutex_lock and
+ *          @ref mutex_trylock
+ *
+ * @details Do not call this function, use @ref mutex_lock or @ref mutex_trylock
+ *          instead
+ *
+ * @param[in,out]   mutex   Mutex object to lock.
+ * @param[in]       block   Whether to block
+ *
+ * @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.
+ *
+ * @retval  true    Mutex obtained
+ * @retval  false   Mutex not obtained (only possible if @p block is `false`)
+ */
+bool mutex_lock_internal(mutex_t *mutex, bool block);
+
 /**
  * @brief   A cancellation structure for use with @ref mutex_lock_cancelable
  *          and @ref mutex_cancel
@@ -219,25 +237,6 @@ static inline mutex_cancel_t mutex_cancel_init(mutex_t *mutex)
     return result;
 }
 
-/**
- * @brief   Tries to get a mutex, non-blocking.
- *
- * @internal
- * @note    This function is intended for use by languages incompatible
- *          with C (such as C++). Code in C should use @ref mutex_trylock
- *          instead
- *
- * @param[in,out]   mutex   Mutex object to lock.
- *
- * @retval  1               if mutex was unlocked, now it is locked.
- * @retval  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
- */
-int mutex_trylock_ffi(mutex_t *mutex);
-
 /**
  * @brief   Tries to get a mutex, non-blocking.
  *
@@ -252,28 +251,7 @@ int mutex_trylock_ffi(mutex_t *mutex);
  */
 static inline int mutex_trylock(mutex_t *mutex)
 {
-#ifdef __cplusplus
-    return mutex_trylock_ffi(mutex);
-#else
-    unsigned irq_state = irq_disable();
-    int retval = 0;
-
-    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);
-    return retval;
-#endif
+    return mutex_lock_internal(mutex, false);
 }
 
 /**
@@ -287,14 +265,12 @@ static inline int mutex_trylock(mutex_t *mutex)
  *
  * @post    The mutex @p is locked and held by the calling thread.
  */
-#if (MAXTHREADS > 1) || DOXYGEN
-void mutex_lock(mutex_t *mutex);
-#else
-/**
- * @brief   dummy implementation for when no scheduler is used
- */
 static inline void mutex_lock(mutex_t *mutex)
 {
+#if (MAXTHREADS > 1)
+    mutex_lock_internal(mutex, true);
+#else
+    /* dummy implementation for when no scheduler is used */
     /* (ab)use next pointer as lock variable */
     volatile uintptr_t *lock = (void *)&mutex->queue.next;
 
@@ -310,8 +286,8 @@ static inline void mutex_lock(mutex_t *mutex)
 
     /* set lock variable */
     *lock = 1;
-}
 #endif
+}
 
 /**
  * @brief   Locks a mutex, blocking. This function can be canceled.
diff --git a/core/mutex.c b/core/mutex.c
index 8d7c283aea..a2529ac34e 100644
--- a/core/mutex.c
+++ b/core/mutex.c
@@ -82,11 +82,12 @@ static inline __attribute__((always_inline)) void _block(mutex_t *mutex,
     /* We were woken up by scheduler. Waker removed us from queue. */
 }
 
-void mutex_lock(mutex_t *mutex)
+bool mutex_lock_internal(mutex_t *mutex, bool block)
 {
     unsigned irq_state = irq_disable();
 
-    DEBUG("PID[%" PRIkernel_pid "] mutex_lock().\n", thread_getpid());
+    DEBUG("PID[%" PRIkernel_pid "] mutex_lock_internal(block=%u).\n",
+          thread_getpid(), (unsigned)block);
 
     if (mutex->queue.next == NULL) {
         /* mutex is unlocked. */
@@ -101,8 +102,14 @@ void mutex_lock(mutex_t *mutex)
         irq_restore(irq_state);
     }
     else {
+        if (!block) {
+            irq_restore(irq_state);
+            return false;
+        }
         _block(mutex, irq_state);
     }
+
+    return true;
 }
 
 int mutex_lock_cancelable(mutex_cancel_t *mc)
@@ -254,12 +261,6 @@ void mutex_cancel(mutex_cancel_t *mc)
     irq_restore(irq_state);
 }
 
-/* Helper for compatibility with C++ or other non-C languages */
-int mutex_trylock_ffi(mutex_t *mutex)
-{
-    return mutex_trylock(mutex);
-}
-
 #else /* MAXTHREADS < 2 */
 typedef int dont_be_pedantic;
 #endif