From f05244a8d55b23428c4a8c097d375d12a9d6e30d Mon Sep 17 00:00:00 2001 From: Felix van Oost Date: Mon, 30 Dec 2024 03:58:49 -0500 Subject: [PATCH] Pass core ID to port lock macros (#1212) Pass core ID to task/ISR lock functions --- include/FreeRTOS.h | 8 +-- portable/CCRH/F1Kx/port.c | 10 ++-- portable/CCRH/F1Kx/portmacro.h | 20 ++++---- .../ThirdParty/GCC/RP2040/include/portmacro.h | 30 ++++++------ .../ThirdParty/xClang/XCOREAI/portmacro.h | 9 ++-- portable/template/portmacro.h | 8 +-- tasks.c | 49 +++++++++++-------- 7 files changed, 71 insertions(+), 63 deletions(-) diff --git a/include/FreeRTOS.h b/include/FreeRTOS.h index 3fecbdd734e..dfccccb404b 100644 --- a/include/FreeRTOS.h +++ b/include/FreeRTOS.h @@ -445,7 +445,7 @@ #ifndef portRELEASE_TASK_LOCK #if ( configNUMBER_OF_CORES == 1 ) - #define portRELEASE_TASK_LOCK() + #define portRELEASE_TASK_LOCK( xCoreID ) #else #error portRELEASE_TASK_LOCK is required in SMP #endif @@ -455,7 +455,7 @@ #ifndef portGET_TASK_LOCK #if ( configNUMBER_OF_CORES == 1 ) - #define portGET_TASK_LOCK() + #define portGET_TASK_LOCK( xCoreID ) #else #error portGET_TASK_LOCK is required in SMP #endif @@ -465,7 +465,7 @@ #ifndef portRELEASE_ISR_LOCK #if ( configNUMBER_OF_CORES == 1 ) - #define portRELEASE_ISR_LOCK() + #define portRELEASE_ISR_LOCK( xCoreID ) #else #error portRELEASE_ISR_LOCK is required in SMP #endif @@ -475,7 +475,7 @@ #ifndef portGET_ISR_LOCK #if ( configNUMBER_OF_CORES == 1 ) - #define portGET_ISR_LOCK() + #define portGET_ISR_LOCK( xCoreID ) #else #error portGET_ISR_LOCK is required in SMP #endif diff --git a/portable/CCRH/F1Kx/port.c b/portable/CCRH/F1Kx/port.c index 0e6116527df..3a43ff418d1 100644 --- a/portable/CCRH/F1Kx/port.c +++ b/portable/CCRH/F1Kx/port.c @@ -258,8 +258,8 @@ void vPortTickISR( void ); * already had lock can acquire lock without waiting. This function could be * call from task and interrupt context, the critical section is called * as in ISR */ - void vPortRecursiveLockAcquire( BaseType_t xFromIsr ); - void vPortRecursiveLockRelease( BaseType_t xFromIsr ); + void vPortRecursiveLockAcquire( BaseType_t xCoreID, BaseType_t xFromIsr ); + void vPortRecursiveLockRelease( BaseType_t xCoreID, BaseType_t xFromIsr ); #endif /* (configNUMBER_OF_CORES > 1) */ @@ -688,10 +688,9 @@ static void prvSetupTimerInterrupt( void ) } /*-----------------------------------------------------------*/ - void vPortRecursiveLockAcquire( BaseType_t xFromIsr ) + void vPortRecursiveLockAcquire( BaseType_t xCoreID, BaseType_t xFromIsr ) { BaseType_t xSavedInterruptStatus; - BaseType_t xCoreID = xPortGET_CORE_ID(); BaseType_t xBitPosition = ( xFromIsr == pdTRUE ); xSavedInterruptStatus = portSET_INTERRUPT_MASK_FROM_ISR(); @@ -705,10 +704,9 @@ static void prvSetupTimerInterrupt( void ) portCLEAR_INTERRUPT_MASK_FROM_ISR( xSavedInterruptStatus ); } - void vPortRecursiveLockRelease( BaseType_t xFromIsr ) + void vPortRecursiveLockRelease( BaseType_t xCoreID, BaseType_t xFromIsr ) { BaseType_t xSavedInterruptStatus; - BaseType_t xCoreID = xPortGET_CORE_ID(); BaseType_t xBitPosition = ( xFromIsr == pdTRUE ); xSavedInterruptStatus = portSET_INTERRUPT_MASK_FROM_ISR(); diff --git a/portable/CCRH/F1Kx/portmacro.h b/portable/CCRH/F1Kx/portmacro.h index 09f9f461450..a665ad23d2a 100644 --- a/portable/CCRH/F1Kx/portmacro.h +++ b/portable/CCRH/F1Kx/portmacro.h @@ -141,18 +141,18 @@ #endif /* if ( configNUMBER_OF_CORES > 1 ) */ #if ( configNUMBER_OF_CORES == 1 ) - #define portGET_ISR_LOCK() - #define portRELEASE_ISR_LOCK() - #define portGET_TASK_LOCK() - #define portRELEASE_TASK_LOCK() + #define portGET_ISR_LOCK( xCoreID ) + #define portRELEASE_ISR_LOCK( xCoreID ) + #define portGET_TASK_LOCK( xCoreID ) + #define portRELEASE_TASK_LOCK( xCoreID ) #else - extern void vPortRecursiveLockAcquire( BaseType_t xFromIsr ); - extern void vPortRecursiveLockRelease( BaseType_t xFromIsr ); + extern void vPortRecursiveLockAcquire( BaseType_t xCoreID, BaseType_t xFromIsr ); + extern void vPortRecursiveLockRelease( BaseType_t xCoreID, BaseType_t xFromIsr ); - #define portGET_ISR_LOCK() vPortRecursiveLockAcquire( pdTRUE ) - #define portRELEASE_ISR_LOCK() vPortRecursiveLockRelease( pdTRUE ) - #define portGET_TASK_LOCK() vPortRecursiveLockAcquire( pdFALSE ) - #define portRELEASE_TASK_LOCK() vPortRecursiveLockRelease( pdFALSE ) + #define portGET_ISR_LOCK( xCoreID ) vPortRecursiveLockAcquire( ( xCoreID ), pdTRUE ) + #define portRELEASE_ISR_LOCK( xCoreID ) vPortRecursiveLockRelease( ( xCoreID ), pdTRUE ) + #define portGET_TASK_LOCK( xCoreID ) vPortRecursiveLockAcquire( ( xCoreID ), pdFALSE ) + #define portRELEASE_TASK_LOCK( xCoreID ) vPortRecursiveLockRelease( ( xCoreID ), pdFALSE ) #endif /* if ( configNUMBER_OF_CORES == 1 ) */ /*-----------------------------------------------------------*/ diff --git a/portable/ThirdParty/GCC/RP2040/include/portmacro.h b/portable/ThirdParty/GCC/RP2040/include/portmacro.h index ed9dbade09e..802470e33f7 100644 --- a/portable/ThirdParty/GCC/RP2040/include/portmacro.h +++ b/portable/ThirdParty/GCC/RP2040/include/portmacro.h @@ -210,8 +210,9 @@ __force_inline static bool spin_try_lock_unsafe(spin_lock_t *lock) { /* Note this is a single method with uxAcquire parameter since we have * static vars, the method is always called with a compile time constant for - * uxAcquire, and the compiler should dothe right thing! */ -static inline void vPortRecursiveLock( uint32_t ulLockNum, + * uxAcquire, and the compiler should do the right thing! */ +static inline void vPortRecursiveLock( BaseType_t xCoreID, + uint32_t ulLockNum, spin_lock_t * pxSpinLock, BaseType_t uxAcquire ) { @@ -219,12 +220,11 @@ static inline void vPortRecursiveLock( uint32_t ulLockNum, static volatile uint8_t ucRecursionCountByLock[ portRTOS_SPINLOCK_COUNT ]; configASSERT( ulLockNum < portRTOS_SPINLOCK_COUNT ); - uint32_t ulCoreNum = get_core_num(); if( uxAcquire ) { if (!spin_try_lock_unsafe(pxSpinLock)) { - if( ucOwnedByCore[ ulCoreNum ][ ulLockNum ] ) + if( ucOwnedByCore[ xCoreID ][ ulLockNum ] ) { configASSERT( ucRecursionCountByLock[ ulLockNum ] != 255u ); ucRecursionCountByLock[ ulLockNum ]++; @@ -234,31 +234,31 @@ static inline void vPortRecursiveLock( uint32_t ulLockNum, } configASSERT( ucRecursionCountByLock[ ulLockNum ] == 0 ); ucRecursionCountByLock[ ulLockNum ] = 1; - ucOwnedByCore[ ulCoreNum ][ ulLockNum ] = 1; + ucOwnedByCore[ xCoreID ][ ulLockNum ] = 1; } else { - configASSERT( ( ucOwnedByCore[ ulCoreNum ] [ulLockNum ] ) != 0 ); + configASSERT( ( ucOwnedByCore[ xCoreID ] [ulLockNum ] ) != 0 ); configASSERT( ucRecursionCountByLock[ ulLockNum ] != 0 ); if( !--ucRecursionCountByLock[ ulLockNum ] ) { - ucOwnedByCore[ ulCoreNum ] [ ulLockNum ] = 0; + ucOwnedByCore[ xCoreID ] [ ulLockNum ] = 0; spin_unlock_unsafe(pxSpinLock); } } } #if ( configNUMBER_OF_CORES == 1 ) - #define portGET_ISR_LOCK() - #define portRELEASE_ISR_LOCK() - #define portGET_TASK_LOCK() - #define portRELEASE_TASK_LOCK() + #define portGET_ISR_LOCK( xCoreID ) + #define portRELEASE_ISR_LOCK( xCoreID ) + #define portGET_TASK_LOCK( xCoreID ) + #define portRELEASE_TASK_LOCK( xCoreID ) #else - #define portGET_ISR_LOCK() vPortRecursiveLock( 0, spin_lock_instance( configSMP_SPINLOCK_0 ), pdTRUE ) - #define portRELEASE_ISR_LOCK() vPortRecursiveLock( 0, spin_lock_instance( configSMP_SPINLOCK_0 ), pdFALSE ) - #define portGET_TASK_LOCK() vPortRecursiveLock( 1, spin_lock_instance( configSMP_SPINLOCK_1 ), pdTRUE ) - #define portRELEASE_TASK_LOCK() vPortRecursiveLock( 1, spin_lock_instance( configSMP_SPINLOCK_1 ), pdFALSE ) + #define portGET_ISR_LOCK( xCoreID ) vPortRecursiveLock( ( xCoreID ), 0, spin_lock_instance( configSMP_SPINLOCK_0 ), pdTRUE ) + #define portRELEASE_ISR_LOCK( xCoreID ) vPortRecursiveLock( ( xCoreID ), 0, spin_lock_instance( configSMP_SPINLOCK_0 ), pdFALSE ) + #define portGET_TASK_LOCK( xCoreID ) vPortRecursiveLock( ( xCoreID ), 1, spin_lock_instance( configSMP_SPINLOCK_1 ), pdTRUE ) + #define portRELEASE_TASK_LOCK( xCoreID ) vPortRecursiveLock( ( xCoreID ), 1, spin_lock_instance( configSMP_SPINLOCK_1 ), pdFALSE ) #endif /*-----------------------------------------------------------*/ diff --git a/portable/ThirdParty/xClang/XCOREAI/portmacro.h b/portable/ThirdParty/xClang/XCOREAI/portmacro.h index 82da9531487..08813331481 100644 --- a/portable/ThirdParty/xClang/XCOREAI/portmacro.h +++ b/portable/ThirdParty/xClang/XCOREAI/portmacro.h @@ -152,10 +152,11 @@ #define portASSERT_IF_IN_ISR() configASSERT( portCHECK_IF_IN_ISR() == 0 ) - #define portGET_ISR_LOCK() rtos_lock_acquire( 0 ) - #define portRELEASE_ISR_LOCK() rtos_lock_release( 0 ) - #define portGET_TASK_LOCK() rtos_lock_acquire( 1 ) - #define portRELEASE_TASK_LOCK() rtos_lock_release( 1 ) + #define portGET_ISR_LOCK( xCoreID ) do{ ( void )( xCoreID ); rtos_lock_acquire( 0 ); } while( 0 ) + #define portRELEASE_ISR_LOCK( xCoreID ) do{ ( void )( xCoreID ); rtos_lock_release( 0 ); } while( 0 ) + #define portGET_TASK_LOCK( xCoreID ) do{ ( void )( xCoreID ); rtos_lock_acquire( 1 ); } while( 0 ) + #define portRELEASE_TASK_LOCK( xCoreID ) do{ ( void )( xCoreID ); rtos_lock_release( 1 ); } while( 0 ) + void vTaskEnterCritical( void ); void vTaskExitCritical( void ); diff --git a/portable/template/portmacro.h b/portable/template/portmacro.h index 4a4a5876c62..a426f0003cb 100644 --- a/portable/template/portmacro.h +++ b/portable/template/portmacro.h @@ -123,19 +123,19 @@ extern void vPortYield( void ); /* Acquire the TASK lock. TASK lock is a recursive lock. * It should be able to be locked by the same core multiple times. */ - #define portGET_TASK_LOCK() do {} while( 0 ) + #define portGET_TASK_LOCK( xCoreID ) do {} while( 0 ) /* Release the TASK lock. If a TASK lock is locked by the same core multiple times, * it should be released as many times as it is locked. */ - #define portRELEASE_TASK_LOCK() do {} while( 0 ) + #define portRELEASE_TASK_LOCK( xCoreID ) do {} while( 0 ) /* Acquire the ISR lock. ISR lock is a recursive lock. * It should be able to be locked by the same core multiple times. */ - #define portGET_ISR_LOCK() do {} while( 0 ) + #define portGET_ISR_LOCK( xCoreID ) do {} while( 0 ) /* Release the ISR lock. If a ISR lock is locked by the same core multiple times, \ * it should be released as many times as it is locked. */ - #define portRELEASE_ISR_LOCK() do {} while( 0 ) + #define portRELEASE_ISR_LOCK( xCoreID ) do {} while( 0 ) #endif /* if ( configNUMBER_OF_CORES > 1 ) */ diff --git a/tasks.c b/tasks.c index f6af37bab8a..d7153f680e2 100644 --- a/tasks.c +++ b/tasks.c @@ -831,7 +831,7 @@ static void prvAddNewTaskToReadyList( TCB_t * pxNewTCB ) PRIVILEGED_FUNCTION; if( uxPrevCriticalNesting > 0U ) { portSET_CRITICAL_NESTING_COUNT( xCoreID, 0U ); - portRELEASE_ISR_LOCK(); + portRELEASE_ISR_LOCK( xCoreID ); } else { @@ -840,7 +840,7 @@ static void prvAddNewTaskToReadyList( TCB_t * pxNewTCB ) PRIVILEGED_FUNCTION; mtCOVERAGE_TEST_MARKER(); } - portRELEASE_TASK_LOCK(); + portRELEASE_TASK_LOCK( xCoreID ); portMEMORY_BARRIER(); configASSERT( pxThisTCB->xTaskRunState == taskTASK_SCHEDULED_TO_YIELD ); @@ -853,15 +853,16 @@ static void prvAddNewTaskToReadyList( TCB_t * pxNewTCB ) PRIVILEGED_FUNCTION; * its run state. */ portDISABLE_INTERRUPTS(); - portGET_TASK_LOCK(); - portGET_ISR_LOCK(); + xCoreID = ( BaseType_t ) portGET_CORE_ID(); + portGET_TASK_LOCK( xCoreID ); + portGET_ISR_LOCK( xCoreID ); portSET_CRITICAL_NESTING_COUNT( xCoreID, uxPrevCriticalNesting ); if( uxPrevCriticalNesting == 0U ) { - portRELEASE_ISR_LOCK(); + portRELEASE_ISR_LOCK( xCoreID ); } } } @@ -3854,6 +3855,7 @@ void vTaskSuspendAll( void ) #else /* #if ( configNUMBER_OF_CORES == 1 ) */ { UBaseType_t ulState; + BaseType_t xCoreID; /* This must only be called from within a task. */ portASSERT_IF_IN_ISR(); @@ -3867,14 +3869,16 @@ void vTaskSuspendAll( void ) * uxSchedulerSuspended since that will prevent context switches. */ ulState = portSET_INTERRUPT_MASK(); + xCoreID = ( BaseType_t ) portGET_CORE_ID(); + /* This must never be called from inside a critical section. */ - configASSERT( portGET_CRITICAL_NESTING_COUNT( portGET_CORE_ID() ) == 0 ); + configASSERT( portGET_CRITICAL_NESTING_COUNT( xCoreID ) == 0 ); /* portSOFTWARE_BARRIER() is only implemented for emulated/simulated ports that * do not otherwise exhibit real time behaviour. */ portSOFTWARE_BARRIER(); - portGET_TASK_LOCK(); + portGET_TASK_LOCK( xCoreID ); /* uxSchedulerSuspended is increased after prvCheckForRunStateChange. The * purpose is to prevent altering the variable when fromISR APIs are readying @@ -3888,12 +3892,17 @@ void vTaskSuspendAll( void ) mtCOVERAGE_TEST_MARKER(); } - portGET_ISR_LOCK(); + /* Query the coreID again as prvCheckForRunStateChange may have + * caused the task to get scheduled on a different core. The correct + * task lock for the core is acquired in prvCheckForRunStateChange. */ + xCoreID = ( BaseType_t ) portGET_CORE_ID(); + + portGET_ISR_LOCK( xCoreID ); /* The scheduler is suspended if uxSchedulerSuspended is non-zero. An increment * is used to allow calls to vTaskSuspendAll() to nest. */ ++uxSchedulerSuspended; - portRELEASE_ISR_LOCK(); + portRELEASE_ISR_LOCK( xCoreID ); portCLEAR_INTERRUPT_MASK( ulState ); } @@ -3998,7 +4007,7 @@ BaseType_t xTaskResumeAll( void ) configASSERT( uxSchedulerSuspended != 0U ); uxSchedulerSuspended = ( UBaseType_t ) ( uxSchedulerSuspended - 1U ); - portRELEASE_TASK_LOCK(); + portRELEASE_TASK_LOCK( xCoreID ); if( uxSchedulerSuspended == ( UBaseType_t ) 0U ) { @@ -5168,8 +5177,8 @@ BaseType_t xTaskIncrementTick( void ) * and move on if another core suspended the scheduler. We should only * do that if the current core has suspended the scheduler. */ - portGET_TASK_LOCK(); /* Must always acquire the task lock first. */ - portGET_ISR_LOCK(); + portGET_TASK_LOCK( xCoreID ); /* Must always acquire the task lock first. */ + portGET_ISR_LOCK( xCoreID ); { /* vTaskSwitchContext() must never be called from within a critical section. * This is not necessarily true for single core FreeRTOS, but it is for this @@ -5250,8 +5259,8 @@ BaseType_t xTaskIncrementTick( void ) #endif } } - portRELEASE_ISR_LOCK(); - portRELEASE_TASK_LOCK(); + portRELEASE_ISR_LOCK( xCoreID ); + portRELEASE_TASK_LOCK( xCoreID ); traceRETURN_vTaskSwitchContext(); } @@ -6997,8 +7006,8 @@ static void prvResetNextTaskUnblockTime( void ) { if( portGET_CRITICAL_NESTING_COUNT( xCoreID ) == 0U ) { - portGET_TASK_LOCK(); - portGET_ISR_LOCK(); + portGET_TASK_LOCK( xCoreID ); + portGET_ISR_LOCK( xCoreID ); } portINCREMENT_CRITICAL_NESTING_COUNT( xCoreID ); @@ -7051,7 +7060,7 @@ static void prvResetNextTaskUnblockTime( void ) if( portGET_CRITICAL_NESTING_COUNT( xCoreID ) == 0U ) { - portGET_ISR_LOCK(); + portGET_ISR_LOCK( xCoreID ); } portINCREMENT_CRITICAL_NESTING_COUNT( xCoreID ); @@ -7143,8 +7152,8 @@ static void prvResetNextTaskUnblockTime( void ) /* Get the xYieldPending stats inside the critical section. */ xYieldCurrentTask = xYieldPendings[ xCoreID ]; - portRELEASE_ISR_LOCK(); - portRELEASE_TASK_LOCK(); + portRELEASE_ISR_LOCK( xCoreID ); + portRELEASE_TASK_LOCK( xCoreID ); portENABLE_INTERRUPTS(); /* When a task yields in a critical section it just sets @@ -7199,7 +7208,7 @@ static void prvResetNextTaskUnblockTime( void ) if( portGET_CRITICAL_NESTING_COUNT( xCoreID ) == 0U ) { - portRELEASE_ISR_LOCK(); + portRELEASE_ISR_LOCK( xCoreID ); portCLEAR_INTERRUPT_MASK_FROM_ISR( uxSavedInterruptStatus ); } else