Fix vTaskSuspendAll assertion for critical nesting count

chinglee-iot commented 2 months ago

Description

The macro portGET_CRITICAL_NESTING_COUNT is implemented like the following:

#define portGET_CRITICAL_NESTING_COUNT()          ( pxCurrentTCBs[ portGET_CORE_ID() ]->uxCriticalNesting )

It involves the following 2 steps:

Get current core ID with portGET_CORE_ID().
Use current core ID to index pxCurrentTCBs and corresponding pxCurrentTCB.

If a context switch happens between step 1 and step 2, the core ID may be changed as the task may be scheduled on a different core. In that case, we would end up accessing the wrong TCB and thereby, wrong critical nesting count.

This PR address the issue by ensuring that portGET_CRITICAL_NESTING_COUNT is called with interrupts disabled to ensure atomicity.

Test Steps

Before this PR There will be assertion in vTaskSuspendAll() if running this XMOS demo.

xrun --xscope bin/RTOSDemo.xe
Starting Scheduler
Logical Core 0 initializing as FreeRTOS Core 0
Logical Core 2 initializing as FreeRTOS Core 1
Logical Core 3 initializing as FreeRTOS Core 2
Logical Core 4 initializing as FreeRTOS Core 3
Logical Core 5 initializing as FreeRTOS Core 4
Logical Core 6 initializing as FreeRTOS Core 5
Logical Core 7 initializing as FreeRTOS Core 6
FreeRTOS Core 1 initialized
FreeRTOS Core 2 initialized
FreeRTOS Core 3 initialized
Starting Scheduler
FreeRTOS Core 4 initialized
FreeRTOS Core 5 initialized
FreeRTOS Core 6 initialized
FreeRTOS Core 0 initialized
Logical Core 0 initializing as FreeRTOS Core 0
Logical Core 2 initializing as FreeRTOS Core 1
Logical Core 3 initializing as FreeRTOS Core 2
Logical Core 4 initializing as FreeRTOS Core 3
Logical Core 5 initializing as FreeRTOS Core 4
Logical Core 6 initializing as FreeRTOS Core 5
Logical Core 7 initializing as FreeRTOS Core 6
FreeRTOS Core 2 initialized
FreeRTOS Core 3 initialized
FreeRTOS Core 4 initialized
FreeRTOS Core 5 initialized
FreeRTOS Core 6 initialized
FreeRTOS Core 0 initialized
FreeRTOS Core 1 initialized
xrun: Program received signal ET_ECALL, Application exception.
      [Switching to tile[1] core[7] (dual issue)]
      vTaskSuspendAll () at ../../../../../Source\tasks.c:3834

      3834                  configASSERT( portGET_CRITICAL_NESTING_COUNT() == 0 );
      Current language:  auto; currently minimal
make: *** [run] Error 125

Checklist:

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

The following unit test is fixed in https://github.com/FreeRTOS/FreeRTOS/pull/1204

/mnt/c/msys64/home/chinglee/kernel/unit-test/FreeRTOS/FreeRTOS/Test/CMock/smp/multiple_priorities_no_timeslice_mock/covg_multiple_priorities_no_timeslice_mock_utest.c:804:test_coverage_prvGetExpectedIdleTime_ready_list_eq_1:FAIL:Function ulFakePortSetInterruptMask. Called earlier than expected.

/mnt/c/msys64/home/chinglee/kernel/unit-test/FreeRTOS/FreeRTOS/Test/CMock/smp/multiple_priorities_no_timeslice_mock/covg_multiple_priorities_no_timeslice_mock_utest.c:910:test_coverage_prvGetExpectedIdleTime_ready_list_eq_2:FAIL:Function ulFakePortSetInterruptMask. Called earlier than expected.

/config_assert_utest.c:587:test_prvGetExpectedIdleTime_assert_nextUnblock_lt_xTickCount:FAIL:Function ulFakePortSetInterruptMask. Called earlier than expected.

Related Issue

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

sonarcloud[bot] commented 2 months ago

Quality Gate passed

Issues
0 New issues
0 Accepted issues

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

See analysis details on SonarCloud

codecov[bot] commented 2 months ago

Codecov Report

Attention: Patch coverage is 0% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 93.00%. Comparing base (8c49c54) to head (ae38d50). Report is 7 commits behind head on main.

Files	Patch %	Lines
tasks.c	0.00%	0 Missing and 1 partial :warning:

Additional details and impacted files

```diff @@ Coverage Diff @@ ## main #1029 +/- ## ======================================= Coverage 93.00% 93.00% ======================================= Files 6 6 Lines 3200 3203 +3 Branches 879 879 ======================================= + Hits 2976 2979 +3 Misses 111 111 Partials 113 113 ``` | [Flag](https://app.codecov.io/gh/FreeRTOS/FreeRTOS-Kernel/pull/1029/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=FreeRTOS) | Coverage Δ | | |---|---|---| | [unittests](https://app.codecov.io/gh/FreeRTOS/FreeRTOS-Kernel/pull/1029/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=FreeRTOS) | `93.00% <0.00%> (+<0.01%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=FreeRTOS#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

FreeRTOS / FreeRTOS-Kernel