Skip to content

Commit 8de35a9

Browse files
Dazza0sudeep-mohanty
Dazza0
and
sudeep-mohanty
committed
change(freertos/smp): Update event_groups.c locking
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 <sudeep.mohanty@espressif.com>
1 parent cf55f11 commit 8de35a9

File tree

2 files changed

+158
-17
lines changed

2 files changed

+158
-17
lines changed

‎event_groups.c

Lines changed: 154 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -63,10 +63,55 @@
6363
#if ( ( configSUPPORT_STATIC_ALLOCATION == 1 ) && ( configSUPPORT_DYNAMIC_ALLOCATION == 1 ) )
6464
uint8_t ucStaticallyAllocated; /**< Set to pdTRUE if the event group is statically allocated to ensure no attempt is made to free the memory. */
6565
#endif
66+
67+
#if ( ( portUSING_GRANULAR_LOCKS == 1 ) && ( configNUMBER_OF_CORES > 1 ) )
68+
portSPINLOCK_TYPE xTaskSpinlock;
69+
portSPINLOCK_TYPE xISRSpinlock;
70+
#endif /* #if ( ( portUSING_GRANULAR_LOCKS == 1 ) && ( configNUMBER_OF_CORES > 1 ) ) */
6671
} EventGroup_t;
6772

6873
/*-----------------------------------------------------------*/
6974

