Skip to content

Commit

Permalink
Merge branch 'bugfix/fix_stack_overflow_on_s3_with_freertos_smp' into…
Browse files Browse the repository at this point in the history 
… 'master'

freertos-smp: Fixed stack overflow on esp32s3 with FreeRTOS SMP

Closes IDF-5509 and IDF-4982

See merge request espressif/esp-idf!20932
  • Loading branch information
Zim Kalinowski committed Nov 7, 2022
2 parents 48e2f38 + 9bb8f0e commit 98ff59b
Show file tree
Hide file tree
Showing 7 changed files with 88 additions and 79 deletions.
16 changes: 1 addition & 15 deletions components/freertos/FreeRTOS-Kernel-SMP/portable/riscv/include/freertos/FreeRTOSConfig_smp.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ This file get's pulled into assembly sources. Therefore, some includes need to b
#define configCPU_CLOCK_HZ (CONFIG_ESP_DEFAULT_CPU_FREQ_MHZ * 1000000)
#define configTICK_RATE_HZ CONFIG_FREERTOS_HZ
#define configMAX_PRIORITIES ( 25 ) //This has impact on speed of search for highest priority
#define configMINIMAL_STACK_SIZE ( 768 + configSTACK_OVERHEAD_TOTAL )
#define configMINIMAL_STACK_SIZE ( CONFIG_FREERTOS_IDLE_TASK_STACKSIZE + configSTACK_OVERHEAD_TOTAL )
#define configUSE_TIME_SLICING 1
#define configUSE_16_BIT_TICKS 0
#define configIDLE_SHOULD_YIELD 0 //Todo: Check this
Expand Down Expand Up @@ -271,20 +271,6 @@ Default values for trace macros added by ESP-IDF and are not part of Vanilla Fre
#define configTASKLIST_INCLUDE_COREID 1
#endif

// ---------------------- Features -------------------------

/* These currently aren't required, but could be useful additions in the future */
#if 0
#ifndef configIDLE_TASK_STACK_SIZE
#define configIDLE_TASK_STACK_SIZE CONFIG_FREERTOS_IDLE_TASK_STACKSIZE
#endif
#if CONFIG_FREERTOS_CHECK_MUTEX_GIVEN_BY_OWNER
#define configCHECK_MUTEX_GIVEN_BY_OWNER 1
#else
#define configCHECK_MUTEX_GIVEN_BY_OWNER 0
#endif
#endif //0

// -------------------- Compatibility ----------------------

