search

FreeRTOS / FreeRTOS-Kernel

FreeRTOS kernel files only, submoduled into https://github.com/FreeRTOS/FreeRTOS and various other repos.
https://www.FreeRTOS.org
MIT License
2.72k stars 1.11k forks source link

[BUG] Replacement of minimal idle hook with passive idle hook introduces a panic on RP2040 #834

Closed kohlerj closed 10 months ago

kohlerj commented 12 months ago

Describe the bug

Jumping from commit 92a4d175e6f6273ac6ec6af8a2bd0310ac098f05 to commit 4bfb9b2d707304917f35fd5e7dcf692abb3d0cb2, an isr_hardfault appears right after launching the scheduler.

Digging a little one quickly sees that it is actually a panic unsupported cause by a task being unexpectedly exited, see screenshot below.

I added #define configUSE_PASSIVE_IDLE_HOOK 0 to be compatible with this new passive idle hook. I previously had #define configUSE_MINIMAL_IDLE_HOOK 0.

Target

Host

To Reproduce

Expected behavior In the case that the minimal idle hook was not used (=0), I expect the same behaviour if passive idle hook is not used as well.

Screenshots

Screenshot 2023-10-14 at 22 21 39

Additional context

Here is my config:

/*
 * FreeRTOS V202111.00
 * Copyright (C) 2020 Amazon.com, Inc. or its affiliates.  All Rights Reserved.
 *
 * Permission is hereby granted, free of charge, to any person obtaining a copy
 * of this software and associated documentation files (the "Software"), to deal
 * in the Software without restriction, including without limitation the rights
 * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
 * copies of the Software, and to permit persons to whom the Software is
 * furnished to do so, subject to the following conditions:
 *
 * The above copyright notice and this permission notice shall be included in
 * all copies or substantial portions of the Software.
 *
 * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
 * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
 * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
 * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
 * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
 * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
 * SOFTWARE.
 *
 * http://www.FreeRTOS.org
 * http://aws.amazon.com/freertos
 *
 * 1 tab == 4 spaces!
 */

#ifndef FREERTOS_CONFIG_H
#define FREERTOS_CONFIG_H

#include "rp2040_config.h"

/*-----------------------------------------------------------
 * Application specific definitions.
 *
 * These definitions should be adjusted for your particular hardware and
 * application requirements.
 *
 * THESE PARAMETERS ARE DESCRIBED WITHIN THE 'CONFIGURATION' SECTION OF THE
 * FreeRTOS API DOCUMENTATION AVAILABLE ON THE FreeRTOS.org WEB SITE.
 *
 * See http://www.freertos.org/a00110.html
 *----------------------------------------------------------*/

/* Scheduler Related */
#define configUSE_PREEMPTION 1
#define configUSE_TICKLESS_IDLE 0
#define configUSE_IDLE_HOOK 0
#define configUSE_PASSIVE_IDLE_HOOK 0
#define configUSE_TICK_HOOK 0
#define configTICK_RATE_HZ ((TickType_t)1000)
#define configMAX_PRIORITIES 7
#define configMINIMAL_STACK_SIZE (configSTACK_DEPTH_TYPE)256
#define configUSE_16_BIT_TICKS 0

#define configIDLE_SHOULD_YIELD 1
#define configUSE_TASK_NOTIFICATIONS 1

/* Synchronization Related */
#define configUSE_MUTEXES 1
#define configUSE_RECURSIVE_MUTEXES 1
#define configUSE_APPLICATION_TASK_TAG 0
#define configUSE_COUNTING_SEMAPHORES 0
#define configQUEUE_REGISTRY_SIZE 8
#define configUSE_QUEUE_SETS 1
#define configUSE_TIME_SLICING 1
#define configUSE_NEWLIB_REENTRANT 0
// todo need this for lwip FreeRTOS sys_arch to compile
#define configENABLE_BACKWARD_COMPATIBILITY 1
#define configNUM_THREAD_LOCAL_STORAGE_POINTERS 5

/* System */
#define configSTACK_DEPTH_TYPE uint32_t
#define configMESSAGE_BUFFER_LENGTH_TYPE size_t

/* Memory allocation related definitions. */
#define configSUPPORT_STATIC_ALLOCATION 1
#define configSUPPORT_DYNAMIC_ALLOCATION 1
#define configTOTAL_HEAP_SIZE (128 * 1024)
#define configAPPLICATION_ALLOCATED_HEAP 0

