From 9bb8f0e38d2232de16ca429aed42cbb2105b6c26 Mon Sep 17 00:00:00 2001 From: Sudeep Mohanty Date: Thu, 3 Nov 2022 13:56:19 +0100 Subject: [PATCH] freertos-smp: Fixed stack overflow on esp32s3 with FreeRTOS SMP The following changes have been made in this commit: 1. configMINIMAL_STACK_SIZE is now defined as CONFIG_FREERTOS_IDLE_TASK_STACKSIZE. 2. Removed configIDLE_TASK_STACK_SIZE config as it was redundant. 3. Updates the order of allocating the TCB and stack memory to avoid the stack memory overriding the TCB memory when the stack grows downwards. 4. CONFIG_FREERTOS_IDLE_TASK_STACKSIZE is now incorporated into the FreeRTOSConfig_smp.h to configure the IDLE0 stack size. --- .../include/freertos/FreeRTOSConfig_smp.h | 16 +---- .../FreeRTOS-Kernel-SMP/portable/riscv/port.c | 62 ++++++++++++------- .../include/freertos/FreeRTOSConfig_smp.h | 16 +---- .../portable/xtensa/port.c | 60 ++++++++++++------ .../FreeRTOS-Kernel/portable/port_common.c | 6 +- components/freertos/FreeRTOS-Kernel/tasks.c | 2 +- .../include/freertos/FreeRTOSConfig.h | 5 +- 7 files changed, 88 insertions(+), 79 deletions(-) diff --git a/components/freertos/FreeRTOS-Kernel-SMP/portable/riscv/include/freertos/FreeRTOSConfig_smp.h b/components/freertos/FreeRTOS-Kernel-SMP/portable/riscv/include/freertos/FreeRTOSConfig_smp.h index 0728e7b10c..1f3c459c20 100644 --- a/components/freertos/FreeRTOS-Kernel-SMP/portable/riscv/include/freertos/FreeRTOSConfig_smp.h +++ b/components/freertos/FreeRTOS-Kernel-SMP/portable/riscv/include/freertos/FreeRTOSConfig_smp.h @@ -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 @@ -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 diff --git a/components/freertos/FreeRTOS-Kernel-SMP/portable/riscv/port.c b/components/freertos/FreeRTOS-Kernel-SMP/portable/riscv/port.c index 32765d8a96..f99f405342 100644 --- a/components/freertos/FreeRTOS-Kernel-SMP/portable/riscv/port.c +++ b/components/freertos/FreeRTOS-Kernel-SMP/portable/riscv/port.c @@ -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 ) diff --git a/components/freertos/FreeRTOS-Kernel-SMP/portable/xtensa/include/freertos/FreeRTOSConfig_smp.h b/components/freertos/FreeRTOS-Kernel-SMP/portable/xtensa/include/freertos/FreeRTOSConfig_smp.h index 66c44f807a..10ab6c91ba 100644 --- a/components/freertos/FreeRTOS-Kernel-SMP/portable/xtensa/include/freertos/FreeRTOSConfig_smp.h +++ b/components/freertos/FreeRTOS-Kernel-SMP/portable/xtensa/include/freertos/FreeRTOSConfig_smp.h @@ -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 @@ -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 diff --git a/components/freertos/FreeRTOS-Kernel-SMP/portable/xtensa/port.c b/components/freertos/FreeRTOS-Kernel-SMP/portable/xtensa/port.c index d56e0f94b7..458c607c33 100644 --- a/components/freertos/FreeRTOS-Kernel-SMP/portable/xtensa/port.c +++ b/components/freertos/FreeRTOS-Kernel-SMP/portable/xtensa/port.c @@ -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, @@ -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 ) diff --git a/components/freertos/FreeRTOS-Kernel/portable/port_common.c b/components/freertos/FreeRTOS-Kernel/portable/port_common.c index 420ad29ded..b7270f30ff 100644 --- a/components/freertos/FreeRTOS-Kernel/portable/port_common.c +++ b/components/freertos/FreeRTOS-Kernel/portable/port_common.c @@ -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 */ @@ -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; } /* diff --git a/components/freertos/FreeRTOS-Kernel/tasks.c b/components/freertos/FreeRTOS-Kernel/tasks.c index de7ffaa0d0..42d932c8eb 100644 --- a/components/freertos/FreeRTOS-Kernel/tasks.c +++ b/components/freertos/FreeRTOS-Kernel/tasks.c @@ -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 ], diff --git a/components/freertos/esp_additions/include/freertos/FreeRTOSConfig.h b/components/freertos/esp_additions/include/freertos/FreeRTOSConfig.h index 96c63210ea..c5b1b1cc57 100644 --- a/components/freertos/esp_additions/include/freertos/FreeRTOSConfig.h +++ b/components/freertos/esp_additions/include/freertos/FreeRTOSConfig.h @@ -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 @@ -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