Skip to content

Commit

Permalink
change(freertos/smp): Update event_groups.c locking
Browse files Browse the repository at this point in the history 
Updated event_groups.c to use granular locking

- Added xTaskSpinlock and xISRSpinlock
- Replaced critical section macros with data group locking macros
such as taskENTER/EXIT_CRITICAL() with egENTER/EXIT_CRITICAL().
- Added prvLockEventGroupForTasks() and prvUnlockEventGroupForTasks() to suspend the event
group when executing non-deterministic code.
- xEventGroupSetBits() and vEventGroupDelete() accesses the kernel data group
directly. Thus, added vTaskSuspendAll()/xTaskResumeAll() to these fucntions.

Co-authored-by: Sudeep Mohanty <[email protected]>
  • Loading branch information
@Dazza0 @sudeep-mohanty
Dazza0 and sudeep-mohanty committed Jan 14, 2025
1 parent 203b09e commit d881cfa
Show file tree
Hide file tree
Showing 2 changed files with 158 additions and 17 deletions.
171 changes: 154 additions & 17 deletions event_groups.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,10 +63,55 @@
#if ( ( configSUPPORT_STATIC_ALLOCATION == 1 ) && ( configSUPPORT_DYNAMIC_ALLOCATION == 1 ) )
uint8_t ucStaticallyAllocated; /**< Set to pdTRUE if the event group is statically allocated to ensure no attempt is made to free the memory. */
#endif

#if ( ( portUSING_GRANULAR_LOCKS == 1 ) && ( configNUMBER_OF_CORES > 1 ) )
portSPINLOCK_TYPE xTaskSpinlock;
portSPINLOCK_TYPE xISRSpinlock;
#endif /* #if ( ( portUSING_GRANULAR_LOCKS == 1 ) && ( configNUMBER_OF_CORES > 1 ) ) */
} EventGroup_t;

/*-----------------------------------------------------------*/

/*
* Macro to mark the start of a critical code region.
*/
#if ( portUSING_GRANULAR_LOCKS == 1 )
#define egENTER_CRITICAL( pxEventBits ) portLOCK_DATA_GROUP( ( portSPINLOCK_TYPE * ) &( pxEventBits->xTaskSpinlock ), &( pxEventBits->xISRSpinlock ) )
#define egENTER_CRITICAL_FROM_ISR( pxEventBits ) portLOCK_DATA_GROUP_FROM_ISR( ( portSPINLOCK_TYPE * ) &( pxEventBits->xISRSpinlock ) )
#else /* #if ( portUSING_GRANULAR_LOCKS == 1 ) */
#define egENTER_CRITICAL( pxEventBits ) do { ( void ) pxEventBits; taskENTER_CRITICAL(); } while( 0 )
#define egENTER_CRITICAL_FROM_ISR( pxEventBits ) do { ( void ) pxEventBits; taskENTER_CRITICAL_FROM_ISR(); } while( 0 )
#endif /* #if ( portUSING_GRANULAR_LOCKS == 1 ) */

/*
* Macro to mark the end of a critical code region.
*/
#if ( portUSING_GRANULAR_LOCKS == 1 )
#define egEXIT_CRITICAL( pxEventBits ) portUNLOCK_DATA_GROUP( ( portSPINLOCK_TYPE * ) &( pxEventBits->xTaskSpinlock ), &( pxEventBits->xISRSpinlock ) )
#define egEXIT_CRITICAL_FROM_ISR( uxSavedInterruptStatus, pxEventBits ) portUNLOCK_DATA_GROUP_FROM_ISR( uxSavedInterruptStatus, ( portSPINLOCK_TYPE * ) &( pxEventBits->xISRSpinlock ) )
#else /* #if ( portUSING_GRANULAR_LOCKS == 1 ) */
#define egEXIT_CRITICAL( pxEventBits ) do { ( void ) pxEventBits; taskEXIT_CRITICAL(); } while( 0 )
#define egEXIT_CRITICAL_FROM_ISR( uxSavedInterruptStatus, pxEventBits ) do { ( void ) pxEventBits; taskEXIT_CRITICAL_FROM_ISR( uxSavedInterruptStatus ); } while( 0 )
#endif /* #if ( portUSING_GRANULAR_LOCKS == 1 ) */

/*
* Locks an event group for tasks. Prevents other tasks from accessing the event group but allows
* ISRs to pend access to the event group. Caller cannot be preempted by other tasks
* after locking the event group, thus allowing the caller to execute non-deterministic
* operations.
*/
#if ( ( portUSING_GRANULAR_LOCKS == 1 ) && ( configNUMBER_OF_CORES > 1 ) )
static void prvLockEventGroupForTasks( EventGroup_t * pxEventBits ) PRIVILEGED_FUNCTION;
#endif /* #if ( ( portUSING_GRANULAR_LOCKS == 1 ) && ( configNUMBER_OF_CORES > 1 ) ) */