75+
/*
76+
* Macro to mark the start of a critical code region.
77+
*/
78+
#if ( portUSING_GRANULAR_LOCKS == 1 )
79+
#define egENTER_CRITICAL( pxEventBits ) portLOCK_DATA_GROUP( ( portSPINLOCK_TYPE * ) &( pxEventBits->xTaskSpinlock ), &( pxEventBits->xISRSpinlock ) )
80+
#define egENTER_CRITICAL_FROM_ISR( pxEventBits ) portLOCK_DATA_GROUP_FROM_ISR( ( portSPINLOCK_TYPE * ) &( pxEventBits->xISRSpinlock ) )
81+
#else /* #if ( portUSING_GRANULAR_LOCKS == 1 ) */
82+
#define egENTER_CRITICAL( pxEventBits ) do { ( void ) pxEventBits; taskENTER_CRITICAL(); } while( 0 )
83+
#define egENTER_CRITICAL_FROM_ISR( pxEventBits ) do { ( void ) pxEventBits; taskENTER_CRITICAL_FROM_ISR(); } while( 0 )
84+
#endif /* #if ( portUSING_GRANULAR_LOCKS == 1 ) */
85+
86+
/*
87+
* Macro to mark the end of a critical code region.
88+
*/
89+
#if ( portUSING_GRANULAR_LOCKS == 1 )
90+
#define egEXIT_CRITICAL( pxEventBits ) portUNLOCK_DATA_GROUP( ( portSPINLOCK_TYPE * ) &( pxEventBits->xTaskSpinlock ), &( pxEventBits->xISRSpinlock ) )
91+
#define egEXIT_CRITICAL_FROM_ISR( uxSavedInterruptStatus, pxEventBits ) portUNLOCK_DATA_GROUP_FROM_ISR( uxSavedInterruptStatus, ( portSPINLOCK_TYPE * ) &( pxEventBits->xISRSpinlock ) )
92+
#else /* #if ( portUSING_GRANULAR_LOCKS == 1 ) */
93+
#define egEXIT_CRITICAL( pxEventBits ) do { ( void ) pxEventBits; taskEXIT_CRITICAL(); } while( 0 )
94+
#define egEXIT_CRITICAL_FROM_ISR( uxSavedInterruptStatus, pxEventBits ) do { ( void ) pxEventBits; taskEXIT_CRITICAL_FROM_ISR( uxSavedInterruptStatus ); } while( 0 )
95+
#endif /* #if ( portUSING_GRANULAR_LOCKS == 1 ) */
96+
97+
/*
98+
* Locks an event group for tasks. Prevents other tasks from accessing the event group but allows
99+
* ISRs to pend access to the event group. Caller cannot be preempted by other tasks
100+
* after locking the event group, thus allowing the caller to execute non-deterministic
101+
* operations.
102+
*/
103+
#if ( ( portUSING_GRANULAR_LOCKS == 1 ) && ( configNUMBER_OF_CORES > 1 ) )
104+
static void prvLockEventGroupForTasks( EventGroup_t * pxEventBits ) PRIVILEGED_FUNCTION;
105+
#endif /* #if ( ( portUSING_GRANULAR_LOCKS == 1 ) && ( configNUMBER_OF_CORES > 1 ) ) */
106+
107+
/*
108+
* Unlocks an event group for tasks. Handles all pended access from ISRs, then reenables
109+
* preemption for the caller.
110+
*/
111+
#if ( ( portUSING_GRANULAR_LOCKS == 1 ) && ( configNUMBER_OF_CORES > 1 ) )
112+
static void prvUnlockEventGroupForTasks( EventGroup_t * pxEventBits ) PRIVILEGED_FUNCTION;
113+
#endif /* #if ( ( portUSING_GRANULAR_LOCKS == 1 ) && ( configNUMBER_OF_CORES > 1 ) ) */
114+
70115
/*
71116
* Test the bits set in uxCurrentEventBits to see if the wait condition is met.
72117
* The wait condition is defined by xWaitForAllBits. If xWaitForAllBits is
@@ -79,6 +124,29 @@
79124
const EventBits_t uxBitsToWaitFor,
80125
const BaseType_t xWaitForAllBits ) PRIVILEGED_FUNCTION;
81126

127+
/*-----------------------------------------------------------*/
128+
129+
/*
130+
* Macro used to lock and unlock an event group. When a task lockss an,
131+
* event group, the task will have thread safe non-deterministic access to
132+
* the event group.
133+
* - Concurrent access from other tasks will be blocked by the xTaskSpinlock
134+
* - Concurrent access from ISRs will be pended
135+
*
136+
* When the task unlocks the event group, all pended access attempts are handled.
137+
*/
138+
#if ( ( portUSING_GRANULAR_LOCKS == 1 ) && ( configNUMBER_OF_CORES > 1 ) )
139+
#define egLOCK( pxEventBits ) prvLockEventGroupForTasks( pxEventBits )
140+
#define egUNLOCK( pxEventBits ) \
141+
( { \
142+
prvUnlockEventGroupForTasks( pxEventBits ); \
143+
pdTRUE; \
144+
} )
145+
#else /* #if ( ( portUSING_GRANULAR_LOCKS == 1 ) && ( configNUMBER_OF_CORES > 1 ) ) */
146+
#define egLOCK( pxEventBits ) vTaskSuspendAll()
147+
#define egUNLOCK( pxEventBits ) xTaskResumeAll()
148+
#endif /* #if ( ( portUSING_GRANULAR_LOCKS == 1 ) && ( configNUMBER_OF_CORES > 1 ) ) */
149+
82150
/*-----------------------------------------------------------*/
83151

84152
#if ( configSUPPORT_STATIC_ALLOCATION == 1 )
@@ -122,6 +190,13 @@
122190
}
123191
#endif /* configSUPPORT_DYNAMIC_ALLOCATION */
124192

193+
#if ( ( portUSING_GRANULAR_LOCKS == 1 ) && ( configNUMBER_OF_CORES > 1 ) )
194+
{
195+
portINIT_EVENT_GROUP_TASK_SPINLOCK( &( pxEventBits->xTaskSpinlock ) );
196+
portINIT_EVENT_GROUP_ISR_SPINLOCK( &( pxEventBits->xISRSpinlock ) );
197+
}
198+
#endif /* #if ( ( portUSING_GRANULAR_LOCKS == 1 ) && ( configNUMBER_OF_CORES > 1 ) ) */
199+
125200
traceEVENT_GROUP_CREATE( pxEventBits );
126201
}
127202
else
@@ -167,6 +242,13 @@
167242
}
168243
#endif /* configSUPPORT_STATIC_ALLOCATION */
169244

