From db58a2732ba1d3ea4975b633d6f17e84242490b3 Mon Sep 17 00:00:00 2001 From: Angus Gratton Date: Thu, 16 Feb 2017 16:03:02 +1100 Subject: [PATCH] freertos: vPortCPUReleaseMutex() no longer returns a value Unlocking a never-locked mutex is an assertion failure in debug mode. In release mode, this further improves performance: No-Contention -> 153 cycles Recursion No-Contention -> 138 cycles Contention -> 378 cycles --- .../freertos/include/freertos/portmacro.h | 6 +-- components/freertos/port.c | 52 +++++++++---------- 2 files changed, 29 insertions(+), 29 deletions(-) diff --git a/components/freertos/include/freertos/portmacro.h b/components/freertos/include/freertos/portmacro.h index 63e4bda667..f55eeefe0c 100644 --- a/components/freertos/include/freertos/portmacro.h +++ b/components/freertos/include/freertos/portmacro.h @@ -127,7 +127,7 @@ typedef unsigned portBASE_TYPE UBaseType_t; typedef struct { - volatile uint32_t owner; + uint32_t owner; uint32_t count; #ifdef CONFIG_FREERTOS_PORTMUX_DEBUG const char *lastLockedFn; @@ -211,9 +211,9 @@ void vTaskExitCritical( portMUX_TYPE *mux, const char *function, int line ); void vTaskExitCritical( portMUX_TYPE *mux ); void vTaskEnterCritical( portMUX_TYPE *mux ); void vPortCPUAcquireMutex(portMUX_TYPE *mux); -portBASE_TYPE vPortCPUReleaseMutex(portMUX_TYPE *mux); +void vPortCPUReleaseMutex(portMUX_TYPE *mux); void vPortCPUAcquireMutexIntsDisabled(portMUX_TYPE *mux); -portBASE_TYPE vPortCPUReleaseMutexIntsDisabled(portMUX_TYPE *mux); +void vPortCPUReleaseMutexIntsDisabled(portMUX_TYPE *mux); #define portENTER_CRITICAL(mux) vTaskEnterCritical(mux) #define portEXIT_CRITICAL(mux) vTaskExitCritical(mux) diff --git a/components/freertos/port.c b/components/freertos/port.c index 48669472ac..fda5ed6cfd 100644 --- a/components/freertos/port.c +++ b/components/freertos/port.c @@ -308,6 +308,9 @@ void vPortCPUInitializeMutex(portMUX_TYPE *mux) { } +/* + * For kernel use: Acquire a per-CPU mux. Spinlocks, so don't hold on to these muxes for too long. + */ #ifdef CONFIG_FREERTOS_PORTMUX_DEBUG void vPortCPUAcquireMutex(portMUX_TYPE *mux, const char *fnName, int line) { unsigned int irqStatus = portENTER_CRITICAL_NESTED(); @@ -325,9 +328,6 @@ void vPortCPUAcquireMutex(portMUX_TYPE *mux) { /* XOR one core ID with this value to get the other core ID */ #define CORE_ID_XOR_SWAP (CORE_ID_PRO ^ CORE_ID_APP) -/* - * For kernel use: Acquire a per-CPU mux. Spinlocks, so don't hold on to these muxes for too long. - */ #ifdef CONFIG_FREERTOS_PORTMUX_DEBUG void vPortCPUAcquireMutexIntsDisabled(portMUX_TYPE *mux, const char *fnName, int line) { #else @@ -372,12 +372,14 @@ void vPortCPUAcquireMutexIntsDisabled(portMUX_TYPE *mux) { #endif } while (res == otherCoreID); - assert(res == coreID || res == portMUX_FREE_VAL); /* any other value implies memory corruption */ + assert(res == coreID || res == portMUX_FREE_VAL); /* any other value implies memory corruption or uninitialized mux */ assert((res == portMUX_FREE_VAL) == (mux->count == 0)); /* we're first to lock iff count is zero */ + assert(mux->count < 0xFF); /* Bad count value implies memory corruption */ /* now we own it, we can increment the refcount */ mux->count++; + #ifdef CONFIG_FREERTOS_PORTMUX_DEBUG if (res==portMUX_FREE_VAL) { //initial lock mux->lastLockedFn=fnName; @@ -390,33 +392,31 @@ void vPortCPUAcquireMutexIntsDisabled(portMUX_TYPE *mux) { #endif } +/* + * For kernel use: Release a per-CPU mux + * + * Mux must be already locked by this core + */ #ifdef CONFIG_FREERTOS_PORTMUX_DEBUG -portBASE_TYPE vPortCPUReleaseMutex(portMUX_TYPE *mux, const char *fnName, int line) { +void vPortCPUReleaseMutex(portMUX_TYPE *mux, const char *fnName, int line) { unsigned int irqStatus = portENTER_CRITICAL_NESTED(); - portBASE_TYPE res = vPortCPUReleaseMutexIntsDisabled(mux, fnName, line); + vPortCPUReleaseMutexIntsDisabled(mux, fnName, line); portEXIT_CRITICAL_NESTED(irqStatus); - return res; } #else -portBASE_TYPE vPortCPUReleaseMutex(portMUX_TYPE *mux) { +void vPortCPUReleaseMutex(portMUX_TYPE *mux) { unsigned int irqStatus = portENTER_CRITICAL_NESTED(); - portBASE_TYPE res = vPortCPUReleaseMutexIntsDisabled(mux); + vPortCPUReleaseMutexIntsDisabled(mux); portEXIT_CRITICAL_NESTED(irqStatus); - return res; } #endif -/* - * For kernel use: Release a per-CPU mux. Returns true if everything is OK, false if mux - * was already unlocked or is locked by a different core. - */ #ifdef CONFIG_FREERTOS_PORTMUX_DEBUG -portBASE_TYPE vPortCPUReleaseMutexIntsDisabled(portMUX_TYPE *mux, const char *fnName, int line) { +void vPortCPUReleaseMutexIntsDisabled(portMUX_TYPE *mux, const char *fnName, int line) { #else -portBASE_TYPE vPortCPUReleaseMutexIntsDisabled(portMUX_TYPE *mux) { +void vPortCPUReleaseMutexIntsDisabled(portMUX_TYPE *mux) { #endif #if !CONFIG_FREERTOS_UNICORE - portBASE_TYPE res; portBASE_TYPE coreID; #ifdef CONFIG_FREERTOS_PORTMUX_DEBUG const char *lastLockedFn=mux->lastLockedFn; @@ -425,20 +425,25 @@ portBASE_TYPE vPortCPUReleaseMutexIntsDisabled(portMUX_TYPE *mux) { mux->lastLockedLine=line; uint32_t owner = mux->owner; if (owner != portMUX_FREE_VAL && owner != CORE_ID_PRO && owner != CORE_ID_APP) { - ets_printf("ERROR: vPortCPUReleaseMutex: mux %p is uninitialized (0x%x)!\n", mux, mux->owner); + ets_printf("ERROR: vPortCPUReleaseMutex: mux %p is invalid (0x%x)!\n", mux, mux->owner); } #endif +#if CONFIG_FREERTOS_PORTMUX_DEBUG || !defined(NDEBUG) RSR(PRID, coreID); - res = (coreID == mux->owner); +#endif #ifdef CONFIG_FREERTOS_PORTMUX_DEBUG - if (!res) { + if (coreID != mux->owner) { ets_printf("ERROR: vPortCPUReleaseMutex: mux %p was already unlocked!\n", mux); ets_printf("Last non-recursive unlock %s line %d, curr unlock %s line %d\n", lastLockedFn, lastLockedLine, fnName, line); } #endif + assert(coreID == mux->owner); // This is a mutex we didn't lock, or it's corrupt + assert(mux->count > 0); // Indicates memory corruption + assert(mux->count < 0x100); // Indicates memory corruption + mux->count--; if(mux->count == 0) { mux->owner = portMUX_FREE_VAL; @@ -448,12 +453,7 @@ portBASE_TYPE vPortCPUReleaseMutexIntsDisabled(portMUX_TYPE *mux) { ets_printf("Recursive unlock: count=%d last locked %s line %d, curr %s line %d\n", mux->count, lastLockedFn, lastLockedLine, fnName, line); } #endif - - return res; - -#else //!CONFIG_FREERTOS_UNICORE - return 0; -#endif +#endif //!CONFIG_FREERTOS_UNICORE } #if CONFIG_FREERTOS_BREAK_ON_SCHEDULER_START_JTAG -- 2.40.0