Implement basic per-CPU / CPU-local storage

[kevinaboos]

• Not yet used anywhere
• Only supported on x86_64

Reminder: remove temp tests and log statements before merging

[tsoutsman]

I've proposed a slightly different API that would decrease the amount of unsafe syncing we have to do between PerCpuData and FixedCpuLocal.

Just to make sure we're on the same page gs is a register that isn't used in regular code and so we can use it for a different purpose right?

[kevinaboos]

Just to make sure we're on the same page gs is a register that isn't used in regular code and so we can use it for a different purpose right?

Correct, the GS segment register is unused and is frequently used for CPU-local storage on x86. It's similar to how the FS segment register is used for thread-local storage.

[kevinaboos]

@tsoutsman I attempted a few different ways to use traits in the CpuLocal type, with the goal being to make the RAII declaration of a CPU-local variable as simple as possible. It's pretty hard to make the type signature simple, but I was able to get it down to a single generic type parameter (which itself has one type parameter). I didn't push it to this branch because I don't love it, but take a look here: 3b24b1d. At the very least, it removes/hides the const OFFSET parameter from CpuLocal, which I do think was a bit of a flawed/leaky design. This design lets the caller do something like:

static PREEMPTION_COUNT: CpuLocal<CpuLocalPreemptionCount<PreemptionCount>> = unsafe { CpuLocal::new() };

---

The only other option I was able to get to work was to parameterize the CpuLocalVariable trait with a type, like so:

pub trait CpuLocalVariable<T: Sized> { ... }

which works, but you then need to have both T and V parameters for CpuLocal:

pub struct CpuLocal<T: Sized, V: CpuLocalVariable<T>> {
    _typ: PhantomData<T>,
    _var: PhantomData<V<T>>,
}

impl<T: Sized, V: CpuLocalVariable<T>> CpuLocal<T, V> {
    pub const unsafe fn new(_variable_marker: V) -> Self {
        Self { _typ: PhantomData, _var: PhantomData }
    }

This is clearer but ergonomically worse, as you have to declare/instantiate it like so:

static PREEMPTION_COUNT: CpuLocal<PreemptionCount, CpuLocalVariable<PreemptionCount>> = unsafe { CpuLocal::new(CpuLocalPreemptionCount) };

---

If you have any ideas on how to further simplify this, please do let me know.

[tsoutsman]

Apologies force push was because I pushed to the wrong remote repo.

Here's a quick enum implementation I made:
b0ce230

It removes size and alignment from cpu_local, removes OFFSET const parameter, prevents CpuLocal from being instantiated with arbitrary offsets and types. The one bad thing is that per_cpu now needs to define a bunch of wrapper types so that it can implement Field for them.

[tsoutsman]

right but that introduces a circular dependency hmm

we could have the Field impls in the crated where the type is defined e.g. cpu for CpuId

[kevinaboos]

Thanks, that enum impl is similar to what I tried, but the addition of the unsafe trait with its associated const does help. I was trying to avoid an unsafe trait -- i wonder if there's a clever way to prevent other crates from implementing the Field trait.

The one bad thing is that per_cpu now needs to define a bunch of wrapper types so that it can implement Field for them.

This is fine, but we should impl Deref+DerefMut on those wrappers so the crates that use cpu_local don't have to be aware of said newtype wrappers.

we could have the Field impls in the crates where the type is defined, e.g. cpu for CpuId

Yeah, i don't really have any other ideas for workarounds here, so I think this is fine. But we should expose it via a macro like define_cpu_local_type!(<type>) such that it's not easy/recommended to directly impl Field. This ties into trying to prevent someone from implementing Field, but I do realize that macros wouldn't prevent that.

I suppose you could also define a proc_macro that would implement hidden traits for a cpu-local type, but that might be too complex right now. My future idea for TLS-style CPU locals incorporates proc_macros for custom attributes, but we can discuss that later.

---

Finally, you left this comment, which I agree with, but yes that's one of the many reasons why CpuLocal::new* was unsafe -- the type provided had to match the offset.

// NOTE: The fact that this previously compiled in cpu_local is
// indicative of an unsafe API, as the offsets (and types) are
// completely arbitrary. There's nothing stopping us from trying to
// access CpuLocal::<16, String> which would be undefined behaviour.

[kevinaboos]

Here's a quick enum implementation I made: b0ce230

It removes size and alignment from cpu_local, removes OFFSET const parameter, prevents CpuLocal from being instantiated with arbitrary offsets and types. The one bad thing is that per_cpu now needs to define a bunch of wrapper types so that it can implement Field for them.

Thanks, I have merged/included that commit into this PR branch, and am going from there.

we could have the Field impls in the crated where the type is defined e.g. cpu for CpuId

Working on this now. I've also addressed some naming issues.

---

Will re-request a review from you when done.

[kevinaboos]

Made the changes, thanks for the suggestion. Though not included in this changeset, I confirmed it's possible to instantiate a CPU-local task type, i.e., this works:

static TASK_SWITCH_PREEMPTION_GUARD: cpu_local::CpuLocal<TaskSwitchPreemptionGuard> = cpu_local::CpuLocal::new(PerCpuField::TaskSwitchPreemptionGuard);
pub fn test_me() -> Option<PreemptionGuard> {
    TASK_SWITCH_PREEMPTION_GUARD.with_mut(|t| t.0.take())
}

see my latest commit msg for other details

[tsoutsman]

One small doc change.