Add priority-limited locks #2684
Open
+480
−105
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
bugadani
force-pushed
the
prio
branch
2 times, most recently
from
4989831 to
8f2075a
Compare
|
I haven't taken the time to see if it matters for this yet but just in case you figure it out before I do: Given priorities are being treated more granular-ly than just a binary on/off thing now, it might a good time to improve the priority restoration instruction here. That should be restoring back to the original level rather than just zero. |
|
Oops I accidentally reverted that change... It's actually needed otherwise that restoration can release a lock by accident. |
bugadani
force-pushed
the
prio
branch
6 times, most recently
from
9a373b5 to
ed34f6c
Compare
This comment was marked as resolved.
This comment was marked as resolved.
bugadani
force-pushed
the
prio
branch
5 times, most recently
from
269d71a to
bae305e
Compare
bjoernQ
reviewed
Dec 10, 2024
|
Running |
|
I see a similar enough crash on main, I don't think this is the PR's fault: |
interesting - I haven't seen crashes on main but after a clean build it seems it's fine on this branch - so might be a red herring |
Dominaezzz
approved these changes
Dec 13, 2024
Comment on lines
+17
to
+18
| unsafe fn enter(&self) -> critical_section::RawRestoreState; | ||
| unsafe fn exit(&self, token: critical_section::RawRestoreState); |
There was a problem hiding this comment.
At some point critical_section should be treated like embedded_hal, where it's a convenient add-on, rather that part of the inherent APIs of the hal.
There was a problem hiding this comment.
Is this comment about the use of critical_section::RawRestoreState as a return value?
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.
However, when it does, we can use it in embassy-executor to lock certain resources without affecting higher priority levels. This will come in handy once we manage our own timer queue, which is only ever accessed from the executor's priority level, and the alarm's level (which we'll be able to reduce to executor+1). Priority-limiting access to the queue will result in less jitter in higher priority executors. Hopefully :)
skip-changelog is purposeful.
syncis a hidden module, which needs to be public because embassy/wifi build on it, but this new addition is pretty experimental - API, documentation or functionality may still change I think.