linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Boqun Feng <boqun@kernel.org>
To: Will Deacon <will@kernel.org>
Cc: "Gary Guo" <gary@garyguo.net>,
	"Peter Zijlstra" <peterz@infradead.org>,
	"Andreas Hindborg" <a.hindborg@kernel.org>,
	"Alice Ryhl" <aliceryhl@google.com>,
	"Lorenzo Stoakes" <lorenzo.stoakes@oracle.com>,
	"Liam R. Howlett" <Liam.Howlett@oracle.com>,
	"Miguel Ojeda" <ojeda@kernel.org>,
	"Boqun Feng" <boqun.feng@gmail.com>,
	"Björn Roy Baron" <bjorn3_gh@protonmail.com>,
	"Benno Lossin" <lossin@kernel.org>,
	"Trevor Gross" <tmgross@umich.edu>,
	"Danilo Krummrich" <dakr@kernel.org>,
	"Mark Rutland" <mark.rutland@arm.com>,
	linux-mm@kvack.org, rust-for-linux@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3] rust: page: add byte-wise atomic memory copy methods
Date: Tue, 17 Feb 2026 09:10:42 -0800	[thread overview]
Message-ID: <aZShEW2b9-71r18I@tardis.local> (raw)
In-Reply-To: <aZRHJ22UtG1YeBMj@willie-the-truck>

On Tue, Feb 17, 2026 at 10:47:03AM +0000, Will Deacon wrote:
> On Tue, Feb 17, 2026 at 09:42:37AM +0000, Gary Guo wrote:
> > On 2026-02-17 08:55, Peter Zijlstra wrote:
> > > On Fri, Feb 13, 2026 at 09:44:18AM -0800, Boqun Feng wrote:
> > >> On Fri, Feb 13, 2026 at 07:42:53AM +0100, Andreas Hindborg wrote:
> > >> [...]
> > >> > diff --git a/rust/kernel/sync/atomic.rs b/rust/kernel/sync/atomic.rs
> > >> > index 4aebeacb961a2..8ab20126a88cf 100644
> > >> > --- a/rust/kernel/sync/atomic.rs
> > >> > +++ b/rust/kernel/sync/atomic.rs
> > >> > @@ -560,3 +560,35 @@ pub fn fetch_add<Rhs, Ordering: ordering::Ordering>(&self, v: Rhs, _: Ordering)
> > >> >          unsafe { from_repr(ret) }
> > >> >      }
> > >> >  }
> > >> > +
> > >> > +/// Copy `len` bytes from `src` to `dst` using byte-wise atomic operations.
> > >> > +///
> > >> 
> > >> Given Greg and Peter's feedback, I think it's better to call out why we
> > >> need `atomic_per_byte_memcpy()` and why we use bindings::memcpy() to
> > >> implement it. How about a paragraph as follow:
> > >> 
> > >> /// This is the concurrent-safe version of `core::ptr::copy()` (the
> > >> /// counterpart of standard C's `memcpy()`). Because of the atomicity at
> > >> /// byte level, when racing with another concurrent atomic access (or
> > >> /// a normal read races with an atomic read) or an external access (from
> > >> /// DMA or userspace), the behavior of this function is defined:
> > >> /// copying memory at the (at least) byte granularity.
> > >> ///
> > >> /// Implementation note: it's currently implemented by kernel's
> > >> /// `memcpy()`, because kernel's `memcpy()` is implemented in a way that
> > >> /// byte-wise atomic memory load/store instructions are used.
> > >> 
> > >> And probably we make it a separate patch for this
> > >> atomic_per_byte_memcpy().
> > >> 
> > >> Thoughts?
> > > 
> > > Its still not making sense; an no kernel memcpy() does not necessarily
> > > use byte wise copy. And please stop talking about 'atomic' here. There
> > > are no atomic ops used (and atomic ops will fundamentally not help).
> > 
> > Byte-wise atomicity means that the guaranteed atomicity is per-byte, not that
> > the copying is per byte. The copying size and order can be arbitrary.
> 
> Curious, but how would you implement a memcpy that _isn't_ "atomic" by
> that definition? Are you worried about accessing bytes multiple times,
> or losing dependency ordering, or something else?
> 

