-
Notifications
You must be signed in to change notification settings - Fork 998
Add goroutine core affinity support for RP2040/RP2350 systems #5092
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
base: dev
Are you sure you want to change the base?
Conversation
- Introduced support for CPU core pinning and affinity for tasks and goroutines. - Updated the scheduler to respect affinity constraints with separate queues for pinned and shared tasks. - Added new runtime API functions `LockToCore`, `UnlockFromCore`, `GetAffinity`, and `CurrentCPU`. - Example program demonstrates core pinning and unpinned execution behavior.
|
Do you actually care about the particular core? If not, are the existing |
|
This is what I see for LockOsThread // LockOSThread wires the calling goroutine to its current operating system thread. // UnlockOSThread undoes an earlier call to LockOSThread. There seems to be no implementation behind it. For the RP2, since it is symmetrical multi processor, it probably doesn't matter which exact core. But for something like StM32h7, it would matter which core you pin to. (I know we don't support multicore on it yet) |
|
I know. What I'm saying is to change |
|
Fair point. That seems reasonable to me. I can use those function names instead. |
|
Right. So
An important issue to think about is what happens if the requested core is busy? |
- Dropped CurrentCPU - Dropped GetAffinity - Renamed LockToCore to LockCore to mimic LockOSThread naming. - Updated examples program
|
Looks like it passed all checks except the macos(13) test with this error This is a scheduled macos-13 brownout. The macOS-13 based runner images are being deprecated. For more details, see actions/runner-images#13046. |
eliasnaur
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. I've commented on the implementation, but I'm still not a fan of the LockCore API, because it may block indefinitely if a long-running goroutine is running on the target core. In a sense, LockCore acts as a per-core mutex that some arbitrary other goroutine may have taken, with the usual deadlock risks.
One way of getting around this issue is by requiring LockCore to be called before any other goroutine has started. A good place would be in an init function.
| // Stub for now | ||
| // On microcontrollers with multiple cores (e.g., RP2040/RP2350), this pins the | ||
| // goroutine to the core it's currently running on. | ||
| // Called by go1.18 standard library on windows, see https://github.com/golang/go/issues/49320 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While here, remove this now irrelevant comment.
| // On microcontrollers with multiple cores (e.g., RP2040/RP2350), this pins the | ||
| // goroutine to the core it's currently running on. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it more precise to say "with the "cores" scheduler"?
src/machine/machine_rp2_cores.go
Outdated
|
|
||
| const numCPU = 2 // RP2040 and RP2350 both have 2 cores | ||
|
|
||
| // LockCore implementation for the cores scheduler. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This needs a more detailed description. For example, it doesn't say what happens if the target core is busy. I believe LockCore returns. If so, this is surprising to me; I would expect that once LockCore returns, the calling goroutine is running on the target core.
…behavior, and limitations with the "cores" scheduler. Updated LockOSThread and UnlockOSThread comments to reflect core pinning behavior on RP2040/RP2350.
eliasnaur
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@aykevl should probably take a look.
| // | ||
| // Valid core values are 0 and 1. Panics if core is out of range. | ||
| // | ||
| // Only available on RP2040 and RP2350 with the "cores" scheduler. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Superfluous comment. The function is only available on rpxxxx by build tags.
| // After calling UnlockCore, the scheduler is free to schedule the goroutine on | ||
| // any core for automatic load balancing. | ||
| // | ||
| // Only available on RP2040 and RP2350 with the "cores" scheduler. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Superfluous comment.
| // To avoid potential blocking on a busy core, consider calling LockCore in an | ||
| // init function before any other goroutines have started. This guarantees the | ||
| // target core is available. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this should be a hard requirements; that is, LockCore should panic if any other goroutine has started.
| // Important: LockCore sets the affinity but does not immediately migrate the | ||
| // goroutine to the target core. The actual migration happens at the next | ||
| // scheduling point (e.g., channel operation, time.Sleep, or Gosched). After | ||
| // that point, the goroutine will wait in the target core's queue if that core | ||
| // is busy running another goroutine. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Exposing this implementation detail seems like an unnecessary burden on the caller. Why can't LockCore switch the goroutine over to the target core before returning? For instance, why can't LockCore call Gosched?
This PR proposes
LockToCore,UnlockFromCore,GetAffinity, andCurrentCPU.API Functions
runtime.NumCPU() intReturns the number of CPU cores available (returns 2 on RP2040/RP2350).
runtime.CurrentCPU() intReturns the current CPU core number (0 or 1).
runtime.LockToCore(core int)Pins the current goroutine to the specified core:
core = 0- Pin to core 0core = 1- Pin to core 1core = -1- Unpin (allow running on any core)Panics if core is invalid (not -1, 0, or 1).
runtime.UnlockFromCore()Unpins the current goroutine, allowing it to run on any core.
Equivalent to
runtime.LockToCore(-1).runtime.GetAffinity() intReturns the current goroutine's CPU affinity:
-1if not pinned (can run on any core)0or1if pinned to that specific coreExample program included in the examples directory
Tested on both pico and pico2
Output of example program