/* Hook function related definitions. */
#define configCHECK_FOR_STACK_OVERFLOW 2
#define configUSE_MALLOC_FAILED_HOOK 1
#define configUSE_DAEMON_TASK_STARTUP_HOOK 0

/* Run time and task stats gathering related definitions. */
#define configGENERATE_RUN_TIME_STATS 0
#define configUSE_TRACE_FACILITY 1
#define configUSE_STATS_FORMATTING_FUNCTIONS 0

/* Co-routine related definitions. */
#define configUSE_CO_ROUTINES 0
#define configMAX_CO_ROUTINE_PRIORITIES 1

/* Software timer related definitions. */
#define configUSE_TIMERS 1
#define configTIMER_TASK_PRIORITY (configMAX_PRIORITIES - 1)
#define configTIMER_QUEUE_LENGTH 10
#define configTIMER_TASK_STACK_DEPTH 1024

/* Interrupt nesting behaviour configuration. */
/*
#define configKERNEL_INTERRUPT_PRIORITY         [dependent of processor]
#define configMAX_SYSCALL_INTERRUPT_PRIORITY    [dependent on processor and
application] #define configMAX_API_CALL_INTERRUPT_PRIORITY   [dependent on
processor and application]
*/

#if FREE_RTOS_KERNEL_SMP  // set by the RP2040 SMP port of FreeRTOS
/* SMP port only */
#define configNUMBER_OF_CORES 2
#define configNUM_CORES \
  configNUMBER_OF_CORES  // for backward compatibility with pico sdk
#define configTICK_CORE 0
#define configRUN_MULTIPLE_PRIORITIES 1
#define configUSE_MINIMAL_IDLE_HOOK 0
#define configUSE_CORE_AFFINITY 1
#endif

/* RP2040 specific */
#define configSUPPORT_PICO_SYNC_INTEROP 1
#define configSUPPORT_PICO_TIME_INTEROP 1

/* It is a good idea to define configASSERT() while
developing. configASSERT() uses the same semantics as the
standard C assert() macro. */
extern void vAssertCalled(unsigned long ulLine, const char* const pcFileName);
#define configASSERT(x) \
  if ((x) == 0) vAssertCalled(__LINE__, __FILE__)

/* Set the following definitions to 1 to include the API
function, or zero to exclude the API function. */
#define INCLUDE_vTaskPrioritySet 1
#define INCLUDE_uxTaskPriorityGet 1
#define INCLUDE_vTaskDelete 1
#define INCLUDE_vTaskSuspend 1
#define INCLUDE_vTaskDelayUntil 1
#define INCLUDE_vTaskDelay 1
#define INCLUDE_xTaskGetSchedulerState 1
#define INCLUDE_xTaskGetCurrentTaskHandle 1
#define INCLUDE_uxTaskGetStackHighWaterMark 1
#define INCLUDE_xTaskGetIdleTaskHandle 1
#define INCLUDE_eTaskGetState 1
#define INCLUDE_xTimerPendFunctionCall 1
#define INCLUDE_xTaskAbortDelay 1
#define INCLUDE_xTaskGetHandle 1
#define INCLUDE_xTaskResumeFromISR 1
#define INCLUDE_xQueueGetMutexHolder 1

/* A header file that defines trace macro can be included
 * here. */

#endif /* FREERTOS_CONFIG_H */
tony-josi-aws commented 12 months ago

@kohlerj, thanks for taking your time to report this bug, will look into it and update.

Also reminding you to open a discussion on the FreeRTOS forum before opening bug report for better visibility.

chinglee-iot commented 11 months ago

@kohlerj

Thank you for reporting this problem. I am also setting up the environment to reproduce. Below is my setting:

Target

Host

To Reproduce

Currently, I am able to run this test application for more than 30 minutes and no hardfault observed. I would like to double confirm with you about my environment setup.

Can you help to take a look at the test project to see if there is something different from yours? It would be great help to us if we can also reproduce the problem.

pressdot commented 11 months ago

I have same issue on Visual Studio 2022 with Visual GDB and Pico W. But when I turn off SMP, It works Fine. Program exited when reached function vPortStartFirstTask(). I guesss maybe the new passive idle hook will conflicts with SMP?

