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

[Boqun Feng <boqun@kernel.org>]

On Sat, Feb 14, 2026 at 09:18:16AM +0100, Andreas Hindborg wrote:
> Boqun Feng <boqun@kernel.org> writes:
> 
> > On Fri, Feb 13, 2026 at 12:28:37PM +0100, Peter Zijlstra 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.
> >> 
> >> 
> >> > +    /// - Callers must ensure that this call does not race with a write to the source page that
> >> > +    ///   overlaps with this read.
> >> 
> >> Yeah, but per the bit above, its user mapped, you *CANNOT* ensure this.
> >> 
> >
> > First, this safety requirement is actually incorrect, because of the
> > user mapped case you mentioned. I believe Andreas put it to prevent
> > others from racing with memcpy(), e.g.
> 
> Since context is a bit washed out here, let's make sure we are talking
> about `Page::read_bytewise_atomic``.
> 
> There are two buffers in play. `src`, which is provided by the `self:
> &Page` and `dst: *mut u8`, which is passed as a function parameter.
> 
> The requirement for `src` is:
> 
>     Callers must ensure that this call does not race with a write to the **source page** that
>     overlaps with this read.
> 
> This requirement is different than the requirement on `dst`. I do not
> want to enforce that all memory operations on `src` be atomic, simply
> that they are synchronized. This is a weaker requirement than the
> requirement on `dst`. As we hold a shared reference to `self` and there
> is no internal synchronization, I think this is the correct requirement.
> 
> For `dst` we have:
> 
>     For the duration of the call, other accesses to the area described by `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.
> 
> Which is also requiring no races, but is specifically mentioning atomic
> operations, which I did not want on `src`.
> 
> With this in mind, do you still think they are redundant?
> 

I see, I've overlooked and I was confused and I guess Peter was too. The
`dst` could be a user mapped page, but the `src` cannot be. I.e. it's
function that copying a public `dst` to a private `src`, of course you
can request no concurrent access to the private `src`. So his objection
is invalid.

It's not redundant, but probably we can make it more clear. Maybe we
start by saying:

/// Notice this function is guaranteed to performs an atomic byte-wise
/// memory write to the `dst` side but it may only perform a normal
/// memory read from the [`Page`] `self`.

Then

/// # Safety
///
/// Callers must ensure that:
/// 
/// - `dst` ..
/// - For ..
/// - No concurrent write to the source page `self` that overlaps with
///   the read.

or anything else that could care out the difference between `src` and
`dst`.

Regards,
Boqun

> 
> Best regards,
> Andreas Hindborg
> 
>