rust: Add Per-CPU Variable API

lib/percpu_test_rust.rs line 25

Quoted 2 lines · Mitchell Levy

lib/percpu_test_rust.rs new file +2 -0

+        let mut native: i64 = 0;
+        let mut pcpu: PerCpuRef<i64> = unsafe { unsafe_get_per_cpu_ref!(PERCPU, CpuGuard::new()) };

A bit complex.

lib/percpu_test_rust.rs line 34

Quoted 8 lines

lib/percpu_test_rust.rs new file +8 -0

+        native += -1;
+        *pcpu += -1;
+        assert!(native == *pcpu && native == -1);
+
+        native += 1;
+        *pcpu += 1;
+        assert!(native == *pcpu && native == 0);
+

That's pretty straightforward..... But is there no symbolic access to the per cpu namespace? How would you access the kernel per cpu variables defined in C?

How do you go about using per cpu atomics like

this_cpu_inc(nr_dentry_unused);

lib/percpu_test_rust.rs line 25

Quoted 2 lines · Mitchell Levy

lib/percpu_test_rust.rs new file +2 -0

+        let mut native: i64 = 0;
+        let mut pcpu: PerCpuRef<i64> = unsafe { unsafe_get_per_cpu_ref!(PERCPU, CpuGuard::new()) };

A bit complex.

lib/percpu_test_rust.rs line 34

Quoted 9 lines

lib/percpu_test_rust.rs new file +8 -0

+        native += -1;
+        *pcpu += -1;
+        assert!(native == *pcpu && native == -1);
+
+        native += 1;
+        *pcpu += 1;
+        assert!(native == *pcpu && native == 0);
+

That's pretty straightforward..... But is there no symbolic access to the per cpu namespace? How would you access the kernel per cpu variables defined in C?

How do you go about using per cpu atomics like

this_cpu_inc(nr_dentry_unused);

Quoted 5 lines · Christoph Lameter (Ampere)

Quoted 2 lines · Mitchell Levy

lib/percpu_test_rust.rs new file +2 -0

+        let mut native: i64 = 0;
+        let mut pcpu: PerCpuRef<i64> = unsafe { unsafe_get_per_cpu_ref!(PERCPU, CpuGuard::new()) };

Quoted 1 line

A bit complex.

I agree with this, maybe a helper function would suffise? Something in terms of,

  unsafe fn get_per_cpu<T>(var: &PerCpuVariable<T>) -> PerCpuRef<T> {
	  unsafe_get_per_cpu_ref!(var, CpuGuard::new())

}

Quoted 17 lines

lib/percpu_test_rust.rs line 34

Quoted 9 lines

lib/percpu_test_rust.rs new file +8 -0

+        native += -1;
+        *pcpu += -1;
+        assert!(native == *pcpu && native == -1);
+
+        native += 1;
+        *pcpu += 1;
+        assert!(native == *pcpu && native == 0);
+

That's pretty straightforward..... But is there no symbolic access to the per cpu namespace? How would you access the kernel per cpu variables defined in C?

How do you go about using per cpu atomics like

this_cpu_inc(nr_dentry_unused);

Quoted 7 lines · Charalampos Mitrodimas

Quoted 5 lines · Christoph Lameter (Ampere)

Quoted 2 lines · Mitchell Levy

lib/percpu_test_rust.rs new file +2 -0

+        let mut native: i64 = 0;
+        let mut pcpu: PerCpuRef<i64> = unsafe { unsafe_get_per_cpu_ref!(PERCPU, CpuGuard::new()) };

Quoted 7 lines

Quoted 1 line

A bit complex.

I agree with this, maybe a helper function would suffise? Something in terms of,

  unsafe fn get_per_cpu<T>(var: &PerCpuVariable<T>) -> PerCpuRef<T> {
	  unsafe_get_per_cpu_ref!(var, CpuGuard::new())

}

I'm certainly open to adding such a helper. Is the main concern here the unwieldy name? Generally, I prefer to keep modifications to global state (disabling preemption via CpuGuard::new()) as explicit as possible, but if there's consensus to the contrary, I'm happy to roll it into the macro/a helper function.

Quoted 18 lines

Quoted 18 lines

lib/percpu_test_rust.rs line 34

Quoted 10 lines

lib/percpu_test_rust.rs new file +8 -0

+        native += -1;
+        *pcpu += -1;
+        assert!(native == *pcpu && native == -1);
+
+        native += 1;
+        *pcpu += 1;
+        assert!(native == *pcpu && native == 0);
+

