Add implementation for critical-section 1.0 #447

Dirbaio · 2022-08-10T16:11:37Z

Picking up #433 since it seems stalled. Changes from #433 are:

Update to critical-section 1.0.0-alpha.2
Use bool restore token
Name Cargo feature critical-section-single-core.

TODO before merging:

Wait for critical-section 1.0 release v1.0.0 release critical-section#19

Co-Authored-By: Markus Reiter @reitermarkus

rust-highfive · 2022-08-10T16:11:40Z

r? @adamgreig

(rust-highfive has picked a reviewer for you, use r? to override)

.github/workflows/clippy.yml

src/critical_section.rs

reitermarkus · 2022-08-11T00:28:40Z

Picking up #433 since it seems stalled.

It's not stalled. I was just waiting for another review, same as my other open PRs. Feel free to ping me next time if you want me to rebase a PR.

Thanks anyways for picking this up.

The workflow pull_request change was meant to be temporary so CI can run on my fork.

adamgreig · 2022-08-11T01:20:05Z

src/interrupt.rs

@@ -54,20 +53,18 @@ pub unsafe fn enable() {
 }

 /// Execute closure `f` in an interrupt-free context.


Perhaps this method should now refer to critical-section, something like this:

Suggested change

/// Execute closure `f` in an interrupt-free context.

/// Execute closure `f` in an interrupt-free context.

///

/// This method does not synchronise multiple cores and may disable required

/// interrupts on some platforms; see the critical-section crate for a cross-platform

/// way to enter a critical section which provides a `CriticalSection` token.

///

/// This crate provides an implementation for critical-section that simply disables

/// all interrupts which may be enabled with the `critical-section-single-core` feature.

Co-Authored-By: Markus Reiter <me@reitermark.us>

448: Add implementation for critical-section 1.0, for cortex-m v0.7.x r=adamgreig a=Dirbaio This is a subset of #447 without any breaking changes, just adding the `critical-section` implementation. The goal is to release it in 0.7.6 so `critical-section` becomes usable without having to wait for cortex-m 0.8. TODO before merging: - [x] Wait for `critical-section 1.0` release rust-embedded/critical-section#19 Co-authored-by: Dario Nieuwenhuis <dirbaio@dirbaio.net>

adamgreig

bors r+

447: Add implementation for critical-section 1.0 r=adamgreig a=Dirbaio Picking up #433 since it seems stalled. Changes from #433 are: - Update to `critical-section 1.0.0-alpha.2` - Use `bool` restore token - Name Cargo feature `critical-section-single-core`. TODO before merging: - [x] Wait for `critical-section 1.0` release rust-embedded/critical-section#19 Co-Authored-By: Markus Reiter `@reitermarkus` Co-authored-by: Dario Nieuwenhuis <dirbaio@dirbaio.net>

bors · 2022-08-12T00:01:42Z

Build failed:

rt-ci-linux (stable)

adamgreig · 2022-08-12T00:32:20Z

bors retry

should go after #450 merges...

bors · 2022-08-12T00:40:22Z

Build succeeded:

651: Use `critical_section` for `Peripherals::take`. r=therealprof a=reitermarkus - `cortex_m` rust-embedded/cortex-m#447 - `msp430` rust-embedded/msp430#13 - `riscv` rust-embedded/riscv#110 - `xtensa_lx` esp-rs/xtensa-lx#20, esp-rs/esp-hal#151 - `mips_mcu` kiffie/pic32-rs#5 Co-authored-by: Markus Reiter <me@reitermark.us>

