freertos: When scheduler is disabled, tasks on other core should immediately resume

... if triggered by a SemaphoreGive/etc.

Previously they would resume after scheduler was resumed, on next
RTOS tick of other CPU.

diff --git a/components/freertos/tasks.c b/components/freertos/tasks.c
index 15d588662c94e11f7c845a4bcbd38a21e1c4e0bd..15cdaad652bf47c9690fef0c5d2689b2884e59be 100644 (file)
--- a/components/freertos/tasks.c
+++ b/components/freertos/tasks.c
@@ -3038,6 +3038,8 @@ BaseType_t xTaskRemoveFromEventList( const List_t * const pxEventList )
 {
 TCB_t *pxUnblockedTCB;
 BaseType_t xReturn;
+BaseType_t xTaskCanBeReady;
+UBaseType_t i;
 
        /* THIS FUNCTION MUST BE CALLED FROM A CRITICAL SECTION.  It can also be
        called from a critical section within an ISR. */
@@ -3061,7 +3063,21 @@ BaseType_t xReturn;
                return pdFALSE;
        }
 
-       if( uxSchedulerSuspended[ xPortGetCoreID() ] == ( UBaseType_t ) pdFALSE )
+       /* Determine if the task can possibly be run on either CPU now, either because the scheduler
+          the task is pinned to is running or because a scheduler is running on any CPU. */
+       xTaskCanBeReady = pdFALSE;
+       if ( pxUnblockedTCB->xCoreID == tskNO_AFFINITY ) {
+               for (i = 0; i < portNUM_PROCESSORS; i++) {
+                       if ( uxSchedulerSuspended[ i ] == ( UBaseType_t ) pdFALSE ) {
+                               xTaskCanBeReady = pdTRUE;
+                               break;
+                       }
+               }
+       } else {
+               xTaskCanBeReady = uxSchedulerSuspended[ pxUnblockedTCB->xCoreID ] == ( UBaseType_t ) pdFALSE;
+       }
+
+       if( xTaskCanBeReady )
        {
                ( void ) uxListRemove( &( pxUnblockedTCB->xGenericListItem ) );
                prvAddTaskToReadyList( pxUnblockedTCB );
@@ -3069,7 +3085,7 @@ BaseType_t xReturn;
        else
        {
                /* The delayed and ready lists cannot be accessed, so hold this task
-               pending until the scheduler is resumed. */
+               pending until the scheduler is resumed on this CPU. */
                vListInsertEnd( &( xPendingReadyList[ xPortGetCoreID() ] ), &( pxUnblockedTCB->xEventListItem ) );
        }
 

diff --git a/components/freertos/test/test_suspend_scheduler.c b/components/freertos/test/test_suspend_scheduler.c
index c96eabbc8cbc51796e0f027838eb1dcdaf327c0d..b429c7c391473cf5c5a101fa23ca430a21849769 100644 (file)
--- a/components/freertos/test/test_suspend_scheduler.c
+++ b/components/freertos/test/test_suspend_scheduler.c
@@ -154,9 +154,8 @@ TEST_CASE("Handle waking multiple tasks while scheduler suspended", "[freertos]"
     /* Suspend scheduler on this CPU */
     vTaskSuspendAll();
 
-    /* Give all the semaphores once. You might expect this will wake up tasks on the other
-       CPU (where the scheduler is not suspended) but it doesn't do this in the current implementation
-       - all tasks are added to xPendingReadyList and woken up later. See note in the freertos-smp docs.
+    /* Give all the semaphores once. This will wake tasks immediately on the other
+       CPU, but they are deferred here until the scheduler resumes.
      */
     for (int p = 0; p < portNUM_PROCESSORS; p++) {
         for (int t = 0; t < TASKS_PER_PROC; t++) {
@@ -164,21 +163,19 @@ TEST_CASE("Handle waking multiple tasks while scheduler suspended", "[freertos]"
         }
    }
 
-    ets_delay_us(2 * 1000); /* Can't vTaskDelay() while scheduler is suspended, but let other CPU do some things */
+    ets_delay_us(1000); /* Let the other CPU do some things */
 
     for (int p = 0; p < portNUM_PROCESSORS; p++) {
         for (int t = 0; t < TASKS_PER_PROC; t++) {
-            /* You might expect that this is '1' for the other CPU, but it's not (see comment above) */
-            ets_printf("Checking CPU %d task %d (expected 0 actual %d)\n", p, t, counters[p][t].counter);
-            TEST_ASSERT_EQUAL(0, counters[p][t].counter);
+            int expected = (p == UNITY_FREERTOS_CPU) ? 0 : 1; // Has run if it was on the other CPU
+            ets_printf("Checking CPU %d task %d (expected %d actual %d)\n", p, t, expected, counters[p][t].counter);
+            TEST_ASSERT_EQUAL(expected, counters[p][t].counter);
         }
     }
 
     /* Resume scheduler */
     xTaskResumeAll();
 
-    vTaskDelay(TASKS_PER_PROC * 2);
-
     /* Now the tasks on both CPUs should have been woken once and counted once. */
     for (int p = 0; p < portNUM_PROCESSORS; p++) {
         for (int t = 0; t < TASKS_PER_PROC; t++) {

diff --git a/docs/api-guides/freertos-smp.rst b/docs/api-guides/freertos-smp.rst
index f135c017a6fec528a485f69a9ca04f9b71106254..a706320d84770c612c5869e8db748709c4929c7b 100644 (file)
--- a/docs/api-guides/freertos-smp.rst
+++ b/docs/api-guides/freertos-smp.rst
@@ -227,11 +227,6 @@ protection against simultaneous access. Consider using critical sections
 (disables interrupts) or semaphores (does not disable interrupts) instead when 
 protecting shared resources in ESP-IDF FreeRTOS.
 
-If the task running on the CPU with scheduler suspended calls a function (like
-``xSemaphoreGive``, ``xQueueSend``) that wakes another task, this task will not run
-until after ``vTaskResumeAll()`` is called. This is true even if the woken task is
-on the other CPU (where the scheduler is still running).
-
 In general, it's better to use other RTOS primitives like mutex semaphores to protect
 against data shared between tasks, rather than ``vTaskSuspendAll()``.