Message-ID: <aHbJUfcsjHA92OlE@tardis-2.local>
Date: Tue, 15 Jul 2025 14:34:09 -0700
From: Boqun Feng <boqun.feng@...il.com>
To: Benno Lossin <lossin@...nel.org>
Cc: Mitchell Levy <levymitchell0@...il.com>,
	Miguel Ojeda <ojeda@...nel.org>,
	Alex Gaynor <alex.gaynor@...il.com>, Gary Guo <gary@...yguo.net>,
	Björn Roy Baron <bjorn3_gh@...tonmail.com>,
	Andreas Hindborg <a.hindborg@...nel.org>,
	Alice Ryhl <aliceryhl@...gle.com>, Trevor Gross <tmgross@...ch.edu>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Dennis Zhou <dennis@...nel.org>, Tejun Heo <tj@...nel.org>,
	Christoph Lameter <cl@...ux.com>,
	Danilo Krummrich <dakr@...nel.org>, linux-kernel@...r.kernel.org,
	rust-for-linux@...r.kernel.org, linux-mm@...ck.org
Subject: Re: [PATCH v2 3/5] rust: percpu: add a rust per-CPU variable test

On Tue, Jul 15, 2025 at 07:44:01PM +0200, Benno Lossin wrote:
[...]
> >> >
> >> > First of all, `thread_local!` has to be implemented by some sys-specific
> >> > unsafe mechanism, right? For example on unix, I think it's using
> >> > pthread_key_t:
> >> >
> >> > 	https://pubs.opengroup.org/onlinepubs/009695399/functions/pthread_key_create.html
> >> >
> >> > what we are implementing (or wrapping) is the very basic unsafe
> >> > mechanism for percpu here. Surely we can explore the design for a safe
> >> > API, but the unsafe mechanism is probably necessary to look into at
> >> > first.
> >> 
> >> But this is intended to be used by drivers, right? If so, then we should
> >
> > Not necessarily only for drivers, we can also use it for implementing
> > other safe abstraction (e.g. hazard pointers, percpu counters etc)
> 
> That's fair, but then it should be `pub(crate)`.
> 

Fine by me, but please see below.

> >> do our usual due diligence and work out a safe abstraction. Only fall
> >> back to unsafe if it isn't possible.
> >> 
> >
> > All I'm saying is instead of figuring out a safe abstraction at first,
> > we should probably focus on identifying how to implement it and which
> > part is really unsafe and the safety requirement for that.
> 
> Yeah. But then we should do that before merging :)
> 

Well, who's talknig about merging? ;-) I thought we just began reviewing
here ;-)

> >> I'm not familiar with percpu, but from the name I assumed that it's
> >> "just a variable for each cpu" so similar to `thread_local!`, but it's
> >> bound to the specific cpu instead of the thread.
> >> 
> >> That in my mind should be rather easy to support in Rust at least with
> >> the thread_local-style API. You just need to ensure that no reference
> >> can escape the cpu, so we can make it `!Send` & `!Sync` + rely on klint
> >
> > Not really, in kernel, we have plenty of use cases that we read the
> > other CPU's percpu variables. For example, each CPU keeps it's own
> > counter and we sum them other in another CPU.
> 
> But then you need some sort of synchronization?
> 

Right, but the synchronization can exist either in the percpu operations
themselves or outside the percpu operations. Some cases, the data types
are small enough to fit in atomic data types, and operations are just
load/store/cmpxchg etc, then operations on the current cpu and remote
read will be naturally synchronized. Sometimes extra synchronization is
needed.

Keyword find all these cases are `per_cpu_ptr()`:

	https://elixir.bootlin.com/linux/v6.15.6/A/ident/per_cpu_ptr

> > If we would like to model it conceptually, it's more like an array
> > that's index by CpuId to me.
> 
> Gotcha, but this model is missing the access control/synchronization. So
> I'm not so sure how useful it is.
> 
> (I think I asked this somewhere else, but the number of CPUs doesn't
> change, right?)
> 

In terms of percpu variable, yes. A percpu variable is even available
for an offline CPU.

> >> to detect context switches.
> >> 
> >> >> >> has:
> >> >> >> 
> >> >> >>     https://doc.rust-lang.org/std/macro.thread_local.html
> >> >> >> 
> >> >> >> So in this example you would store a `Cell<i64>` instead.
> >> >> >> 
> >> >> >> I'm not familiar with per CPU variables, but if you're usually storing
> >> >> >> `Copy` types, then this is much better wrt not having unsafe code
> >> >> >> everywhere.
> >> >> >> 
> >> >> >> If one also often stores `!Copy` types, then we might be able to get
> >> >> >> away with `RefCell`, but that's a small runtime overhead -- which is
> >> >> >> probably bad given that per cpu variables are most likely used for
> >> >> >> performance reasons? In that case the user might just need to store
> >> >> >> `UnsafeCell` and use unsafe regardless. (or we invent something
> >> >
> >> > This sounds reasonable to me.
> >> >
> >> >> >> specifically for that case, eg tokens that are statically known to be
> >> >> >> unique etc)
> >> >> >
> >> >> > I'm open to including a specialization for `T: Copy` in a similar vein
> >> >> > to what I have here for numeric types. Off the top of my head, that
> >> >> > shouldn't require any user-facing `unsafe`. But yes, I believe there is
> >> >> > a significant amount of interest in having `!Copy` per-CPU variables.
> >> >> > (At least, I'm interested in having them around for experimenting with
> >> >> > using Rust for HV drivers.)
> >> >> 
> >> >> What kinds of types would you like to store? Allocations? Just integers
> >> >> in bigger structs? Mutexes?
> >> >> 
> >> >
> >> > In the VMBus driver, there is a percpu work_struct.
> >> 
> >> Do you have a link? Or better yet a Rust struct description of what you
> >> think it will look like :)
> >> 
> >
> > Not Rust code yet, but here is the corresponding C code:
> >
> > 	https://github.com/Rust-for-Linux/linux/blob/rust-next/drivers/hv/vmbus_drv.c#L1396
> 
> Thanks!
> 
> > But please note that we are not solely developing the abstraction for
> > this usage, but more for generally understand how to wrap percpu
> > functionality similar to the usage in C.
> 
> Well, I have to start somewhere for looking at the use-cases :)
> 
> If you have more, just let me see. (probably won't have enough time to
> look at them now, but maybe in a couple weeks)
> 

If you have time, feel free to take a look at hazard pointers;

https://lore.kernel.org/lkml/20240917143402.930114-1-boqun.feng@gmail.com/
https://lore.kernel.org/lkml/20250625031101.12555-1-boqun.feng@gmail.com/

You can also take a look at existing usage of percpu, e.g. SRCU uses to
track how many readers are active:

	https://elixir.bootlin.com/linux/v6.15.6/source/kernel/rcu/srcutree.c#L577

[...]

Regards,
Boqun

Powered by blists - more mailing lists