245+
#if ( ( portUSING_GRANULAR_LOCKS == 1 ) && ( configNUMBER_OF_CORES > 1 ) )
246+
{
247+
portINIT_EVENT_GROUP_TASK_SPINLOCK( &( pxEventBits->xTaskSpinlock ) );
248+
portINIT_EVENT_GROUP_ISR_SPINLOCK( &( pxEventBits->xISRSpinlock ) );
249+
}
250+
#endif /* #if ( ( portUSING_GRANULAR_LOCKS == 1 ) && ( configNUMBER_OF_CORES > 1 ) ) */
251+
170252
traceEVENT_GROUP_CREATE( pxEventBits );
171253
}
172254
else
@@ -202,7 +284,7 @@
202284
}
203285
#endif
204286

205-
vTaskSuspendAll();
287+
egLOCK( pxEventBits );
206288
{
207289
uxOriginalBitValue = pxEventBits->uxEventBits;
208290

@@ -245,7 +327,7 @@
245327
}
246328
}
247329
}
248-
xAlreadyYielded = xTaskResumeAll();
330+
xAlreadyYielded = egUNLOCK( pxEventBits );
249331

250332
if( xTicksToWait != ( TickType_t ) 0 )
251333
{
@@ -267,7 +349,7 @@
267349
if( ( uxReturn & eventUNBLOCKED_DUE_TO_BIT_SET ) == ( EventBits_t ) 0 )
268350
{
269351
/* The task timed out, just return the current event bit value. */
270-
taskENTER_CRITICAL();
352+
egENTER_CRITICAL( pxEventBits );
271353
{
272354
uxReturn = pxEventBits->uxEventBits;
273355

@@ -284,7 +366,7 @@
284366
mtCOVERAGE_TEST_MARKER();
285367
}
286368
}
287-
taskEXIT_CRITICAL();
369+
egEXIT_CRITICAL( pxEventBits );
288370

289371
xTimeoutOccurred = pdTRUE;
290372
}
@@ -333,7 +415,7 @@
333415
}
334416
#endif
335417

336-
vTaskSuspendAll();
418+
egLOCK( pxEventBits );
337419
{
338420
const EventBits_t uxCurrentEventBits = pxEventBits->uxEventBits;
339421

@@ -401,7 +483,7 @@
401483
traceEVENT_GROUP_WAIT_BITS_BLOCK( xEventGroup, uxBitsToWaitFor );
402484
}
403485
}
404-
xAlreadyYielded = xTaskResumeAll();
486+
xAlreadyYielded = egUNLOCK( pxEventBits );
405487

