-
Notifications
You must be signed in to change notification settings - Fork 31
Add portable-atomic support for spin-based locking on no-atomic targets #43
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
base: master
Are you sure you want to change the base?
Conversation
This allows buddy_system_allocator to run on targets without native atomic support, such as RV32IM platforms without the A extension, while keeping the default behavior unchanged.
| default = ["alloc", "use_spin"] | ||
| alloc = [] | ||
| use_spin = ["spin"] | ||
| soft_atomic_spin_single_core = ["spin/portable-atomic", "portable-atomic/unsafe-assume-single-core"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we really need this: By https://doc.rust-lang.org/cargo/reference/features.html#feature-unification, you can enable spin/portable-atomic in a downstream project if necessary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The real feature wanted here is to use spin without spin_mutex?
|
spin_mutex used core::atomic by default. If you want to use 'LockedHeap', and there is no hardware support for atomic instructions, such as when the target is 'rv32im", it will become unavailable.At this point, you need to enable the portable-atomic software shim for support.The features are separated for convenience of combination, so that configuration can be easily done in the dependency section instead of leaving the configuration task to downstream tasks. However, it's not a big problem.Currently, one of my template projects is being developed based on the modified fork.
|
When you need to specify the additional feature for |
|
okay,thanks.
However, if there are projects that depend on this project for development, and then new projects depend on those projects that in turn depend on this project, it can easily lead to configuration hell if we do not expose the underlying features of spin to allow complete configuration at a certain layer, but instead push all configuration tasks to the top-level project. If users then discover that dependency package A relies on feature1 of the underlying package BASE1, while dependency B relies on another version or a different feature of the underlying package BASE1, the only available strategy would be to fork the original project and write an additional wrapper layer. By exposing these features, such situations can be better avoided at a certain abstraction layer.
If all configuration is pushed to the top level, then the initial Cargo.toml of a large project will be anything but elegant.
|
Please give a concrete example where your approach is cleaner that the existing one. |
|
Since my goal is a simple RV32IM board, I need to use an allocator to initialize the heap. However, I don’t want to develop it myself (QwQ—never mind for now). Subsequent projects based on this board will have a simple template (see: github cargo-ecos/template/c1), and later there will also be UI adaptation layers, etc. If every template contains a patch during initialization, it will be... inelegant, and the project will no longer be transparent to users.
|
You can specify the portable-atomic feature in the board support crate. |
|
Thank you. I'll check the manual later. It seems that during previous testing, it was only effective when configured at the top level, so this PR might not be necessary. Thanks and bye.
Message ID: ***@***.***>
|
eg. rv32im