Implement more ergonomic CPU-local storage #1012
Merged
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
Signed-off-by: Klimenty Tsoutsman <[email protected]>
Signed-off-by: Klimenty Tsoutsman <[email protected]>
Signed-off-by: Klimenty Tsoutsman <[email protected]>
tsoutsman
commented
Jul 23, 2023
Signed-off-by: Klimenty Tsoutsman <[email protected]>
Signed-off-by: Klimenty Tsoutsman <[email protected]>
Signed-off-by: Klimenty Tsoutsman <[email protected]>
kevinaboos
requested changes
Jul 26, 2023
Signed-off-by: Klimenty Tsoutsman <[email protected]>
kevinaboos
requested changes
Jul 27, 2023
Signed-off-by: Klimenty Tsoutsman <[email protected]>
Signed-off-by: Klimenty Tsoutsman <[email protected]>
Signed-off-by: Klimenty Tsoutsman <[email protected]>
Signed-off-by: Klimenty Tsoutsman <[email protected]>
kevinaboos
approved these changes
Jul 27, 2023
| use crate::cpu_local::{CpuLocal, CpuLocalField, PerCpuField}; | ||
| use no_drop::NoDrop; | ||
| #![no_std] | ||
| #![feature(negative_impls, thread_local)] |
There was a problem hiding this comment.
note for next PR: i think we can avoid the need for every user of cls_macro to declare #![feature(thread_local)] by using the #[allow_internal_unstable(...)] attribute in the cls_macro crate, either on the proc macro definition or as a crate-level attribute. I recall using that attribute in the thread_local_macro crate, based on std's thread_local!() macro trickery.
This isn't a blocker to merging/approving this PR, but we should try to improve that rough edge, especially once we have dozens of CPU local vars everywhere.
There was a problem hiding this comment.
(i just realized this is intended for regular macros, not procedural macros, but you should try it anyway)
github-actions bot
pushed a commit
that referenced
this pull request
Jul 27, 2023
* The new `cls` crate offers a `#[cpu_local]` attribute that can be applied
to a static variable to declare it as Cpu-Local Storage (CLS),
much like `#[thread_local]` and Thread-Local Storage (TLS).
* The `cls_macro` crate provides a procedural macro that implements
the `#[cpu_local]` attribute. Currently, this only supports basic
primitive types: `u8`, `u16`, `u32`, `u64`. More coming later.
* The API for accessing CPU-local primitive types is quite simple:
only `load`, `fetch_add`, `fetch_sub`. Note that these functions are
implemented as atomic with respect to ONLY the single current CPU,
not atomic with respect to all memory accesses across all other CPUs.
This is intentional as it is more efficient, and CPU-local variables are
only accessible to a single CPU and not by another "foreign" CPU.
* Currently this is only used for the preemption counter on each CPU,
but it will be used for more variables in future PRs.
* This simplifies the implementation of `preemption` management,
as we can rely on the `#[cpu_local]` API's semantics to be atomic
with respect to the current CPU.
* This also avoids a minor latent bug that existed in the old preemption
implementation which made it theoretically possible for a task migration
to occur between accessing the CPU-local preemption count and
modifying that count. This wasn't a problem since Theseus does not yet
perform task migration, but the new design makes this interleaving
impossible and is thus future-proof for later task migration support.
* Re-separate `preemption` and `cpu_local` into two different crates,
as they previously were. This is possible because the preemption count
now uses the new `#[cpu_local]` attribute, which allows `preemption`
and `cpu_local` to not have cyclic dependencies on each other.
* This is responsible for most of the changes in this PR: changing
`cpu_local_preemption` back to `cpu_local` or `preemption`.
* Another current limitation is that we must use the `#[cpu_local]` attribute
to _manually_ specify the offset of each CPU-local variable from the start
of the `PerCpuData` storage type. This will be removed in a future PR.
Signed-off-by: Klimenty Tsoutsman <[email protected]>
Co-authored-by: Kevin Boos <[email protected]> 1f620ac
github-actions bot
pushed a commit
to tsoutsman/Theseus
that referenced
this pull request
Jul 27, 2023
…us-os#1012) * The new `cls` crate offers a `#[cpu_local]` attribute that can be applied to a static variable to declare it as Cpu-Local Storage (CLS), much like `#[thread_local]` and Thread-Local Storage (TLS). * The `cls_macro` crate provides a procedural macro that implements the `#[cpu_local]` attribute. Currently, this only supports basic primitive types: `u8`, `u16`, `u32`, `u64`. More coming later. * The API for accessing CPU-local primitive types is quite simple: only `load`, `fetch_add`, `fetch_sub`. Note that these functions are implemented as atomic with respect to ONLY the single current CPU, not atomic with respect to all memory accesses across all other CPUs. This is intentional as it is more efficient, and CPU-local variables are only accessible to a single CPU and not by another "foreign" CPU. * Currently this is only used for the preemption counter on each CPU, but it will be used for more variables in future PRs. * This simplifies the implementation of `preemption` management, as we can rely on the `#[cpu_local]` API's semantics to be atomic with respect to the current CPU. * This also avoids a minor latent bug that existed in the old preemption implementation which made it theoretically possible for a task migration to occur between accessing the CPU-local preemption count and modifying that count. This wasn't a problem since Theseus does not yet perform task migration, but the new design makes this interleaving impossible and is thus future-proof for later task migration support. * Re-separate `preemption` and `cpu_local` into two different crates, as they previously were. This is possible because the preemption count now uses the new `#[cpu_local]` attribute, which allows `preemption` and `cpu_local` to not have cyclic dependencies on each other. * This is responsible for most of the changes in this PR: changing `cpu_local_preemption` back to `cpu_local` or `preemption`. * Another current limitation is that we must use the `#[cpu_local]` attribute to _manually_ specify the offset of each CPU-local variable from the start of the `PerCpuData` storage type. This will be removed in a future PR. Signed-off-by: Klimenty Tsoutsman <[email protected]> Co-authored-by: Kevin Boos <[email protected]> 1f620ac
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.
MVP of a
cpu_localattribute.Limitations that will be addressed in a future PR:
Draft PR because the macro needs to be implemented foraarch64.