689: defmt-rtt: Update to critical-section 1.0 r=Urhengulas a=Dirbaio This is a breaking change, because 1.0 no longer supplies any critical section implementation by default. The user has to opt-in to one instead (for example, by enabling the `critical-section-single-core` feature in `cortex-m`: rust-embedded/cortex-m#447). Thankfully it needs bumping only `defmt-rtt`, and not `defmt`. So it won't cause the painful ecosystem split like the `defmt 0.2 -> 0.3` update. TODO before merging: - [x] Wait for `critical-section 1.0` release rust-embedded/critical-section#19 Co-authored-by: Dario Nieuwenhuis <dirbaio@dirbaio.net>

9: Update to critical-section 1.0 r=jonas-schievink a=Dirbaio This is a breaking change, because 1.0 no longer supplies any critical section implementation by default. The user has to opt-in to one instead (for example, by enabling the `critical-section-single-core` feature in `cortex-m`: rust-embedded/cortex-m#447 (comment)). TODO before merging: - [x] Wait for `critical-section 1.0` release rust-embedded/critical-section#19 Co-authored-by: Dario Nieuwenhuis <dirbaio@dirbaio.net>

110: Add critical-section 1.0 implementation, fix multicore unsoundness. r=almindor a=Dirbaio ~~Requires #109~~ This adds a [critical-section](https://github.com/rust-embedded/critical-section) implementation for single-core chips, based on disabling all interrupts. `interrupt::free` is is unsound on multicore systems because it only disables interrupts in the current core. For multicore chips, a chip-specific critical section implementationis needed instead. Unsoundness is fixed by not returning the `CriticalSection` token. This is a breaking change. This is the riscv equivalent of rust-embedded/cortex-m#447 and rust-embedded/cortex-m#448 Co-authored-by: Dario Nieuwenhuis <dirbaio@dirbaio.net>

Dirbaio requested a review from a team as a code owner August 10, 2022 16:11

rust-highfive assigned adamgreig Aug 10, 2022

rust-highfive added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-cortex-m labels Aug 10, 2022

Dirbaio mentioned this pull request Aug 10, 2022

Add implementation for critical-section 1.0, for cortex-m v0.7.x #448

Merged

1 task

Dirbaio force-pushed the cs branch from 67034bf to 4453ebb Compare August 10, 2022 17:04

Dirbaio changed the title ~~Add implementation for crticial-section 1.0~~ Add implementation for critical-section 1.0 Aug 10, 2022

This was referenced Aug 10, 2022

defmt-rtt: Update to critical-section 1.0 knurling-rs/defmt#689

Merged

Update to critical-section 1.0 nrf-rs/nrf-usbd#9

Merged

adamgreig reviewed Aug 10, 2022

View reviewed changes

.github/workflows/clippy.yml Outdated Show resolved Hide resolved

adamgreig reviewed Aug 10, 2022

View reviewed changes

src/critical_section.rs Outdated Show resolved Hide resolved

Dirbaio force-pushed the cs branch from 4453ebb to f22c650 Compare August 10, 2022 23:49

adamgreig reviewed Aug 11, 2022

View reviewed changes

Dirbaio force-pushed the cs branch from f22c650 to 82c170a Compare August 11, 2022 12:13

Add implementation for critical-section 1.0

3a15a6b

Co-Authored-By: Markus Reiter <me@reitermark.us>

Dirbaio force-pushed the cs branch from 82c170a to 3a15a6b Compare August 11, 2022 21:31

adamgreig approved these changes Aug 11, 2022

View reviewed changes

bors bot merged commit 0e53054 into rust-embedded:master Aug 12, 2022

This was referenced Aug 21, 2022

Fix interrupt::free() unsoundness on multicore systems. rust-embedded/riscv#111

Closed

Add critical-section 1.0 implementation, fix multicore unsoundness. rust-embedded/riscv#110

Merged

reitermarkus mentioned this pull request Aug 24, 2022

Use critical_section for Peripherals::take. rust-embedded/svd2rust#651

Merged

reitermarkus mentioned this pull request Dec 4, 2023

Fix interrupt::free. #433

Closed

urdh mentioned this pull request Dec 3, 2024

Consider using critical_section for the init-context CriticalSection token rtic-rs/rtic#1002

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add implementation for critical-section 1.0 #447

Add implementation for critical-section 1.0 #447

Uh oh!

Dirbaio commented Aug 10, 2022 •

edited

Loading

Uh oh!

rust-highfive commented Aug 10, 2022

Uh oh!

Uh oh!

Uh oh!

reitermarkus commented Aug 11, 2022

Uh oh!

adamgreig Aug 11, 2022

Uh oh!

Dirbaio Aug 11, 2022

Uh oh!

adamgreig left a comment

Uh oh!

bors bot commented Aug 12, 2022

Uh oh!

adamgreig commented Aug 12, 2022

Uh oh!

bors bot commented Aug 12, 2022

Uh oh!

Uh oh!

		@@ -54,20 +53,18 @@ pub unsafe fn enable() {
		}

		/// Execute closure `f` in an interrupt-free context.

-/// Execute closure `f` in an interrupt-free context.
+/// Execute closure `f` in an interrupt-free context.
+///
+/// This method does not synchronise multiple cores and may disable required
+/// interrupts on some platforms; see the critical-section crate for a cross-platform
+/// way to enter a critical section which provides a `CriticalSection` token.
+///
+/// This crate provides an implementation for critical-section that simply disables
+/// all interrupts which may be enabled with the `critical-section-single-core` feature.

Add implementation for critical-section 1.0 #447

Add implementation for critical-section 1.0 #447

Uh oh!

Conversation

Dirbaio commented Aug 10, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rust-highfive commented Aug 10, 2022

Uh oh!

Uh oh!

Uh oh!

reitermarkus commented Aug 11, 2022

Uh oh!

adamgreig Aug 11, 2022

Choose a reason for hiding this comment

Uh oh!

Dirbaio Aug 11, 2022

Choose a reason for hiding this comment

Uh oh!

adamgreig left a comment

Choose a reason for hiding this comment

Uh oh!

bors bot commented Aug 12, 2022

Uh oh!

adamgreig commented Aug 12, 2022

Uh oh!

bors bot commented Aug 12, 2022

Uh oh!

Uh oh!

Dirbaio commented Aug 10, 2022 •

edited

Loading