diff --git a/demos/rt/RT-STM32F303-DISCOVERY/chconf.h b/demos/rt/RT-STM32F303-DISCOVERY/chconf.h index 8a2d5efa1..491c2dfdd 100644 --- a/demos/rt/RT-STM32F303-DISCOVERY/chconf.h +++ b/demos/rt/RT-STM32F303-DISCOVERY/chconf.h @@ -183,6 +183,15 @@ */ #define CH_CFG_USE_MUTEXES TRUE +/** + * @brief Enables recursive behavior on mutexes. + * @note Recursive mutexes are heavier and have an increased + * memory footprint. + * + * @note The default is @p FALSE. + */ +#define CH_CFG_USE_MUTEXES_RECURSIVE TRUE + /** * @brief Conditional Variables APIs. * @details If enabled then the conditional variables APIs are included diff --git a/os/hal/osal/rt/osal.h b/os/hal/osal/rt/osal.h index 74044616a..ab648d720 100644 --- a/os/hal/osal/rt/osal.h +++ b/os/hal/osal/rt/osal.h @@ -839,8 +839,7 @@ static inline void osalMutexLock(mutex_t *mp) { static inline void osalMutexUnlock(mutex_t *mp) { #if CH_CFG_USE_MUTEXES - (void)mp; - chMtxUnlock(); + chMtxUnlock(mp); #elif CH_CFG_USE_SEMAPHORES chSemSignal((semaphore_t *)mp); #else diff --git a/os/rt/include/chmtx.h b/os/rt/include/chmtx.h index 0444d936c..a85fb84e7 100644 --- a/os/rt/include/chmtx.h +++ b/os/rt/include/chmtx.h @@ -62,6 +62,9 @@ struct mutex { @p NULL. */ mutex_t *m_next; /**< @brief Next @p mutex_t into an owner-list or @p NULL. */ +#if CH_CFG_USE_MUTEXES_RECURSIVE || defined(__DOXYGEN__) + cnt_t m_taken; /**< @brief Mutex recursion counter. */ +#endif }; /*===========================================================================*/ @@ -75,7 +78,11 @@ struct mutex { * * @param[in] name the name of the mutex variable */ +#if CH_CFG_USE_MUTEXES_RECURSIVE || defined(__DOXYGEN__) +#define _MUTEX_DATA(name) {_threads_queue_t_DATA(name.m_queue), NULL, NULL, 0} +#else #define _MUTEX_DATA(name) {_threads_queue_t_DATA(name.m_queue), NULL, NULL} +#endif /** * @brief Static mutex initializer. @@ -99,8 +106,8 @@ extern "C" { void chMtxLockS(mutex_t *mp); bool chMtxTryLock(mutex_t *mp); bool chMtxTryLockS(mutex_t *mp); - mutex_t *chMtxUnlock(void); - mutex_t *chMtxUnlockS(void); + void chMtxUnlock(mutex_t *mp); + void chMtxUnlockS(mutex_t *mp); void chMtxUnlockAll(void); #ifdef __cplusplus } @@ -124,6 +131,17 @@ static inline bool chMtxQueueNotEmptyS(mutex_t *mp) { return queue_notempty(&mp->m_queue); } +/** + * @brief Returns the next mutex in the mutexes stack of the current thread. + * + * @return A pointer to the next mutex in the stack. + * @retval NULL if the stack is empty. + */ +static inline mutex_t *chMtxGetNextMutex(void) { + + return chThdGetSelfX()->p_mtxlist; +} + #endif /* CH_CFG_USE_MUTEXES */ #endif /* _CHMTX_H_ */ diff --git a/os/rt/src/chcond.c b/os/rt/src/chcond.c index cc3ee2e40..18c0e1c8c 100644 --- a/os/rt/src/chcond.c +++ b/os/rt/src/chcond.c @@ -210,7 +210,8 @@ msg_t chCondWaitS(condition_variable_t *cp) { chDbgCheck(cp != NULL); chDbgAssert(ctp->p_mtxlist != NULL, "not owning a mutex"); - mp = chMtxUnlockS(); + mp = chMtxGetNextMutex(); + chMtxUnlockS(mp); ctp->p_u.wtobjp = cp; queue_prio_insert(ctp, &cp->c_queue); chSchGoSleepS(CH_STATE_WTCOND); @@ -293,7 +294,8 @@ msg_t chCondWaitTimeoutS(condition_variable_t *cp, systime_t time) { chDbgCheck((cp != NULL) && (time != TIME_IMMEDIATE)); chDbgAssert(currp->p_mtxlist != NULL, "not owning a mutex"); - mp = chMtxUnlockS(); + mp = chMtxGetNextMutex(); + chMtxUnlockS(mp); currp->p_u.wtobjp = cp; queue_prio_insert(currp, &cp->c_queue); msg = chSchGoSleepTimeoutS(CH_STATE_WTCOND, time); diff --git a/os/rt/src/chheap.c b/os/rt/src/chheap.c index abda4ff10..5e1e20c3c 100644 --- a/os/rt/src/chheap.c +++ b/os/rt/src/chheap.c @@ -47,7 +47,7 @@ */ #if CH_CFG_USE_MUTEXES || defined(__DOXYGEN__) #define H_LOCK(h) chMtxLock(&(h)->h_mtx) -#define H_UNLOCK(h) chMtxUnlock() +#define H_UNLOCK(h) chMtxUnlock(&(h)->h_mtx) #else #define H_LOCK(h) chSemWait(&(h)->h_sem) #define H_UNLOCK(h) chSemSignal(&(h)->h_sem) diff --git a/os/rt/src/chmtx.c b/os/rt/src/chmtx.c index 001fa1dc3..cf0f2e7b3 100644 --- a/os/rt/src/chmtx.c +++ b/os/rt/src/chmtx.c @@ -41,12 +41,18 @@ * owner of the mutex. * . *

Constraints

- * In ChibiOS/RT the Unlock operations are always performed in - * lock-reverse order. The unlock API does not even have a parameter, - * the mutex to unlock is selected from an internal, per-thread, stack - * of owned mutexes. This both improves the performance and is - * required for an efficient implementation of the priority - * inheritance mechanism. + * In ChibiOS/RT the Unlock operations must always be performed + * in lock-reverse order. This restriction both improves the + * performance and is required for an efficient implementation + * of the priority inheritance mechanism.
+ * Operating under this restriction also ensures that deadlocks + * are no possible. + * + *

Recursive mode

+ * By default mutexes are not recursive, this mean that it is not + * possible to take a mutex already owned by the same thread. + * It is possible to enable the recursive behavior by enabling the + * option @p CH_CFG_USE_MUTEXES_RECURSIVE. * *

The priority inversion problem

* The mutexes in ChibiOS/RT implements the full priority @@ -270,39 +276,40 @@ bool chMtxTryLockS(mutex_t *mp) { * @post The mutex is unlocked and removed from the per-thread stack of * owned mutexes. * - * @return A pointer to the unlocked mutex. + * @param[in] mp pointer to the @p mutex_t structure * * @api */ -mutex_t *chMtxUnlock(void) { +void chMtxUnlock(mutex_t *mp) { thread_t *ctp = currp; - mutex_t *ump, *mp; + mutex_t *lmp; chSysLock(); chDbgAssert(ctp->p_mtxlist != NULL, "owned mutexes list empty"); + chDbgAssert(ctp->p_mtxlist != mp, "not next in list"); chDbgAssert(ctp->p_mtxlist->m_owner == ctp, "ownership failure"); - /* Removes the top mutex from the thread's owned mutexes list and marks it - as not owned.*/ - ump = ctp->p_mtxlist; - ctp->p_mtxlist = ump->m_next; + /* Removes the top mutex from the thread's owned mutexes list and marks + it as not owned. Note, it is assumed to be the same mutex passed as + parameter of this function.*/ + ctp->p_mtxlist = mp->m_next; /* If a thread is waiting on the mutex then the fun part begins.*/ - if (chMtxQueueNotEmptyS(ump)) { + if (chMtxQueueNotEmptyS(mp)) { thread_t *tp; /* Recalculates the optimal thread priority by scanning the owned mutexes list.*/ tprio_t newprio = ctp->p_realprio; - mp = ctp->p_mtxlist; - while (mp != NULL) { + lmp = ctp->p_mtxlist; + while (lmp != NULL) { /* If the highest priority thread waiting in the mutexes list has a greater priority than the current thread base priority then the final priority will have at least that priority.*/ - if (chMtxQueueNotEmptyS(mp) && (mp->m_queue.p_next->p_prio > newprio)) - newprio = mp->m_queue.p_next->p_prio; - mp = mp->m_next; + if (chMtxQueueNotEmptyS(lmp) && (lmp->m_queue.p_next->p_prio > newprio)) + newprio = lmp->m_queue.p_next->p_prio; + lmp = lmp->m_next; } /* Assigns to the current thread the highest priority among all the @@ -311,16 +318,16 @@ mutex_t *chMtxUnlock(void) { /* Awakens the highest priority thread waiting for the unlocked mutex and assigns the mutex to it.*/ - tp = queue_fifo_remove(&ump->m_queue); - ump->m_owner = tp; - ump->m_next = tp->p_mtxlist; - tp->p_mtxlist = ump; + tp = queue_fifo_remove(&mp->m_queue); + mp->m_owner = tp; + mp->m_next = tp->p_mtxlist; + tp->p_mtxlist = mp; chSchWakeupS(tp, MSG_OK); } else - ump->m_owner = NULL; + mp->m_owner = NULL; + chSysUnlock(); - return ump; } /** @@ -331,53 +338,54 @@ mutex_t *chMtxUnlock(void) { * @post This function does not reschedule so a call to a rescheduling * function must be performed before unlocking the kernel. * - * @return A pointer to the unlocked mutex. + * @param[in] mp pointer to the @p mutex_t structure * * @sclass */ -mutex_t *chMtxUnlockS(void) { +void chMtxUnlockS(mutex_t *mp) { thread_t *ctp = currp; - mutex_t *ump, *mp; + mutex_t *lmp; - chDbgCheckClassS(); chDbgAssert(ctp->p_mtxlist != NULL, "owned mutexes list empty"); + chDbgAssert(ctp->p_mtxlist != mp, "not next in list"); chDbgAssert(ctp->p_mtxlist->m_owner == ctp, "ownership failure"); - /* Removes the top mutex from the owned mutexes list and marks it as not - owned.*/ - ump = ctp->p_mtxlist; - ctp->p_mtxlist = ump->m_next; + /* Removes the top mutex from the thread's owned mutexes list and marks + it as not owned. Note, it is assumed to be the same mutex passed as + parameter of this function.*/ + ctp->p_mtxlist = mp->m_next; /* If a thread is waiting on the mutex then the fun part begins.*/ - if (chMtxQueueNotEmptyS(ump)) { + if (chMtxQueueNotEmptyS(mp)) { thread_t *tp; /* Recalculates the optimal thread priority by scanning the owned mutexes list.*/ tprio_t newprio = ctp->p_realprio; - mp = ctp->p_mtxlist; - while (mp != NULL) { + lmp = ctp->p_mtxlist; + while (lmp != NULL) { /* If the highest priority thread waiting in the mutexes list has a greater priority than the current thread base priority then the final priority will have at least that priority.*/ - if (chMtxQueueNotEmptyS(mp) && (mp->m_queue.p_next->p_prio > newprio)) - newprio = mp->m_queue.p_next->p_prio; - - mp = mp->m_next; + if (chMtxQueueNotEmptyS(lmp) && (lmp->m_queue.p_next->p_prio > newprio)) + newprio = lmp->m_queue.p_next->p_prio; + lmp = lmp->m_next; } + + /* Assigns to the current thread the highest priority among all the + waiting threads.*/ ctp->p_prio = newprio; /* Awakens the highest priority thread waiting for the unlocked mutex and assigns the mutex to it.*/ - tp = queue_fifo_remove(&ump->m_queue); - ump->m_owner = tp; - ump->m_next = tp->p_mtxlist; - tp->p_mtxlist = ump; + tp = queue_fifo_remove(&mp->m_queue); + mp->m_owner = tp; + mp->m_next = tp->p_mtxlist; + tp->p_mtxlist = mp; chSchReadyI(tp); } else - ump->m_owner = NULL; - return ump; + mp->m_owner = NULL; } /** diff --git a/test/rt/testbmk.c b/test/rt/testbmk.c index 2a3b7c1b5..4f648fa14 100644 --- a/test/rt/testbmk.c +++ b/test/rt/testbmk.c @@ -601,13 +601,13 @@ static void bmk12_execute(void) { test_start_timer(1000); do { chMtxLock(&mtx1); - chMtxUnlock(); + chMtxUnlock(&mtx1); chMtxLock(&mtx1); - chMtxUnlock(); + chMtxUnlock(&mtx1); chMtxLock(&mtx1); - chMtxUnlock(); + chMtxUnlock(&mtx1); chMtxLock(&mtx1); - chMtxUnlock(); + chMtxUnlock(&mtx1); n++; #if defined(SIMULATOR) ChkIntSources(); diff --git a/test/rt/testmtx.c b/test/rt/testmtx.c index c9ba9e093..f2a3c5f5a 100644 --- a/test/rt/testmtx.c +++ b/test/rt/testmtx.c @@ -90,7 +90,7 @@ static msg_t thread1(void *p) { chMtxLock(&m1); test_emit_token(*(char *)p); - chMtxUnlock(); + chMtxUnlock(&m1); return 0; } @@ -103,7 +103,7 @@ static void mtx1_execute(void) { threads[2] = chThdCreateStatic(wa[2], WA_SIZE, prio+3, thread1, "C"); threads[3] = chThdCreateStatic(wa[3], WA_SIZE, prio+4, thread1, "B"); threads[4] = chThdCreateStatic(wa[4], WA_SIZE, prio+5, thread1, "A"); - chMtxUnlock(); + chMtxUnlock(&m1); test_wait_threads(); test_assert(1, prio == chThdGetPriorityX(), "wrong priority level"); test_assert_sequence(2, "ABCDE"); @@ -349,7 +349,7 @@ static msg_t thread4a(void *p) { (void)p; chThdSleepMilliseconds(50); chMtxLock(&m2); - chMtxUnlock(); + chMtxUnlock(&m2); return 0; } @@ -358,7 +358,7 @@ static msg_t thread4b(void *p) { (void)p; chThdSleepMilliseconds(150); chMtxLock(&m1); - chMtxUnlock(); + chMtxUnlock(&m1); return 0; } @@ -378,7 +378,7 @@ static void mtx4_execute(void) { test_assert(3, chThdGetPriorityX() == p1, "wrong priority level"); chThdSleepMilliseconds(100); test_assert(4, chThdGetPriorityX() == p2, "wrong priority level"); - chMtxUnlock(); + chMtxUnlock(&m1); test_assert(5, chThdGetPriorityX() == p1, "wrong priority level"); chThdSleepMilliseconds(100); test_assert(6, chThdGetPriorityX() == p1, "wrong priority level"); @@ -398,7 +398,7 @@ static void mtx4_execute(void) { chThdSleepMilliseconds(100); test_assert(11, chThdGetPriorityX() == p2, "wrong priority level"); chSysLock(); - chMtxUnlockS(); + chMtxUnlockS(&m1); chSchRescheduleS(); chSysUnlock(); test_assert(12, chThdGetPriorityX() == p1, "wrong priority level"); @@ -444,7 +444,7 @@ static void mtx5_execute(void) { test_assert(2, !b, "not locked"); chSysLock(); - chMtxUnlockS(); + chMtxUnlockS(&m1); chSysUnlock(); test_assert(3, queue_isempty(&m1.m_queue), "queue not empty"); @@ -487,7 +487,7 @@ static msg_t thread10(void *p) { chMtxLock(&m1); chCondWait(&c1); test_emit_token(*(char *)p); - chMtxUnlock(); + chMtxUnlock(&m1); return 0; } @@ -580,8 +580,8 @@ static msg_t thread11(void *p) { chCondWait(&c1); #endif test_emit_token(*(char *)p); - chMtxUnlock(); - chMtxUnlock(); + chMtxUnlock(&m1); + chMtxUnlock(&m2); return 0; } @@ -589,7 +589,7 @@ static msg_t thread12(void *p) { chMtxLock(&m2); test_emit_token(*(char *)p); - chMtxUnlock(); + chMtxUnlock(&m2); return 0; }