Pass core ID to critical nesting count macros

[felixvanoost]

Description

Passes the core ID as an argument to the critical nesting count macros port{GET/SET/INCREMENT/DECREMENT}_CRITICAL_NESTING_COUNT in tasks.c. This allows the functions that use these macros to read and store the core ID once per call using portGET_CORE_ID() when using FreeRTOS SMP, thereby reducing the number of unnecessary, and generally slow, accesses to a peripheral register. I have summarised the worst-case number of calls to portGET_CORE_ID() before and after these changes:

Function	# of calls (before)	# of calls (after)
prvCheckForRunStateChange()	4	1
prvYieldForTask()	5	1
vTaskSwitchContext()	2	1
vTaskYieldWithinAPI()	2	1
vTaskEnterCritical()	3	1
vTaskEnterCriticalFromISR()	2	1
vTaskExitCritical()	4	1
vTaskExitCriticalFromISR()	4	1

The execution time of each of the above functions has been reduced on the architecture I tested (an ARM Cortex-M7), though I expect this will yield similar improvements on most ISAs. Further improvements are possible by also passing the core ID to the port{GET/RELEASE}_{ISR/TASK}_LOCK}() macros, but this involves changes to the port API so I haven't included them here.

Test Steps

I compared the execution time of the functions listed in the table above using ETM trace on an NXP S32K396 microcontroller running FreeRTOS v11.1 in SMP mode. I looked at the execution time for 10 random calls to each function while the system is running:

Function	Mean execution time (before)	Mean execution time (after)	% reduction
prvCheckForRunStateChange()	768 ns	769 ns	0%
prvYieldForTask()	2995 ns	3050 ns	-2%
vTaskSwitchContext()	11963 ns	11055 ns	8%
vTaskEnterCritical()	4936 ns	4695 ns	5%
vTaskEnterCriticalFromISR()	3095 ns	2799 ns	10%
vTaskExitCritical()	5342 ns	5352 ns	0%
vTaskExitCriticalFromISR()	3974 ns	2665 ns	33%

I was not able to get individual results for vTaskYieldWithinAPI() because it calls some of the other functions above. For prvCheckForRunStateChange() and prvYieldForTask(), I noticed that the number of calls to portGET_CORE_ID() on my system were the same as before, so my code is not branching to the worst-case path. These are average-case results on my particular project and not definitive, but certain functions do see a clear improvement.

These changes are made under the assumption that the core ID cannot change within the context of each function, which I believe is true.

Checklist:

• 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

#1204

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

[chinglee-iot]

I give my initial review suggest here. I will feedback my further suggestion about the followings topic:

• portXXX_CRITICAL_NESTING_COUNT prototype change. Currently, only RP2040 supports portCRITICAL_NESTING_IN_TCB = 0. We will discuss the backward compatibility problem and update in this PR.
• vTaskYieldWithinAPI() implementation. The critical nesting count should be acquired with interrupt disabled. It would be great if we can also fix this in the PR.

[felixvanoost]

@chinglee-iot I see that some of the tests are failing:

1. Not sure what's going on with the link-verifier checks, but I was able to access all three of the reportedly broken links on my browser.
2. Since the number of expected function calls in the CMocks has changed, should I be updating the test cases myself in this PR?
3. I see that the kernel demos SMP test is generating an unused variable error in vTaskSuspendAll() because the function is called when the scheduler is not running. I believe xCoreID is used in exactly the same way as ulState here, so I'm confused why only one error iis being flagged. Is there a preferred way of handling this error? I could move the assignment outside the if statement or cast xCoreID to void in the else.

[aggarg]

Thank you for this change and thorough testing @felixvanoost ! Even though this is a backward incompatible change, we can take it because 1) not many SMP ports store critical nesting in context and 2) the fix should be trivial for the port author.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[chinglee-iot]

Two or the community supported port uses portCRITICAL_NESTING_IN_TCB = 0

• RP2350 RISC-V and RP2350 ARM NTZ
  We should create a PR to update these two ports and submodule pointer as well.