From 40b96cf596115fa9d5357245309aa59b422c3b55 Mon Sep 17 00:00:00 2001 From: Darian Leung Date: Tue, 1 Nov 2022 23:11:48 +0800 Subject: [PATCH] freertos: Refactor stream buffer spinlock initialization This commit refactors the way stream buffers initialize their spinlock. - "prvInitialiseNewStreamBuffer()" now initializes the stream buffer fields manually (instead of using memset()) to avoid resetting the spin lock - Stream buffer creation functions now manually initialize the spinlock after the other fields are initialized using "prvInitialiseNewStreamBuffer()" Also added comments to event group spinlock initializtion. --- .../freertos/FreeRTOS-Kernel/event_groups.c | 9 +- .../freertos/FreeRTOS-Kernel/stream_buffer.c | 98 +++++-------------- 2 files changed, 31 insertions(+), 76 deletions(-) diff --git a/components/freertos/FreeRTOS-Kernel/event_groups.c b/components/freertos/FreeRTOS-Kernel/event_groups.c index d1b6ba4987..9f36c460b3 100644 --- a/components/freertos/FreeRTOS-Kernel/event_groups.c +++ b/components/freertos/FreeRTOS-Kernel/event_groups.c @@ -136,10 +136,10 @@ static BaseType_t prvTestWaitCondition( const EventBits_t uxCurrentEventBits, } #endif /* configSUPPORT_DYNAMIC_ALLOCATION */ - traceEVENT_GROUP_CREATE( pxEventBits ); -#ifdef ESP_PLATFORM + /* Initialize the event group's spinlock. */ portMUX_INITIALIZE( &pxEventBits->xEventGroupLock ); -#endif // ESP_PLATFORM + + traceEVENT_GROUP_CREATE( pxEventBits ); } else { @@ -190,9 +190,8 @@ static BaseType_t prvTestWaitCondition( const EventBits_t uxCurrentEventBits, } #endif /* configSUPPORT_STATIC_ALLOCATION */ -#ifdef ESP_PLATFORM + /* Initialize the event group's spinlock. */ portMUX_INITIALIZE( &pxEventBits->xEventGroupLock ); -#endif // ESP_PLATFORM traceEVENT_GROUP_CREATE( pxEventBits ); } diff --git a/components/freertos/FreeRTOS-Kernel/stream_buffer.c b/components/freertos/FreeRTOS-Kernel/stream_buffer.c index 38c4fef59c..db7903f9b3 100644 --- a/components/freertos/FreeRTOS-Kernel/stream_buffer.c +++ b/components/freertos/FreeRTOS-Kernel/stream_buffer.c @@ -189,9 +189,8 @@ typedef struct StreamBufferDef_t /*lint !e9058 Style convention #if ( configUSE_TRACE_FACILITY == 1 ) UBaseType_t uxStreamBufferNumber; /* Used for tracing purposes. */ #endif -#ifdef ESP_PLATFORM + portMUX_TYPE xStreamBufferLock; /* Spinlock required for SMP critical sections */ -#endif // ESP_PLATFORM } StreamBuffer_t; /* @@ -254,17 +253,6 @@ static void prvInitialiseNewStreamBuffer( StreamBuffer_t * const pxStreamBuffer, size_t xTriggerLevelBytes, uint8_t ucFlags ) PRIVILEGED_FUNCTION; -#ifdef ESP_PLATFORM -/** - * Called by xStreamBufferReset() to reset the members of the StreamBuffer, excluding - * its spinlock. - */ -static void prvResetStreamBufferFields( StreamBuffer_t * const pxStreamBuffer, - uint8_t * const pucBuffer, - size_t xBufferSizeBytes, - size_t xTriggerLevelBytes, - uint8_t ucFlags ) PRIVILEGED_FUNCTION; -#endif /*-----------------------------------------------------------*/ #if ( configSUPPORT_DYNAMIC_ALLOCATION == 1 ) @@ -328,6 +316,11 @@ static void prvResetStreamBufferFields( StreamBuffer_t * const pxStreamBuffer, xTriggerLevelBytes, ucFlags ); + /* Initialize the stream buffer's spinlock separately, as + * prvInitialiseNewStreamBuffer() is also called from + * xStreamBufferReset(). */ + portMUX_INITIALIZE( &( ( ( StreamBuffer_t * ) pucAllocatedMemory )->xStreamBufferLock ) ); + traceSTREAM_BUFFER_CREATE( ( ( StreamBuffer_t * ) pucAllocatedMemory ), xIsMessageBuffer ); } else @@ -403,6 +396,11 @@ static void prvResetStreamBufferFields( StreamBuffer_t * const pxStreamBuffer, * again. */ pxStreamBuffer->ucFlags |= sbFLAGS_IS_STATICALLY_ALLOCATED; + /* Initialize the stream buffer's spinlock separately, as + * prvInitialiseNewStreamBuffer() is also called from + * xStreamBufferReset(). */ + portMUX_INITIALIZE( &( pxStreamBuffer->xStreamBufferLock ) ); + traceSTREAM_BUFFER_CREATE( pxStreamBuffer, xIsMessageBuffer ); xReturn = ( StreamBufferHandle_t ) pxStaticStreamBuffer; /*lint !e9087 Data hiding requires cast to opaque type. */ @@ -478,23 +476,11 @@ BaseType_t xStreamBufferReset( StreamBufferHandle_t xStreamBuffer ) { if( pxStreamBuffer->xTaskWaitingToSend == NULL ) { - #ifdef ESP_PLATFORM - /* As we just entered a critical section, we must NOT reset the spinlock field. - * Thus, call `prvResetStreamBufferFields` instead of `prvInitialiseNewStreamBuffer` - */ - prvResetStreamBufferFields( pxStreamBuffer, - pxStreamBuffer->pucBuffer, - pxStreamBuffer->xLength, - pxStreamBuffer->xTriggerLevelBytes, - pxStreamBuffer->ucFlags ); - - #else // ESP_PLATFORM - prvInitialiseNewStreamBuffer( pxStreamBuffer, - pxStreamBuffer->pucBuffer, - pxStreamBuffer->xLength, - pxStreamBuffer->xTriggerLevelBytes, - pxStreamBuffer->ucFlags ); - #endif // ESP_PLATFORM + prvInitialiseNewStreamBuffer( pxStreamBuffer, + pxStreamBuffer->pucBuffer, + pxStreamBuffer->xLength, + pxStreamBuffer->xTriggerLevelBytes, + pxStreamBuffer->ucFlags ); xReturn = pdPASS; #if ( configUSE_TRACE_FACILITY == 1 ) @@ -1338,53 +1324,23 @@ static void prvInitialiseNewStreamBuffer( StreamBuffer_t * const pxStreamBuffer, } /*lint !e529 !e438 xWriteValue is only used if configASSERT() is defined. */ #endif - ( void ) memset( ( void * ) pxStreamBuffer, 0x00, sizeof( StreamBuffer_t ) ); /*lint !e9087 memset() requires void *. */ + /* This function could be called from xStreamBufferReset(), so we reset the + * stream buffer fields manually in order to avoid clearing + * xStreamBufferLock. The xStreamBufferLock is initialized separately on + * stream buffer creation. */ + pxStreamBuffer->xTail = ( size_t ) 0; + pxStreamBuffer->xHead = ( size_t ) 0; + pxStreamBuffer->xTaskWaitingToReceive = ( TaskHandle_t ) 0; + pxStreamBuffer->xTaskWaitingToSend = ( TaskHandle_t ) 0; + #if ( configUSE_TRACE_FACILITY == 1 ) + pxStreamBuffer->uxStreamBufferNumber = ( UBaseType_t ) 0; + #endif pxStreamBuffer->pucBuffer = pucBuffer; pxStreamBuffer->xLength = xBufferSizeBytes; pxStreamBuffer->xTriggerLevelBytes = xTriggerLevelBytes; pxStreamBuffer->ucFlags = ucFlags; -#ifdef ESP_PLATFORM - portMUX_INITIALIZE( &( pxStreamBuffer->xStreamBufferLock ) ); -#endif // ESP_PLATFORM } - -#ifdef ESP_PLATFORM - - /** The goal of this function is to (re)set all the fields of the given StreamBuffer, except - * its lock. - */ - static void prvResetStreamBufferFields( StreamBuffer_t * const pxStreamBuffer, - uint8_t * const pucBuffer, - size_t xBufferSizeBytes, - size_t xTriggerLevelBytes, - uint8_t ucFlags ) - { - #if ( configASSERT_DEFINED == 1 ) - { - /* The value written just has to be identifiable when looking at the - * memory. Don't use 0xA5 as that is the stack fill value and could - * result in confusion as to what is actually being observed. */ - const BaseType_t xWriteValue = 0x55; - configASSERT( memset( pucBuffer, ( int ) xWriteValue, xBufferSizeBytes ) == pucBuffer ); - } /*lint !e529 !e438 xWriteValue is only used if configASSERT() is defined. */ - #endif - - /* Do not include the spinlock in the part to reset! - * Thus, make sure the spinlock is the last field of the structure. */ - _Static_assert( offsetof(StreamBuffer_t, xStreamBufferLock) == sizeof( StreamBuffer_t ) - sizeof(portMUX_TYPE), - "xStreamBufferLock must be the last field of structure StreamBuffer_t" ); - const size_t erasable = sizeof( StreamBuffer_t ) - sizeof(portMUX_TYPE); - ( void ) memset( ( void * ) pxStreamBuffer, 0x00, erasable ); /*lint !e9087 memset() requires void *. */ - pxStreamBuffer->pucBuffer = pucBuffer; - pxStreamBuffer->xLength = xBufferSizeBytes; - pxStreamBuffer->xTriggerLevelBytes = xTriggerLevelBytes; - pxStreamBuffer->ucFlags = ucFlags; - } - -#endif // ESP_PLATFORM - - #if ( configUSE_TRACE_FACILITY == 1 ) UBaseType_t uxStreamBufferGetStreamBufferNumber( StreamBufferHandle_t xStreamBuffer )