embedonomicon icon indicating copy to clipboard operation
embedonomicon copied to clipboard

concurrency chapter

Open japaric opened this issue 6 years ago • 11 comments

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

japaric avatar Apr 08 '19 21:04 japaric

r? @korken89

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

rust-highfive avatar Apr 08 '19 21:04 rust-highfive

Other than those two nits LGTM

therealprof avatar Apr 09 '19 11:04 therealprof

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

  • Atomics
  • Spinlock
  • Mutex

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: :)

andre-richter avatar Apr 09 '19 12:04 andre-richter

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

korken89 avatar Apr 09 '19 17:04 korken89

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

korken89 avatar Apr 09 '19 17:04 korken89

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.^^

RalfJung avatar Apr 09 '19 20:04 RalfJung

@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

japaric avatar Apr 13 '19 16:04 japaric

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

  • Both SysTick and SVCall have fixed (and different) priorities.
  • They are both exceptions, not interrupts. That's a bit misleading.

Disasm avatar Apr 14 '19 09:04 Disasm

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"!

jounathaen avatar Jan 22 '20 14:01 jounathaen

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

jamesmunns avatar Apr 26 '20 18:04 jamesmunns

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

TDHolmes avatar Jul 03 '21 15:07 TDHolmes