Skip to content

Commit f073b35

Browse files
Dazza0sudeep-mohanty
Dazza0
and
sudeep-mohanty
committed
change(freertos/smp): Update stream_buffer.c locking
Updated stream_buffer.c to use granular locking - Added xTaskSpinlock and xISRSpinlock - Replaced critical section macros with data group locking macros such as taskENTER/EXIT_CRITICAL() with sbENTER/EXIT_CRITICAL(). - Added prvLockStreamBufferForTasks() and prvUnlockStreamBufferForTasks() to suspend the stream buffer when executing non-deterministic code. Co-authored-by: Sudeep Mohanty <sudeep.mohanty@espressif.com>
1 parent 8de35a9 commit f073b35

File tree

2 files changed

+134
-19
lines changed

2 files changed

+134
-19
lines changed

‎include/FreeRTOS.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3498,6 +3498,9 @@ typedef struct xSTATIC_STREAM_BUFFER
34983498
void * pvDummy5[ 2 ];
34993499
#endif
35003500
UBaseType_t uxDummy6;
3501+
#if ( ( portUSING_GRANULAR_LOCKS == 1 ) && ( configNUMBER_OF_CORES > 1 ) )
3502+
portSPINLOCK_TYPE xDummySpinlock[ 2 ];
3503+
#endif /* #if ( ( portUSING_GRANULAR_LOCKS == 1 ) && ( configNUMBER_OF_CORES > 1 ) ) */
35013504
} StaticStreamBuffer_t;
35023505

35033506
/* Message buffers are built on stream buffers. */

‎stream_buffer.c

Lines changed: 131 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -58,14 +58,54 @@
5858
#error INCLUDE_xTaskGetCurrentTaskHandle must be set to 1 to build stream_buffer.c
5959
#endif
6060

61+
62+
/*
63+
* Macro to mark the start of a critical code region.
64+
*/
65+
#if ( portUSING_GRANULAR_LOCKS == 1 )
66+
#define sbENTER_CRITICAL( pxStreamBuffer ) portLOCK_DATA_GROUP( ( portSPINLOCK_TYPE * ) &( pxStreamBuffer->xTaskSpinlock ), &( pxStreamBuffer->xISRSpinlock ) )
67+
#define sbENTER_CRITICAL_FROM_ISR( pxStreamBuffer ) portLOCK_DATA_GROUP_FROM_ISR( ( portSPINLOCK_TYPE * ) &( pxStreamBuffer->xISRSpinlock ) )
68+
#else /* #if ( portUSING_GRANULAR_LOCKS == 1 ) */
69+
#define sbENTER_CRITICAL( pxEventBits ) do { ( void ) pxStreamBuffer; taskENTER_CRITICAL(); } while( 0 )
70+
#define sbENTER_CRITICAL_FROM_ISR( pxEventBits ) do { ( void ) pxStreamBuffer; taskENTER_CRITICAL_FROM_ISR(); } while( 0 )
71+
#endif /* #if ( portUSING_GRANULAR_LOCKS == 1 ) */
72+
73+
/*
74+
* Macro to mark the end of a critical code region.
75+
*/
76+
#if ( portUSING_GRANULAR_LOCKS == 1 )
77+
#define sbEXIT_CRITICAL( pxEventBits ) portUNLOCK_DATA_GROUP( ( portSPINLOCK_TYPE * ) &( pxStreamBuffer->xTaskSpinlock ), &( pxStreamBuffer->xISRSpinlock ) )
78+
#define sbEXIT_CRITICAL_FROM_ISR( uxSavedInterruptStatus, pxEventBits ) portUNLOCK_DATA_GROUP_FROM_ISR( uxSavedInterruptStatus, ( portSPINLOCK_TYPE * ) &( pxStreamBuffer->xISRSpinlock ) )
79+
#else /* #if ( portUSING_GRANULAR_LOCKS == 1 ) */
80+
#define egEXIT_CRITICAL( pxEventBits ) do { ( void ) pxEventBits; taskEXIT_CRITICAL(); } while( 0 )
81+
#define egEXIT_CRITICAL_FROM_ISR( uxSavedInterruptStatus, pxStreamBuffer ) do { ( void ) pxStreamBuffer; taskEXIT_CRITICAL_FROM_ISR( uxSavedInterruptStatus ); } while( 0 )
82+
#endif /* #if ( portUSING_GRANULAR_LOCKS == 1 ) */
83+
84+
/*
85+
* Macro used to lock and unlock a stream buffer. When a task locks a stream
86+
* buffer, the task will have thread safe non-deterministic access to the stream
87+
* buffer.
88+
* - Concurrent access from other tasks will be blocked by the xTaskSpinlock
89+
* - Concurrent access from ISRs will be pended
90+
*
91+
* When the task unlocks the stream buffer, all pended access attempts are handled.
92+
*/
93+
#if ( ( portUSING_GRANULAR_LOCKS == 1 ) && ( configNUMBER_OF_CORES > 1 ) )
94+
#define sbLOCK( pxStreamBuffer ) prvLockStreamBufferForTasks( pxStreamBuffer )
95+
#define sbUNLOCK( pxStreamBuffer ) prvUnlockStreamBufferForTasks( pxStreamBuffer )
96+
#else /* #if ( ( portUSING_GRANULAR_LOCKS == 1 ) && ( configNUMBER_OF_CORES > 1 ) ) */
97+
#define sbLOCK( pxStreamBuffer ) vTaskSuspendAll()
98+
#define sbUNLOCK( pxStreamBuffer ) ( void ) xTaskResumeAll()
99+
#endif /* #if ( ( portUSING_GRANULAR_LOCKS == 1 ) && ( configNUMBER_OF_CORES > 1 ) ) */
100+
61101
/* If the user has not provided application specific Rx notification macros,
62102
* or #defined the notification macros away, then provide default implementations
63103
* that uses task notifications. */
64104
#ifndef sbRECEIVE_COMPLETED
65105
#define sbRECEIVE_COMPLETED( pxStreamBuffer ) \
66106
do \
67107
{ \
68-
vTaskSuspendAll(); \
108+
sbLOCK( pxStreamBuffer ); \
69109
{ \
70110
if( ( pxStreamBuffer )->xTaskWaitingToSend != NULL ) \
71111
{ \
@@ -76,7 +116,7 @@
76116
( pxStreamBuffer )->xTaskWaitingToSend = NULL; \
77117
} \
78118
} \
79-
( void ) xTaskResumeAll(); \
119+
( void ) sbUNLOCK( pxStreamBuffer ); \
80120
} while( 0 )
81121
#endif /* sbRECEIVE_COMPLETED */
82122

