Add no_std implementation based on critical-section.
#195
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
reitermarkus
force-pushed
the
critical-section
branch
from
771a98b to
3435d24
Compare
matklad
reviewed
Sep 6, 2022
There was a problem hiding this comment.
🤔 so, historically I've been pretty opposed to "just works" implementation on no_std, as how exactly one does locking is pretty important, and spinlocks is decidedly not a way to go, there's some discussion in #61.
The approach roughly like this could work though:
- if we use a critical section rather than spinning, that counts as proper blocking in at least some contexts
- the experience is that the user has to explicitly enable the
critical_sectionfeature, so they are opting into this behavior
So, yeah, I think I am generally on board with it. However, it is important that we don't actually spin here (the current implementation does spin).
|
Okay, so I have a different implementation which doesn't spin and simply completely blocks all threads during |
reitermarkus
force-pushed
the
critical-section
branch
2 times, most recently
from
89a5da8 to
bdb7107
Compare
reitermarkus
changed the title
no_std implementation based on critical-section/atomic-polyfill.no_std implementation based on critical-section.
reitermarkus
force-pushed
the
critical-section
branch
3 times, most recently
from
ff0a4ba to
42e9c79
Compare
|
@matklad, can you have another look? |
matklad
reviewed
Sep 12, 2022
matklad
reviewed
Sep 16, 2022
reitermarkus
force-pushed
the
critical-section
branch
3 times, most recently
from
ced0703 to
124c01d
Compare
|
@matklad, can you have another look at this? Thanks. |
…res are enabled.
reitermarkus
force-pushed
the
critical-section
branch
from
589c61e to
7d9afdb
Compare
|
lgtm! bors r+ |
|
Build succeeded: |
crapStone
pushed a commit
to Calciumdibromid/CaBr2
that referenced
this pull request
Nov 1, 2022
This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [once_cell](https://github.com/matklad/once_cell) | dependencies | minor | `1.15.0` -> `1.16.0` | --- ### Release Notes <details> <summary>matklad/once_cell</summary> ### [`v1.16.0`](https://github.com/matklad/once_cell/blob/HEAD/CHANGELOG.md#​1160) [Compare Source](matklad/once_cell@v1.15.0...v1.16.0) - Add `no_std` implementation based on `critical-section`, [#​195](matklad/once_cell#195). - Deprecate `atomic-polyfill` feature (use the new `critical-section` instead) </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNC44LjAiLCJ1cGRhdGVkSW5WZXIiOiIzNC45LjEifQ==--> Co-authored-by: cabr2-bot <[email protected]> Reviewed-on: https://codeberg.org/Calciumdibromid/CaBr2/pulls/1615 Reviewed-by: crapStone <[email protected]> Co-authored-by: Calciumdibromid Bot <[email protected]> Co-committed-by: Calciumdibromid Bot <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add implementation based on
critical-sectionfor embedded targets.This started out using
critical_section::Mutex, but tests were failing sincecritical_section::withblocks all threads during initialization, so I changed it to be basically the same asimp_plwithout theparking_lotparts, sinceatomic-polyfillis based oncritical-section.Depends on rust-embedded/critical-section#26.