Re: [PATCH v5 2/4] rust: sync: atomic: add atomic_per_byte_memcpy

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [Low] rust: sync: atomic: missing #[inline] attribute on
  atomic_per_byte_memcpy()
- [Low] rust: sync: atomic: wrong core::ptr::copy() doc on
  atomic_per_byte_memcpy()
--

commit cade864abc69fc627ff4e6283efdc3170c94fccf
Author: Andreas Hindborg <a.hindborg@kernel.org>

rust: sync: atomic: add atomic_per_byte_memcpy

This commit adds atomic_per_byte_memcpy() in kernel::sync::atomic, wrapping
the kernel's memcpy() to perform byte-wise atomic memory copies.

> diff --git a/rust/kernel/sync/atomic.rs b/rust/kernel/sync/atomic.rs
> index 6d27898add42c..cbcb081d8674e 100644
> --- a/rust/kernel/sync/atomic.rs
> +++ b/rust/kernel/sync/atomic.rs
> @@ -848,3 +848,38 @@ pub unsafe fn cmpxchg<T: AtomicType, Ordering: ordering::Ordering>(
>      // per LKMM.
>      unsafe { Atomic::from_ptr(ptr) }.cmpxchg(old, new, o)
>  }
> +
> +/// Copy `len` bytes from `src` to `dst` using byte-wise atomic operations.
> +///
> +/// This is the concurrent-safe counterpart of `core::ptr::copy()` (the equivalent of standard
> +/// C's `memcpy()`). Because of the atomicity at byte level, when racing with another concurrent

[Severity: Low]
Should this documentation (and the commit message) refer to
core::ptr::copy_nonoverlapping() instead of core::ptr::copy()?

core::ptr::copy() is the equivalent of C's memmove() and permits overlapping
regions, whereas core::ptr::copy_nonoverlapping() corresponds to C's
memcpy() and matches the safety requirement below that src and dst do not
overlap.

> +/// atomic access (or when a normal read races with an atomic read), or with an external access
> +/// (from DMA or userspace), the behavior of this function is defined: the memory is copied at
> +/// (at least) byte granularity.
> +///
> +/// Implementation note: this is currently implemented by the kernel's `memcpy()`, which is
> +/// implemented in a way such that byte-wise atomic memory load/store instructions are used.
> +///
> +/// # Safety
> +///
> +/// Callers must ensure that:
> +///
> +/// - `src` is valid for atomic reads for `len` bytes for the duration of the call.
> +/// - `dst` is valid for atomic writes for `len` bytes for the duration of the call.
> +/// - The memory regions `src .. src + len` and `dst .. dst + len` do not overlap.
> +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.
> +    //  - Copy between overlapping regions, which `memcpy()` does not support.
> +    unsafe {
> +        bindings::memcpy(
> +            dst.cast::<kernel::ffi::c_void>(),
> +            src.cast::<kernel::ffi::c_void>(),
> +            len,
> +        )
> +    };
> +}

[Severity: Low]
Should atomic_per_byte_memcpy() be marked #[inline]?  It is a non-generic
function in the kernel crate that forwards directly to bindings::memcpy(),
so without #[inline] callers in other crates cannot inline the wrapper
unless LTO is enabled.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261110-page-volatile-io-v5-0-a91ef252c24b@kernel.org?part=2