That's pretty straightforward..... But is there no symbolic access to the per cpu namespace? How would you access the kernel per cpu variables defined in C?

How do you go about using per cpu atomics like

this_cpu_inc(nr_dentry_unused);

This series adds an API for declaring an using per-CPU variables from Rust, and it also adds support for Rust access to C per-CPU variables (subject to some soundness requirements). It also adds a small test module, lib/percpu_test_rust.rs, in the vein of lib/percpu_test.c.

---
I am sending this patch as an RFC to gather feedback on the API as well as get some input on correctness. In particular, the unsafe getter macro is somewhat cumbersome, though its safety requirements for pure-Rust code aren't too difficult to verify (interoperation with C code is certainly much more tricky).

It was suggested that I base my implementation on Rust's thread-local storage API. However, this API requires that for a thread-local T, any particular usage must be via a &T. Thus, many times it is required that thread-local variables be in the form RefCell<T> or Cell<T>. This has some pretty severe disadvantages in the context of per-CPU variables. Namely, RefCell<T> may panic if .borrow_mut is called multiple times at different points in the call stack. This is similar to the problem inherent in my current implementation (that is, unsafe_get_per_cpu_ref must be used carefully so as to not create aliasing mut references), but this implementation flags this potential problem via an unsafe block and the resulting PerCpuRef can be easily passed along and used fairly easily. Cell<T> on the other hand requires a copy any time the underlying value is used, which is entirely avoided here.

Signed-off-by Mitchell Levy <[email protected]>

Hello,

Quoted 5 lines · Mitchell Levy

This series adds an API for declaring an using per-CPU variables from Rust, and it also adds support for Rust access to C per-CPU variables (subject to some soundness requirements). It also adds a small test module, lib/percpu_test_rust.rs, in the vein of lib/percpu_test.c.

Quoted 7 lines

---
I am sending this patch as an RFC to gather feedback on the API as well as get some input on correctness. In particular, the unsafe getter macro is somewhat cumbersome, though its safety requirements for pure-Rust code aren't too difficult to verify (interoperation with C code is certainly much more tricky).

I've been thinking a little bit about this over the holidays and fwiw am not really the right person to evaluate the rust side of this. I did briefly discuss this with Hudson (cced) who is more familiar with rust.

In addition to Christoph's comments:
1. This implementation seems targeted at static allocations and the safety of this api relies on the end user doing the right thing.

There are many percpu variables created dynamically via alloc_percpu() and the lifetime of those variables are tied to a larger object. So access to those variables is safe via the understanding the larger object exists. I feel like there is room here for rust to do this checking somehow? Maybe in rust-only percpu variables, but something better nonetheless.

2. As Christoph mentioned, the percpu api relies heavily on macros for architecture specific optimizations. Your implementation seems asm-generic correct, but we should find a way to export the architecture specific percpu macros.

this_cpu_off is a percpu variable that helps x86 arch_raw_cpu_ptr().

3. Maybe a good use case to start off with would be percpu_refcount and how rust can use it or how it can make something like that safer?

Some of Hudson's direct comments about the Rust implementation:
1. DerefMut for PerCpuRef is ultimately constructing a reference from a raw integer without preserving the provenance of the original pointer in any way (since this integer is being constructed from raw register reads). I believe this violates Rust's pointer provenance rules for unsafe code.

2. The safety requirements of unsafe_get_per_cpu_ref seem like they would often not be possible to verify at the callsite of the macro without global knowledge -- especially for percpu variables defined in C code. Is this a misunderstanding? Can you show an example of where it would be straightforward to build a higher-level safe API on top of this unsafe one?

Quoted 14 lines

It was suggested that I base my implementation on Rust's thread-local storage API. However, this API requires that for a thread-local T, any particular usage must be via a &T. Thus, many times it is required that thread-local variables be in the form RefCell<T> or Cell<T>. This has some pretty severe disadvantages in the context of per-CPU variables. Namely, RefCell<T> may panic if .borrow_mut is called multiple times at different points in the call stack. This is similar to the problem inherent in my current implementation (that is, unsafe_get_per_cpu_ref must be used carefully so as to not create aliasing mut references), but this implementation flags this potential problem via an unsafe block and the resulting PerCpuRef can be easily passed along and used fairly easily. Cell<T> on the other hand requires a copy any time the underlying value is used, which is entirely avoided here.

Quoted 2 lines

Signed-off-by Mitchell Levy <[email protected]>

I think this is a cool topic and has helped inspire me to learn a bit more rust. Hopefully I can be more helpful in the future.

Thanks,
Dennis