All of lore.kernel.org
 help / color / mirror / Atom feed
From: Boqun Feng <boqun.feng@gmail.com>
To: Benno Lossin <lossin@kernel.org>
Cc: "Mitchell Levy" <levymitchell0@gmail.com>,
	"Miguel Ojeda" <ojeda@kernel.org>,
	"Alex Gaynor" <alex.gaynor@gmail.com>,
	"Gary Guo" <gary@garyguo.net>,
	"Björn Roy Baron" <bjorn3_gh@protonmail.com>,
	"Andreas Hindborg" <a.hindborg@kernel.org>,
	"Alice Ryhl" <aliceryhl@google.com>,
	"Trevor Gross" <tmgross@umich.edu>,
	"Andrew Morton" <akpm@linux-foundation.org>,
	"Dennis Zhou" <dennis@kernel.org>, "Tejun Heo" <tj@kernel.org>,
	"Christoph Lameter" <cl@linux.com>,
	"Danilo Krummrich" <dakr@kernel.org>,
	linux-kernel@vger.kernel.org, rust-for-linux@vger.kernel.org,
	linux-mm@kvack.org
Subject: Re: [PATCH v2 3/5] rust: percpu: add a rust per-CPU variable test
Date: Tue, 15 Jul 2025 14:34:09 -0700	[thread overview]
Message-ID: <aHbJUfcsjHA92OlE@tardis-2.local> (raw)
In-Reply-To: <DBCTCZ5HUZOF.2DJX63Q0VWWFN@kernel.org>

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

  reply	other threads:[~2025-07-15 21:34 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-07-12 21:31 [PATCH v2 0/5] rust: Add Per-CPU Variable API Mitchell Levy
2025-07-12 21:31 ` [PATCH v2 1/5] rust: percpu: introduce a rust API for per-CPU variables Mitchell Levy
2025-07-12 21:31 ` [PATCH v2 2/5] rust: rust-analyzer: add lib to dirs searched for crates Mitchell Levy
2025-07-12 21:31 ` [PATCH v2 3/5] rust: percpu: add a rust per-CPU variable test Mitchell Levy
2025-07-13  9:30   ` Benno Lossin
2025-07-15 10:31     ` Mitchell Levy
2025-07-15 11:31       ` Benno Lossin
2025-07-15 14:10         ` Boqun Feng
2025-07-15 15:55           ` Benno Lossin
2025-07-15 16:31             ` Boqun Feng
2025-07-15 17:44               ` Benno Lossin
2025-07-15 21:34                 ` Boqun Feng [this message]
2025-07-16 10:32                   ` Benno Lossin
2025-07-16 15:33                     ` Boqun Feng
2025-07-16 17:21                       ` Benno Lossin
2025-07-16 17:52                         ` Boqun Feng
2025-07-16 18:22                           ` Benno Lossin
2025-07-16 15:35                 ` Boqun Feng
2025-07-12 21:31 ` [PATCH v2 4/5] rust: percpu: Add pin-hole optimizations for numerics Mitchell Levy
2025-07-12 21:31 ` [PATCH v2 5/5] rust: percpu: cache per-CPU pointers in the dynamic case Mitchell Levy

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=aHbJUfcsjHA92OlE@tardis-2.local \
    --to=boqun.feng@gmail.com \
    --cc=a.hindborg@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=alex.gaynor@gmail.com \
    --cc=aliceryhl@google.com \
    --cc=bjorn3_gh@protonmail.com \
    --cc=cl@linux.com \
    --cc=dakr@kernel.org \
    --cc=dennis@kernel.org \
    --cc=gary@garyguo.net \
    --cc=levymitchell0@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=lossin@kernel.org \
    --cc=ojeda@kernel.org \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=tj@kernel.org \
    --cc=tmgross@umich.edu \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.