Convert Zephyr primitives to a better initialization and use framework #57
Conversation
This file contains hidden or 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
d3zd3z
force-pushed
the
atomic-objects
branch
2 times, most recently
from
b86831b to
29db006
Compare
Implement a new wrapping system for Zephyr Objects. Instead of trying to either use a static or a pinned-box, use an atomic to detect initialization, as well as improper moves. This allows for objects to have `const` constructors, allowing them to be declared as simple statics and just shared. Subsequent patches will implement this for various primitives. Signed-off-by: David Brown <[email protected]>
Move Semaphores to the new `ZephyrObject` system, allowing the constructor to be const. Unfortunately, `Result` isn't generally useful with const, and as such, this changes the API for `Semaphore::new()` to just return the semaphore instead of a Result. Instead, if a semaphore constructor has inconsitent arguments, it will panic. Given that semaphore constructors are generally quite simple, this is a minor issue. However, it will involve removing a lot of `.unwrap()` or `.expect(...)` calls on semaphores. The samples have been changed for the API change, but mostly not changed to take advantage of the easier ways to use them. It is generally no longer necessary to put semaphores in an Arc or Rc, but just to allocate them in something like a StaticCell, and then pass them around by reference. Signed-off-by: David Brown <[email protected]>
Convert to the new atomic initializer format. This makes the `new()` constructor `const` allowing queues to be used as statics. Similar to the API change to Semaphore, the constructor no longer returns a Result, just the Queue directly. Signed-off-by: David Brown <[email protected]>
Adapt to the API change in both Semaphore and Queue. This still allocates using Arc, though. Signed-off-by: David Brown <[email protected]>
Update the sys Mutex/Condvar to use the new object initialization, and update the uses. This requires a few fixes: - The SimpleTls no longer needs the StaticTls helper, and we can just use a plain global static Mutex. - Bounded channels add items to a free queue during init. To avoid moving this queue after items have been inserted, we place it in a box. Given that the data of bounded queues is also boxed, this isn't really a concern. Signed-off-by: David Brown <[email protected]>
This was an experiment to see if using an atomic to manage semaphore initialization was workable. Now that the main Semaphore implementation works this way, this can just be removed. Signed-off-by: David Brown <[email protected]>
Drop the iterations so not as much time is spent in CI. This still gives meaning functional results for CI, and the increased value can be used for actual benchmarking. Signed-off-by: David Brown <[email protected]>
Although there is still a lot of allocation left (the crossbeam API for channels is kind of built around it), convert the Semaphores in the benchmark to be static. This reduces overhead a little bit, because the reference counts of Arc don't need to be maintained, and also demonstrates how StaticCell can be used to allocate things like arrays of objects statically. Signed-off-by: David Brown <[email protected]>
Clean up formatting Signed-off-by: David Brown <[email protected]>
Run rustfmt to clean up formatting of the code. Signed-off-by: David Brown <[email protected]>
With Mutex having a const constructor, simplify the initializaion, and use a simple static for the Stats container. Signed-off-by: David Brown <[email protected]>
cfriedt
approved these changes
Mar 9, 2025
teburd
approved these changes
Mar 12, 2025
|
I wouldn't be super scared of Pin, but really most IPCs in Zephyr really do want to be static I'd think. Otherwise there's going to be lots of issues trying to track lifetimes of what lives longer than what else. Particularly because Zephyr loves callbacks and there's not a great way I don't think to inform Rust something must live longer than the callback. I could be wrong though! |
I think it is quite reasonable to have various Zephyr APIs that use things like callbacks and such to accept |
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.
The existing object wrappers for Zephyr objects centers around a "Fixed" type, which is a enum, containing either a static reference, or a
Pin<Box<T>>. This means that anything not declared with thekernel_objmacros will require allocation.We can get rid of the need for allocation and also improve performance (very slightly). Instead of this enum which then hides another reference, a ZephyrObject is just a regular struct, with an atomic and an UnsafeCell holding the real Zephyr object. The atomic is used to allow the initialization to happen on first use instead of at constructor time. It also detects if one of these objects has been moved (resulting in a panic), because requiring the use of Pin on these types would make this mechanism even less hygienic than the
kernel_objmacros.The new objects have const constructors, which allows them to be plainly declared as statics, or used in regular code, without requiring an alloc to happen under the hood. The only constraint is that the objects cannot be moved after their first use. For embedded, this is generally not difficult, as the objects will usually be static, put in something like a StaticCell, or allocated in an Rc or Arc.
There is, unfortunately, a bit of a minor API change. Semaphore's currently return a Result, and the Result type is not particularly usable in const.
Semaphore::newnow directly returns the Semaphore, and if the constructor values are nonsensical (either a zero limit, or an initial value larger than the limit), it will panic instead of returning an error.