@@ -105,7 +145,7 @@
105145
do { \
106146
UBaseType_t uxSavedInterruptStatus; \
107147
\
108-
uxSavedInterruptStatus = taskENTER_CRITICAL_FROM_ISR(); \
148+
uxSavedInterruptStatus = sbENTER_CRITICAL_FROM_ISR( pxStreamBuffer ); \
109149
{ \
110150
if( ( pxStreamBuffer )->xTaskWaitingToSend != NULL ) \
111151
{ \
@@ -117,7 +157,7 @@
117157
( pxStreamBuffer )->xTaskWaitingToSend = NULL; \
118158
} \
119159
} \
120-
taskEXIT_CRITICAL_FROM_ISR( uxSavedInterruptStatus ); \
160+
sbEXIT_CRITICAL_FROM_ISR( uxSavedInterruptStatus, pxStreamBuffer ); \
121161
} while( 0 )
122162
#endif /* sbRECEIVE_COMPLETED_FROM_ISR */
123163

@@ -145,7 +185,7 @@
145185
*/
146186
#ifndef sbSEND_COMPLETED
147187
#define sbSEND_COMPLETED( pxStreamBuffer ) \
148-
vTaskSuspendAll(); \
188+
sbLOCK( pxStreamBuffer ); \
149189
{ \
150190
if( ( pxStreamBuffer )->xTaskWaitingToReceive != NULL ) \
151191
{ \
@@ -156,7 +196,7 @@
156196
( pxStreamBuffer )->xTaskWaitingToReceive = NULL; \
157197
} \
158198
} \
159-
( void ) xTaskResumeAll()
199+
( void ) sbUNLOCK( pxStreamBuffer )
160200
#endif /* sbSEND_COMPLETED */
161201

162202
/* If user has provided a per-instance send completed callback, then
@@ -184,7 +224,7 @@
184224
do { \
185225
UBaseType_t uxSavedInterruptStatus; \
186226
\
187-
uxSavedInterruptStatus = taskENTER_CRITICAL_FROM_ISR(); \
227+
uxSavedInterruptStatus = sbENTER_CRITICAL_FROM_ISR( pxStreamBuffer ); \
188228
{ \
189229
if( ( pxStreamBuffer )->xTaskWaitingToReceive != NULL ) \
190230
{ \
@@ -196,7 +236,7 @@
196236
( pxStreamBuffer )->xTaskWaitingToReceive = NULL; \
197237
} \
198238
} \
199-
taskEXIT_CRITICAL_FROM_ISR( uxSavedInterruptStatus ); \
239+
sbEXIT_CRITICAL_FROM_ISR( uxSavedInterruptStatus, pxStreamBuffer ); \
200240
} while( 0 )
201241
#endif /* sbSEND_COMPLETE_FROM_ISR */
202242