/*
* Unlocks an event group for tasks. Handles all pended access from ISRs, then reenables
* preemption for the caller.
*/
#if ( ( portUSING_GRANULAR_LOCKS == 1 ) && ( configNUMBER_OF_CORES > 1 ) )
static void prvUnlockEventGroupForTasks( EventGroup_t * pxEventBits ) PRIVILEGED_FUNCTION;
#endif /* #if ( ( portUSING_GRANULAR_LOCKS == 1 ) && ( configNUMBER_OF_CORES > 1 ) ) */

/*
* Test the bits set in uxCurrentEventBits to see if the wait condition is met.
* The wait condition is defined by xWaitForAllBits. If xWaitForAllBits is
Expand All @@ -79,6 +124,29 @@
const EventBits_t uxBitsToWaitFor,
const BaseType_t xWaitForAllBits ) PRIVILEGED_FUNCTION;

/*-----------------------------------------------------------*/

/*
* Macro used to lock and unlock an event group. When a task lockss an,
* event group, the task will have thread safe non-deterministic access to
* the event group.
* - Concurrent access from other tasks will be blocked by the xTaskSpinlock
* - Concurrent access from ISRs will be pended
*
* When the task unlocks the event group, all pended access attempts are handled.
*/
#if ( ( portUSING_GRANULAR_LOCKS == 1 ) && ( configNUMBER_OF_CORES > 1 ) )
#define egLOCK( pxEventBits ) prvLockEventGroupForTasks( pxEventBits )
#define egUNLOCK( pxEventBits ) \
( { \
prvUnlockEventGroupForTasks( pxEventBits ); \
pdTRUE; \
} )
#else /* #if ( ( portUSING_GRANULAR_LOCKS == 1 ) && ( configNUMBER_OF_CORES > 1 ) ) */
#define egLOCK( pxEventBits ) vTaskSuspendAll()
#define egUNLOCK( pxEventBits ) xTaskResumeAll()
#endif /* #if ( ( portUSING_GRANULAR_LOCKS == 1 ) && ( configNUMBER_OF_CORES > 1 ) ) */

/*-----------------------------------------------------------*/

#if ( configSUPPORT_STATIC_ALLOCATION == 1 )
Expand Down Expand Up @@ -122,6 +190,13 @@
}
#endif /* configSUPPORT_DYNAMIC_ALLOCATION */

#if ( ( portUSING_GRANULAR_LOCKS == 1 ) && ( configNUMBER_OF_CORES > 1 ) )
{
portINIT_EVENT_GROUP_TASK_SPINLOCK( &( pxEventBits->xTaskSpinlock ) );
portINIT_EVENT_GROUP_ISR_SPINLOCK( &( pxEventBits->xISRSpinlock ) );
}
#endif /* #if ( ( portUSING_GRANULAR_LOCKS == 1 ) && ( configNUMBER_OF_CORES > 1 ) ) */

traceEVENT_GROUP_CREATE( pxEventBits );
}
else
Expand Down Expand Up @@ -167,6 +242,13 @@
}
#endif /* configSUPPORT_STATIC_ALLOCATION */

#if ( ( portUSING_GRANULAR_LOCKS == 1 ) && ( configNUMBER_OF_CORES > 1 ) )
{
portINIT_EVENT_GROUP_TASK_SPINLOCK( &( pxEventBits->xTaskSpinlock ) );
portINIT_EVENT_GROUP_ISR_SPINLOCK( &( pxEventBits->xISRSpinlock ) );
}
#endif /* #if ( ( portUSING_GRANULAR_LOCKS == 1 ) && ( configNUMBER_OF_CORES > 1 ) ) */

traceEVENT_GROUP_CREATE( pxEventBits );
}
else
Expand Down Expand Up @@ -202,7 +284,7 @@
}
#endif

vTaskSuspendAll();
egLOCK( pxEventBits );
{
uxOriginalBitValue = pxEventBits->uxEventBits;

Expand Down Expand Up @@ -245,7 +327,7 @@
}
}
}
xAlreadyYielded = xTaskResumeAll();
xAlreadyYielded = egUNLOCK( pxEventBits );

if( xTicksToWait != ( TickType_t ) 0 )
{
Expand All @@ -267,7 +349,7 @@
if( ( uxReturn & eventUNBLOCKED_DUE_TO_BIT_SET ) == ( EventBits_t ) 0 )
{
/* The task timed out, just return the current event bit value. */
taskENTER_CRITICAL();
egENTER_CRITICAL( pxEventBits );
{
uxReturn = pxEventBits->uxEventBits;

Expand All @@ -284,7 +366,7 @@
mtCOVERAGE_TEST_MARKER();
}
}
taskEXIT_CRITICAL();
egEXIT_CRITICAL( pxEventBits );

xTimeoutOccurred = pdTRUE;
}
Expand Down Expand Up @@ -333,7 +415,7 @@
}
#endif