We are worried about two racing memcpy()s end up being data race and
that's undefined behavior. And "atomic" is the key word in C (and Rust)
to "lift" normal accesses to non-data-race, for example:

	thread 1		thread 2
	--------		--------
    	*a = 1;			r1 = *a;

is data race, and 

	thread 1		thread 2
	--------		--------
    	atomic_store(a,1);	r1 = atomic_load(a);

is not.

In memcpy() case, since we don't need the whole copy to be a single
atomic operation, so as long as the atomicity is guaranteed at byte
level (larger is fine because 2byte atomic is still byte atomic), it
should be sufficient as a concurrent-safe memcpy().

So either we want to live in a world where

"concurrent normal accesses with at least one being write are data race
therefore UBs, use the corresponding atomic API in this case and handle
the data carefully with concurrent accesses in mind".

or we want to live in a world where

"concurrent normal accesses with at least one being write are data race
therefore UBs, but there are 17 and more API which are technically UBs,
but they are not considered as UBs in kernel, use them"

To me, having a atomic_bytewise_memcpy() at least clear things out about
what is actually needed (at the very minimal) to have a concurrent-safe
memcpy(). Moving forward, since the concept has been already somehow
proposed to C/C++, it's likely to be standardized (we can push it from
the kernel end as well) so we don't need to implement a concurrent-safe
memcpy() for all architectures on our own.

Hope this makes some sense ;-)

Regards,
Boqun

> This all feels like playing tricks to placate the type system for
> something that isn't actually a problem in practice. But I think I'm
> probably at least as confused as Peter :)
> 

> Will


  reply	other threads:[~2026-02-17 17:10 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-02-13  6:42 Andreas Hindborg
2026-02-13 11:28 ` Peter Zijlstra
2026-02-13 12:45   ` Andreas Hindborg
2026-02-13 14:35     ` Peter Zijlstra
2026-02-13 16:42   ` Boqun Feng
2026-02-14  8:18     ` Andreas Hindborg
2026-02-17 18:47       ` Boqun Feng
2026-02-13 17:44 ` Boqun Feng
2026-02-14  8:04   ` Andreas Hindborg
2026-02-17  8:55   ` Peter Zijlstra
2026-02-17  9:42     ` Gary Guo
2026-02-17 10:47       ` Will Deacon
2026-02-17 17:10         ` Boqun Feng [this message]
2026-02-18  8:53           ` Peter Zijlstra
2026-02-18 11:20           ` Peter Zijlstra
2026-02-17 12:03 ` Alice Ryhl
2026-02-17 17:32   ` Boqun Feng
2026-02-17 23:10   ` Gary Guo
2026-02-18  9:40     ` Alice Ryhl
2026-02-18 10:20     ` Peter Zijlstra
2026-02-18 11:36       ` Gary Guo
2026-02-18 12:12         ` Peter Zijlstra
2026-02-18 11:56 ` Miguel Ojeda
2026-02-18 12:00   ` Alice Ryhl
2026-02-18 12:07     ` Miguel Ojeda
2026-02-18 12:33       ` Andreas Hindborg
2026-02-18 14:42     ` Benno Lossin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aZShEW2b9-71r18I@tardis.local \
    --to=boqun@kernel.org \
    --cc=Liam.Howlett@oracle.com \
    --cc=a.hindborg@kernel.org \
    --cc=aliceryhl@google.com \
    --cc=bjorn3_gh@protonmail.com \
    --cc=boqun.feng@gmail.com \
    --cc=dakr@kernel.org \
    --cc=gary@garyguo.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=lorenzo.stoakes@oracle.com \
    --cc=lossin@kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=ojeda@kernel.org \
    --cc=peterz@infradead.org \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=tmgross@umich.edu \
    --cc=will@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox