Skip to content

Commit

Permalink
core/mutex: clean up
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
maribu committed Sep 21, 2022
1 parent 4c1684f commit 5ff92dc
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 59 deletions.
80 changes: 28 additions & 52 deletions core/include/mutex.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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.
*
Expand All @@ -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);
}

/**
Expand All @@ -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;

Expand All @@ -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.
Expand Down
14 changes: 7 additions & 7 deletions core/mutex.c
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ 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();

Expand All @@ -101,8 +101,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)
Expand Down Expand Up @@ -254,12 +260,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

0 comments on commit 5ff92dc

Please # to comment.