vTaskSuspendAll();
egLOCK( pxEventBits );
{
const EventBits_t uxCurrentEventBits = pxEventBits->uxEventBits;

Expand Down Expand Up @@ -401,7 +483,7 @@
traceEVENT_GROUP_WAIT_BITS_BLOCK( xEventGroup, uxBitsToWaitFor );
}
}
xAlreadyYielded = xTaskResumeAll();
xAlreadyYielded = egUNLOCK( pxEventBits );

if( xTicksToWait != ( TickType_t ) 0 )
{
Expand All @@ -422,7 +504,7 @@

if( ( uxReturn & eventUNBLOCKED_DUE_TO_BIT_SET ) == ( EventBits_t ) 0 )
{
taskENTER_CRITICAL();
egENTER_CRITICAL( pxEventBits );
{
/* The task timed out, just return the current event bit value. */
uxReturn = pxEventBits->uxEventBits;
Expand All @@ -447,7 +529,7 @@

xTimeoutOccurred = pdTRUE;
}
taskEXIT_CRITICAL();
egEXIT_CRITICAL( pxEventBits );
}
else
{
Expand Down Expand Up @@ -482,7 +564,7 @@
configASSERT( xEventGroup );
configASSERT( ( uxBitsToClear & eventEVENT_BITS_CONTROL_BYTES ) == 0 );

taskENTER_CRITICAL();
egENTER_CRITICAL( pxEventBits );
{
traceEVENT_GROUP_CLEAR_BITS( xEventGroup, uxBitsToClear );

Expand All @@ -493,7 +575,7 @@
/* Clear the bits. */
pxEventBits->uxEventBits &= ~uxBitsToClear;
}
taskEXIT_CRITICAL();
egEXIT_CRITICAL( pxEventBits );

traceRETURN_xEventGroupClearBits( uxReturn );

Expand Down Expand Up @@ -524,19 +606,19 @@
EventBits_t xEventGroupGetBitsFromISR( EventGroupHandle_t xEventGroup )
{
UBaseType_t uxSavedInterruptStatus;
EventGroup_t const * const pxEventBits = xEventGroup;
EventGroup_t * const pxEventBits = xEventGroup;
EventBits_t uxReturn;

traceENTER_xEventGroupGetBitsFromISR( xEventGroup );

/* MISRA Ref 4.7.1 [Return value shall be checked] */
/* More details at: https://github.com/FreeRTOS/FreeRTOS-Kernel/blob/main/MISRA.md#dir-47 */
/* coverity[misra_c_2012_directive_4_7_violation] */
uxSavedInterruptStatus = taskENTER_CRITICAL_FROM_ISR();
uxSavedInterruptStatus = egENTER_CRITICAL_FROM_ISR( pxEventBits );
{
uxReturn = pxEventBits->uxEventBits;
}
taskEXIT_CRITICAL_FROM_ISR( uxSavedInterruptStatus );
egEXIT_CRITICAL_FROM_ISR( uxSavedInterruptStatus, pxEventBits );

traceRETURN_xEventGroupGetBitsFromISR( uxReturn );

Expand Down Expand Up @@ -564,10 +646,17 @@

pxList = &( pxEventBits->xTasksWaitingForBits );
pxListEnd = listGET_END_MARKER( pxList );
vTaskSuspendAll();
egLOCK( pxEventBits );
{
traceEVENT_GROUP_SET_BITS( xEventGroup, uxBitsToSet );

#if ( ( portUSING_GRANULAR_LOCKS == 1 ) && ( configNUMBER_OF_CORES > 1 ) )

/* We are about to access the kernel data group non-deterministically,
* thus we suspend the kernel data group.*/
vTaskSuspendAll();
#endif /* #if ( ( portUSING_GRANULAR_LOCKS == 1 ) && ( configNUMBER_OF_CORES > 1 ) ) */

pxListItem = listGET_HEAD_ENTRY( pxList );

/* Set the bits. */
Expand Down Expand Up @@ -638,8 +727,12 @@

/* Snapshot resulting bits. */
uxReturnBits = pxEventBits->uxEventBits;

#if ( ( portUSING_GRANULAR_LOCKS == 1 ) && ( configNUMBER_OF_CORES > 1 ) )
( void ) xTaskResumeAll();
#endif /* #if ( ( portUSING_GRANULAR_LOCKS == 1 ) && ( configNUMBER_OF_CORES > 1 ) ) */
}
( void ) xTaskResumeAll();
( void ) egUNLOCK( pxEventBits );

traceRETURN_xEventGroupSetBits( uxReturnBits );

Expand All @@ -658,19 +751,30 @@

pxTasksWaitingForBits = &( pxEventBits->xTasksWaitingForBits );

vTaskSuspendAll();
egLOCK( pxEventBits );
{
traceEVENT_GROUP_DELETE( xEventGroup );

#if ( ( portUSING_GRANULAR_LOCKS == 1 ) && ( configNUMBER_OF_CORES > 1 ) )

/* We are about to access the kernel data group non-deterministically,
* thus we suspend the kernel data group.*/
vTaskSuspendAll();
#endif /* #if ( ( portUSING_GRANULAR_LOCKS == 1 ) && ( configNUMBER_OF_CORES > 1 ) ) */

while( listCURRENT_LIST_LENGTH( pxTasksWaitingForBits ) > ( UBaseType_t ) 0 )
{
/* Unblock the task, returning 0 as the event list is being deleted
* and cannot therefore have any bits set. */
configASSERT( pxTasksWaitingForBits->xListEnd.pxNext != ( const ListItem_t * ) &( pxTasksWaitingForBits->xListEnd ) );
vTaskRemoveFromUnorderedEventList( pxTasksWaitingForBits->xListEnd.pxNext, eventUNBLOCKED_DUE_TO_BIT_SET );
}

#if ( ( portUSING_GRANULAR_LOCKS == 1 ) && ( configNUMBER_OF_CORES > 1 ) )
( void ) xTaskResumeAll();
#endif /* #if ( ( portUSING_GRANULAR_LOCKS == 1 ) && ( configNUMBER_OF_CORES > 1 ) ) */
}
( void ) xTaskResumeAll();
( void ) egUNLOCK( pxEventBits );

#if ( ( configSUPPORT_DYNAMIC_ALLOCATION == 1 ) && ( configSUPPORT_STATIC_ALLOCATION == 0 ) )
{
Expand Down Expand Up @@ -775,6 +879,39 @@
}
/*-----------------------------------------------------------*/

#if ( ( portUSING_GRANULAR_LOCKS == 1 ) && ( configNUMBER_OF_CORES > 1 ) )
static void prvLockEventGroupForTasks( EventGroup_t * pxEventBits )
{
/* Disable preempt so that current task cannot be preempted by another task */
vTaskPreemptionDisable( NULL );

portDISABLE_INTERRUPTS();

/* Keep holding xTaskSpinlock to prevent tasks on other cores from accessing
* the event group while it is suspended. */
portGET_SPINLOCK( &( pxEventBits->xTaskSpinlock ) );

portENABLE_INTERRUPTS();
}
#endif /* #if ( ( portUSING_GRANULAR_LOCKS == 1 ) && ( configNUMBER_OF_CORES > 1 ) ) */
/*-----------------------------------------------------------*/

#if ( ( portUSING_GRANULAR_LOCKS == 1 ) && ( configNUMBER_OF_CORES > 1 ) )
static void prvUnlockEventGroupForTasks( EventGroup_t * pxEventBits )
{
portDISABLE_INTERRUPTS();

/* Release the previously held task spinlock */
portRELEASE_SPINLOCK( &( pxEventBits->xTaskSpinlock ) );

portENABLE_INTERRUPTS();

/* Re-enable preemption so that current task cannot be preempted by other tasks */
vTaskPreemptionEnable( NULL );
}
#endif /* #if ( ( portUSING_GRANULAR_LOCKS == 1 ) && ( configNUMBER_OF_CORES > 1 ) ) */
/*-----------------------------------------------------------*/

static BaseType_t prvTestWaitCondition( const EventBits_t uxCurrentEventBits,
const EventBits_t uxBitsToWaitFor,
const BaseType_t xWaitForAllBits )
Expand Down
4 changes: 4 additions & 0 deletions include/FreeRTOS.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3429,6 +3429,10 @@ typedef struct xSTATIC_EVENT_GROUP
#if ( ( configSUPPORT_STATIC_ALLOCATION == 1 ) && ( configSUPPORT_DYNAMIC_ALLOCATION == 1 ) )
uint8_t ucDummy4;
#endif

#if ( ( portUSING_GRANULAR_LOCKS == 1 ) && ( configNUMBER_OF_CORES > 1 ) )
portSPINLOCK_TYPE xDummySpinlock[ 2 ];
#endif /* #if ( ( portUSING_GRANULAR_LOCKS == 1 ) && ( configNUMBER_OF_CORES > 1 ) ) */
} StaticEventGroup_t;

/*
Expand Down

0 comments on commit d881cfa

Please sign in to comment.