Merge branch 'feature/newlib_locking' into 'master'

newlib locking support Has matching tests here: http://gitlab.espressif.cn/idf/esp-idf-tests/commits/feature/newlib_locking_tests This is ready, the "bug" I saw was me not understanding how newlib stream locking works (I still don't understand why it works like it does, but that's less important for now.) See merge request !17
2016-09-01 16:17:22 +08:00 · 2016-09-01 16:17:22 +08:00 · fb87346864
parent 9db4732861 00ea21f736
commit fb87346864
8 changed files with 316 additions and 50 deletions
--- a/components/esp32/include/soc/cpu.h
+++ b/components/esp32/include/soc/cpu.h
@ -0,0 +1,36 @@
+// Copyright 2010-2016 Espressif Systems (Shanghai) PTE LTD
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#ifndef _SOC_CPU_H
+#define _SOC_CPU_H
+
+#include "xtensa/corebits.h"
+
+/* C macros for xtensa special register read/write/exchange */
+
+#define RSR(reg, curval)  asm volatile ("rsr %0, " #reg : "=r" (curval));
+#define WSR(reg, newval)  asm volatile ("wsr %0, " #reg : : "r" (newval));
+#define XSR(reg, swapval) asm volatile ("xsr %0, " #reg : "+r" (swapval));
+
+/* Return true if the CPU is in an interrupt context
+   (PS.UM == 0)
+*/
+static inline bool cpu_in_interrupt_context(void)
+{
+    uint32_t ps;
+    RSR(PS, ps);
+    return (ps & PS_UM) == 0;
+}
+
+#endif
--- a/components/esp32/syscalls.c
+++ b/components/esp32/syscalls.c
@ -19,13 +19,15 @@
 #include <errno.h>
 #include <sys/reent.h>
 #include <stdlib.h>
+#include "esp_attr.h"
 #include "rom/libc_stubs.h"
+#include "rom/uart.h"
+#include "soc/cpu.h"
 #include "freertos/FreeRTOS.h"
+#include "freertos/semphr.h"
 #include "freertos/portmacro.h"
 #include "freertos/task.h"

