Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Feature Request] Pass core ID to SMP port lock functions #1204

Open
felixvanoost opened this issue Dec 11, 2024 · 4 comments
Open

[Feature Request] Pass core ID to SMP port lock functions #1204

felixvanoost opened this issue Dec 11, 2024 · 4 comments
Assignees
Labels
enhancement New feature or request

Comments

@felixvanoost
Copy link

felixvanoost commented Dec 11, 2024

Is your feature request related to a problem? Please describe.
In SMP mode, several functions must execute in critical sections by making sequential calls to portGET_TASK_LOCK()/portGET_ISR_LOCK() and portRELEASE_TASK_LOCK()/portRELEASE_ISR_LOCK(). On some SMP ports (e.g. the Cortex-M7 and Cortex-R5 port from NXP), these locks are implemented using software spinlocks that use the macro portGET_CORE_ID() to determine the ID of the currently executing core.

On these same ports, portGET_CORE_ID() requires reading from peripheral registers that have non-trivial (and sometimes non-deterministic) access times. This impacts the overall execution time of certain critical functions and increases the overhead caused by FreeRTOS in SMP mode. Some examples include:

  • vTaskSwitchContext(), which calls portGET_CORE_ID() once internally, but in the worst case 1 additional time per call to portGET_TASK_LOCK()/portGET_ISR_LOCK() and portRELEASE_TASK_LOCK()/portRELEASE_ISR_LOCK() depending on the port (up to 5 calls in total).
  • vTaskEnterCritical() and vTaskExitCritical() which in the worst case each require 2 calls to portGET_CORE_ID() (one to release each lock).

These functions are used frequently by the scheduler so would benefit the most from optimization.

Describe the solution you'd like
The number of calls to portGET_CORE_ID() could be potentially reduced in the aforementioned functions by storing the core ID in a local variable and passing it as an argument to portGET_TASK_LOCK()/portGET_ISR_LOCK() and portRELEASE_TASK_LOCK()/portRELEASE_ISR_LOCK().

On my particular hardware (an NXP S32K3-series MCU with a Cortex-M7), a read of the peripheral register containing the core ID takes ~200-600ns each time, so a potential speedup of ~400-1,200ns could be possible with each get/take lock function call.

Describe alternatives you've considered
None so far.

How many devices will this feature impact?
Potentially all MCUs that must read the core ID from a register that is not local to the core itself (i.e, from a memory-mapped peripheral).

What are your project timelines?
ASAP 🙃

Additional context
None

@felixvanoost felixvanoost added the enhancement New feature or request label Dec 11, 2024
@felixvanoost
Copy link
Author

I realised that portGET_CORE_ID() is actually used in the CRITICAL_NESTING_COUNT macros in tasks.c as well, resulting in an additional function call each time. I've summed the number of (what I believe to be) unnecessary calls to portGET_CORE_ID() in the worst case for some of the most used functions:

Function name Unnecessary calls to portGET_CORE_ID()
prvCheckForStateRunChange() 8
prvYieldForTask() 1
vTaskSuspendAll() 3
vTaskResumeAll() 1
vTaskSwitchContext() 5
vTaskYieldWithinAPI() 1
vTaskEnterCritical() 4
vTaskEnterCriticalFromISR() 2
vTaskExitCritical() 6
vTaskExitCriticalFromISR() 4

In my initial testing with storing the core ID in a local variable per function call, I was able to reduce the execution time of some of these functions by ~30% on my system.

I can submit a PR to resolve at least part of these if that would be helpful!

@amazonKamath
Copy link
Member

@felixvanoost
Copy link
Author

I've opened a PR (#1206) to reduce the number of calls to portGET_CORE_ID() in the CRITICAL_NESTING_COUNT macros.

The changes I described in the initial issue would result in an additional execution time reduction but would require the port API to change slightly. The macros port{GET/RELEASE}_ISR_LOCK and port{GET/RELEASE}_TASK_LOCK would need to be updated to pass the core ID as a parameter, which will affect existing SMP ports.

@chinglee-iot
Copy link
Member

Thank you for creating this PR. I will look into the detail and discuss with you in the PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants