From 48e03e4cbdd28f29a2d09e3bcff2f24688a87630 Mon Sep 17 00:00:00 2001 From: Darian Leung Date: Wed, 24 Aug 2022 21:02:43 +0800 Subject: [PATCH] freeRTOS: Synchronize queue functions to v10.4.3 This commit synchronizes multiple functions in queue.c with upstream v10.4.3. Multi-core modifications are then reapplied to these functions. The following functions were modified: prvNotifyQueueSetContainer() xQueueGenericCreateStatic() xQueueGenericCreate() xQueueGetMutexHolder() xQueueCreateCountingSemaphoreStatic() xQueueCreateCountingSemaphore() xQueueGenericSend() xQueueGenericSendFromISR() xQueueReceiveFromISR() uxQueueMessagesWaiting() prvUnlockQueue() prvIsQueueFull() xQueueAddToSet() xQueueRemoveFromSet() prvNotifyQueueSetContainer() Note: The SEGGER_SYSVIEW traceQUEUE_SEND() macro was updated as the xCopyPosition argument is no longer available in scenarios where the macro is called. --- .../Sample/OS/SEGGER_SYSVIEW_FreeRTOS.h | 2 +- components/freertos/FreeRTOS-Kernel/queue.c | 107 +++++++++--------- 2 files changed, 54 insertions(+), 55 deletions(-) diff --git a/components/app_trace/sys_view/Sample/OS/SEGGER_SYSVIEW_FreeRTOS.h b/components/app_trace/sys_view/Sample/OS/SEGGER_SYSVIEW_FreeRTOS.h index 82e6859a59..b03fba82cd 100644 --- a/components/app_trace/sys_view/Sample/OS/SEGGER_SYSVIEW_FreeRTOS.h +++ b/components/app_trace/sys_view/Sample/OS/SEGGER_SYSVIEW_FreeRTOS.h @@ -220,7 +220,7 @@ Notes: #if ( configUSE_QUEUE_SETS != 1 ) #define traceQUEUE_SEND( pxQueue ) SYSVIEW_RecordU32x4(apiFastID_OFFSET + apiID_XQUEUEGENERICSEND, SEGGER_SYSVIEW_ShrinkId((U32)pxQueue), (U32)pvItemToQueue, xTicksToWait, xCopyPosition) #else - #define traceQUEUE_SEND( pxQueue ) SYSVIEW_RecordU32x4(apiFastID_OFFSET + apiID_XQUEUEGENERICSEND, SEGGER_SYSVIEW_ShrinkId((U32)pxQueue), 0, 0, xCopyPosition) + #define traceQUEUE_SEND( pxQueue ) SYSVIEW_RecordU32x4(apiFastID_OFFSET + apiID_XQUEUEGENERICSEND, SEGGER_SYSVIEW_ShrinkId((U32)pxQueue), 0, 0, 0) #endif #endif // CONFIG_FREERTOS_SMP diff --git a/components/freertos/FreeRTOS-Kernel/queue.c b/components/freertos/FreeRTOS-Kernel/queue.c index 5d797765a2..6c473aa822 100644 --- a/components/freertos/FreeRTOS-Kernel/queue.c +++ b/components/freertos/FreeRTOS-Kernel/queue.c @@ -220,7 +220,7 @@ static void prvCopyDataFromQueue( Queue_t * const pxQueue, * Checks to see if a queue is a member of a queue set, and if so, notifies * the queue set that the queue contains data. */ - static BaseType_t prvNotifyQueueSetContainer( const Queue_t * const pxQueue, const BaseType_t xCopyPosition ) PRIVILEGED_FUNCTION; + static BaseType_t prvNotifyQueueSetContainer( const Queue_t * const pxQueue ) PRIVILEGED_FUNCTION; #endif /* @@ -362,10 +362,8 @@ BaseType_t xQueueGenericReset( QueueHandle_t xQueue, * variable of type StaticQueue_t or StaticSemaphore_t equals the size of * the real queue and semaphore structures. */ volatile size_t xSize = sizeof( StaticQueue_t ); - - /* This assertion cannot be branch covered in unit tests */ - configASSERT( xSize == sizeof( Queue_t ) ); /* LCOV_EXCL_BR_LINE */ - ( void ) xSize; /* Keeps lint quiet when configASSERT() is not defined. */ + configASSERT( xSize == sizeof( Queue_t ) ); + ( void ) xSize; /* Keeps lint quiet when configASSERT() is not defined. */ } #endif /* configASSERT_DEFINED */ @@ -405,30 +403,22 @@ BaseType_t xQueueGenericReset( QueueHandle_t xQueue, const UBaseType_t uxItemSize, const uint8_t ucQueueType ) { - Queue_t * pxNewQueue = NULL; + Queue_t * pxNewQueue; size_t xQueueSizeInBytes; uint8_t * pucQueueStorage; configASSERT( uxQueueLength > ( UBaseType_t ) 0 ); - if( uxItemSize == ( UBaseType_t ) 0 ) - { - /* There is not going to be a queue storage area. */ - xQueueSizeInBytes = ( size_t ) 0; - } - else - { - /* Allocate enough space to hold the maximum number of items that - * can be in the queue at any time. It is valid for uxItemSize to be - * zero in the case the queue is used as a semaphore. */ - xQueueSizeInBytes = ( size_t ) ( uxQueueLength * uxItemSize ); /*lint !e961 MISRA exception as the casts are only redundant for some ports. */ - } + /* Allocate enough space to hold the maximum number of items that + * can be in the queue at any time. It is valid for uxItemSize to be + * zero in the case the queue is used as a semaphore. */ + xQueueSizeInBytes = ( size_t ) ( uxQueueLength * uxItemSize ); /*lint !e961 MISRA exception as the casts are only redundant for some ports. */ /* Check for multiplication overflow. */ configASSERT( ( uxItemSize == 0 ) || ( uxQueueLength == ( xQueueSizeInBytes / uxItemSize ) ) ); /* Check for addition overflow. */ - configASSERT( ( sizeof( Queue_t ) + xQueueSizeInBytes ) > xQueueSizeInBytes ); + configASSERT( ( sizeof( Queue_t ) + xQueueSizeInBytes ) > xQueueSizeInBytes ); /* Allocate the queue and storage area. Justification for MISRA * deviation as follows: pvPortMalloc() always ensures returned memory @@ -598,10 +588,7 @@ static void prvInitialiseNewQueue( const UBaseType_t uxQueueLength, * calling task is the mutex holder, but not a good way of determining the * identity of the mutex holder, as the holder may change between the * following critical section exiting and the function returning. */ -#ifdef ESP_PLATFORM - Queue_t * const pxQueue = (Queue_t *)pxSemaphore; -#endif - taskENTER_CRITICAL( &( pxQueue->xQueueLock ) ); + taskENTER_CRITICAL( &( pxSemaphore->xQueueLock ) ); { if( pxSemaphore->uxQueueType == queueQUEUE_IS_MUTEX ) { @@ -612,7 +599,7 @@ static void prvInitialiseNewQueue( const UBaseType_t uxQueueLength, pxReturn = NULL; } } - taskEXIT_CRITICAL( &( pxQueue->xQueueLock ) ); + taskEXIT_CRITICAL( &( pxSemaphore->xQueueLock ) ); return pxReturn; } /*lint !e818 xSemaphore cannot be a pointer to const because it is a typedef. */ @@ -750,7 +737,7 @@ static void prvInitialiseNewQueue( const UBaseType_t uxQueueLength, const UBaseType_t uxInitialCount, StaticQueue_t * pxStaticQueue ) { - QueueHandle_t xHandle = NULL; + QueueHandle_t xHandle; configASSERT( uxMaxCount != 0 ); configASSERT( uxInitialCount <= uxMaxCount ); @@ -779,7 +766,7 @@ static void prvInitialiseNewQueue( const UBaseType_t uxQueueLength, QueueHandle_t xQueueCreateCountingSemaphore( const UBaseType_t uxMaxCount, const UBaseType_t uxInitialCount ) { - QueueHandle_t xHandle = NULL; + QueueHandle_t xHandle; configASSERT( uxMaxCount != 0 ); configASSERT( uxInitialCount <= uxMaxCount ); @@ -844,7 +831,7 @@ BaseType_t xQueueGenericSend( QueueHandle_t xQueue, #if ( configUSE_QUEUE_SETS == 1 ) { - UBaseType_t uxPreviousMessagesWaiting = pxQueue->uxMessagesWaiting; + const UBaseType_t uxPreviousMessagesWaiting = pxQueue->uxMessagesWaiting; xYieldRequired = prvCopyDataToQueue( pxQueue, pvItemToQueue, xCopyPosition ); @@ -857,7 +844,7 @@ BaseType_t xQueueGenericSend( QueueHandle_t xQueue, * in the queue has not changed. */ mtCOVERAGE_TEST_MARKER(); } - else if( prvNotifyQueueSetContainer( pxQueue, xCopyPosition ) != pdFALSE ) + else if( prvNotifyQueueSetContainer( pxQueue ) != pdFALSE ) { /* The queue is a member of a queue set, and posting * to the queue set caused a higher priority task to @@ -1079,6 +1066,7 @@ BaseType_t xQueueGenericSendFromISR( QueueHandle_t xQueue, if( ( pxQueue->uxMessagesWaiting < pxQueue->uxLength ) || ( xCopyPosition == queueOVERWRITE ) ) { const int8_t cTxLock = pxQueue->cTxLock; + const UBaseType_t uxPreviousMessagesWaiting = pxQueue->uxMessagesWaiting; traceQUEUE_SEND_FROM_ISR( pxQueue ); @@ -1097,7 +1085,14 @@ BaseType_t xQueueGenericSendFromISR( QueueHandle_t xQueue, { if( pxQueue->pxQueueSetContainer != NULL ) { - if( prvNotifyQueueSetContainer( pxQueue, xCopyPosition ) != pdFALSE ) + if( ( xCopyPosition == queueOVERWRITE ) && ( uxPreviousMessagesWaiting != ( UBaseType_t ) 0 ) ) + { + /* Do not notify the queue set as an existing item + * was overwritten in the queue so the number of items + * in the queue has not changed. */ + mtCOVERAGE_TEST_MARKER(); + } + else if( prvNotifyQueueSetContainer( pxQueue ) != pdFALSE ) { /* The queue is a member of a queue set, and posting * to the queue set caused a higher priority task to @@ -1170,6 +1165,9 @@ BaseType_t xQueueGenericSendFromISR( QueueHandle_t xQueue, { mtCOVERAGE_TEST_MARKER(); } + + /* Not used in this path. */ + ( void ) uxPreviousMessagesWaiting; } #endif /* configUSE_QUEUE_SETS */ } @@ -1267,7 +1265,7 @@ BaseType_t xQueueGiveFromISR( QueueHandle_t xQueue, { if( pxQueue->pxQueueSetContainer != NULL ) { - if( prvNotifyQueueSetContainer( pxQueue, queueSEND_TO_BACK ) != pdFALSE ) + if( prvNotifyQueueSetContainer( pxQueue ) != pdFALSE ) { /* The semaphore is a member of a queue set, and * posting to the queue set caused a higher priority @@ -1347,6 +1345,8 @@ BaseType_t xQueueGiveFromISR( QueueHandle_t xQueue, { /* Increment the lock count so the task that unlocks the queue * knows that data was posted while it was locked. */ + configASSERT( cTxLock != queueINT8_MAX ); + pxQueue->cTxLock = ( int8_t ) ( cTxLock + 1 ); } @@ -2007,6 +2007,8 @@ BaseType_t xQueueReceiveFromISR( QueueHandle_t xQueue, { /* Increment the lock count so the task that unlocks the queue * knows that data was removed while it was locked. */ + configASSERT( cRxLock != queueINT8_MAX ); + pxQueue->cRxLock = ( int8_t ) ( cRxLock + 1 ); } @@ -2085,15 +2087,14 @@ BaseType_t xQueuePeekFromISR( QueueHandle_t xQueue, UBaseType_t uxQueueMessagesWaiting( const QueueHandle_t xQueue ) { UBaseType_t uxReturn; - Queue_t * const pxQueue = ( Queue_t * ) xQueue; configASSERT( xQueue ); - taskENTER_CRITICAL( &( pxQueue->xQueueLock ) ); + taskENTER_CRITICAL( &( ( ( Queue_t * ) xQueue )->xQueueLock ) ); { uxReturn = ( ( Queue_t * ) xQueue )->uxMessagesWaiting; } - taskEXIT_CRITICAL( &( pxQueue->xQueueLock ) ); + taskEXIT_CRITICAL( &( ( ( Queue_t * ) xQueue )->xQueueLock ) ); return uxReturn; } /*lint !e818 Pointer cannot be declared const as xQueue is a typedef not pointer. */ @@ -2353,7 +2354,7 @@ static void prvUnlockQueue( Queue_t * const pxQueue ) { if( pxQueue->pxQueueSetContainer != NULL ) { - if( prvNotifyQueueSetContainer( pxQueue, queueSEND_TO_BACK ) != pdFALSE ) + if( prvNotifyQueueSetContainer( pxQueue ) != pdFALSE ) { /* The queue is a member of a queue set, and posting to * the queue set caused a higher priority task to unblock. @@ -2496,6 +2497,9 @@ static BaseType_t prvIsQueueFull( const Queue_t * pxQueue ) { BaseType_t xReturn; +#ifndef ESP_PLATFORM + taskENTER_CRITICAL( &( pxQueue->xQueueLock ) ); +#endif { if( pxQueue->uxMessagesWaiting == pxQueue->uxLength ) { @@ -2506,6 +2510,9 @@ static BaseType_t prvIsQueueFull( const Queue_t * pxQueue ) xReturn = pdFALSE; } } +#ifndef ESP_PLATFORM + taskEXIT_CRITICAL( &( pxQueue->xQueueLock ) ); +#endif return xReturn; } @@ -2977,11 +2984,8 @@ BaseType_t xQueueIsQueueFullFromISR( const QueueHandle_t xQueue ) QueueSetHandle_t xQueueSet ) { BaseType_t xReturn; -#ifdef ESP_PLATFORM - Queue_t * pxQueue = (Queue_t * )xQueueOrSemaphore; -#endif - taskENTER_CRITICAL( &( pxQueue->xQueueLock ) ); + taskENTER_CRITICAL( &( ( ( Queue_t * ) xQueueOrSemaphore )->xQueueLock ) ); { if( ( ( Queue_t * ) xQueueOrSemaphore )->pxQueueSetContainer != NULL ) { @@ -3000,7 +3004,7 @@ BaseType_t xQueueIsQueueFullFromISR( const QueueHandle_t xQueue ) xReturn = pdPASS; } } - taskEXIT_CRITICAL( &( pxQueue->xQueueLock ) ); + taskEXIT_CRITICAL( &( ( ( Queue_t * ) xQueueOrSemaphore )->xQueueLock ) ); return xReturn; } @@ -3030,15 +3034,12 @@ BaseType_t xQueueIsQueueFullFromISR( const QueueHandle_t xQueue ) } else { -#ifdef ESP_PLATFORM - Queue_t* pxQueue = (Queue_t*)pxQueueOrSemaphore; -#endif - taskENTER_CRITICAL( &( pxQueue->xQueueLock ) ); + taskENTER_CRITICAL( &( ( ( Queue_t * ) pxQueueOrSemaphore )->xQueueLock ) ); { /* The queue is no longer contained in the set. */ pxQueueOrSemaphore->pxQueueSetContainer = NULL; } - taskEXIT_CRITICAL( &( pxQueue->xQueueLock ) ); + taskEXIT_CRITICAL( &( ( ( Queue_t * ) pxQueueOrSemaphore )->xQueueLock ) ); xReturn = pdPASS; } @@ -3077,20 +3078,16 @@ BaseType_t xQueueIsQueueFullFromISR( const QueueHandle_t xQueue ) #if ( configUSE_QUEUE_SETS == 1 ) - static BaseType_t prvNotifyQueueSetContainer( const Queue_t * const pxQueue, - const BaseType_t xCopyPosition ) + static BaseType_t prvNotifyQueueSetContainer( const Queue_t * const pxQueue ) { Queue_t * pxQueueSetContainer = pxQueue->pxQueueSetContainer; BaseType_t xReturn = pdFALSE; /* This function must be called form a critical section. */ - /* The following line is not reachable in unit tests because every call - * to prvNotifyQueueSetContainer is preceded by a check that - * pxQueueSetContainer != NULL */ - configASSERT( pxQueueSetContainer ); /* LCOV_EXCL_BR_LINE */ + configASSERT( pxQueueSetContainer ); - //Acquire the Queue set's spinlock + /* Acquire the Queue set's spinlock */ taskENTER_CRITICAL( &( pxQueueSetContainer->xQueueLock ) ); configASSERT( pxQueueSetContainer->uxMessagesWaiting < pxQueueSetContainer->uxLength ); @@ -3099,10 +3096,10 @@ BaseType_t xQueueIsQueueFullFromISR( const QueueHandle_t xQueue ) { const int8_t cTxLock = pxQueueSetContainer->cTxLock; - traceQUEUE_SEND( pxQueueSetContainer ); + traceQUEUE_SET_SEND( pxQueueSetContainer ); /* The data copied is the handle of the queue that contains data. */ - xReturn = prvCopyDataToQueue( pxQueueSetContainer, &pxQueue, xCopyPosition ); + xReturn = prvCopyDataToQueue( pxQueueSetContainer, &pxQueue, queueSEND_TO_BACK ); if( cTxLock == queueUNLOCKED ) { @@ -3125,6 +3122,8 @@ BaseType_t xQueueIsQueueFullFromISR( const QueueHandle_t xQueue ) } else { + configASSERT( cTxLock != queueINT8_MAX ); + pxQueueSetContainer->cTxLock = ( int8_t ) ( cTxLock + 1 ); } } @@ -3133,7 +3132,7 @@ BaseType_t xQueueIsQueueFullFromISR( const QueueHandle_t xQueue ) mtCOVERAGE_TEST_MARKER(); } - //Release the Queue set's spinlock + /* Release the Queue set's spinlock */ taskEXIT_CRITICAL( &( pxQueueSetContainer->xQueueLock ) ); return xReturn;