-int uart_tx_one_char(uint8_t c);
-
 void abort() {
    do
    {
@ -136,6 +138,21 @@ int _open_r(struct _reent *r, const char * path, int flags, int mode) {
 ssize_t _write_r(struct _reent *r, int fd, const void * data, size_t size) {
    const char* p = (const char*) data;
    if (fd == STDOUT_FILENO) {
+        static _lock_t stdout_lock; /* lazily initialised */
+        /* Even though newlib does stream locking on stdout, we need
+           a dedicated stdout UART lock...
+
+           This is because each task has its own _reent structure with
+           unique FILEs for stdin/stdout/stderr, so these are
+           per-thread (lazily initialised by __sinit the first time a
+           stdio function is used, see findfp.c:235.
+
+           It seems like overkill to allocate a FILE-per-task and lock
+           a thread-local stream, but I see no easy way to fix this
+           (pre-__sinit_, tasks have "fake" FILEs ie __sf_fake_stdout
+           which aren't fully valid.)
+        */
+        _lock_acquire_recursive(&stdout_lock);
        while(size--) {
 #if CONFIG_NEWLIB_STDOUT_ADDCR
            if (*p=='\n') {
@ -145,6 +162,7 @@ ssize_t _write_r(struct _reent *r, int fd, const void * data, size_t size) {
            uart_tx_one_char(*p);
            ++p;
        }
+        _lock_release_recursive(&stdout_lock);
    }
    return size;
 }
@ -158,37 +176,188 @@ ssize_t _read_r(struct _reent *r, int fd, void * dst, size_t size) {
    return 0;
 }

-// TODO: implement locks via FreeRTOS mutexes
-void _lock_init(_lock_t *lock) {
+/* Notes on our newlib lock implementation:
+ *
+ * - Use FreeRTOS mutex semaphores as locks.
+ * - lock_t is int, but we store an xSemaphoreHandle there.
+ * - Locks are no-ops until the FreeRTOS scheduler is running.
+ * - Due to this, locks need to be lazily initialised the first time
+ *   they are acquired. Initialisation/deinitialisation of locks is
+ *   protected by lock_init_spinlock.
+ * - Race conditions around lazy initialisation (via lock_acquire) are
+ *   protected against.
+ * - Anyone calling lock_close is reponsible for ensuring noone else
+ *   is holding the lock at this time.
+ * - Race conditions between lock_close & lock_init (for the same lock)
+ *   are the responsibility of the caller.
+ */
+
+static portMUX_TYPE lock_init_spinlock = portMUX_INITIALIZER_UNLOCKED;
+
+/* Initialise the given lock by allocating a new mutex semaphore
+   as the _lock_t value.
+*/
+static void IRAM_ATTR lock_init_generic(_lock_t *lock, uint8_t mutex_type) {
+    portENTER_CRITICAL(&lock_init_spinlock);
+    if (xTaskGetSchedulerState() == taskSCHEDULER_NOT_STARTED) {
+        /* nothing to do until the scheduler is running */
+        *lock = 0; /* ensure lock is zeroed out, in case it's an automatic variable */
+        portEXIT_CRITICAL(&lock_init_spinlock);
+        return;
+    }
+
+    if (*lock) {
+         /* Lock already initialised (either we didn't check earlier,
+          or it got initialised while we were waiting for the
+          spinlock.) */
+    }
+    else
+    {
+        /* Create a new semaphore
+
+           this is a bit of an API violation, as we're calling the
+           private function xQueueCreateMutex(x) directly instead of
+           the xSemaphoreCreateMutex / xSemaphoreCreateRecursiveMutex
+           wrapper functions...
+
+           The better alternative would be to pass pointers to one of
+           the two xSemaphoreCreate___Mutex functions, but as FreeRTOS
+           implements these as macros instead of inline functions
+           (*party like it's 1998!*) it's not possible to do this
+           without writing wrappers. Doing it this way seems much less
+           spaghetti-like.
+        */
+        xSemaphoreHandle new_sem = xQueueCreateMutex(mutex_type);
+        if (!new_sem) {
+            abort(); /* No more semaphores available or OOM */
+        }
+        *lock = (_lock_t)new_sem;
+    }
+    portEXIT_CRITICAL(&lock_init_spinlock);
 }

-void _lock_init_recursive(_lock_t *lock) {
+void IRAM_ATTR _lock_init(_lock_t *lock) {
+    lock_init_generic(lock, queueQUEUE_TYPE_MUTEX);
 }

-void _lock_close(_lock_t *lock) {
+void IRAM_ATTR _lock_init_recursive(_lock_t *lock) {
+    lock_init_generic(lock, queueQUEUE_TYPE_RECURSIVE_MUTEX);
 }

-void _lock_close_recursive(_lock_t *lock) {
+/* Free the mutex semaphore pointed to by *lock, and zero it out.
+
+   Note that FreeRTOS doesn't account for deleting mutexes while they
+   are held, and neither do we... so take care not to delete newlib
+   locks while they may be held by other tasks!
+*/
+void IRAM_ATTR _lock_close(_lock_t *lock) {
+    portENTER_CRITICAL(&lock_init_spinlock);
+    if (*lock) {
+        xSemaphoreHandle h = (xSemaphoreHandle)(*lock);
+#if (INCLUDE_xSemaphoreGetMutexHolder == 1)
+        configASSERT(xSemaphoreGetMutexHolder(h) == NULL); /* mutex should not be held */
+#endif
+        vSemaphoreDelete(h);
+        *lock = 0;
+    }
+    portEXIT_CRITICAL(&lock_init_spinlock);
 }

-void _lock_acquire(_lock_t *lock) {
+/* Acquire the mutex semaphore for lock. wait up to delay ticks.
+   mutex_type is queueQUEUE_TYPE_RECURSIVE_MUTEX or queueQUEUE_TYPE_MUTEX
+*/
+static int IRAM_ATTR lock_acquire_generic(_lock_t *lock, uint32_t delay, uint8_t mutex_type) {
+    xSemaphoreHandle h = (xSemaphoreHandle)(*lock);
+    if (!h) {
+        if (xTaskGetSchedulerState() == taskSCHEDULER_NOT_STARTED) {
+            return 0; /* locking is a no-op before scheduler is up, so this "succeeds" */
+        }
+        /* lazy initialise lock - might have had a static initializer in newlib (that we don't use),
+           or _lock_init might have been called before the scheduler was running... */
+        lock_init_generic(lock, mutex_type);
+        h = (xSemaphoreHandle)(*lock);
+        configASSERT(h != NULL);
+    }
+
+    BaseType_t success;
+    if (cpu_in_interrupt_context()) {
+        /* In ISR Context */
+        if (mutex_type == queueQUEUE_TYPE_RECURSIVE_MUTEX) {
+            abort(); /* recursive mutexes make no sense in ISR context */
+        }
+        BaseType_t higher_task_woken = false;
+        success = xSemaphoreTakeFromISR(h, &higher_task_woken);
+        if (!success && delay > 0) {
+            abort(); /* Tried to block on mutex from ISR, couldn't... rewrite your program to avoid libc interactions in ISRs! */
+        }
+        if (higher_task_woken) {
+            portYIELD_FROM_ISR();
+        }
+    }
+    else {
+        /* In task context */
+        if (mutex_type == queueQUEUE_TYPE_RECURSIVE_MUTEX) {
+            success = xSemaphoreTakeRecursive(h, delay);
+        } else {
+            success = xSemaphoreTake(h, delay);
+        }
+    }
+
+    return (success == pdTRUE) ? 0 : -1;
 }

-void _lock_acquire_recursive(_lock_t *lock) {
+void IRAM_ATTR _lock_acquire(_lock_t *lock) {
+    lock_acquire_generic(lock, portMAX_DELAY, queueQUEUE_TYPE_MUTEX);
 }

-int _lock_try_acquire(_lock_t *lock) {
-    return 0;
+void IRAM_ATTR _lock_acquire_recursive(_lock_t *lock) {
+    lock_acquire_generic(lock, portMAX_DELAY, queueQUEUE_TYPE_RECURSIVE_MUTEX);
 }

-int _lock_try_acquire_recursive(_lock_t *lock) {
-    return 0;
+int IRAM_ATTR _lock_try_acquire(_lock_t *lock) {
+    return lock_acquire_generic(lock, 0, queueQUEUE_TYPE_MUTEX);
 }

-void _lock_release(_lock_t *lock) {
+int IRAM_ATTR _lock_try_acquire_recursive(_lock_t *lock) {
+    return lock_acquire_generic(lock, 0, queueQUEUE_TYPE_RECURSIVE_MUTEX);
 }

-void _lock_release_recursive(_lock_t *lock) {
+/* Release the mutex semaphore for lock.
+   mutex_type is queueQUEUE_TYPE_RECURSIVE_MUTEX or queueQUEUE_TYPE_MUTEX
+*/
+static void IRAM_ATTR lock_release_generic(_lock_t *lock, uint8_t mutex_type) {
+    xSemaphoreHandle h = (xSemaphoreHandle)(*lock);
+    if (h == NULL) {
+        /* This is probably because the scheduler isn't running yet,
+           or the scheduler just started running and some code was
+           "holding" a not-yet-initialised lock... */
+        return;
+    }
+
+    if (cpu_in_interrupt_context()) {
+        if (mutex_type == queueQUEUE_TYPE_RECURSIVE_MUTEX) {
+            abort(); /* indicates logic bug, it shouldn't be possible to lock recursively in ISR */
+        }
+        BaseType_t higher_task_woken = false;
+        xSemaphoreGiveFromISR(h, &higher_task_woken);
+        if (higher_task_woken) {
+            portYIELD_FROM_ISR();
+        }
+    } else {
+        if (mutex_type == queueQUEUE_TYPE_RECURSIVE_MUTEX) {
+            xSemaphoreGiveRecursive(h);
+        } else {
+            xSemaphoreGive(h);
+        }
+    }
+}
+
+void IRAM_ATTR _lock_release(_lock_t *lock) {
+    lock_release_generic(lock, queueQUEUE_TYPE_MUTEX);
+}
+
+void IRAM_ATTR _lock_release_recursive(_lock_t *lock) {
+    lock_release_generic(lock, queueQUEUE_TYPE_RECURSIVE_MUTEX);
 }

 static struct _reent s_reent;
@ -239,7 +408,7 @@ static struct syscall_stub_table s_stub_table = {
    ._lock_init = &_lock_init,
    ._lock_init_recursive = &_lock_init_recursive,
    ._lock_close = &_lock_close,
-    ._lock_close_recursive = &_lock_close_recursive,
+    ._lock_close_recursive = &_lock_close,
    ._lock_acquire = &_lock_acquire,
    ._lock_acquire_recursive = &_lock_acquire_recursive,
    ._lock_try_acquire = &_lock_try_acquire,
--- a/components/freertos/Kconfig
+++ b/components/freertos/Kconfig
@ -121,6 +121,32 @@ config FREERTOS_DEBUG_OCDAWARE
 		The FreeRTOS panic and unhandled exception handers can detect a JTAG OCD debugger and
 		instead of panicking, have the debugger stop on the offending instruction.

+choice FREERTOS_ASSERT
+	prompt "FreeRTOS assertions"
+	default FREERTOS_ASSERT_FAIL_ABORT
+	help
+		Failed FreeRTOS configASSERT() assertions can be configured to
+		behave in different ways.
+
+config FREERTOS_ASSERT_FAIL_ABORT
+	bool "abort() on failed assertions"
+	help
+		If a FreeRTOS configASSERT() fails, FreeRTOS will abort() and
+		halt execution. The panic handler can be configured to handle
+		the outcome of an abort() in different ways.
+
+config FREERTOS_ASSERT_FAIL_PRINT_CONTINUE
+	bool "Print and continue failed assertions"
+	help
+		If a FreeRTOS assertion fails, print it out and continue.
+
+config FREERTOS_ASSERT_DISABLE
+	bool "Disable FreeRTOS assertions"
+	help
+		FreeRTOS configASSERT() will not be compiled into the binary.
+
+endchoice
+
 config FREERTOS_BREAK_ON_SCHEDULER_START_JTAG
 	bool "Stop program on scheduler start when JTAG/OCD is detected"
 	depends on FREERTOS_DEBUG_OCDAWARE
@ -129,4 +155,31 @@ config FREERTOS_BREAK_ON_SCHEDULER_START_JTAG
 		If JTAG/OCD is connected, stop execution when the scheduler is started and the first
 		task is executed.

+menuconfig FREERTOS_DEBUG_INTERNALS
+	bool "Debug FreeRTOS internals"
+	default n
+	help
+		Enable this option to show the menu with internal FreeRTOS debugging features.
+		This option does not change any code by itself, it just shows/hides some options.
+
+if FREERTOS_DEBUG_INTERNALS
+
+config FREERTOS_PORTMUX_DEBUG
+	bool "Debug portMUX portENTER_CRITICAL/portEXIT_CRITICAL"
+	depends on FREERTOS_DEBUG_INTERNALS
+	default n
+	help
+		If enabled, debug information (including integrity checks) will be printed
+	 	to UART for the port-specific MUX implementation.
+
+config FREERTOS_PORTMUX_DEBUG_RECURSIVE
+	bool "Debug portMUX Recursion"
+	depends on FREERTOS_PORTMUX_DEBUG
+	default n
+	help
+		If enabled, additional debug information will be printed for recursive
+		portMUX usage.
+
+endif # FREERTOS_DEBUG_INTERNALS
+
 endmenu
--- a/components/freertos/include/freertos/FreeRTOSConfig.h
+++ b/components/freertos/include/freertos/FreeRTOSConfig.h
@ -106,12 +106,25 @@
 #include "xtensa_config.h"


-#if 1
+/* configASSERT behaviour */
 #ifndef __ASSEMBLER__
 #include "rom/ets_sys.h"
-#define configASSERT(a) if (!(a)) ets_printf("%s:%d (%s)- assert failed!\n", __FILE__, __LINE__, __FUNCTION__)
-#endif
+
+#if defined(CONFIG_FREERTOS_ASSERT_DISABLE)
+#define configASSERT(a) /* assertions disabled */
+#elif defined(CONFIG_FREERTOS_ASSERT_FAIL_PRINT_CONTINUE)
+#define configASSERT(a) if (!(a)) {                                     \
+        ets_printf("%s:%d (%s)- assert failed!\n", __FILE__, __LINE__,  \
+                   __FUNCTION__);                                       \
+    }
+#else /* CONFIG_FREERTOS_ASSERT_FAIL_ABORT */
+#define configASSERT(a) if (!(a)) {                                     \
+        ets_printf("%s:%d (%s)- assert failed!\n", __FILE__, __LINE__,  \
+                   __FUNCTION__);                                       \
+        abort();                                                        \
+        }
 #endif
+#endif /* def __ASSEMBLER__ */


 /*-----------------------------------------------------------
@ -209,6 +222,8 @@
 #define INCLUDE_vTaskDelay					1
 #define INCLUDE_uxTaskGetStackHighWaterMark	1

+#define INCLUDE_xSemaphoreGetMutexHolder    1
+
 /* The priority at which the tick interrupt runs.  This should probably be
   kept at 1. */
 #define configKERNEL_INTERRUPT_PRIORITY		1
--- a/components/freertos/include/freertos/portmacro.h
+++ b/components/freertos/include/freertos/portmacro.h
@ -120,13 +120,12 @@ typedef unsigned portBASE_TYPE	UBaseType_t;

 #include "sdkconfig.h"

-#define portMUX_DEBUG
 #define portFIRST_TASK_HOOK CONFIG_FREERTOS_BREAK_ON_SCHEDULER_START_JTAG


 typedef struct {
 	volatile uint32_t mux;
-#ifdef portMUX_DEBUG
+#ifdef CONFIG_FREERTOS_PORTMUX_DEBUG
 	const char *lastLockedFn;
 	int lastLockedLine;
 #endif
@ -151,7 +150,7 @@ typedef struct {
 #define portMUX_VAL_SHIFT		0

 //Keep this in sync with the portMUX_TYPE struct definition please.
-#ifdef portMUX_DEBUG
+#ifndef CONFIG_FREERTOS_PORTMUX_DEBUG
 #define portMUX_INITIALIZER_UNLOCKED { 					\
 		.mux = portMUX_MAGIC_VAL|portMUX_FREE_VAL 		\
 	}
@ -170,13 +169,6 @@ typedef struct {

 #define portCRITICAL_NESTING_IN_TCB 1 

-
-/*
-Enable this to enable mux debugging. With this on, the mux code will warn you for deadlocks
-and double releases etc.
-*/
-#define portMUX_DEBUG
-
 /*
 Modifications to portENTER_CRITICAL:

@ -198,7 +190,7 @@ This all assumes that interrupts are either entirely disabled or enabled. Interr
 will break this scheme.
 */
 void vPortCPUInitializeMutex(portMUX_TYPE *mux);
-#ifdef portMUX_DEBUG
+#ifdef CONFIG_FREERTOS_PORTMUX_DEBUG
 void vPortCPUAcquireMutex(portMUX_TYPE *mux, const char *function, int line);
 portBASE_TYPE vPortCPUReleaseMutex(portMUX_TYPE *mux, const char *function, int line);
 void vTaskEnterCritical( portMUX_TYPE *mux, const char *function, int line );
--- a/components/freertos/port.c
+++ b/components/freertos/port.c
@ -251,7 +251,7 @@ void vPortStoreTaskMPUSettings( xMPU_SETTINGS *xMPUSettings, const struct xMEMOR

 /*
 * Wrapper for the Xtensa compare-and-set instruction. This subroutine will atomically compare
- * *addr to compare, and if it's the same, will set *addr to set. It will return the old value 
+ * *mux to compare, and if it's the same, will set *mux to set. It will return the old value
 * of *addr.
 *
 * Note: the NOPs are needed on the ESP31 processor but superfluous on the ESP32.
@ -276,7 +276,7 @@ uint32_t uxPortCompareSet(volatile uint32_t *mux, uint32_t compare, uint32_t set
 * For kernel use: Initialize a per-CPU mux. Mux will be initialized unlocked.
 */
 void vPortCPUInitializeMutex(portMUX_TYPE *mux) {
-#ifdef portMUX_DEBUG
+#ifdef CONFIG_FREERTOS_PORTMUX_DEBUG
 	ets_printf("Initializing mux %p\n", mux);
 	mux->lastLockedFn="(never locked)";
 	mux->lastLockedLine=-1;
@ -288,7 +288,7 @@ 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 portMUX_DEBUG
+#ifdef CONFIG_FREERTOS_PORTMUX_DEBUG
 void vPortCPUAcquireMutex(portMUX_TYPE *mux, const char *fnName, int line) {
 #else
 void vPortCPUAcquireMutex(portMUX_TYPE *mux) {
@ -296,7 +296,7 @@ void vPortCPUAcquireMutex(portMUX_TYPE *mux) {
 	uint32_t res;
 	uint32_t recCnt;
 	unsigned int irqStatus;
-#ifdef portMUX_DEBUG
+#ifdef CONFIG_FREERTOS_PORTMUX_DEBUG
 	uint32_t cnt=(1<<16);
 	if ( (mux->mux & portMUX_MAGIC_MASK) != portMUX_MAGIC_VAL ) {
 		ets_printf("ERROR: vPortCPUAcquireMutex: mux %p is uninitialized (0x%X)! Called from %s line %d.\n", mux, mux->mux, fnName, line);
@ -313,21 +313,21 @@ void vPortCPUAcquireMutex(portMUX_TYPE *mux) {
 			//Mux was already locked by us. Just bump the recurse count by one.
 			recCnt=(res&portMUX_CNT_MASK)>>portMUX_CNT_SHIFT;
 			recCnt++;
-#ifdef portMUX_DEBUG_RECURSIVE
+#ifdef CONFIG_FREERTOS_PORTMUX_DEBUG_RECURSIVE
 			ets_printf("Recursive lock: recCnt=%d last non-recursive lock %s line %d, curr %s line %d\n", recCnt, mux->lastLockedFn, mux->lastLockedLine, fnName, line);
 #endif
 			mux->mux=portMUX_MAGIC_VAL|(recCnt<<portMUX_CNT_SHIFT)|(xPortGetCoreID()<<portMUX_VAL_SHIFT);
 			break;
 		}
-		cnt--;
-#ifdef portMUX_DEBUG
+#ifdef CONFIG_FREERTOS_PORTMUX_DEBUG
+        cnt--;
 		if (cnt==0) {
 			ets_printf("Timeout on mux! last non-recursive lock %s line %d, curr %s line %d\n", mux->lastLockedFn, mux->lastLockedLine, fnName, line);
 			ets_printf("Mux value %X\n", mux->mux);
 		}
 #endif
 	} while (res!=portMUX_FREE_VAL);
-#ifdef portMUX_DEBUG
+#ifdef CONFIG_FREERTOS_PORTMUX_DEBUG
 	if (res==portMUX_FREE_VAL) { //initial lock
 		mux->lastLockedFn=fnName;
 		mux->lastLockedLine=line;
@ -340,7 +340,7 @@ void vPortCPUAcquireMutex(portMUX_TYPE *mux) {
 * 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 portMUX_DEBUG
+#ifdef CONFIG_FREERTOS_PORTMUX_DEBUG
 portBASE_TYPE vPortCPUReleaseMutex(portMUX_TYPE *mux, const char *fnName, int line) {
 #else
 portBASE_TYPE vPortCPUReleaseMutex(portMUX_TYPE *mux) {
@ -351,7 +351,7 @@ portBASE_TYPE vPortCPUReleaseMutex(portMUX_TYPE *mux) {
 	portBASE_TYPE ret=pdTRUE;
 //	ets_printf("Unlock %p\n", mux);
 	irqStatus=portENTER_CRITICAL_NESTED();
-#ifdef portMUX_DEBUG
+#ifdef CONFIG_FREERTOS_PORTMUX_DEBUG
 	const char *lastLockedFn=mux->lastLockedFn;
 	int lastLockedLine=mux->lastLockedLine;
 	mux->lastLockedFn=fnName;
@ -362,13 +362,13 @@ portBASE_TYPE vPortCPUReleaseMutex(portMUX_TYPE *mux) {
 	res=uxPortCompareSet(&mux->mux, (xPortGetCoreID()<<portMUX_VAL_SHIFT)|portMUX_MAGIC_VAL, portMUX_FREE_VAL);

 	if ( res == portMUX_FREE_VAL ) {
-#ifdef portMUX_DEBUG
+#ifdef CONFIG_FREERTOS_PORTMUX_DEBUG
 		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
 		ret=pdFALSE;
 	} else if ( ((res&portMUX_VAL_MASK)>>portMUX_VAL_SHIFT) != xPortGetCoreID() ) {
-#ifdef portMUX_DEBUG
+#ifdef CONFIG_FREERTOS_PORTMUX_DEBUG
 		ets_printf("ERROR: vPortCPUReleaseMutex: mux %p wasn't locked by this core (%d) but by core %d (ret=%x, mux=%x).\n", mux, xPortGetCoreID(), ((res&portMUX_VAL_MASK)>>portMUX_VAL_SHIFT), res, mux->mux);
 		ets_printf("Last non-recursive lock %s line %d\n", lastLockedFn, lastLockedLine);
 		ets_printf("Called by %s line %d\n", fnName, line);
@ -378,7 +378,7 @@ portBASE_TYPE vPortCPUReleaseMutex(portMUX_TYPE *mux) {
 		//We locked this, but the reccount isn't zero. Decrease refcount and continue.
 		recCnt=(res&portMUX_CNT_MASK)>>portMUX_CNT_SHIFT;
 		recCnt--;
-#ifdef portMUX_DEBUG_RECURSIVE
+#ifdef CONFIG_FREERTOS_PORTMUX_DEBUG_RECURSIVE
 		ets_printf("Recursive unlock: recCnt=%d last locked %s line %d, curr %s line %d\n", recCnt, lastLockedFn, lastLockedLine, fnName, line);
 #endif
 		mux->mux=portMUX_MAGIC_VAL|(recCnt<<portMUX_CNT_SHIFT)|(xPortGetCoreID()<<portMUX_VAL_SHIFT);
--- a/components/freertos/queue.c
+++ b/components/freertos/queue.c
@ -483,14 +483,15 @@ int8_t *pcAllocatedBuffer;

 	void* xQueueGetMutexHolder( QueueHandle_t xSemaphore )
 	{
-	void *pxReturn;
+		Queue_t * const pxQueue = ( Queue_t * ) xSemaphore;
+		void *pxReturn;

 		/* This function is called by xSemaphoreGetMutexHolder(), and should not
 		be called directly.  Note:  This is a good way of determining if the
 		calling task is the mutex holder, but not a good way of determining the
 		identity of the mutex holder, as the holder may change between the
 		following critical section exiting and the function returning. */
-		taskENTER_CRITICAL();
+		taskENTER_CRITICAL(&pxQueue->mux);
 		{
 			if( ( ( Queue_t * ) xSemaphore )->uxQueueType == queueQUEUE_IS_MUTEX )
 			{
@ -501,7 +502,7 @@ int8_t *pcAllocatedBuffer;
 				pxReturn = NULL;
 			}
 		}
-		taskEXIT_CRITICAL();
+		taskEXIT_CRITICAL(&pxQueue->mux);

 		return pxReturn;
 	} /*lint !e818 xSemaphore cannot be a pointer to const because it is a typedef. */
--- a/components/freertos/tasks.c
+++ b/components/freertos/tasks.c
@ -3754,14 +3754,14 @@ scheduler will re-enable the interrupts instead. */
 #if ( portCRITICAL_NESTING_IN_TCB == 1 )


-#ifdef portMUX_DEBUG
+#ifdef CONFIG_FREERTOS_PORTMUX_DEBUG
 	void vTaskEnterCritical( portMUX_TYPE *mux, const char *function, int line )
 #else
 	void vTaskEnterCritical( portMUX_TYPE *mux )
 #endif
 	{
 		portDISABLE_INTERRUPTS();
-#ifdef portMUX_DEBUG
+#ifdef CONFIG_FREERTOS_PORTMUX_DEBUG
 		vPortCPUAcquireMutex( mux, function, line );
 #else
 		vPortCPUAcquireMutex( mux );
@ -3794,13 +3794,13 @@ scheduler will re-enable the interrupts instead. */

 #if ( portCRITICAL_NESTING_IN_TCB == 1 )

-#ifdef portMUX_DEBUG
+#ifdef CONFIG_FREERTOS_PORTMUX_DEBUG
 	void vTaskExitCritical( portMUX_TYPE *mux, const char *function, int line )
 #else
 	void vTaskExitCritical( portMUX_TYPE *mux )
 #endif
 	{
-#ifdef portMUX_DEBUG
+#ifdef CONFIG_FREERTOS_PORTMUX_DEBUG
 		vPortCPUReleaseMutex( mux, function, line );
 #else
 		vPortCPUReleaseMutex( mux );