Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

First draft at concurrency chapter #49

Merged
merged 3 commits into from
Oct 17, 2018
Merged

First draft at concurrency chapter #49

merged 3 commits into from
Oct 17, 2018

Conversation

adamgreig
Copy link
Member

I've mostly focused on building up to safely sharing peripherals, without very much focus on RTFM or higher level things like an RTOS, since I don't think there's very much we can usefully say about them at the moment. Very happy to be told I'm wrong on that front though!

Closes #7.

r? @rust-embedded/resources

@adamgreig adamgreig requested a review from a team as a code owner October 16, 2018 00:56
@adamgreig adamgreig added T-resources S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 16, 2018
@andre-richter
Copy link
Member

This is high quality content. Hats off!

I am wondering about one point: The mutex-inside-critical-section example confuses me a bit. In a single-core scenario, it doesn't make much sense, because a critical section is already enough to keep the only contender for the shared data away? In a multi-core example, we could still get weird behavior if one core currently holds the lock when the other core in an ISR tries to lock it. Maybe I am missing something here?

@adamgreig
Copy link
Member Author

Thanks!

In the single-core case, the mutex is really only ensuring you are in fact inside a critical section - it doesn't have its own state or concept of being locked or any other overhead. Really all it's doing is only letting you borrow its contents if you can give it a CriticalSection token. We need it rather than just using the shared data directly only because the shared data would not be Sync (e.g. a Cell or RefCell) but the Mutex<Cell> is Sync, so can be placed in statics.

In a multi-core scenario the cortex_m::interrupt::Mutex is unsound as far as I can see...

@andre-richter
Copy link
Member

I was just asking because the introduction speaks about multi-core. Would it help to set the scope for each of the examples (single or multi or both) so that the reader can differentiate?

@adamgreig
Copy link
Member Author

Good idea, I'll add a note that the cortex_m::interrupt::Mutex (and in general relying on critical sections for exclusive access) only applies on single core systems.

@adamgreig
Copy link
Member Author

@andre-richter I've added some notes about multiple cores. Good observation: it's lead to needing to also note that cortex_m::interrupt::Mutex is not actually safe on multi-core systems!

andre-richter
andre-richter previously approved these changes Oct 16, 2018
Copy link
Member

@andre-richter andre-richter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome. This is an excellent read now.

therealprof
therealprof previously approved these changes Oct 16, 2018
@andre-richter
Copy link
Member

Shall we bors this @adamgreig ?

@adamgreig
Copy link
Member Author

I've just had a typo pointed out so give me a moment to fix that...

@adamgreig
Copy link
Member Author

Okay, fixed, if you're happy then go ahead and bors. Thanks!

Copy link
Member

@andre-richter andre-richter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bors r+

bors bot added a commit that referenced this pull request Oct 17, 2018
49: First draft at concurrency chapter r=andre-richter a=adamgreig

I've mostly focused on building up to safely sharing peripherals, without very much focus on RTFM or higher level things like an RTOS, since I don't think there's very much we can usefully say about them at the moment. Very happy to be told I'm wrong on that front though!

Closes #7.

r? @rust-embedded/resources 

Co-authored-by: Adam Greig <adam@adamgreig.com>
@bors
Copy link
Contributor

bors bot commented Oct 17, 2018

Build succeeded

@bors bors bot merged commit 9d3594b into master Oct 17, 2018
@bors bors bot deleted the concurrency branch October 17, 2018 19:31
njmartin10 pushed a commit to njmartin10/book that referenced this pull request Nov 10, 2018
49: First draft at concurrency chapter r=andre-richter a=adamgreig

I've mostly focused on building up to safely sharing peripherals, without very much focus on RTFM or higher level things like an RTOS, since I don't think there's very much we can usefully say about them at the moment. Very happy to be told I'm wrong on that front though!

Closes rust-embedded#7.

r? @rust-embedded/resources 

Co-authored-by: Adam Greig <adam@adamgreig.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-resources
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants