Skip to content

Commit

Permalink
spin_lock: added new spinlock interface and decoupled it from RTOS
Browse files Browse the repository at this point in the history 
spin_lock: cleaned-up port files and removed portmux files

components/soc: decoupled compare and set operations from FreeRTOS

soc/spinlock: filled initial implementation of spinlock refactor

It will decouple the spinlocks into separated components with not depencences of freertos
an similar interface was provided focusing the readabillity and maintenance, also
naming to spinlocks were adopted. On FreeRTOS side the legacy portMUX macros
gained a form of wrapper functions that calls the spinlocks component thus
minimizing the impact on RTOS side.

This feature aims to close IDF-967

soc/spinlock: spinlocks passed on unit test, missing test corner cases

components/compare_set: added better function namings plus minor performance optimization on spinlocks

soc/spinlock: code reordering to remove ISC C90 mix error

freertos/portmacro: gor rid of critical sections multiline macros, placed inline functions instead

soc/spinlock: improved spinlock performance from internal RAM

For cases where the spinlock is executed from IRAM, there is no
need to check where the spinlock object is placed on memory,
removing this checks caused a great improvement on performance.
  • Loading branch information
@projectgus
Felipe Neves authored and projectgus committed Jan 21, 2020
1 parent a7bbc74 commit 73592d9
Show file tree
Hide file tree
Showing 12 changed files with 422 additions and 656 deletions.
29 changes: 0 additions & 29 deletions components/freertos/Kconfig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -389,35 +389,6 @@ menu "FreeRTOS"
FreeRTOS will enter light sleep mode if no tasks need to run for this number
of ticks.

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.

if !FREERTOS_UNICORE
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_UNICORE

endif # FREERTOS_DEBUG_INTERNALS

config FREERTOS_TASK_FUNCTION_WRAPPER
bool "Enclose all task functions in a wrapper function"
depends on COMPILER_OPTIMIZATION_DEFAULT
Expand Down
13 changes: 6 additions & 7 deletions components/freertos/include/freertos/portable.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -179,12 +179,6 @@ void vPortYieldOtherCore( BaseType_t coreid) PRIVILEGED_FUNCTION;
*/
void vPortSetStackWatchpoint( void* pxStackStart );

/*
* Returns true if the current core is in ISR context; low prio ISR, med prio ISR or timer tick ISR. High prio ISRs
* aren't detected here, but they normally cannot call C code, so that should not be an issue anyway.
*/
BaseType_t xPortInIsrContext(void);

/*
* This function will be called in High prio ISRs. Returns true if the current core was in ISR context
* before calling into high prio ISR context.
Expand Down Expand Up @@ -239,7 +233,12 @@ static inline bool IRAM_ATTR xPortCanYield(void)
}
#endif

void uxPortCompareSetExtram(volatile uint32_t *addr, uint32_t compare, uint32_t *set);
static inline void uxPortCompareSetExtram(volatile uint32_t *addr, uint32_t compare, uint32_t *set)
{
#if defined(CONFIG_ESP32_SPIRAM_SUPPORT)
compare_and_set_extram(addr, compare, set);
#endif
}

#endif /* PORTABLE_H */

305 changes: 113 additions & 192 deletions components/freertos/include/freertos/portmacro.h
View file

Large diffs are not rendered by default.

138 changes: 38 additions & 100 deletions components/freertos/port.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -127,10 +127,10 @@ extern void _xt_coproc_init(void);
_Static_assert(tskNO_AFFINITY == CONFIG_FREERTOS_NO_AFFINITY, "incorrect tskNO_AFFINITY value");

/*-----------------------------------------------------------*/

unsigned port_xSchedulerRunning[portNUM_PROCESSORS] = {0}; // Duplicate of inaccessible xSchedulerRunning; needed at startup to avoid counting nesting
unsigned port_interruptNesting[portNUM_PROCESSORS] = {0}; // Interrupt nesting level. Increased/decreased in portasm.c, _frxt_int_enter/_frxt_int_exit

BaseType_t port_uxCriticalNesting[portNUM_PROCESSORS] = {0};
BaseType_t port_uxOldInterruptState[portNUM_PROCESSORS] = {0};
/*-----------------------------------------------------------*/

// User exception dispatcher when exiting
Expand Down Expand Up @@ -355,74 +355,6 @@ void vPortAssertIfInISR(void)
configASSERT(xPortInIsrContext());
}

/*
* For kernel use: Initialize a per-CPU mux. Mux will be initialized unlocked.
*/
void vPortCPUInitializeMutex(portMUX_TYPE *mux) {

#ifdef CONFIG_FREERTOS_PORTMUX_DEBUG
ets_printf("Initializing mux %p\n", mux);
mux->lastLockedFn="(never locked)";
mux->lastLockedLine=-1;
#endif
mux->owner=portMUX_FREE_VAL;
mux->count=0;
}

#include "portmux_impl.h"

