From d0686d5f2bb8b5c07863f16e0e20a99921d9c105 Mon Sep 17 00:00:00 2001 From: Jeff Tenney Date: Mon, 11 Nov 2024 15:27:11 -0700 Subject: [PATCH 1/2] Don't suspend scheduler if task already notified --- tasks.c | 139 ++++++++++++++++++++++++++++---------------------------- 1 file changed, 70 insertions(+), 69 deletions(-) diff --git a/tasks.c b/tasks.c index 421dea71dff..9dfc732dfd9 100644 --- a/tasks.c +++ b/tasks.c @@ -7653,30 +7653,34 @@ TickType_t uxTaskResetEventItemValue( void ) TickType_t xTicksToWait ) { uint32_t ulReturn; - BaseType_t xAlreadyYielded, xShouldBlock = pdFALSE; traceENTER_ulTaskGenericNotifyTake( uxIndexToWaitOn, xClearCountOnExit, xTicksToWait ); configASSERT( uxIndexToWaitOn < configTASK_NOTIFICATION_ARRAY_ENTRIES ); - /* We suspend the scheduler here as prvAddCurrentTaskToDelayedList is a - * non-deterministic operation. */ - vTaskSuspendAll(); + /* If the notification count is zero, and if we are willing to wait for a + * notification, then block the task and wait. */ + if( ( pxCurrentTCB->ulNotifiedValue[ uxIndexToWaitOn ] == 0U ) && ( xTicksToWait > ( TickType_t ) 0 ) ) { - /* We MUST enter a critical section to atomically check if a notification - * has occurred and set the flag to indicate that we are waiting for - * a notification. If we do not do so, a notification sent from an ISR - * will get lost. */ - taskENTER_CRITICAL(); + BaseType_t xAlreadyYielded, xShouldBlock = pdFALSE; + + /* We suspend the scheduler here as prvAddCurrentTaskToDelayedList is a + * non-deterministic operation. */ + vTaskSuspendAll(); { - /* Only block if the notification count is not already non-zero. */ - if( pxCurrentTCB->ulNotifiedValue[ uxIndexToWaitOn ] == 0U ) + /* We MUST enter a critical section to atomically check if a notification + * has occurred and set the flag to indicate that we are waiting for + * a notification. If we do not do so, a notification sent from an ISR + * will get lost. */ + taskENTER_CRITICAL(); { - /* Mark this task as waiting for a notification. */ - pxCurrentTCB->ucNotifyState[ uxIndexToWaitOn ] = taskWAITING_NOTIFICATION; - - if( xTicksToWait > ( TickType_t ) 0 ) + /* Only block if the notification count is not already non-zero. */ + if( pxCurrentTCB->ulNotifiedValue[ uxIndexToWaitOn ] == 0U ) { + /* Mark this task as waiting for a notification. */ + pxCurrentTCB->ucNotifyState[ uxIndexToWaitOn ] = taskWAITING_NOTIFICATION; + + /* Arrange to wait for a notification. */ xShouldBlock = pdTRUE; } else @@ -7684,37 +7688,33 @@ TickType_t uxTaskResetEventItemValue( void ) mtCOVERAGE_TEST_MARKER(); } } + taskEXIT_CRITICAL(); + + /* We are now out of the critical section but the scheduler is still + * suspended, so we are safe to do non-deterministic operations such + * as prvAddCurrentTaskToDelayedList. */ + if( xShouldBlock == pdTRUE ) + { + traceTASK_NOTIFY_TAKE_BLOCK( uxIndexToWaitOn ); + prvAddCurrentTaskToDelayedList( xTicksToWait, pdTRUE ); + } else { mtCOVERAGE_TEST_MARKER(); } } - taskEXIT_CRITICAL(); + xAlreadyYielded = xTaskResumeAll(); - /* We are now out of the critical section but the scheduler is still - * suspended, so we are safe to do non-deterministic operations such - * as prvAddCurrentTaskToDelayedList. */ - if( xShouldBlock == pdTRUE ) + /* Force a reschedule if xTaskResumeAll has not already done so. */ + if( ( xShouldBlock == pdTRUE ) && ( xAlreadyYielded == pdFALSE ) ) { - traceTASK_NOTIFY_TAKE_BLOCK( uxIndexToWaitOn ); - prvAddCurrentTaskToDelayedList( xTicksToWait, pdTRUE ); + taskYIELD_WITHIN_API(); } else { mtCOVERAGE_TEST_MARKER(); } } - xAlreadyYielded = xTaskResumeAll(); - - /* Force a reschedule if xTaskResumeAll has not already done so. */ - if( ( xShouldBlock == pdTRUE ) && ( xAlreadyYielded == pdFALSE ) ) - { - taskYIELD_WITHIN_API(); - } - else - { - mtCOVERAGE_TEST_MARKER(); - } taskENTER_CRITICAL(); { @@ -7757,34 +7757,39 @@ TickType_t uxTaskResetEventItemValue( void ) uint32_t * pulNotificationValue, TickType_t xTicksToWait ) { - BaseType_t xReturn, xAlreadyYielded, xShouldBlock = pdFALSE; + BaseType_t xReturn; traceENTER_xTaskGenericNotifyWait( uxIndexToWaitOn, ulBitsToClearOnEntry, ulBitsToClearOnExit, pulNotificationValue, xTicksToWait ); configASSERT( uxIndexToWaitOn < configTASK_NOTIFICATION_ARRAY_ENTRIES ); - /* We suspend the scheduler here as prvAddCurrentTaskToDelayedList is a - * non-deterministic operation. */ - vTaskSuspendAll(); + /* If the task hasn't received a notification, and if we are willing to wait + * for it, then block the task and wait. */ + if( ( pxCurrentTCB->ucNotifyState[ uxIndexToWaitOn ] != taskNOTIFICATION_RECEIVED ) && ( xTicksToWait > ( TickType_t ) 0 ) ) { - /* We MUST enter a critical section to atomically check and update the - * task notification value. If we do not do so, a notification from - * an ISR will get lost. */ - taskENTER_CRITICAL(); + BaseType_t xAlreadyYielded, xShouldBlock = pdFALSE; + + /* We suspend the scheduler here as prvAddCurrentTaskToDelayedList is a + * non-deterministic operation. */ + vTaskSuspendAll(); { - /* Only block if a notification is not already pending. */ - if( pxCurrentTCB->ucNotifyState[ uxIndexToWaitOn ] != taskNOTIFICATION_RECEIVED ) + /* We MUST enter a critical section to atomically check and update the + * task notification value. If we do not do so, a notification from + * an ISR will get lost. */ + taskENTER_CRITICAL(); { - /* Clear bits in the task's notification value as bits may get - * set by the notifying task or interrupt. This can be used - * to clear the value to zero. */ - pxCurrentTCB->ulNotifiedValue[ uxIndexToWaitOn ] &= ~ulBitsToClearOnEntry; + /* Only block if a notification is not already pending. */ + if( pxCurrentTCB->ucNotifyState[ uxIndexToWaitOn ] != taskNOTIFICATION_RECEIVED ) + { + /* Clear bits in the task's notification value as bits may get + * set by the notifying task or interrupt. This can be used + * to clear the value to zero. */ + pxCurrentTCB->ulNotifiedValue[ uxIndexToWaitOn ] &= ~ulBitsToClearOnEntry; - /* Mark this task as waiting for a notification. */ - pxCurrentTCB->ucNotifyState[ uxIndexToWaitOn ] = taskWAITING_NOTIFICATION; + /* Mark this task as waiting for a notification. */ + pxCurrentTCB->ucNotifyState[ uxIndexToWaitOn ] = taskWAITING_NOTIFICATION; - if( xTicksToWait > ( TickType_t ) 0 ) - { + /* Arrange to wait for a notification. */ xShouldBlock = pdTRUE; } else @@ -7792,37 +7797,33 @@ TickType_t uxTaskResetEventItemValue( void ) mtCOVERAGE_TEST_MARKER(); } } + taskEXIT_CRITICAL(); + + /* We are now out of the critical section but the scheduler is still + * suspended, so we are safe to do non-deterministic operations such + * as prvAddCurrentTaskToDelayedList. */ + if( xShouldBlock == pdTRUE ) + { + traceTASK_NOTIFY_WAIT_BLOCK( uxIndexToWaitOn ); + prvAddCurrentTaskToDelayedList( xTicksToWait, pdTRUE ); + } else { mtCOVERAGE_TEST_MARKER(); } } - taskEXIT_CRITICAL(); + xAlreadyYielded = xTaskResumeAll(); - /* We are now out of the critical section but the scheduler is still - * suspended, so we are safe to do non-deterministic operations such - * as prvAddCurrentTaskToDelayedList. */ - if( xShouldBlock == pdTRUE ) + /* Force a reschedule if xTaskResumeAll has not already done so. */ + if( ( xShouldBlock == pdTRUE ) && ( xAlreadyYielded == pdFALSE ) ) { - traceTASK_NOTIFY_WAIT_BLOCK( uxIndexToWaitOn ); - prvAddCurrentTaskToDelayedList( xTicksToWait, pdTRUE ); + taskYIELD_WITHIN_API(); } else { mtCOVERAGE_TEST_MARKER(); } } - xAlreadyYielded = xTaskResumeAll(); - - /* Force a reschedule if xTaskResumeAll has not already done so. */ - if( ( xShouldBlock == pdTRUE ) && ( xAlreadyYielded == pdFALSE ) ) - { - taskYIELD_WITHIN_API(); - } - else - { - mtCOVERAGE_TEST_MARKER(); - } taskENTER_CRITICAL(); { From 84d26497f972d5c64ef659349545e46854a942fb Mon Sep 17 00:00:00 2001 From: Jeff Tenney Date: Mon, 11 Nov 2024 21:49:47 -0700 Subject: [PATCH 2/2] Put declarations back at beginning of function --- tasks.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/tasks.c b/tasks.c index 9dfc732dfd9..22e11f0fb21 100644 --- a/tasks.c +++ b/tasks.c @@ -7653,6 +7653,7 @@ TickType_t uxTaskResetEventItemValue( void ) TickType_t xTicksToWait ) { uint32_t ulReturn; + BaseType_t xAlreadyYielded, xShouldBlock = pdFALSE; traceENTER_ulTaskGenericNotifyTake( uxIndexToWaitOn, xClearCountOnExit, xTicksToWait ); @@ -7662,8 +7663,6 @@ TickType_t uxTaskResetEventItemValue( void ) * notification, then block the task and wait. */ if( ( pxCurrentTCB->ulNotifiedValue[ uxIndexToWaitOn ] == 0U ) && ( xTicksToWait > ( TickType_t ) 0 ) ) { - BaseType_t xAlreadyYielded, xShouldBlock = pdFALSE; - /* We suspend the scheduler here as prvAddCurrentTaskToDelayedList is a * non-deterministic operation. */ vTaskSuspendAll(); @@ -7757,7 +7756,7 @@ TickType_t uxTaskResetEventItemValue( void ) uint32_t * pulNotificationValue, TickType_t xTicksToWait ) { - BaseType_t xReturn; + BaseType_t xReturn, xAlreadyYielded, xShouldBlock = pdFALSE; traceENTER_xTaskGenericNotifyWait( uxIndexToWaitOn, ulBitsToClearOnEntry, ulBitsToClearOnExit, pulNotificationValue, xTicksToWait ); @@ -7767,8 +7766,6 @@ TickType_t uxTaskResetEventItemValue( void ) * for it, then block the task and wait. */ if( ( pxCurrentTCB->ucNotifyState[ uxIndexToWaitOn ] != taskNOTIFICATION_RECEIVED ) && ( xTicksToWait > ( TickType_t ) 0 ) ) { - BaseType_t xAlreadyYielded, xShouldBlock = pdFALSE; - /* We suspend the scheduler here as prvAddCurrentTaskToDelayedList is a * non-deterministic operation. */ vTaskSuspendAll();