406488
if( xTicksToWait != ( TickType_t ) 0 )
407489
{
@@ -422,7 +504,7 @@
422504

423505
if( ( uxReturn & eventUNBLOCKED_DUE_TO_BIT_SET ) == ( EventBits_t ) 0 )
424506
{
425-
taskENTER_CRITICAL();
507+
egENTER_CRITICAL( pxEventBits );
426508
{
427509
/* The task timed out, just return the current event bit value. */
428510
uxReturn = pxEventBits->uxEventBits;
@@ -447,7 +529,7 @@
447529

448530
xTimeoutOccurred = pdTRUE;
449531
}
450-
taskEXIT_CRITICAL();
532+
egEXIT_CRITICAL( pxEventBits );
451533
}
452534
else
453535
{
@@ -482,7 +564,7 @@
482564
configASSERT( xEventGroup );
483565
configASSERT( ( uxBitsToClear & eventEVENT_BITS_CONTROL_BYTES ) == 0 );
484566

485-
taskENTER_CRITICAL();
567+
egENTER_CRITICAL( pxEventBits );
486568
{
487569
traceEVENT_GROUP_CLEAR_BITS( xEventGroup, uxBitsToClear );
488570

@@ -493,7 +575,7 @@
493575
/* Clear the bits. */
494576
pxEventBits->uxEventBits &= ~uxBitsToClear;
495577
}
496-
taskEXIT_CRITICAL();
578+
egEXIT_CRITICAL( pxEventBits );
497579

498580
traceRETURN_xEventGroupClearBits( uxReturn );
499581

@@ -524,19 +606,19 @@
524606
EventBits_t xEventGroupGetBitsFromISR( EventGroupHandle_t xEventGroup )
525607
{
526608
UBaseType_t uxSavedInterruptStatus;
527-
EventGroup_t const * const pxEventBits = xEventGroup;
609+
EventGroup_t * const pxEventBits = xEventGroup;
528610
EventBits_t uxReturn;
529611

530612
traceENTER_xEventGroupGetBitsFromISR( xEventGroup );
531613

532614
/* MISRA Ref 4.7.1 [Return value shall be checked] */
533615
/* More details at: https://github.com/FreeRTOS/FreeRTOS-Kernel/blob/main/MISRA.md#dir-47 */
534616
/* coverity[misra_c_2012_directive_4_7_violation] */
535-
uxSavedInterruptStatus = taskENTER_CRITICAL_FROM_ISR();
617+
uxSavedInterruptStatus = egENTER_CRITICAL_FROM_ISR( pxEventBits );
536618
{
537619
uxReturn = pxEventBits->uxEventBits;
538620
}
539-
taskEXIT_CRITICAL_FROM_ISR( uxSavedInterruptStatus );
621+
egEXIT_CRITICAL_FROM_ISR( uxSavedInterruptStatus, pxEventBits );
540622

541623
traceRETURN_xEventGroupGetBitsFromISR( uxReturn );
542624

@@ -564,10 +646,17 @@
564646

565647
pxList = &( pxEventBits->xTasksWaitingForBits );
566648
pxListEnd = listGET_END_MARKER( pxList );
567-
vTaskSuspendAll();
649+
egLOCK( pxEventBits );
568650
{
569651
traceEVENT_GROUP_SET_BITS( xEventGroup, uxBitsToSet );
570652

653+
#if ( ( portUSING_GRANULAR_LOCKS == 1 ) && ( configNUMBER_OF_CORES > 1 ) )
654+
655+
/* We are about to access the kernel data group non-deterministically,
656+
* thus we suspend the kernel data group.*/
657+
vTaskSuspendAll();
658+
#endif /* #if ( ( portUSING_GRANULAR_LOCKS == 1 ) && ( configNUMBER_OF_CORES > 1 ) ) */
659+
571660
pxListItem = listGET_HEAD_ENTRY( pxList );
572661

573662
/* Set the bits. */
@@ -638,8 +727,12 @@
638727

639728
/* Snapshot resulting bits. */
640729
uxReturnBits = pxEventBits->uxEventBits;
730+
731+
#if ( ( portUSING_GRANULAR_LOCKS == 1 ) && ( configNUMBER_OF_CORES > 1 ) )
732+
( void ) xTaskResumeAll();
733+
#endif /* #if ( ( portUSING_GRANULAR_LOCKS == 1 ) && ( configNUMBER_OF_CORES > 1 ) ) */
641734
}
642-
( void ) xTaskResumeAll();
735+
( void ) egUNLOCK( pxEventBits );
643736

644737
traceRETURN_xEventGroupSetBits( uxReturnBits );
645738

@@ -658,19 +751,30 @@
658751

659752
pxTasksWaitingForBits = &( pxEventBits->xTasksWaitingForBits );
660753

661-
vTaskSuspendAll();
754+
egLOCK( pxEventBits );
662755
{
663756
traceEVENT_GROUP_DELETE( xEventGroup );
664757

758+
#if ( ( portUSING_GRANULAR_LOCKS == 1 ) && ( configNUMBER_OF_CORES > 1 ) )
759+
760+
/* We are about to access the kernel data group non-deterministically,
761+
* thus we suspend the kernel data group.*/
762+
vTaskSuspendAll();
763+
#endif /* #if ( ( portUSING_GRANULAR_LOCKS == 1 ) && ( configNUMBER_OF_CORES > 1 ) ) */
764+
665765
while( listCURRENT_LIST_LENGTH( pxTasksWaitingForBits ) > ( UBaseType_t ) 0 )
666766
{
667767
/* Unblock the task, returning 0 as the event list is being deleted
668768
* and cannot therefore have any bits set. */
669769
configASSERT( pxTasksWaitingForBits->xListEnd.pxNext != ( const ListItem_t * ) &( pxTasksWaitingForBits->xListEnd ) );
670770
vTaskRemoveFromUnorderedEventList( pxTasksWaitingForBits->xListEnd.pxNext, eventUNBLOCKED_DUE_TO_BIT_SET );
671771
}
772+
773+
#if ( ( portUSING_GRANULAR_LOCKS == 1 ) && ( configNUMBER_OF_CORES > 1 ) )
774+
( void ) xTaskResumeAll();
775+
#endif /* #if ( ( portUSING_GRANULAR_LOCKS == 1 ) && ( configNUMBER_OF_CORES > 1 ) ) */
672776
}
673-
( void ) xTaskResumeAll();
777+
( void ) egUNLOCK( pxEventBits );
674778

675779
#if ( ( configSUPPORT_DYNAMIC_ALLOCATION == 1 ) && ( configSUPPORT_STATIC_ALLOCATION == 0 ) )
676780
{
@@ -775,6 +879,39 @@
775879
}
776880
/*-----------------------------------------------------------*/
777881

882+
#if ( ( portUSING_GRANULAR_LOCKS == 1 ) && ( configNUMBER_OF_CORES > 1 ) )
883+
static void prvLockEventGroupForTasks( EventGroup_t * pxEventBits )
884+
{
885+
/* Disable preempt so that current task cannot be preempted by another task */
886+
vTaskPreemptionDisable( NULL );
887+
888+
portDISABLE_INTERRUPTS();
889+
890+
/* Keep holding xTaskSpinlock to prevent tasks on other cores from accessing
891+
* the event group while it is suspended. */
892+
portGET_SPINLOCK( portGET_CORE_ID(), &( pxEventBits->xTaskSpinlock ) );
893+
894+
portENABLE_INTERRUPTS();
895+
}
896+
#endif /* #if ( ( portUSING_GRANULAR_LOCKS == 1 ) && ( configNUMBER_OF_CORES > 1 ) ) */
897+
/*-----------------------------------------------------------*/
898+
899+
#if ( ( portUSING_GRANULAR_LOCKS == 1 ) && ( configNUMBER_OF_CORES > 1 ) )
900+
static void prvUnlockEventGroupForTasks( EventGroup_t * pxEventBits )
901+
{
902+
portDISABLE_INTERRUPTS();
903+
904+
/* Release the previously held task spinlock */
905+
portRELEASE_SPINLOCK( portGET_CORE_ID(), &( pxEventBits->xTaskSpinlock ) );
906+
907+
portENABLE_INTERRUPTS();
908+
909+
/* Re-enable preemption so that current task cannot be preempted by other tasks */
910+
vTaskPreemptionEnable( NULL );
911+
}
912+
#endif /* #if ( ( portUSING_GRANULAR_LOCKS == 1 ) && ( configNUMBER_OF_CORES > 1 ) ) */
913+
/*-----------------------------------------------------------*/
914+
778915
static BaseType_t prvTestWaitCondition( const EventBits_t uxCurrentEventBits,
779916
const EventBits_t uxBitsToWaitFor,
780917
const BaseType_t xWaitForAllBits )

‎include/FreeRTOS.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3439,6 +3439,10 @@ typedef struct xSTATIC_EVENT_GROUP
34393439
#if ( ( configSUPPORT_STATIC_ALLOCATION == 1 ) && ( configSUPPORT_DYNAMIC_ALLOCATION == 1 ) )
34403440
uint8_t ucDummy4;
34413441
#endif
3442+
3443+
#if ( ( portUSING_GRANULAR_LOCKS == 1 ) && ( configNUMBER_OF_CORES > 1 ) )
3444+
portSPINLOCK_TYPE xDummySpinlock[ 2 ];
3445+
#endif /* #if ( ( portUSING_GRANULAR_LOCKS == 1 ) && ( configNUMBER_OF_CORES > 1 ) ) */
34423446
} StaticEventGroup_t;
34433447

34443448
/*

0 commit comments

Comments
 (0)