Update task notification scheduler suspension usage

[Dazza0]

Description

#612 fixed an issue with ulTaskGenericNotifyTake() and xTaskGenericNotifyWait() being non deterministic by ensuring that those functions call prvAddCurrentTaskToDelayedList() while the scheduler was suspended. The merged fix would suspend the scheduler while still in a critical section, then exit the critical section while the scheduler was suspended:

taskENTER_CRITICAL();
vTaskSuspendAll();
taskEXIT_CRITICAL();
prvAddCurrentTaskToDelayedList( xTicksToWait, pdTRUE );
xTaskResumeAll();

The method above technically works but is a bit unintuitive due to interleaving critical sections and scheduler suspension.

This PR updates the behavior described above by wrapping the critical section in a scheduler suspension block:

vTaskSuspendAll();
taskENTER_CRITICAL();
// Check if block required
taskEXIT_CRITICAL();
if (blocking) {
    prvAddCurrentTaskToDelayedList( xTicksToWait, pdTRUE );
}
xTaskResumeAll();

This method is more intuitive. As an added bonus, it also allows the SMP implementation of vTaskSuspendAll() to be simplified (as we can assume the vTaskSuspendAll() is now never called in a critical section). As such, an extra assert has been added to vTaskSuspendAll() to ensure that it is never called in a critical section.

• I have tested my changes. No regression in existing tests.
• I have modified and/or added unit-tests to cover the code changes in this Pull Request.

Related Issue

• See Update unit tests due to task notification changes FreeRTOS#1177 for CMock test updates
• Related to [BUG] xTaskNotifyWait & ulTaskNotifyTake perform a non-deterministic operation in a critical section. #612

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

[Dazza0]

@chinglee-iot I've updated the CMock unit tests as well. PTAL.

[Skptak]

@aggarg @RichardBarry

Is this change alright? I know some ports such as the ARM_CRx_No_GIC push the critical nesting count as part of the task context:

    /* Push the critical nesting count. */
    LDR     R2, ulCriticalNestingConst
    LDR     R1, [R2]
    PUSH    {R1}

I'm worried that this change might be a breaking change for some ports/applications and was hoping for a deeper look into this PR from you two

[chinglee-iot]

Using portGET_CRITICAL_NESTING_COUNT() here is okay to me. Sharing my observation here.
Critical nesting count should only be modified in critical section and context switch inside a critical section is not allowed in SMP. Use the following scenarios for discussion:

1. portGET_CRITICAL_NESTING_COUNT() is greater than 1. This task is in a critical section and can read or update the critical nesting count without problem.
2. portGET_CRITICAL_NESTING_COUNT() is 0. The task A may get preempted by task B while reading critical nesting count. Critical nesting count will be restore to 0 when the task B left critical section.

[sonarcloud]

Quality Gate passed

The SonarCloud Quality Gate passed, but some issues were introduced.

2 New issues
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

[codecov]

Codecov Report

Attention: 3 lines in your changes are missing coverage. Please review.

Comparison is base (57a5ed7) 93.55% compared to head (63c8d73) 93.52%.

Files	Patch %	Lines
tasks.c	89.65%	0 Missing and 3 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #982      +/-   ##
==========================================
- Coverage   93.55%   93.52%   -0.03%     
==========================================
  Files           6        6              
  Lines        3197     3199       +2     
  Branches      887      889       +2     
==========================================
+ Hits         2991     2992       +1     
  Misses         92       92              
- Partials      114      115       +1     

Flag	Coverage Δ
unittests	93.52% <89.65%> (-0.03%)	⬇️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.