concurrency chapter #48
Conversation
|
r? @korken89 (rust_highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
S-waiting-on-review
src/concurrency.md
Outdated
| {{#include ../ci/concurrency/examples/systick.rs}}``` | ||
|
|
||
| If you are not familiar with embedded / Cortex-M programs the most important | ||
| thing to point note here is that the function marked with the `entry` attribute |
There was a problem hiding this comment.
Should probably either be point out or note.
|
Other than those two nits LGTM |
|
EDIT ---------------------- 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 I think readers new to these terms will be left very confused (same as me :D) 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 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 👍 :) |
|
I'm happy with this, but lets include @therealprof comments. After that I will merge. |
|
Something seems amiss with the asm format, can you have a look @japaric ? |
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.^^ |
Co-Authored-By: japaric <[email protected]>
also note the bounds and multi-core requirements of the cooperative handlers pattern
|
@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 In Priorities section I would suggest using some different interrupts as an example:
|
|
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"! |
|
Note: will probably need to be rebased on #66 |
| [this repository]: https://github.com/rust-embedded/embedonomicon | ||
|
|
||
| ``` rust | ||
| {{#include ../ci/concurrency/examples/systick.rs}}``` |
There was a problem hiding this comment.
The backticks on the same line break with the current mdbook version (there's multiple instances of this).
| {{#include ../ci/concurrency/examples/systick.rs}}``` | |
| {{#include ../ci/concurrency/examples/systick.rs}} | |
| ``` |
|
|
||
| ``` rust | ||
| #[link_name = "SysTick"] // places this function in the vector table | ||
| fn randomly_generated_identifier() { |
| @@ -0,0 +1,65 @@ | |||
| // source: examples/init.rs | |||
|
|
|||
| #![feature(maybe_uninit)] | |||
| // TODO does T require a Sync / Send bound? | ||
| unsafe impl<T> Sync for Mutex<T> {} |
There was a problem hiding this comment.
| // TODO does T require a Sync / Send bound? | |
| unsafe impl<T> Sync for Mutex<T> {} | |
| unsafe impl<T: Send> Sync for Mutex<T> {} |
| contexts* that may preempt each other. | ||
|
|
||
| - `Send`: types that can be transferred between *execution contexts* that may | ||
| preempt each other. |
There was a problem hiding this comment.
It might make sense to update this wrt. the RFC to restrict Send/Sync reasoning to single cores
| The problem with `Mutex` is not the critical section that uses; it's the fact | ||
| that it can be stored in a `static` variable making accessible to all cores. | ||
| Thus in multi-core context the `Mutex` abstraction should not implement the | ||
| `Sync` trait. |
There was a problem hiding this comment.
Oh yeah this is outdated with the RFC now
|
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 |
|
|
||
| #[exception] | ||
| fn SysTick() { | ||
| X.store(true, Ordering::Relaxed); |
There was a problem hiding this comment.
Using relaxed ordering is probably fine for this toy example since there's no other important memory accesses that could be reordered across the load or store, but if someone copies this example and starts making the wrong assumptions, it would be quite easy for their code to become unsound/incorrect. It might be better to default to sequentially consistent atomics for the examples.
This PR adds a chapter that documents the most common (memory) safe uses of
unsafecode in concurrent bare metal (no_std) programs. The patterns described here are used to implement the safe concurrency abstractions provided by thecortex-m,cortex-m-rtandcortex-m-rtfmcrates.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