rust-embedded / embedonomicon

How to bootstrap support for a no_std target
https://docs.rust-embedded.org/embedonomicon/
Apache License 2.0
206 stars 33 forks source link

concurrency chapter #48

Open japaric opened 5 years ago

japaric commented 5 years ago

This PR adds a chapter that documents the most common (memory) safe uses of unsafe code in concurrent bare metal (no_std) programs. The patterns described here are used to implement the safe concurrency abstractions provided by the cortex-m, cortex-m-rt and cortex-m-rtfm crates.

Rendered

@RalfJung would it be possible to get the UCG WG to (at some point) review / audit these patterns? I'm particularly concerned about LLVM attributes / semantics not matching the intended semantics.

cc @jamesmunns this is relevant to your recent work on shared-rs

rust-highfive commented 5 years ago

r? @korken89

(rust_highfive has picked a reviewer for you, use r? to override)

therealprof commented 5 years ago

Other than those two nits LGTM

andre-richter commented 5 years ago

EDIT ---------------------- I just realized we are in fact showing a spinlock under the "Atomic" chapter. Sorry for the confusion.

In this case, it is probably needed to explain a bit more about the terms

and the unfortunate case that due to the naming in the spin crate, this example

use spin::Mutex;

static COUNTER: Mutex<u64> = Mutex::new(0);

actually shows a spinlock which happens to be named Mutex.

I think readers new to these terms will be left very confused (same as me :D) EDIT END ----------------------

I think the end of the document is missing a subchapter to conclude the multi-core story and therefore the whole document nicely.

First, we tell that Mutex as implemented in the current form is not Sync. Then we introduce Atomics as an alternative, and only casually mention they can be used to build spinlocks too.

IMO, we should showcase exactly that after the Atomics chapter. We can probably cite a minimal implementation, as for example it can be found in the spin crate.

Other than that, I like :+1: :)

korken89 commented 5 years ago

I'm happy with this, but lets include @therealprof comments. After that I will merge.

korken89 commented 5 years ago

Something seems amiss with the asm format, can you have a look @japaric ?

RalfJung commented 5 years ago

would it be possible to get the UCG WG to (at some point) review / audit these patterns? I'm particularly concerned about LLVM attributes / semantics not matching the intended semantics.

I guess? If you have concrete questions, I suggest to open an issue at https://github.com/rust-rfcs/unsafe-code-guidelines/. However, in this PR I see 1700 lines of code and I am not sure what to look at.^^

japaric commented 5 years ago

@therealprof thanks for the review; I have addressed your comments

@andre-richter I have added some notes; let me know if you think something else needs to be clarified

@korken89 if you mean the CI failure; it should have been fixed by #47

@RalfJung great, I'll open a few issues over there

Disasm commented 5 years ago

@japaric In Priorities section I would suggest using some different interrupts as an example:

jounathaen commented 4 years ago

I'd like to bump this PR, because the chapter contains a lot of useful information and it would be great to have them in the "documentation"!

jamesmunns commented 4 years ago

Note: will probably need to be rebased on https://github.com/rust-embedded/embedonomicon/pull/66

TDHolmes commented 3 years ago

Any chance this will be going in at some point? Sounds very useful. I'm coming here from the "under the hood" section of the RTIC documentation