feat(threads): add mutex with priority inheritance #398

elenaf9 · 2024-09-05T14:42:56Z

Description

Add a mutex implementation analogous to the existing Lock, with additional priority inheritance.
Unfortunately I don't think we can directly reuse Lock, because we need access to the inner ThreadList.

The PR also moves all synchronization primitives into a new sync submodule. Any objections?

Issues/PRs references

Issue: #321.

Depends on

Open Questions

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-runqueue/src/runqueue.rs

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

src/riot-rs-threads/src/sync/mutex.rs

elenaf9 · 2024-10-02T13:46:27Z

I've rebased now, and moved the threading-mutex example into a threading-mutex test instead, analogous to the thread-lock test.

kaspar030 · 2024-10-02T14:09:35Z

The PR also moves all synchronization primitives into a new sync submodule. Any objections?

No objections. How hard is it to chop that into another PR that we can merge right away?

elenaf9 · 2024-10-02T14:57:52Z

The PR also moves all synchronization primitives into a new sync submodule. Any objections?

No objections. How hard is it to chop that into another PR that we can merge right away?

Not hard at all 🙂 #452.

ROMemories

I'll leave @kaspar030 review the rest of the implementation regarding priority inheritance; I have a couple of important questions regarding safety that I left as inline comments.

On another note, when should we use that new mutex? Should we replace our uses of embassy_sync::mutex::Mutex with it?

tests/threading-mutex/Cargo.toml

src/riot-rs-threads/src/sync/mutex.rs

src/riot-rs-threads/src/threadlist.rs

src/riot-rs-threads/src/sync/mutex.rs

elenaf9 · 2024-10-03T08:50:05Z

On another note, when should we use that new mutex? Should we replace our uses of embassy_sync::mutex::Mutex with it?

Tldr: embassy_sync::mutex::Mutex is for usage inside of async tasks, riot_rs_threads::sync::Mutex for threads without an embassy executor.

embassy_sync::mutex::Mutex is the correct choice if we have (multiple) async tasks, because it only blocks the waiting task, and other tasks on the same executor can still run. The riot_rs_threads::sync::Mutex can a) only be used with a executor_thread (otherwise it'd panic when its not inside a thread context, e.g. in case of the executor-interrupt), and b) always block the whole thread and thus also all other tasks that are running on the same executor.

The riot_rs_threads::sync::Mutex is for non-async contexts. This could also be realized with the embassy Mutex + block_on, but the new Mutex has a smaller overhead and the priority inheritance feature.

This demonstrates how multiple threads can wait for the same mutex, get unblocked by priority, and the owning thread inherits the priority of the highest waiting thread.

Address review comments.

src/riot-rs-threads/src/sync/mutex.rs

Apply suggestions from code review Co-authored-by: ROMemories <152802150+ROMemories@users.noreply.github.com>

src/riot-rs-threads/src/sync/mutex.rs

Co-authored-by: ROMemories <152802150+ROMemories@users.noreply.github.com>