Fix context switch when time slicing is off

[aggarg]

Description

When time slicing is off, context switch should only happen when a task with priority higher than the currently executing one is unblocked. Earlier the code was invoking a context switch even when a task with priority equal to the currently executing task was unblocked. This commit fixes the code to only do a context switch when a higher priority task is unblocked.

Test Steps

Create 2 tasks with same priority:

1. Task T1 - Creates task T2 and then blocks itself by calling vTaskDelay.
2. Task T2 - Never blocks.

void TaskT1( void *pvParams )
{
    xTaskCreate( prvTaskT2, "t2", 512, NULL, 1, NULL );

    for( ;; )
    {
        vTaskDelay(pdMS_TO_TICKS(1000));
    }
}
/*-----------------------------------------------------------*/
void TaskT2( void *pvParams )
{
    volatile uint32_t ulCounter = 0;

    for( ;; )
    {
        ulCounter++;
    }
}
/*-----------------------------------------------------------*/

When time slicing is off, the task T1 must never be scheduled again after it blocks.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

[codecov[bot]]

Codecov Report

Base: 94.30% // Head: 94.30% // No change to project coverage :thumbsup:

Coverage data is based on head (15f92eb) compared to base (44e02bf). Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #568   +/-   ##
=======================================
  Coverage   94.30%   94.30%           
=======================================
  Files           6        6           
  Lines        2370     2370           
  Branches      579      579           
=======================================
  Hits         2235     2235           
  Misses         85       85           
  Partials       50       50           

Flag	Coverage Δ
unittests	94.30% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
tasks.c	94.69% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

[feilipu]

This breaks the default "Round Robin" behaviour for equal priority Tasks that has been the default for FreeRTOS since practically forever.

People rely, often inadvertently, on the former default behaviour.

[aggarg]

This case is handled here - https://github.com/FreeRTOS/FreeRTOS-Kernel/blob/main/tasks.c#L2841

Are you facing any problem with round robin scheduling of equal priority tasks?

[feilipu]

Provided that configUSE_TIME_SLICING remains enabled as the default, then there should be no problem. And, that will remain the default case, right?

People unaccustomed to FreeRTOS do not know that "busy wait" is not an acceptable option for pended Tasks, and therefore often tend to end up using the "Round Robin" feature inadvertently.

[aggarg]

And, that will remain the default case, right?

Yes - https://github.com/FreeRTOS/FreeRTOS-Kernel/blob/main/include/FreeRTOS.h#L888

Here is the description of the configUSE_TIME_SLICING which describes the intended behavior - https://www.freertos.org/a00110.html#configUSE_TIME_SLICING

This PR just fixes a bug when the application writer has turned off the time slicing explicitly. If you are saying that someone depended on that bug, then yes, their application will break and the fix would be as simple as setting configUSE_TIME_SLICING to 1 or leaving it undefined.