@@ -249,8 +289,30 @@ typedef struct StreamBufferDef_t
249289
StreamBufferCallbackFunction_t pxReceiveCompletedCallback; /* Optional callback called on receive complete. sbRECEIVE_COMPLETED is called if this is NULL. */
250290
#endif
251291
UBaseType_t uxNotificationIndex; /* The index we are using for notification, by default tskDEFAULT_INDEX_TO_NOTIFY. */
292+
#if ( ( portUSING_GRANULAR_LOCKS == 1 ) && ( configNUMBER_OF_CORES > 1 ) )
293+
portSPINLOCK_TYPE xTaskSpinlock;
294+
portSPINLOCK_TYPE xISRSpinlock;
295+
#endif /* #if ( ( portUSING_GRANULAR_LOCKS == 1 ) && ( configNUMBER_OF_CORES > 1 ) ) */
252296
} StreamBuffer_t;
253297

298+
/*
299+
* Locks a stream buffer for tasks. Prevents other tasks from accessing the stream buffer
300+
* but allows ISRs to pend access to the stream buffer. Caller cannot be preempted
301+
* by other tasks after locking the stream buffer, thus allowing the caller to
302+
* execute non-deterministic operations.
303+
*/
304+
#if ( ( portUSING_GRANULAR_LOCKS == 1 ) && ( configNUMBER_OF_CORES > 1 ) )
305+
static void prvLockStreamBufferForTasks( StreamBuffer_t * const pxStreamBuffer ) PRIVILEGED_FUNCTION;
306+
#endif /* #if ( ( portUSING_GRANULAR_LOCKS == 1 ) && ( configNUMBER_OF_CORES > 1 ) ) */
307+
308+
/*
309+
* Unlocks a stream buffer for tasks. Handles all pended access from ISRs, then reenables preemption
310+
* for the caller.
311+
*/
312+
#if ( ( portUSING_GRANULAR_LOCKS == 1 ) && ( configNUMBER_OF_CORES > 1 ) )
313+
static void prvUnlockStreamBufferForTasks( StreamBuffer_t * const pxStreamBuffer ) PRIVILEGED_FUNCTION;
314+
#endif /* #if ( ( portUSING_GRANULAR_LOCKS == 1 ) && ( configNUMBER_OF_CORES > 1 ) ) */
315+
254316
/*
255317
* The number of bytes available to be read from the buffer.
256318
*/
@@ -327,6 +389,42 @@ static void prvInitialiseNewStreamBuffer( StreamBuffer_t * const pxStreamBuffer,
327389
StreamBufferCallbackFunction_t pxReceiveCompletedCallback ) PRIVILEGED_FUNCTION;
328390

