[BUG] Scheduling stop in the prvCheckForRunStateChange function for SMP

[raceant]

Describe the bug
Running on the SMP system, the system stops scheduling, and an assertion is triggered at configASSERT( pxThisTCB->xTaskRunState == taskTASK_SCHEDULED_TO_YIELD ) in the code of the prvCheckForRunStateChange function.

In the vTaskSuspendAll function, the portSET_INTERRUPT_MASK() macro is used to disable interrupts before calling prvCheckForRunStateChange. However, the prvCheckForRunStateChange function code uses the portENABLE_INTERRUPTS() macro to temporarily enable task switching, which results in task switching not actually being enabled. After calling portENABLE_INTERRUPTS(), the interrupts used for scheduling remain masked.

Target

• Development board: [user-defined]
• Instruction Set Architecture: [ARM_CR82]
• IDE and version: [IAR 9.20]

To Reproduce

• Use multi-task with xSemaphoreXXX function on multi-core system.
• Frequent calls to the vTaskSuspendAll function
• Frequent calls to the prvYieldCore( xCoreID ) macro

Additional context
It might be appropriate to use the portDISABLE_INTERRUPTS() macro instead of the portSET_INTERRUPT_MASK() macro in the vTaskSuspendAll function.

[raceant]

Attached is the relevant code

FreeRTOS-Kernel/tasks.c

Lines 3896 to 3933 in e5987bb

	ulState = portSET_INTERRUPT_MASK();
	xCoreID = ( BaseType_t ) portGET_CORE_ID();
	/* This must never be called from inside a critical section. */
	configASSERT( portGET_CRITICAL_NESTING_COUNT( xCoreID ) == 0 );
	/* portSOFTWARE_BARRIER() is only implemented for emulated/simulated ports that
	* do not otherwise exhibit real time behaviour. */
	portSOFTWARE_BARRIER();
	portGET_TASK_LOCK( xCoreID );
	/* uxSchedulerSuspended is increased after prvCheckForRunStateChange. The
	* purpose is to prevent altering the variable when fromISR APIs are readying
	* it. */
	if( uxSchedulerSuspended == 0U )
	{
	prvCheckForRunStateChange();
	}
	else
	{
	mtCOVERAGE_TEST_MARKER();
	}
	/* Query the coreID again as prvCheckForRunStateChange may have
	* caused the task to get scheduled on a different core. The correct
	* task lock for the core is acquired in prvCheckForRunStateChange. */
	xCoreID = ( BaseType_t ) portGET_CORE_ID();
	portGET_ISR_LOCK( xCoreID );
	/* The scheduler is suspended if uxSchedulerSuspended is non-zero. An increment
	* is used to allow calls to vTaskSuspendAll() to nest. */
	++uxSchedulerSuspended;
	portRELEASE_ISR_LOCK( xCoreID );
	portCLEAR_INTERRUPT_MASK( ulState );

FreeRTOS-Kernel/tasks.c

Lines 863 to 871 in e5987bb

	portENABLE_INTERRUPTS();
	/* Enabling interrupts should cause this core to immediately service
	* the pending interrupt and yield. After servicing the pending interrupt,
	* the task needs to re-evaluate its run state within this loop, as
	* other cores may have requested this task to yield, potentially altering
	* its run state. */
	portDISABLE_INTERRUPTS();

[chinglee-iot]

@raceant
Thank you for reporting this issue.
The implementation assumes the following porting macros operate on the same interrupt mask register.

Using Cortex-M as an example:

• portDISABLE_INTERRUPTS - Disable interrupts (set PRIMASK bit to 1)

cpsid i

• portENABLE_INTERRUPTS - Enable interrupts (clear PRIMASK bit to 0)

cpsie i

• portSET_INTERRUPT_MASK - Save current PRIMASK state, then disable interrupts

mrs %0, PRIMASK    ; Read current state
cpsid i            ; Disable interrupts

Returns the saved state for later restoration.

• portCLEAR_INTERRUPT_MASK - Restore previous PRIMASK state

msr PRIMASK, %0    ; Restore saved state

This restores interrupts to their previous state (not unconditionally enabling them), which is critical for nested critical sections.

Using portDISABLE_INTERRUPTS() and portENABLE_INTERRUPTS() in vTaskSuspendAll() will need more consideration because these macros unconditionally disable and enable interrupts without considering nested critical section scenarios.

Could you please update your port to follow this behavior and let us know the result?

[raceant]

Thank you very much for your reply.
According to the modifications above, the issue was resolved after testing.

There are many platform-specific demo code, the macros portDISABLE_INTERRUPTS macro is used to set the interrupt control bits of the processor, while the macros portSET_INTERRUPT_MASK macro is used to control the mask bits of the interrupt controller.

Using ARM_AARCH64 as an example:

• portDISABLE_INTERRUPTS
  

  FreeRTOS-Kernel/portable/GCC/ARM_AARCH64/portmacro.h

  Lines 108 to 111 in e5987bb

  	#define portDISABLE_INTERRUPTS() \
  	__asm volatile ( "MSR DAIFSET, #2" ::: "memory" ); \
  	__asm volatile ( "DSB SY" ); \
  	__asm volatile ( "ISB SY" );

• portENABLE_INTERRUPTS
  

  FreeRTOS-Kernel/portable/GCC/ARM_AARCH64/portmacro.h

  Lines 113 to 116 in e5987bb

  	#define portENABLE_INTERRUPTS() \
  	__asm volatile ( "MSR DAIFCLR, #2" ::: "memory" ); \
  	__asm volatile ( "DSB SY" ); \
  	__asm volatile ( "ISB SY" );

• portSET_INTERRUPT_MASK
  

  FreeRTOS-Kernel/portable/GCC/ARM_AARCH64/port.c

  Lines 495 to 519 in e5987bb

  	UBaseType_t uxPortSetInterruptMask( void )
  	{
  	uint32_t ulReturn;
  	/* Interrupt in the CPU must be turned off while the ICCPMR is being
  	* updated. */
  	portDISABLE_INTERRUPTS();
  	if( portICCPMR_PRIORITY_MASK_REGISTER == ( uint32_t ) ( configMAX_API_CALL_INTERRUPT_PRIORITY << portPRIORITY_SHIFT ) )
  	{
  	/* Interrupts were already masked. */
  	ulReturn = pdTRUE;
  	}
  	else
  	{
  	ulReturn = pdFALSE;
  	portICCPMR_PRIORITY_MASK_REGISTER = ( uint32_t ) ( configMAX_API_CALL_INTERRUPT_PRIORITY << portPRIORITY_SHIFT );
  	__asm volatile ( "dsb sy \n"
  	"isb sy \n" ::: "memory" );
  	}
  	portENABLE_INTERRUPTS();
  	return ulReturn;
  	}

• portCLEAR_INTERRUPT_MASK
  

  FreeRTOS-Kernel/portable/GCC/ARM_AARCH64/port.c

  Lines 122 to 129 in e5987bb

  	#define portCLEAR_INTERRUPT_MASK() \
  	{ \
  	portDISABLE_INTERRUPTS(); \
  	portICCPMR_PRIORITY_MASK_REGISTER = portUNMASK_VALUE; \
  	__asm volatile ( "DSB SY \n" \
  	"ISB SY \n" ); \
  	portENABLE_INTERRUPTS(); \
  	}

Issues may arise on platforms where the two macros define the interrupt mask register differently.
It might be more suitable to support interrupt nesting.
Do you have any advice?

Looking forward to optimizing this code.
Thank you.