Pass core ID to critical nesting count macros #1206
Conversation
There was a problem hiding this comment.
I give my initial review suggest here. I will feedback my further suggestion about the followings topic:
portXXX_CRITICAL_NESTING_COUNTprototype change. Currently, only RP2040 supports portCRITICAL_NESTING_IN_TCB = 0. We will discuss the backward compatibility problem and update in this PR.vTaskYieldWithinAPI()implementation. The critical nesting count should be acquired with interrupt disabled. It would be great if we can also fix this in the PR.
| @@ -855,7 +856,7 @@ static void prvAddNewTaskToReadyList( TCB_t * pxNewTCB ) PRIVILEGED_FUNCTION; | |||
| portGET_TASK_LOCK(); | |||
| portGET_ISR_LOCK(); | |||
|
|
|||
| portSET_CRITICAL_NESTING_COUNT( uxPrevCriticalNesting ); | |||
| portSET_CRITICAL_NESTING_COUNT( xCoreID, uxPrevCriticalNesting ); | |||
There was a problem hiding this comment.
The running state of the task may be altered after enabling or disabling interrupts. We need to obtain the core ID again to determine which core the task is currently running on.
tasks.c
Outdated
| @@ -3866,6 +3868,7 @@ void vTaskSuspendAll( void ) | |||
| #else /* #if ( configNUMBER_OF_CORES == 1 ) */ | |||
| { | |||
| UBaseType_t ulState; | |||
| const BaseType_t xCoreID = ( BaseType_t ) portGET_CORE_ID(); | |||
There was a problem hiding this comment.
The xCoreID must be acquired with interrupts disabled to prevent the task from being scheduled to another core.
There was a problem hiding this comment.
Is it sufficient to do this after the call to portSET_INTERRUPT_MASK()?
tasks.c
Outdated
| @@ -6992,27 +6996,29 @@ static void prvResetNextTaskUnblockTime( void ) | |||
|
|
|||
| void vTaskEnterCritical( void ) | |||
| { | |||
| const BaseType_t xCoreID = ( BaseType_t ) portGET_CORE_ID(); | |||
There was a problem hiding this comment.
Similar reason above. Suggest to acquire xCoreID with interrupt disabled.
|
Description
Passes the core ID as an argument to the critical nesting count macros
port{GET/SET/INCREMENT/DECREMENT}_CRITICAL_NESTING_COUNTintasks.c. This allows the functions that use these macros to read and store the core ID once per call usingportGET_CORE_ID()when using FreeRTOS SMP, thereby reducing the number of unnecessary, and generally slow, accesses to a peripheral register. I have summarised the worst-case number of calls toportGET_CORE_ID()before and after these changes:prvCheckForRunStateChange()prvYieldForTask()vTaskSwitchContext()vTaskYieldWithinAPI()vTaskEnterCritical()vTaskEnterCriticalFromISR()vTaskExitCritical()vTaskExitCriticalFromISR()The execution time of each of the above functions has been reduced on the architecture I tested (an ARM Cortex-M7), though I expect this will yield similar improvements on most ISAs. Further improvements are possible by also passing the core ID to the
port{GET/RELEASE}_{ISR/TASK}_LOCK}()macros, but this involves changes to the port API so I haven't included them here.Test Steps
I compared the execution time of the functions listed in the table above using ETM trace on an NXP S32K396 microcontroller running FreeRTOS v11.1 in SMP mode. I looked at the execution time for 10 random calls to each function while the system is running:
prvCheckForRunStateChange()prvYieldForTask()vTaskSwitchContext()vTaskEnterCritical()vTaskEnterCriticalFromISR()vTaskExitCritical()vTaskExitCriticalFromISR()I was not able to get individual results for
vTaskYieldWithinAPI()because it calls some of the other functions above. ForprvCheckForRunStateChange()andprvYieldForTask(), I noticed that the number of calls toportGET_CORE_ID()on my system were the same as before, so my code is not branching to the worst-case path. These are average-case results on my particular project and not definitive, but certain functions do see a clear improvement.These changes are made under the assumption that the core ID cannot change within the context of each function, which I believe is true.
Checklist:
Related Issue
#1204
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.