Re: [PATCH v3] rust: page: add byte-wise atomic memory copy methods

[Gary Guo <gary@garyguo.net>]

On 2026-02-17 12:03, Alice Ryhl wrote:
> On Fri, Feb 13, 2026 at 07:42:53AM +0100, Andreas Hindborg wrote:
>> When copying data from buffers that are mapped to user space, it is
>> impossible to guarantee absence of concurrent memory operations on 
>> those
>> buffers. Copying data to/from `Page` from/to these buffers would be
>> undefined behavior if no special considerations are made.
>> 
>> Add methods on `Page` to read and write the contents using byte-wise 
>> atomic
>> operations.
>> 
>> Also improve clarity by specifying additional requirements on
>> `read_raw`/`write_raw` methods regarding concurrent operations on 
>> involved
>> buffers.
>> 
>> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
> 
>> +/// Copy `len` bytes from `src` to `dst` using byte-wise atomic 
>> operations.
>> +///
>> +/// This copy operation is volatile.
>> +///
>> +/// # Safety
>> +///
>> +/// Callers must ensure that:
>> +///
>> +/// - `src` is valid for reads for `len` bytes for the duration of 
>> the call.
>> +/// - `dst` is valid for writes for `len` bytes for the duration of 
>> the call.
>> +/// - For the duration of the call, other accesses to the areas 
>> described by `src`, `dst` and `len`,
>> +///   must not cause data races (defined by [`LKMM`]) against atomic 
>> operations executed by this
>> +///   function. Note that if all other accesses are atomic, then this 
>> safety requirement is
>> +///   trivially fulfilled.
>> +///
>> +/// [`LKMM`]: srctree/tools/memory-model
>> +pub unsafe fn atomic_per_byte_memcpy(src: *const u8, dst: *mut u8, 
>> len: usize) {
>> +    // SAFETY: By the safety requirements of this function, the 
>> following operation will not:
>> +    //  - Trap.
>> +    //  - Invalidate any reference invariants.
>> +    //  - Race with any operation by the Rust AM, as 
>> `bindings::memcpy` is a byte-wise atomic
>> +    //    operation and all operations by the Rust AM to the involved 
>> memory areas use byte-wise
>> +    //    atomic semantics.
>> +    unsafe {
>> +        bindings::memcpy(
>> +            dst.cast::<kernel::ffi::c_void>(),
>> +            src.cast::<kernel::ffi::c_void>(),
>> +            len,
> 
> Are we sure that LLVM will not say "memcpy is a special function name, 
> I
> know what it means" and optimize this like a non-atomic memcpy?

This "treating special symbol name as intrinsics" logic is done in 
Clang,
and won't be performed once lower to LLVM IR, so Rust is immune to that 
(even
when LTO'ed together with Clang generated IR). So calling to bindings is 
fine.

> 
> I think we should consider using the
> 
> 	std::intrinsics::volatile_copy_nonoverlapping_memory
> 
> intrinsic until Rust stabilizes a built-in atomic per-byte memcpy. Yes 
> I
> know the intrinsic is unstable, but we should at least ask the Rust
> folks about it. They are plausibly ok with this particular usage.

If we have this in stable, I think it's sufficient for LKMM. However for 
Rust/C11 MM
says that volatile ops are not atomic and use them for concurrency is 
UB.

I recall in last Rust all hands the vibe at discussion is that it's 
desirable to define
volatile as being byte-wise atomic, so if that actually happens, this 
would indeed be
what we want (but I think semantics w.r.t. mixed-size atomics need to be 
figured out first).

Best,
Gary

> 
> Alice