/*
* 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();
vPortCPUAcquireMutexIntsDisabled(mux, portMUX_NO_TIMEOUT, fnName, line);
portEXIT_CRITICAL_NESTED(irqStatus);
}

bool vPortCPUAcquireMutexTimeout(portMUX_TYPE *mux, int timeout_cycles, const char *fnName, int line) {
unsigned int irqStatus = portENTER_CRITICAL_NESTED();
bool result = vPortCPUAcquireMutexIntsDisabled(mux, timeout_cycles, fnName, line);
portEXIT_CRITICAL_NESTED(irqStatus);
return result;
}

#else
void vPortCPUAcquireMutex(portMUX_TYPE *mux) {
unsigned int irqStatus = portENTER_CRITICAL_NESTED();
vPortCPUAcquireMutexIntsDisabled(mux, portMUX_NO_TIMEOUT);
portEXIT_CRITICAL_NESTED(irqStatus);
}

bool vPortCPUAcquireMutexTimeout(portMUX_TYPE *mux, int timeout_cycles) {
unsigned int irqStatus = portENTER_CRITICAL_NESTED();
bool result = vPortCPUAcquireMutexIntsDisabled(mux, timeout_cycles);
portEXIT_CRITICAL_NESTED(irqStatus);
return result;
}
#endif


/*
* For kernel use: Release a per-CPU mux
*
* Mux must be already locked by this core
*/
#ifdef CONFIG_FREERTOS_PORTMUX_DEBUG
void vPortCPUReleaseMutex(portMUX_TYPE *mux, const char *fnName, int line) {
unsigned int irqStatus = portENTER_CRITICAL_NESTED();
vPortCPUReleaseMutexIntsDisabled(mux, fnName, line);
portEXIT_CRITICAL_NESTED(irqStatus);
}
#else
void vPortCPUReleaseMutex(portMUX_TYPE *mux) {
unsigned int irqStatus = portENTER_CRITICAL_NESTED();
vPortCPUReleaseMutexIntsDisabled(mux);
portEXIT_CRITICAL_NESTED(irqStatus);
}
#endif

void vPortSetStackWatchpoint( void* pxStackStart ) {
//Set watchpoint 1 to watch the last 32 bytes of the stack.
//Unfortunately, the Xtensa watchpoints can't set a watchpoint on a random [base - base+n] region because
Expand All @@ -435,39 +367,45 @@ void vPortSetStackWatchpoint( void* pxStackStart ) {
esp_set_watchpoint(1, (char*)addr, 32, ESP_WATCHPOINT_STORE);
}

#if defined(CONFIG_SPIRAM)
/*
* Compare & set (S32C1) does not work in external RAM. Instead, this routine uses a mux (in internal memory) to fake it.
*/
static portMUX_TYPE extram_mux = portMUX_INITIALIZER_UNLOCKED;

void uxPortCompareSetExtram(volatile uint32_t *addr, uint32_t compare, uint32_t *set) {
uint32_t prev;

uint32_t oldlevel = portENTER_CRITICAL_NESTED();
uint32_t xPortGetTickRateHz(void) {
return (uint32_t)configTICK_RATE_HZ;
}

#ifdef CONFIG_FREERTOS_PORTMUX_DEBUG
vPortCPUAcquireMutexIntsDisabled(&extram_mux, portMUX_NO_TIMEOUT, __FUNCTION__, __LINE__);
#else
vPortCPUAcquireMutexIntsDisabled(&extram_mux, portMUX_NO_TIMEOUT);
#endif
prev=*addr;
if (prev==compare) {
*addr=*set;
void __attribute__((optimize("-O3"))) vPortEnterCritical(portMUX_TYPE *mux)
{
BaseType_t oldInterruptLevel = portENTER_CRITICAL_NESTED();
/* Interrupts may already be disabled (because we're doing this recursively)
* but we can't get the interrupt level after
* vPortCPUAquireMutex, because it also may mess with interrupts.
* Get it here first, then later figure out if we're nesting
* and save for real there.
*/
vPortCPUAcquireMutex( mux );
BaseType_t coreID = xPortGetCoreID();
BaseType_t newNesting = port_uxCriticalNesting[coreID] + 1;
port_uxCriticalNesting[coreID] = newNesting;

if( newNesting == 1 )
{
//This is the first time we get called. Save original interrupt level.
port_uxOldInterruptState[coreID] = oldInterruptLevel;
}
*set=prev;
#ifdef CONFIG_FREERTOS_PORTMUX_DEBUG
vPortCPUReleaseMutexIntsDisabled(&extram_mux, __FUNCTION__, __LINE__);
#else
vPortCPUReleaseMutexIntsDisabled(&extram_mux);
#endif

portEXIT_CRITICAL_NESTED(oldlevel);
}
#endif //defined(CONFIG_SPIRAM)


void __attribute__((optimize("-O3"))) vPortExitCritical(portMUX_TYPE *mux)
{
vPortCPUReleaseMutex( mux );
BaseType_t coreID = xPortGetCoreID();
BaseType_t nesting = port_uxCriticalNesting[coreID];

if(nesting > 0U)
{
nesting--;
port_uxCriticalNesting[coreID] = nesting;

uint32_t xPortGetTickRateHz(void) {
return (uint32_t)configTICK_RATE_HZ;
if( nesting == 0U )
{
portEXIT_CRITICAL_NESTED(port_uxOldInterruptState[coreID]);
}
}
}
118 changes: 0 additions & 118 deletions components/freertos/portmux_impl.h
View file

This file was deleted.

Loading

0 comments on commit 73592d9

Please sign in to comment.