From 3e62c2e0520324f41531fe7f8a2ea2271f833c52 Mon Sep 17 00:00:00 2001
From: Angus Gratton <angus@espressif.com>
Date: Thu, 28 Sep 2017 11:46:53 +1000
Subject: [PATCH] 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.
---
 components/freertos/tasks.c                   | 20 +++++++++++++++++--
 .../freertos/test/test_suspend_scheduler.c    | 15 ++++++--------
 docs/api-guides/freertos-smp.rst              |  5 -----
 3 files changed, 24 insertions(+), 16 deletions(-)

diff --git a/components/freertos/tasks.c b/components/freertos/tasks.c
index 15d588662c..15cdaad652 100644
--- 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 c96eabbc8c..b429c7c391 100644
--- 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 f135c017a6..a706320d84 100644
--- 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()``.
 
-- 
2.40.0