feat(threads): multicore support #397

elenaf9 · 2024-09-05T13:02:42Z

Description

This PR adds support for symmetric multiprocessing (SMP) for riot-rs-threads. It's a refactor and continuation of #241.
Right now, only the RP2040 is supported. I am planning to also add support for esp32s3 in a follow-up PR.

The implementation follows a global scheduling approach where the highest n ready threads are scheduled on the n available cores.
The main change in the scheduler logic is that it now removes the current thread from the runqueue, which is necessary so that a thread doesn't get picked twice. This also means that the thread has to be added to the runqueue again each time the scheduler is triggered (if it's still running).
This introduces some overhead if the scheduler is triggered unnecessarily. The PR therefore also includes optimizations that make sure that the scheduler is only triggered when a context switch is actually needed.
The implementation furthermore also supports affinity masks, which allow restricting a thread to certain cores.

All multicore logic is feature-gated, so that the single-core implementation largely remains the same. I still notice some difference in preliminary benchmark data, which I am still investigating.

Issues/PRs references

Tracking issue: #243

Depends on:

Open Questions

Idle threads

If there are no ready threads in the runqueue, the current (= on main) sched implementation WFIs in a loop. The context of the previous thread is only saved after a new thread is ready and the context is actually switched.
This causes some issues on multicore:

there is a race condition where it can happen that the still "unsaved" thread becomes ready again, and scheduled on the other core. That core would then pick up an outdated state.
we can't WFI inside the critical section because it would also block the other core

By adding idle threads, both of these issues are fixed and we furthermore avoid footguns related to interrupt priorities (e.g. right now we require that all embassy interrupts have a larger priority than PendSV so that they can preempt our scheduler when it's stuck in WFI). Wdyt?

Open TODOS

Drop commits that have later been reverted (I kept them in the history for now for transparency)
Post preliminary benchmark data
- compare: main vs this PR (multicore not enabled) vs this PR (multicore enabled)
- boards: RP2040 and nrf52840
- benchmarks: bench_sched_yield, bench_sched_flags (new)

Change checklist

I have cleaned up my commit history and squashed fixup commits.
I have followed the Coding Conventions.
I have performed a self-review of my own code.
I have made corresponding changes to the documentation.

src/riot-rs-rt/src/cortexm.rs

src/riot-rs-runqueue/src/runqueue.rs

src/riot-rs-threads/src/arch/cortex_m.rs

src/riot-rs-threads/src/lib.rs

Cargo.toml

src/riot-rs-runqueue/src/runqueue.rs

src/riot-rs-threads/src/smp/mod.rs

src/riot-rs-threads/src/smp/rp2040.rs

examples/threading-multicore/Cargo.toml

elenaf9 · 2024-09-07T12:31:52Z

I am planning to also add support for esp32s3 in a follow-up PR.

Brief update: I was able to also add multicore support for the dual-core esp32s3, based on #399!

It's in a separate branch for now, until #399 is merged.

branch elenaf9:multicore/v2_esp32s3
diff: f38b142...e5d64c8

I still need to do some more testing; the benchmarks that I am using for rp2040 only support cortex_m right now.

elenaf9 · 2024-09-27T16:58:24Z

I have now rebased my branch and cleaned up my commit history quite a lot.
(I have saved the old history with all WIP intermediate steps in a separate branch elenaf9:multicore/v2_save).

Additionally, with #399 merged, the last commit now also adds SMP support for the dual-core xtensa ESP32S3 🎉

elenaf9 · 2024-10-04T15:16:03Z

Rebased again to include the recently merged dynamic priority feature & adapt it for multicore.

src/riot-rs-rt/Cargo.toml

kaspar030 · 2024-10-24T12:29:24Z

You mean if it's rebased onto main, right? I believe that's still due to the most recent esp-hal update and esp-rs/esp-hal#2386.

Ah yes. I was confused b/c the fix for riscv was merged and I thought that would fix the issue also on xtensa. I guess there's a reason why we use INTR0 on xtensa and INTR1 on riscv?

kaspar030 · 2024-10-24T12:32:27Z

I guess there's a reason why we use INTR0 on xtensa and INTR1 on riscv?

Talked offline, no reason.

The issue seems to be resolved in upstream esp-hal by now. Since we depend on a fork anyway, could we maybe just update our fork to the most recent upstream main?

I'm trying to not jump in between versions (anymore ...). We already have a fix, if we unify use of intr for xtensa or riscv, we're good. Or, if we update the fix. (I'd prefer unifying).

Initial symmetric multiprocessing support for the RP2040 dual-core chip.

Add `Multicore::schedule_on_core`, which uses the FIFO queues between the cores to trigger the scheduler on the other core.

When a new thread is ready, schedule it only if it has higher prio that on of the current running threads. Select the core with the lower prio thread and schedule it there. This avoids unnecessary invocations of the scheduler and reduces priority inversions.

Feature-gate all multicore related code and logic so that there is no overhead when the feature is not enabled.

When a thread becomes ready, the scheduler on the core with the lowest prio running thread will be triggered. If now another thread becomes ready as well and the scheduler didn't have a chance to run yet (e.g. because interrupts are still disabled), the same scheduler will be triggered again, but only one thread is then selected and can run. The other thread is "skipped". To solve this, the scheduler on the other core should be triggered as well in this scenario so that both schedulers get the most recent state and the two highest prio threads are run.

elenaf9 · 2024-10-24T14:27:20Z

Unfortunately this PR breaks threading on the esp32-s3 for both single and multi-core. That needs fixing.

You mean if it's rebased onto main, right? I believe that's still due to the most recent esp-hal update and esp-rs/esp-hal#2386. The issue seems to be resolved in upstream esp-hal by now. Since we depend on a fork anyway, could we maybe just update our fork to the most recent upstream main?

Solved now by re-enabling the correct CPU interrupt in riot-rs-esp based on the Core ID. Single-Core was changed with #484 to now use CPU Intr 0 as well.

kaspar030

LGTM. Good job!

kaspar030 · 2024-10-24T16:46:21Z

Gratulations @elenaf9!

elenaf9 mentioned this pull request Sep 5, 2024

(WIP) threads: multicore support for RP2040 #241

Closed

elenaf9 commented Sep 5, 2024

View reviewed changes

src/riot-rs-rt/src/cortexm.rs Show resolved Hide resolved

elenaf9 commented Sep 5, 2024

View reviewed changes

src/riot-rs-runqueue/src/runqueue.rs Outdated Show resolved Hide resolved

src/riot-rs-threads/src/arch/cortex_m.rs Outdated Show resolved Hide resolved

src/riot-rs-threads/src/lib.rs Show resolved Hide resolved

Cargo.toml Outdated Show resolved Hide resolved