Fix task notification determinism.

- Delayed task list is no longer walked while interrupts are disabled. - Fixed comment that could lead to similar mistakes in the future. - Minimize performance impact.
FreeRTOS · Feb 13, 2023 · eeed340 · eeed340
1 parent 050cf0d
commit eeed340
Showing 1 changed file with 46 additions and 34 deletions.
diff --git a/tasks.c b/tasks.c
@@ -3087,8 +3087,8 @@ void vTaskPlaceOnEventList( List_t * const pxEventList,
 {
     configASSERT( pxEventList );
 
-    /* THIS FUNCTION MUST BE CALLED WITH EITHER INTERRUPTS DISABLED OR THE
-     * SCHEDULER SUSPENDED AND THE QUEUE BEING ACCESSED LOCKED. */
+    /* THIS FUNCTION MUST BE CALLED WITH THE SCHEDULER SUSPENDED AND THE QUEUE
+     * BEING ACCESSED LOCKED. */
 
     /* Place the event list item of the TCB in the appropriate event list.
      * This is placed in the list in priority order so the highest priority task
@@ -4693,22 +4693,25 @@ TickType_t uxTaskResetEventItemValue( void )
         configASSERT( uxIndexToWait < configTASK_NOTIFICATION_ARRAY_ENTRIES );
 
         taskENTER_CRITICAL();
+
+        /* Only block if the notification count is not already non-zero. */
+        if( pxCurrentTCB->ulNotifiedValue[ uxIndexToWait ] == 0UL )
         {
-            /* Only block if the notification count is not already non-zero. */
-            if( pxCurrentTCB->ulNotifiedValue[ uxIndexToWait ] == 0UL )
+            /* Mark this task as waiting for a notification. */
+            pxCurrentTCB->ucNotifyState[ uxIndexToWait ] = taskWAITING_NOTIFICATION;
+
+            if( xTicksToWait > ( TickType_t ) 0 )
             {
-                /* Mark this task as waiting for a notification. */
-                pxCurrentTCB->ucNotifyState[ uxIndexToWait ] = taskWAITING_NOTIFICATION;
+                traceTASK_NOTIFY_TAKE_BLOCK( uxIndexToWait );
 
-                if( xTicksToWait > ( TickType_t ) 0 )
-                {
-                    prvAddCurrentTaskToDelayedList( xTicksToWait, pdTRUE );
-                    traceTASK_NOTIFY_TAKE_BLOCK( uxIndexToWait );
+                /* Suspend the scheduler before enabling interrupts. */
+                vTaskSuspendAll();
+                taskEXIT_CRITICAL();
+
+                prvAddCurrentTaskToDelayedList( xTicksToWait, pdTRUE );
 
-                    /* All ports are written to allow a yield in a critical
-                     * section (some will yield immediately, others wait until the
-                     * critical section exits) - but it is not something that
-                     * application code should ever do. */
+                if( xTaskResumeAll() == pdFALSE )
+                {
                     portYIELD_WITHIN_API();
                 }
                 else
@@ -4718,10 +4721,13 @@ TickType_t uxTaskResetEventItemValue( void )
             }
             else
             {
-                mtCOVERAGE_TEST_MARKER();
+                taskEXIT_CRITICAL();
             }
         }
-        taskEXIT_CRITICAL();
+        else
+        {
+            taskEXIT_CRITICAL();
+        }
 
         taskENTER_CRITICAL();
         {
@@ -4767,27 +4773,30 @@ TickType_t uxTaskResetEventItemValue( void )
         configASSERT( uxIndexToWait < configTASK_NOTIFICATION_ARRAY_ENTRIES );
 
         taskENTER_CRITICAL();
+
+        /* Only block if a notification is not already pending. */
+        if( pxCurrentTCB->ucNotifyState[ uxIndexToWait ] != taskNOTIFICATION_RECEIVED )
         {
-            /* Only block if a notification is not already pending. */
-            if( pxCurrentTCB->ucNotifyState[ uxIndexToWait ] != 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[ uxIndexToWait ] &= ~ulBitsToClearOnEntry;
+
+            /* Mark this task as waiting for a notification. */
+            pxCurrentTCB->ucNotifyState[ uxIndexToWait ] = taskWAITING_NOTIFICATION;
+
+            if( xTicksToWait > ( TickType_t ) 0 )
             {
-                /* 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[ uxIndexToWait ] &= ~ulBitsToClearOnEntry;
+                traceTASK_NOTIFY_WAIT_BLOCK( uxIndexToWait );
 
-                /* Mark this task as waiting for a notification. */
-                pxCurrentTCB->ucNotifyState[ uxIndexToWait ] = taskWAITING_NOTIFICATION;
+                /* Suspend the scheduler before enabling interrupts. */
+                vTaskSuspendAll();
+                taskEXIT_CRITICAL();
 
-                if( xTicksToWait > ( TickType_t ) 0 )
-                {
-                    prvAddCurrentTaskToDelayedList( xTicksToWait, pdTRUE );
-                    traceTASK_NOTIFY_WAIT_BLOCK( uxIndexToWait );
+                prvAddCurrentTaskToDelayedList( xTicksToWait, pdTRUE );
 
-                    /* All ports are written to allow a yield in a critical
-                     * section (some will yield immediately, others wait until the
-                     * critical section exits) - but it is not something that
-                     * application code should ever do. */
+                if( xTaskResumeAll() == pdFALSE )
+                {
                     portYIELD_WITHIN_API();
                 }
                 else
@@ -4797,10 +4806,13 @@ TickType_t uxTaskResetEventItemValue( void )
             }
             else
             {
-                mtCOVERAGE_TEST_MARKER();
+                taskEXIT_CRITICAL();
             }
         }
-        taskEXIT_CRITICAL();
+        else
+        {
+            taskEXIT_CRITICAL();
+        }
 
         taskENTER_CRITICAL();
         {