From 63fee6c23acb9089621a4a4867feb9f7c866b740 Mon Sep 17 00:00:00 2001 From: Darian Leung Date: Thu, 16 Nov 2023 19:18:17 +0800 Subject: [PATCH] refactor(freertos/idf): Refactor yield and affinity macros This commit refactors the following macros so that calling them no longer requires referencing pxTCB->xCoreID. - taskIS_YIELD_REQUIRED() - taskIS_YIELD_REQUIRED_USING_PRIORITY() - taskIS_AFFINITY_COMPATIBLE() --- components/freertos/FreeRTOS-Kernel/tasks.c | 100 ++++++++++---------- 1 file changed, 52 insertions(+), 48 deletions(-) diff --git a/components/freertos/FreeRTOS-Kernel/tasks.c b/components/freertos/FreeRTOS-Kernel/tasks.c index 5569d89309..d0cbbcf799 100644 --- a/components/freertos/FreeRTOS-Kernel/tasks.c +++ b/components/freertos/FreeRTOS-Kernel/tasks.c @@ -130,27 +130,29 @@ /* Macros to check if an unblocked task causes a yield on the current core. * - pxTCB is the TCB of the task to check - * - xCurCoreID is the current core's ID - * - xYieldEqualPriority indicates whether a yield should occur if the unblocked - * task's priority is equal to the priority of the task currently running on the - * current core. * - uxTaskPriority is the task's priority - * - xTaskCoreID is the task's core affinity */ + * - xYieldEqualPriority if the task having equal priority as the currently + * executing task should cause a yield. + * + * In single-core, this macro simply checks the unblocked task has a high enough + * priority to preempt the current task, and returns pdTRUE if so. + * + * In SMP, this macro checks if the unblocked task can preempt either core: + * - If a yield is required on the current core, this macro return pdTRUE + * - if a yield is required on the other core, this macro will internally + * trigger it. + */ #if ( configNUMBER_OF_CORES > 1 ) - #define taskIS_YIELD_REQUIRED( pxTCB, xCurCoreID, xYieldEqualPriority ) prvIsYieldUsingPrioritySMP( ( pxTCB )->uxPriority, ( pxTCB )->xCoreID, xCurCoreID, xYieldEqualPriority ) - #define taskIS_YIELD_REQUIRED_USING_PRIORITY( uxTaskPriority, xTaskCoreID, xCurCoreID, xYieldEqualPriority ) prvIsYieldUsingPrioritySMP( uxTaskPriority, xTaskCoreID, xCurCoreID, xYieldEqualPriority ) + #define taskIS_YIELD_REQUIRED( pxTCB, xYieldEqualPriority ) prvIsYieldRequiredSMP( ( pxTCB ), ( pxTCB )->uxPriority, xYieldEqualPriority ) + #define taskIS_YIELD_REQUIRED_USING_PRIORITY( pxTCB, uxTaskPriority, xYieldEqualPriority ) prvIsYieldRequiredSMP( ( pxTCB ), uxTaskPriority, xYieldEqualPriority ) #else - #define taskIS_YIELD_REQUIRED( pxTCB, xCurCoreID, xYieldEqualPriority ) \ + #define taskIS_YIELD_REQUIRED( pxTCB, xYieldEqualPriority ) \ ( { \ - /* xCurCoreID is unused */ \ - ( void ) xCurCoreID; \ ( ( ( pxTCB )->uxPriority + ( ( xYieldEqualPriority == pdTRUE ) ? 1 : 0 ) ) > pxCurrentTCBs[ 0 ]->uxPriority ) ? pdTRUE : pdFALSE; \ } ) - #define taskIS_YIELD_REQUIRED_USING_PRIORITY( uxTaskPriority, xTaskCoreID, xCurCoreID, xYieldEqualPriority ) \ + #define taskIS_YIELD_REQUIRED_USING_PRIORITY( pxTCB, uxTaskPriority, xYieldEqualPriority ) \ ( { \ - /* xTaskCoreID and xCurCoreID are unused */ \ - ( void ) xTaskCoreID; \ - ( void ) xCurCoreID; \ + ( void ) pxTCB; \ ( ( uxTaskPriority + ( ( xYieldEqualPriority == pdTRUE ) ? 1 : 0 ) ) >= pxCurrentTCBs[ 0 ]->uxPriority ) ? pdTRUE : pdFALSE; \ } ) #endif /* configNUMBER_OF_CORES > 1 */ @@ -158,18 +160,19 @@ /* Macros to check if a task has a compatible affinity with a particular core. * - xCore is the target core - * - xCoreID is the affinity of the task to check + * - pxTCB is the task to check * * This macro will always return true on single core as the concept of core * affinity doesn't exist. */ #if ( configNUMBER_OF_CORES > 1 ) - #define taskIS_AFFINITY_COMPATIBLE( xCore, xCoreID ) ( ( ( ( xCoreID ) == xCore ) || ( ( xCoreID ) == tskNO_AFFINITY ) ) ? pdTRUE : pdFALSE ) + #define taskIS_AFFINITY_COMPATIBLE( xCore, pxTCB ) ( ( ( ( pxTCB )->xCoreID == xCore ) || ( ( pxTCB )->xCoreID == tskNO_AFFINITY ) ) ? pdTRUE : pdFALSE ) #else - #define taskIS_AFFINITY_COMPATIBLE( xCore, xCoreID ) \ - ( { \ - /* xCoreID is unused */ \ - ( void ) xCoreID; \ - pdTRUE; \ + #define taskIS_AFFINITY_COMPATIBLE( xCore, pxTCB ) \ + ( { \ + /* xCore and pxTCB are unused */ \ + ( void ) xCore; \ + ( void ) pxTCB; \ + pdTRUE; \ } ) #endif /* configNUMBER_OF_CORES > 1 */ /*-----------------------------------------------------------*/ @@ -568,10 +571,9 @@ static BaseType_t prvCreateIdleTasks( void ); */ #if ( configNUMBER_OF_CORES > 1 ) - static BaseType_t prvIsYieldUsingPrioritySMP( UBaseType_t uxTaskPriority, - BaseType_t xTaskCoreID, - BaseType_t xCurCoreID, - BaseType_t xYieldEqualPriority ) PRIVILEGED_FUNCTION; + static BaseType_t prvIsYieldRequiredSMP( TCB_t * pxTCB, + UBaseType_t uxTaskPriority, + BaseType_t xYieldEqualPriority ) PRIVILEGED_FUNCTION; #endif /* configNUMBER_OF_CORES > 1 */ @@ -765,13 +767,15 @@ static void prvAddNewTaskToReadyList( TCB_t * pxNewTCB ) PRIVILEGED_FUNCTION; #if ( configNUMBER_OF_CORES > 1 ) - static BaseType_t prvIsYieldUsingPrioritySMP( UBaseType_t uxTaskPriority, - BaseType_t xTaskCoreID, - BaseType_t xCurCoreID, - BaseType_t xYieldEqualPriority ) + static BaseType_t prvIsYieldRequiredSMP( TCB_t * pxTCB, + UBaseType_t uxTaskPriority, + BaseType_t xYieldEqualPriority ) { configASSERT( uxTaskPriority < configMAX_PRIORITIES ); + /* Save core ID as we can no longer be preempted. */ + const BaseType_t xCurCoreID = portGET_CORE_ID(); + if( xYieldEqualPriority == pdTRUE ) { /* Increment the task priority to achieve the same affect as @@ -786,7 +790,7 @@ static void prvAddNewTaskToReadyList( TCB_t * pxNewTCB ) PRIVILEGED_FUNCTION; * priority than the current core, and the core has not suspended * scheduling, then yield the current core. * Todo: Make fair scheduling a configurable option (IDF-5772). */ - if( ( taskIS_AFFINITY_COMPATIBLE( xCurCoreID, xTaskCoreID ) == pdTRUE ) && + if( ( taskIS_AFFINITY_COMPATIBLE( xCurCoreID, pxTCB ) == pdTRUE ) && ( uxTaskPriority > pxCurrentTCBs[ xCurCoreID ]->uxPriority ) && ( uxSchedulerSuspended[ xCurCoreID ] == ( UBaseType_t ) 0U ) ) { @@ -797,7 +801,7 @@ static void prvAddNewTaskToReadyList( TCB_t * pxNewTCB ) PRIVILEGED_FUNCTION; /* If the target task can run on the other core, and has a higher * priority then the other core, and the other core has not suspended * scheduling, then yield the other core */ - else if( ( taskIS_AFFINITY_COMPATIBLE( !xCurCoreID, xTaskCoreID ) == pdTRUE ) && + else if( ( taskIS_AFFINITY_COMPATIBLE( !xCurCoreID, pxTCB ) == pdTRUE ) && ( uxTaskPriority > pxCurrentTCBs[ !xCurCoreID ]->uxPriority ) && ( uxSchedulerSuspended[ !xCurCoreID ] == ( UBaseType_t ) 0U ) ) { @@ -1194,7 +1198,7 @@ static void prvAddNewTaskToReadyList( TCB_t * pxNewTCB ) mtCOVERAGE_TEST_MARKER(); } - if( ( pxCurrentTCBs[ 0 ] == NULL ) && ( taskIS_AFFINITY_COMPATIBLE( 0, pxNewTCB->xCoreID ) == pdTRUE ) ) + if( ( pxCurrentTCBs[ 0 ] == NULL ) && ( taskIS_AFFINITY_COMPATIBLE( 0, pxNewTCB ) == pdTRUE ) ) { /* On core 0, there are no other tasks, or all the other tasks * are in the suspended state - make this the current task. */ @@ -1202,7 +1206,7 @@ static void prvAddNewTaskToReadyList( TCB_t * pxNewTCB ) } #if ( configNUMBER_OF_CORES > 1 ) - else if( ( pxCurrentTCBs[ 1 ] == NULL ) && ( taskIS_AFFINITY_COMPATIBLE( 1, pxNewTCB->xCoreID ) == pdTRUE ) ) + else if( ( pxCurrentTCBs[ 1 ] == NULL ) && ( taskIS_AFFINITY_COMPATIBLE( 1, pxNewTCB ) == pdTRUE ) ) { /* On core 1, there are no other tasks, or all the other tasks * are in the suspended state - make this the current task. */ @@ -1217,7 +1221,7 @@ static void prvAddNewTaskToReadyList( TCB_t * pxNewTCB ) if( xSchedulerRunning == pdFALSE ) { if( ( pxCurrentTCBs[ 0 ] != NULL ) && - ( taskIS_AFFINITY_COMPATIBLE( 0, pxNewTCB->xCoreID ) == pdTRUE ) && + ( taskIS_AFFINITY_COMPATIBLE( 0, pxNewTCB ) == pdTRUE ) && ( pxCurrentTCBs[ 0 ]->uxPriority <= pxNewTCB->uxPriority ) ) { pxCurrentTCBs[ 0 ] = pxNewTCB; @@ -1225,7 +1229,7 @@ static void prvAddNewTaskToReadyList( TCB_t * pxNewTCB ) #if ( configNUMBER_OF_CORES > 1 ) else if( ( pxCurrentTCBs[ 1 ] != NULL ) && - ( taskIS_AFFINITY_COMPATIBLE( 1, pxNewTCB->xCoreID ) == pdTRUE ) && + ( taskIS_AFFINITY_COMPATIBLE( 1, pxNewTCB ) == pdTRUE ) && ( pxCurrentTCBs[ 1 ]->uxPriority <= pxNewTCB->uxPriority ) ) { pxCurrentTCBs[ 1 ] = pxNewTCB; @@ -1260,7 +1264,7 @@ static void prvAddNewTaskToReadyList( TCB_t * pxNewTCB ) { /* If the created task is of a higher priority than the current task * then it should run now. */ - if( taskIS_YIELD_REQUIRED( pxNewTCB, portGET_CORE_ID(), pdTRUE ) == pdTRUE ) + if( taskIS_YIELD_REQUIRED( pxNewTCB, pdTRUE ) == pdTRUE ) { taskYIELD_IF_USING_PREEMPTION(); } @@ -1777,7 +1781,7 @@ static void prvAddNewTaskToReadyList( TCB_t * pxNewTCB ) /* The priority of a task other than the currently * running task is being raised. Is the priority being * raised above that of the running task? */ - if( taskIS_YIELD_REQUIRED_USING_PRIORITY( uxNewPriority, pxTCB->xCoreID, portGET_CORE_ID(), pdTRUE ) == pdTRUE ) + if( taskIS_YIELD_REQUIRED_USING_PRIORITY( pxTCB, uxNewPriority, pdTRUE ) == pdTRUE ) { xYieldRequired = pdTRUE; } @@ -2106,7 +2110,7 @@ static void prvAddNewTaskToReadyList( TCB_t * pxNewTCB ) prvAddTaskToReadyList( pxTCB ); /* A higher priority task may have just been resumed. */ - if( taskIS_YIELD_REQUIRED( pxTCB, portGET_CORE_ID(), pdTRUE ) == pdTRUE ) + if( taskIS_YIELD_REQUIRED( pxTCB, pdTRUE ) == pdTRUE ) { /* This yield may not cause the task just resumed to run, * but will leave the lists in the correct state for the @@ -2177,7 +2181,7 @@ static void prvAddNewTaskToReadyList( TCB_t * pxNewTCB ) { /* Ready lists can be accessed so move the task from the * suspended list to the ready list directly. */ - if( taskIS_YIELD_REQUIRED( pxTCB, xCurCoreID, pdTRUE ) == pdTRUE ) + if( taskIS_YIELD_REQUIRED( pxTCB, pdTRUE ) == pdTRUE ) { xYieldRequired = pdTRUE; @@ -2575,7 +2579,7 @@ BaseType_t xTaskResumeAll( void ) /* If the moved task has a priority higher than or equal to * the current task then a yield must be performed. */ - if( taskIS_YIELD_REQUIRED( pxTCB, xCurCoreID, pdTRUE ) == pdTRUE ) + if( taskIS_YIELD_REQUIRED( pxTCB, pdTRUE ) == pdTRUE ) { xYieldPending[ xCurCoreID ] = pdTRUE; } @@ -3137,7 +3141,7 @@ BaseType_t xTaskCatchUpTicks( TickType_t xTicksToCatchUp ) /* Preemption is on, but a context switch should only be * performed if the unblocked task has a priority that is * higher than the currently executing task. */ - if( taskIS_YIELD_REQUIRED( pxTCB, xCurCoreID, pdFALSE ) == pdTRUE ) + if( taskIS_YIELD_REQUIRED( pxTCB, pdFALSE ) == pdTRUE ) { /* Pend the yield to be performed when the scheduler * is unsuspended. */ @@ -3279,7 +3283,7 @@ BaseType_t xTaskIncrementTick( void ) * 0, we only need to context switch if the unblocked * task can run on core 0 and has a higher priority * than the current task. */ - if( ( taskIS_AFFINITY_COMPATIBLE( 0, pxTCB->xCoreID ) == pdTRUE ) && ( pxTCB->uxPriority > pxCurrentTCBs[ 0 ]->uxPriority ) ) + if( ( taskIS_AFFINITY_COMPATIBLE( 0, pxTCB ) == pdTRUE ) && ( pxTCB->uxPriority > pxCurrentTCBs[ 0 ]->uxPriority ) ) { xSwitchRequired = pdTRUE; } @@ -3542,7 +3546,7 @@ BaseType_t xTaskIncrementTick( void ) } /* Check if the current task has a compatible affinity */ - if( taskIS_AFFINITY_COMPATIBLE( xCurCoreID, pxTCBCur->xCoreID ) == pdFALSE ) + if( taskIS_AFFINITY_COMPATIBLE( xCurCoreID, pxTCBCur ) == pdFALSE ) { goto get_next_task; } @@ -3858,7 +3862,7 @@ void vTaskPlaceOnUnorderedEventList( List_t * pxEventList, listINSERT_END( &( xPendingReadyList[ uxPendCore ] ), &( pxUnblockedTCB->xEventListItem ) ); } - if( taskIS_YIELD_REQUIRED( pxUnblockedTCB, xCurCoreID, pdFALSE ) == pdTRUE ) + if( taskIS_YIELD_REQUIRED( pxUnblockedTCB, pdFALSE ) == pdTRUE ) { /* The unblocked task requires a the current core to yield */ xReturn = pdTRUE; @@ -4043,7 +4047,7 @@ void vTaskRemoveFromUnorderedEventList( ListItem_t * pxEventListItem, listREMOVE_ITEM( &( pxUnblockedTCB->xStateListItem ) ); prvAddTaskToReadyList( pxUnblockedTCB ); - if( taskIS_YIELD_REQUIRED( pxUnblockedTCB, xCurCoreID, pdFALSE ) == pdTRUE ) + if( taskIS_YIELD_REQUIRED( pxUnblockedTCB, pdFALSE ) == pdTRUE ) { /* The unblocked task has a priority above that of the calling task, so * a context switch is required. This function is called with the @@ -5883,7 +5887,7 @@ TickType_t uxTaskResetEventItemValue( void ) } #endif - if( taskIS_YIELD_REQUIRED( pxTCB, portGET_CORE_ID(), pdFALSE ) == pdTRUE ) + if( taskIS_YIELD_REQUIRED( pxTCB, pdFALSE ) == pdTRUE ) { /* The notified task has a priority above the currently * executing task so a yield is required. */ @@ -6021,7 +6025,7 @@ TickType_t uxTaskResetEventItemValue( void ) listINSERT_END( &( xPendingReadyList[ xCurCoreID ] ), &( pxTCB->xEventListItem ) ); } - if( taskIS_YIELD_REQUIRED( pxTCB, xCurCoreID, pdFALSE ) == pdTRUE ) + if( taskIS_YIELD_REQUIRED( pxTCB, pdFALSE ) == pdTRUE ) { /* The notified task has a priority above the currently * executing task so a yield is required. */ @@ -6115,7 +6119,7 @@ TickType_t uxTaskResetEventItemValue( void ) listINSERT_END( &( xPendingReadyList[ xCurCoreID ] ), &( pxTCB->xEventListItem ) ); } - if( taskIS_YIELD_REQUIRED( pxTCB, xCurCoreID, pdFALSE ) == pdTRUE ) + if( taskIS_YIELD_REQUIRED( pxTCB, pdFALSE ) == pdTRUE ) { /* The notified task has a priority above the currently * executing task so a yield is required. */