pressdot commented 11 months ago

Update: After Changed to the SMP branch on https://github.com/FreeRTOS/FreeRTOS-Kernel/tree/ae3a498e435cecdb25b889f2740ea99027dd0cb1 the problem disappears, so I guess it may be the code problem.

pressdot commented 11 months ago

@kohlerj

Thank you for reporting this problem. I am also setting up the environment to reproduce. Below is my setting:

Target

  • Development board: Raspberry Pi Pico ( no wifi version )
  • cmake and ninja are used
  • Toolchain and version: GCC 9.2.1 (GNU Tools for Arm Embedded Processors 9-2019-q4-major)

Host

  • Host OS: Windows 10

To Reproduce

  • Reference the test project in this commit
  • main function
void vLaunch( void )
{
    vTaskStartScheduler();
    for(;;);
}
  • FreeRTOSConfig.h only change assert and static memory configuration
$ diff FreeRTOSConfig.h ori_FreeRTOSConfig.h
80d79
< #define configKERNEL_PROVIDED_STATIC_MEMORY 1
130c129,131
< #define configASSERT(x) assert(x)
---
> extern void vAssertCalled(unsigned long ulLine, const char* const pcFileName);
> #define configASSERT(x) \
>   if ((x) == 0) vAssertCalled(__LINE__, __FILE__)

Currently, I am able to run this test application for more than 30 minutes and no hardfault observed. I would like to double confirm with you about my environment setup.

Can you help to take a look at the test project to see if there is something different from yours? It would be great help to us if we can also reproduce the problem.

Hi @chinglee-iot, and @kohlerj After 5 hours work, I finally reproduce this problem in VScode. Target

Host

chinglee-iot commented 11 months ago

@pressdot Thank you for your information. The API to get idle task memory for SMP is updated in this PR. vApplicationGetIdleTaskMemory is different from single core with xCoreID parameter. The task memory for passive idle tasks should now be provided with this API as well. It may cause the problem if the vApplicationGetIdleTaskMemory is not implemented correctly.

Thank you for your information. This is something we should improve to prevent user from this problem.

pressdot commented 11 months ago

@pressdot Thank you for your information. The API to get idle task memory for SMP is updated in this PR. vApplicationGetIdleTaskMemory is different from single core with xCoreID parameter. The task memory for passive idle tasks should now be provided with this API as well. It may cause the problem if the vApplicationGetIdleTaskMemory is not implemented correctly.

Thank you for your information. This is something we should improve to prevent user from this problem.

I am glad to help, Thank you for replying!

kohlerj commented 11 months ago

Hi all,

Sorry for me not replying sooner, I had a lot going at the moment.

I can confirm that I was missing the xCoreID parameter in vApplicationGetIdleTaskMemory. I took @pressdot's implementation of it and the problem is gone.

Maybe a note here https://www.freertos.org/Static_Vs_Dynamic_Memory_Allocation.html or here https://www.freertos.org/symmetric-multiprocessing-introduction.html would suffice to avoid this issue for other users?

Thank you for your help both of you!

chinglee-iot commented 10 months ago

@kohlerj @pressdot PR #890 which also addresses the problem is merged. Instead of changing the prototype of vApplicationGetIdleTaskMemory for SMP, another callback function vApplicationGetPassiveIdleTaskMemory is introduced in this PR. This ensures that a SMP application can be built with configNUMBER_OF_CORES == 1 without any modification or using compile option.

Sorry for the inconvenient for updating the prototype.

void vApplicationGetIdleTaskMemory( StaticTask_t ** ppxIdleTaskTCBBuffer,
                                    StackType_t ** ppxIdleTaskStackBuffer,
                                    uint32_t * pulIdleTaskStackSize );

#if ( configNUMBER_OF_CORES > 1 )
    void vApplicationGetPassiveIdleTaskMemory( StaticTask_t ** ppxIdleTaskTCBBuffer,
                                               StackType_t ** ppxIdleTaskStackBuffer,
                                               uint32_t * pulIdleTaskStackSize,
                                               BaseType_t xPassiveIdleTaskIndex );
#endif /* #if ( configNUMBER_OF_CORES > 1 ) */

The issue will be closed. Feel free to reopen it if any problem.