329391
/*-----------------------------------------------------------*/
392+
393+
#if ( ( portUSING_GRANULAR_LOCKS == 1 ) && ( configNUMBER_OF_CORES > 1 ) )
394+
static void prvLockStreamBufferForTasks( StreamBuffer_t * const pxStreamBuffer )
395+
{
396+
/* Disable preempt so that current task cannot be preempted by another task */
397+
vTaskPreemptionDisable( NULL );
398+
399+
/* Lock the stream buffer data group so that we can suspend the stream buffer atomically */
400+
sbENTER_CRITICAL( pxStreamBuffer );
401+
402+
/* Keep holding xTaskSpinlock after unlocking the data group to prevent tasks
403+
* on other cores from accessing the stream buffer while it is suspended. */
404+
portGET_SPINLOCK( portGET_CORE_ID(), &( pxStreamBuffer->xTaskSpinlock ) );
405+
406+
sbEXIT_CRITICAL( pxStreamBuffer );
407+
}
408+
#endif /* #if ( ( portUSING_GRANULAR_LOCKS == 1 ) && ( configNUMBER_OF_CORES > 1 ) ) */
409+
/*-----------------------------------------------------------*/
410+
411+
#if ( ( portUSING_GRANULAR_LOCKS == 1 ) && ( configNUMBER_OF_CORES > 1 ) )
412+
static void prvUnlockStreamBufferForTasks( StreamBuffer_t * const pxStreamBuffer )
413+
{
414+
/* Lock the stream buffer data group so that we can handle any pended accesses atomically */
415+
sbENTER_CRITICAL( pxStreamBuffer );
416+
417+
/* Release the previously held task spinlock */
418+
portRELEASE_SPINLOCK( portGET_CORE_ID(), &( pxStreamBuffer->xTaskSpinlock ) );
419+
420+
sbEXIT_CRITICAL( pxStreamBuffer );
421+
422+
/* Re-enable preemption so that current task cannot be preempted by other tasks */
423+
vTaskPreemptionEnable( NULL );
424+
}
425+
#endif /* #if ( ( portUSING_GRANULAR_LOCKS == 1 ) && ( configNUMBER_OF_CORES > 1 ) ) */
426+
/*-----------------------------------------------------------*/
427+
330428
#if ( configSUPPORT_DYNAMIC_ALLOCATION == 1 )
331429
StreamBufferHandle_t xStreamBufferGenericCreate( size_t xBufferSizeBytes,
332430
size_t xTriggerLevelBytes,
@@ -405,6 +503,13 @@ static void prvInitialiseNewStreamBuffer( StreamBuffer_t * const pxStreamBuffer,
405503
pxSendCompletedCallback,
406504
pxReceiveCompletedCallback );
407505

506+
#if ( ( portUSING_GRANULAR_LOCKS == 1 ) && ( configNUMBER_OF_CORES > 1 ) )
507+
{
508+
portINIT_STREAM_BUFFER_TASK_SPINLOCK( &( ( ( StreamBuffer_t * ) pvAllocatedMemory )->xTaskSpinlock ) );
509+
portINIT_STREAM_BUFFER_ISR_SPINLOCK( &( ( ( StreamBuffer_t * ) pvAllocatedMemory )->xISRSpinlock ) );
510+
}
511+
#endif /* #if ( ( portUSING_GRANULAR_LOCKS == 1 ) && ( configNUMBER_OF_CORES > 1 ) ) */
512+
408513
traceSTREAM_BUFFER_CREATE( ( ( StreamBuffer_t * ) pvAllocatedMemory ), xStreamBufferType );
409514
}
410515
else
@@ -499,6 +604,13 @@ static void prvInitialiseNewStreamBuffer( StreamBuffer_t * const pxStreamBuffer,
499604
* again. */
500605
pxStreamBuffer->ucFlags |= sbFLAGS_IS_STATICALLY_ALLOCATED;
501606

607+
#if ( ( portUSING_GRANULAR_LOCKS == 1 ) && ( configNUMBER_OF_CORES > 1 ) )
608+
{
609+
portINIT_STREAM_BUFFER_TASK_SPINLOCK( &( pxStreamBuffer->xTaskSpinlock ) );
610+
portINIT_STREAM_BUFFER_ISR_SPINLOCK( &( pxStreamBuffer->xISRSpinlock ) );
611+
}
612+
#endif /* #if ( ( portUSING_GRANULAR_LOCKS == 1 ) && ( configNUMBER_OF_CORES > 1 ) ) */
613+
502614
traceSTREAM_BUFFER_CREATE( pxStreamBuffer, xStreamBufferType );
503615

504616
/* MISRA Ref 11.3.1 [Misaligned access] */
@@ -614,7 +726,7 @@ BaseType_t xStreamBufferReset( StreamBufferHandle_t xStreamBuffer )
614726
#endif
615727

616728
/* Can only reset a message buffer if there are no tasks blocked on it. */
617-
taskENTER_CRITICAL();
729+
sbENTER_CRITICAL( pxStreamBuffer );
618730
{
619731
if( ( pxStreamBuffer->xTaskWaitingToReceive == NULL ) && ( pxStreamBuffer->xTaskWaitingToSend == NULL ) )
620732
{
@@ -644,7 +756,7 @@ BaseType_t xStreamBufferReset( StreamBufferHandle_t xStreamBuffer )
644756
xReturn = pdPASS;
645757
}
646758
}
647-
taskEXIT_CRITICAL();
759+
sbEXIT_CRITICAL( pxStreamBuffer );
648760

649761
traceRETURN_xStreamBufferReset( xReturn );
650762

@@ -872,7 +984,7 @@ size_t xStreamBufferSend( StreamBufferHandle_t xStreamBuffer,
872984
{
873985
/* Wait until the required number of bytes are free in the message
874986
* buffer. */
875-
taskENTER_CRITICAL();
987+
sbENTER_CRITICAL( pxStreamBuffer );
876988
{
877989
xSpace = xStreamBufferSpacesAvailable( pxStreamBuffer );
878990

@@ -887,11 +999,11 @@ size_t xStreamBufferSend( StreamBufferHandle_t xStreamBuffer,
887999
}
8881000
else
8891001
{
890-
taskEXIT_CRITICAL();
1002+
sbEXIT_CRITICAL( pxStreamBuffer );
8911003
break;
8921004
}
8931005
}
894-
taskEXIT_CRITICAL();
1006+
sbEXIT_CRITICAL( pxStreamBuffer );
8951007

8961008
traceBLOCKING_ON_STREAM_BUFFER_SEND( xStreamBuffer );
8971009
( void ) xTaskNotifyWaitIndexed( pxStreamBuffer->uxNotificationIndex, ( uint32_t ) 0, ( uint32_t ) 0, NULL, xTicksToWait );
@@ -1087,7 +1199,7 @@ size_t xStreamBufferReceive( StreamBufferHandle_t xStreamBuffer,
10871199
{
10881200
/* Checking if there is data and clearing the notification state must be
10891201
* performed atomically. */
1090-
taskENTER_CRITICAL();
1202+
sbENTER_CRITICAL( pxStreamBuffer );
10911203
{
10921204
xBytesAvailable = prvBytesInBuffer( pxStreamBuffer );
10931205

@@ -1112,7 +1224,7 @@ size_t xStreamBufferReceive( StreamBufferHandle_t xStreamBuffer,
11121224
mtCOVERAGE_TEST_MARKER();
11131225
}
11141226
}
1115-
taskEXIT_CRITICAL();
1227+
sbEXIT_CRITICAL( pxStreamBuffer );
11161228

11171229
if( xBytesAvailable <= xBytesToStoreMessageLength )
11181230
{
@@ -1409,7 +1521,7 @@ BaseType_t xStreamBufferSendCompletedFromISR( StreamBufferHandle_t xStreamBuffer
14091521
/* MISRA Ref 4.7.1 [Return value shall be checked] */
14101522
/* More details at: https://github.com/FreeRTOS/FreeRTOS-Kernel/blob/main/MISRA.md#dir-47 */
14111523
/* coverity[misra_c_2012_directive_4_7_violation] */
1412-
uxSavedInterruptStatus = taskENTER_CRITICAL_FROM_ISR();
1524+
uxSavedInterruptStatus = sbENTER_CRITICAL_FROM_ISR( pxStreamBuffer );
14131525
{
14141526
if( ( pxStreamBuffer )->xTaskWaitingToReceive != NULL )
14151527
{
@@ -1426,7 +1538,7 @@ BaseType_t xStreamBufferSendCompletedFromISR( StreamBufferHandle_t xStreamBuffer
14261538
xReturn = pdFALSE;
14271539
}
14281540
}
1429-
taskEXIT_CRITICAL_FROM_ISR( uxSavedInterruptStatus );
1541+
sbEXIT_CRITICAL_FROM_ISR( uxSavedInterruptStatus, pxStreamBuffer );
14301542

14311543
traceRETURN_xStreamBufferSendCompletedFromISR( xReturn );
14321544

@@ -1448,7 +1560,7 @@ BaseType_t xStreamBufferReceiveCompletedFromISR( StreamBufferHandle_t xStreamBuf
14481560
/* MISRA Ref 4.7.1 [Return value shall be checked] */
14491561
/* More details at: https://github.com/FreeRTOS/FreeRTOS-Kernel/blob/main/MISRA.md#dir-47 */
14501562
/* coverity[misra_c_2012_directive_4_7_violation] */
1451-
uxSavedInterruptStatus = taskENTER_CRITICAL_FROM_ISR();
1563+
uxSavedInterruptStatus = sbENTER_CRITICAL_FROM_ISR( pxStreamBuffer );
14521564
{
14531565
if( ( pxStreamBuffer )->xTaskWaitingToSend != NULL )
14541566
{
@@ -1465,7 +1577,7 @@ BaseType_t xStreamBufferReceiveCompletedFromISR( StreamBufferHandle_t xStreamBuf
14651577
xReturn = pdFALSE;
14661578
}
14671579
}
1468-
taskEXIT_CRITICAL_FROM_ISR( uxSavedInterruptStatus );
1580+
sbEXIT_CRITICAL_FROM_ISR( uxSavedInterruptStatus, pxStreamBuffer );
14691581

14701582
traceRETURN_xStreamBufferReceiveCompletedFromISR( xReturn );
14711583

0 commit comments

Comments
 (0)