// backward compatibility for 4.4
Expand Down
62 changes: 41 additions & 21 deletions components/freertos/FreeRTOS-Kernel-SMP/portable/riscv/port.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -473,41 +473,61 @@ void vApplicationGetIdleTaskMemory(StaticTask_t **ppxIdleTaskTCBBuffer,
{
StackType_t *pxStackBufferTemp;
StaticTask_t *pxTCBBufferTemp;
/* Stack always grows downwards (from high address to low address) on all
* ESP RISC-V targets. Given that the heap allocator likely allocates memory
* from low to high address, we allocate the stack first and then the TCB so
* that the stack does not grow downwards into the TCB.
*
* Allocate TCB and stack buffer in internal memory. */
pxStackBufferTemp = pvPortMalloc(CONFIG_FREERTOS_IDLE_TASK_STACKSIZE);
pxTCBBufferTemp = pvPortMalloc(sizeof(StaticTask_t));

/* If the stack grows down then allocate the stack then the TCB so the stack
* does not grow into the TCB. Likewise if the stack grows up then allocate
* the TCB then the stack. */
#if (portSTACK_GROWTH > 0)
{
//Allocate TCB and stack buffer in internal memory
pxTCBBufferTemp = pvPortMalloc(sizeof(StaticTask_t));
pxStackBufferTemp = pvPortMalloc(configMINIMAL_STACK_SIZE);
}
#else /* portSTACK_GROWTH */
{
//Allocate TCB and stack buffer in internal memory
pxStackBufferTemp = pvPortMalloc(configMINIMAL_STACK_SIZE);
pxTCBBufferTemp = pvPortMalloc(sizeof(StaticTask_t));
}
#endif /* portSTACK_GROWTH */

assert(pxStackBufferTemp != NULL);
assert(pxTCBBufferTemp != NULL);
// Write back pointers
*ppxIdleTaskStackBuffer = pxStackBufferTemp;
*ppxIdleTaskTCBBuffer = pxTCBBufferTemp;
*pulIdleTaskStackSize = CONFIG_FREERTOS_IDLE_TASK_STACKSIZE;
*pulIdleTaskStackSize = configMINIMAL_STACK_SIZE;
}

void vApplicationGetTimerTaskMemory(StaticTask_t **ppxTimerTaskTCBBuffer,
StackType_t **ppxTimerTaskStackBuffer,
uint32_t *pulTimerTaskStackSize )
{
StackType_t *pxStackBufferTemp;
StaticTask_t *pxTCBBufferTemp;
/* Stack always grows downwards (from high address to low address) on all
* ESP RISC-V targets. Given that the heap allocator likely allocates memory
* from low to high address, we allocate the stack first and then the TCB so
* that the stack does not grow downwards into the TCB.
*
* Allocate TCB and stack buffer in internal memory. */
pxStackBufferTemp = pvPortMalloc(configTIMER_TASK_STACK_DEPTH);
pxTCBBufferTemp = pvPortMalloc(sizeof(StaticTask_t));
assert(pxStackBufferTemp != NULL);
StackType_t *pxStackBufferTemp;

/* If the stack grows down then allocate the stack then the TCB so the stack
* does not grow into the TCB. Likewise if the stack grows up then allocate
* the TCB then the stack. */
#if (portSTACK_GROWTH > 0)
{
//Allocate TCB and stack buffer in internal memory
pxTCBBufferTemp = pvPortMalloc(sizeof(StaticTask_t));
pxStackBufferTemp = pvPortMalloc(configTIMER_TASK_STACK_DEPTH);
}
#else /* portSTACK_GROWTH */
{
//Allocate TCB and stack buffer in internal memory
pxStackBufferTemp = pvPortMalloc(configTIMER_TASK_STACK_DEPTH);
pxTCBBufferTemp = pvPortMalloc(sizeof(StaticTask_t));
}
#endif /* portSTACK_GROWTH */

assert(pxTCBBufferTemp != NULL);
// Write back pointers
*ppxTimerTaskStackBuffer = pxStackBufferTemp;
assert(pxStackBufferTemp != NULL);
//Write back pointers
*ppxTimerTaskTCBBuffer = pxTCBBufferTemp;
*ppxTimerTaskStackBuffer = pxStackBufferTemp;
*pulTimerTaskStackSize = configTIMER_TASK_STACK_DEPTH;
}
#endif //( configSUPPORT_STATIC_ALLOCATION == 1 )
Expand Down
16 changes: 1 addition & 15 deletions ...onents/freertos/FreeRTOS-Kernel-SMP/portable/xtensa/include/freertos/FreeRTOSConfig_smp.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ This file get's pulled into assembly sources. Therefore, some includes need to b
#define configCPU_CLOCK_HZ (CONFIG_ESP_DEFAULT_CPU_FREQ_MHZ * 1000000)
#define configTICK_RATE_HZ CONFIG_FREERTOS_HZ
#define configMAX_PRIORITIES ( 25 ) //This has impact on speed of search for highest priority
#define configMINIMAL_STACK_SIZE ( 768 + configSTACK_OVERHEAD_TOTAL )
#define configMINIMAL_STACK_SIZE ( CONFIG_FREERTOS_IDLE_TASK_STACKSIZE + configSTACK_OVERHEAD_TOTAL )
#define configUSE_TIME_SLICING 1
#define configUSE_16_BIT_TICKS 0
#define configIDLE_SHOULD_YIELD 0 //Todo: Check this
Expand Down Expand Up @@ -312,20 +312,6 @@ extern volatile uint32_t port_switch_flag[portNUM_PROCESSORS];
#endif
#endif

// ---------------------- Features -------------------------

/* These currently aren't required, but could be useful additions in the future */
#if 0
#ifndef configIDLE_TASK_STACK_SIZE
#define configIDLE_TASK_STACK_SIZE CONFIG_FREERTOS_IDLE_TASK_STACKSIZE
#endif
#if CONFIG_FREERTOS_CHECK_MUTEX_GIVEN_BY_OWNER
#define configCHECK_MUTEX_GIVEN_BY_OWNER 1
#else
#define configCHECK_MUTEX_GIVEN_BY_OWNER 0
#endif
#endif //0

// -------------------- Compatibility ----------------------

// backward compatibility for 4.4
Expand Down
60 changes: 40 additions & 20 deletions components/freertos/FreeRTOS-Kernel-SMP/portable/xtensa/port.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -537,20 +537,30 @@ void vApplicationGetIdleTaskMemory(StaticTask_t **ppxIdleTaskTCBBuffer,
{
StackType_t *pxStackBufferTemp;
StaticTask_t *pxTCBBufferTemp;
/* Stack always grows downwards (from high address to low address) on all
* ESP Xtensa targets. Given that the heap allocator likely allocates memory
* from low to high address, we allocate the stack first and then the TCB so
* that the stack does not grow downwards into the TCB.
*
* Allocate TCB and stack buffer in internal memory. */
pxStackBufferTemp = pvPortMalloc(CONFIG_FREERTOS_IDLE_TASK_STACKSIZE);
pxTCBBufferTemp = pvPortMalloc(sizeof(StaticTask_t));

/* If the stack grows down then allocate the stack then the TCB so the stack
* does not grow into the TCB. Likewise if the stack grows up then allocate
* the TCB then the stack. */
#if (portSTACK_GROWTH > 0)
{
//Allocate TCB and stack buffer in internal memory
pxTCBBufferTemp = pvPortMalloc(sizeof(StaticTask_t));
pxStackBufferTemp = pvPortMalloc(configMINIMAL_STACK_SIZE);
}
#else /* portSTACK_GROWTH */
{
//Allocate TCB and stack buffer in internal memory
pxStackBufferTemp = pvPortMalloc(configMINIMAL_STACK_SIZE);
pxTCBBufferTemp = pvPortMalloc(sizeof(StaticTask_t));
}
#endif /* portSTACK_GROWTH */

assert(pxStackBufferTemp != NULL);
assert(pxTCBBufferTemp != NULL);
// Write back pointers
*ppxIdleTaskStackBuffer = pxStackBufferTemp;
*ppxIdleTaskTCBBuffer = pxTCBBufferTemp;
*pulIdleTaskStackSize = CONFIG_FREERTOS_IDLE_TASK_STACKSIZE;
*pulIdleTaskStackSize = configMINIMAL_STACK_SIZE;
}

void vApplicationGetTimerTaskMemory(StaticTask_t **ppxTimerTaskTCBBuffer,
Expand All @@ -559,19 +569,29 @@ void vApplicationGetTimerTaskMemory(StaticTask_t **ppxTimerTaskTCBBuffer,
{
StaticTask_t *pxTCBBufferTemp;
StackType_t *pxStackBufferTemp;
/* Stack always grows downwards (from high address to low address) on all
* ESP Xtensa targets. Given that the heap allocator likely allocates memory
* from low to high address, we allocate the stack first and then the TCB so
* that the stack does not grow downwards into the TCB.
*
* Allocate TCB and stack buffer in internal memory. */
pxStackBufferTemp = pvPortMalloc(configTIMER_TASK_STACK_DEPTH);
pxTCBBufferTemp = pvPortMalloc(sizeof(StaticTask_t));
assert(pxStackBufferTemp != NULL);

/* If the stack grows down then allocate the stack then the TCB so the stack
* does not grow into the TCB. Likewise if the stack grows up then allocate
* the TCB then the stack. */
#if (portSTACK_GROWTH > 0)
{
//Allocate TCB and stack buffer in internal memory
pxTCBBufferTemp = pvPortMalloc(sizeof(StaticTask_t));
pxStackBufferTemp = pvPortMalloc(configTIMER_TASK_STACK_DEPTH);
}
#else /* portSTACK_GROWTH */
{
//Allocate TCB and stack buffer in internal memory
pxStackBufferTemp = pvPortMalloc(configTIMER_TASK_STACK_DEPTH);
pxTCBBufferTemp = pvPortMalloc(sizeof(StaticTask_t));
}
#endif /* portSTACK_GROWTH */

assert(pxTCBBufferTemp != NULL);
// Write back pointers
*ppxTimerTaskStackBuffer = pxStackBufferTemp;
assert(pxStackBufferTemp != NULL);
//Write back pointers
*ppxTimerTaskTCBBuffer = pxTCBBufferTemp;
*ppxTimerTaskStackBuffer = pxStackBufferTemp;
*pulTimerTaskStackSize = configTIMER_TASK_STACK_DEPTH;
}
#endif //( configSUPPORT_STATIC_ALLOCATION == 1 )
Expand Down
6 changes: 3 additions & 3 deletions components/freertos/FreeRTOS-Kernel/portable/port_common.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -171,12 +171,12 @@ void vApplicationGetIdleTaskMemory(StaticTask_t **ppxIdleTaskTCBBuffer,
{
//Allocate TCB and stack buffer in internal memory
pxTCBBufferTemp = pvPortMallocTcbMem(sizeof(StaticTask_t));
pxStackBufferTemp = pvPortMallocStackMem(configIDLE_TASK_STACK_SIZE);
pxStackBufferTemp = pvPortMallocStackMem(configMINIMAL_STACK_SIZE);
}
#else /* portSTACK_GROWTH */
{
//Allocate TCB and stack buffer in internal memory
pxStackBufferTemp = pvPortMallocStackMem(configIDLE_TASK_STACK_SIZE);
pxStackBufferTemp = pvPortMallocStackMem(configMINIMAL_STACK_SIZE);
pxTCBBufferTemp = pvPortMallocTcbMem(sizeof(StaticTask_t));
}
#endif /* portSTACK_GROWTH */
Expand All @@ -186,7 +186,7 @@ void vApplicationGetIdleTaskMemory(StaticTask_t **ppxIdleTaskTCBBuffer,
//Write back pointers
*ppxIdleTaskTCBBuffer = pxTCBBufferTemp;
*ppxIdleTaskStackBuffer = pxStackBufferTemp;
*pulIdleTaskStackSize = configIDLE_TASK_STACK_SIZE;
*pulIdleTaskStackSize = configMINIMAL_STACK_SIZE;
}

/*
Expand Down
2 changes: 1 addition & 1 deletion components/freertos/FreeRTOS-Kernel/tasks.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2355,7 +2355,7 @@ void vTaskStartScheduler( void )
/* The Idle task is being created using dynamically allocated RAM. */
xReturn = xTaskCreatePinnedToCore( prvIdleTask,
configIDLE_TASK_NAME,
configIDLE_TASK_STACK_SIZE,
configMINIMAL_STACK_SIZE,
( void * ) NULL,
portPRIVILEGE_BIT, /* In effect ( tskIDLE_PRIORITY | portPRIVILEGE_BIT ), but tskIDLE_PRIORITY is zero. */
&xIdleTaskHandle[ xCoreID ],
Expand Down
5 changes: 1 addition & 4 deletions components/freertos/esp_additions/include/freertos/FreeRTOSConfig.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ This file get's pulled into assembly sources. Therefore, some includes need to b
#define configMINIMAL_STACK_SIZE ( ( unsigned short ) (0x4000 + 40) / sizeof(portSTACK_TYPE) )
#else
#define configMAX_PRIORITIES ( 25 ) //This has impact on speed of search for highest priority
#define configMINIMAL_STACK_SIZE ( 768 + configSTACK_OVERHEAD_TOTAL )
#define configMINIMAL_STACK_SIZE ( CONFIG_FREERTOS_IDLE_TASK_STACKSIZE + configSTACK_OVERHEAD_TOTAL )
#endif
#define configUSE_TIME_SLICING 1
#define configUSE_16_BIT_TICKS 0
Expand Down Expand Up @@ -267,9 +267,6 @@ Note: Include trace macros here and not above as trace macros are dependent on s
// ---------------------- Features -------------------------

#define configTHREAD_LOCAL_STORAGE_DELETE_CALLBACKS CONFIG_FREERTOS_TLSP_DELETION_CALLBACKS
#ifndef configIDLE_TASK_STACK_SIZE
#define configIDLE_TASK_STACK_SIZE CONFIG_FREERTOS_IDLE_TASK_STACKSIZE
#endif

#if CONFIG_FREERTOS_CHECK_MUTEX_GIVEN_BY_OWNER
#define configCHECK_MUTEX_GIVEN_BY_OWNER 1
Expand Down

0 comments on commit 98ff59b

Please sign in to comment.