From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 7060EC83F1D for ; Tue, 15 Jul 2025 11:31:15 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id CABAB6B00B9; Tue, 15 Jul 2025 07:31:14 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id C5CD76B00BA; Tue, 15 Jul 2025 07:31:14 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id B72656B00BB; Tue, 15 Jul 2025 07:31:14 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0016.hostedemail.com [216.40.44.16]) by kanga.kvack.org (Postfix) with ESMTP id A45E46B00B9 for ; Tue, 15 Jul 2025 07:31:14 -0400 (EDT) Received: from smtpin13.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay07.hostedemail.com (Postfix) with ESMTP id 57A8A16046B for ; Tue, 15 Jul 2025 11:31:14 +0000 (UTC) X-FDA: 83666282868.13.E245AF9 Received: from nyc.source.kernel.org (nyc.source.kernel.org [147.75.193.91]) by imf29.hostedemail.com (Postfix) with ESMTP id 94599120006 for ; Tue, 15 Jul 2025 11:31:12 +0000 (UTC) Authentication-Results: imf29.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=PzOVpspQ; dmarc=pass (policy=quarantine) header.from=kernel.org; spf=pass (imf29.hostedemail.com: domain of lossin@kernel.org designates 147.75.193.91 as permitted sender) smtp.mailfrom=lossin@kernel.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1752579072; a=rsa-sha256; cv=none; b=pshJOVX40Tw3MnzS/7E27rpc4jmpcg3oH+eQV6xGnjn34PorXdTdXMd0fU0a2mpNY30b/8 tajX5phDGnkNTb/6iZANovt2oWQtO/9wzIEaXXCLTYQixJ8zk9QXJb53z6dR+ajfZTOIgJ jKeTXxJ7nD1KEJIdkFF7DNovQODNvns= ARC-Authentication-Results: i=1; imf29.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=PzOVpspQ; dmarc=pass (policy=quarantine) header.from=kernel.org; spf=pass (imf29.hostedemail.com: domain of lossin@kernel.org designates 147.75.193.91 as permitted sender) smtp.mailfrom=lossin@kernel.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1752579072; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=WpLdKFh/1KJsNWECjwYNR+L7FlFhuLqYL4EMqtBn6Ls=; b=0gBKJVPvIWaiWdGvaOhXk/9+09jfQqg13UEQwgz2MtfYcVXco8UANmZewMQg+CR8LvTaRp cdRnuZZ8txPG9W96I6Co4LlgyRtE5rkGPJzSztR8XBWd0t7fC8VIYF5d/v+oeP5hxPbJrf lcttxW17PJZ5+GMKrRYoRcEuQfwjBS0= Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by nyc.source.kernel.org (Postfix) with ESMTP id DDCEBA56FC9; Tue, 15 Jul 2025 11:31:11 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 50CB7C4CEE3; Tue, 15 Jul 2025 11:31:08 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1752579071; bh=qTB8vzTixTpm3wEfYNlb54btZpsBf8r9fBjMKLDVrWc=; h=Date:Cc:Subject:From:To:References:In-Reply-To:From; b=PzOVpspQqciglPtD7dzDo2/PwrO0glyWo2YflyoOSrzXK3b+H0V8gu5N4uUE0GeK9 bS00ebjJsg9bs+SlBBZBt/d5vgKYt3V4Rq6eLYl79c+ic1IHDiXWQZ2jq768Zn9QgD Ayt+vfbCpyYRb5n8b9e3mxUe/g/WrmrchCrQULgA2WHGzNo8Fb8U8zT0+vP9IwlNEN neYOZTsxHrsLiml56Tfbhj6PPLFRYET2v7tH7S7in0kH5Ur04eBCRTl6Qxc7tamPUS Bu/F2CM1NIoR23Zz9p3TEIPQhtxyFfsW8zMr03FK4qX0kZrefEE0kM6VqVSx27Mex9 wlY2ERxyeOryQ== Mime-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=UTF-8 Date: Tue, 15 Jul 2025 13:31:06 +0200 Message-Id: Cc: "Miguel Ojeda" , "Alex Gaynor" , "Boqun Feng" , "Gary Guo" , =?utf-8?q?Bj=C3=B6rn_Roy_Baron?= , "Andreas Hindborg" , "Alice Ryhl" , "Trevor Gross" , "Andrew Morton" , "Dennis Zhou" , "Tejun Heo" , "Christoph Lameter" , "Danilo Krummrich" , , , Subject: Re: [PATCH v2 3/5] rust: percpu: add a rust per-CPU variable test From: "Benno Lossin" To: "Mitchell Levy" X-Mailer: aerc 0.20.1 References: <20250712-rust-percpu-v2-0-826f2567521b@gmail.com> <20250712-rust-percpu-v2-3-826f2567521b@gmail.com> <68762e19.170a0220.33e203.a0b7@mx.google.com> In-Reply-To: <68762e19.170a0220.33e203.a0b7@mx.google.com> X-Rspamd-Server: rspam12 X-Rspamd-Queue-Id: 94599120006 X-Stat-Signature: nkh6zi16nfeun88fansxodkr676tupd1 X-Rspam-User: X-HE-Tag: 1752579072-666664 X-HE-Meta: U2FsdGVkX1/xJYDLWbYEEiUo2Q1bVJej6bPCUJp+m1WBrMfUFwzkvq8OFZMcdvQXM6IB9oIlD946r614IlulI+5/NFzigCaJ66/8xcgW5q+FzptOo68SaqGEXx9HaOrgsZA50rNEsqe0kz50IM5y2Nw65oP8ikk18rcpSOmZ1dY6f/G4n2PIL+AvJDmHQJ+ztfVEsFWhhnxBuJvUfMt0XIWGRyhzEWMOkw6DsjjvVvYOIiTyoCfuLRattSlhSmyiB+z8Viif/uaESGlFK0o2gnBqxBdZqr4mA0KImsZMQqj8ejVJfGLFmcenFeyA43dlmO/kr/R5rTrqcaYMIYSqA3bAzsSntG8hZwIdazTrxAr5nYxJHSIEg0NeDL5m5B/RnSiAf/RE7anQQJxxXrs53KkLIsyUHXEVC+0c66WoZdybjb9u7F/8TSGH3KMRbJjY+Qa7pa6FKR+oTx+b+CUr84NQn3eEiwfmuz/MGDyVYVgLoduir4eCClDWiXSAgpeCfrVP8xwNAsaDsEbwEbFJ8LCkQuq0cHpb5UJwkPi7bfR03iRWviooT08uc2n5iQ2meoa59/wDvbXhoEzjdE6aN6iZNZO6Cov3t9KowFiEE4sqBUiDVOmKRCfIu98bh3cfTwhb9H7u//UXwQLHizbPHC5SLHAy7Kvfro7dPJuwaQveZo9xG02bRc5jOa5Dzlx93OVacvlfIIvx+/H4d+tQd5fD4mZDb8wdkvwRRMYNK3zNH7PhWr06L4njSZ94e9VZtzXSZZ6coXMTnrn6waknsVpy/4joxVoVAwLB0MMeL1bi6e7YCoITRcU5dzG8FeClTH8zKldqJVf2LitUG3yv80CsJM+VcBLSX31CsbqRNgFPEVV3VaI4VZBGfeq7DNqbJW8TH23JMwLQrRBWry30P3HgxT7ep0OJPzrIy+8aQCkP1wJiQ0HPdleFAV9T1aPNo6vLv+OrlOxhJjXTZ4F 7uAMMYXo mpW4aGCe9xdsv2yDFIPDeahcDGT2VvIkSnKeN6hkcgDecnrfAZzE8BW/tjg66lVCO3VNX+v6z16zybnil+CHwYHY0sUUB3P3AbN1Dc0O3lc8uFf/RbMAd1edMGCuLzU/U5xLmzJrgE/D5JDJoprmp7P5v5kED0kJpFqjrKPWyBuD5oc+nlgQx1mLe2VrCdSltvR7m/hH7svnZRQAWBEnIRNJ4u8/hdNzNfu8zqByjba5NvckPib+vcWyH1fYOL7MxfnbMoD4AB0/7BNfGOb+bEeNUQNnBQjZu/rgS0xIYLqjX5P0CyMBeRJYzYeExLr3KZSs8oNSlrE9ydc2VW5W15rssd8tiQ3a7wfQcHIiWha4Jb0Tc5AP0um0YfIHwdfDDH3o8ugnt+LM/d4VJsGn6AU54obyg7NBkWxxB3V1vQpyZPxo= X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: On Tue Jul 15, 2025 at 12:31 PM CEST, Mitchell Levy wrote: > On Sun, Jul 13, 2025 at 11:30:31AM +0200, Benno Lossin wrote: >> On Sat Jul 12, 2025 at 11:31 PM CEST, Mitchell Levy wrote: >> > Add a short exercise for Rust's per-CPU variable API, modelled after >> > lib/percpu_test.c >> > >> > Signed-off-by: Mitchell Levy >> > --- >> > lib/Kconfig.debug | 9 ++++ >> > lib/Makefile | 1 + >> > lib/percpu_test_rust.rs | 120 +++++++++++++++++++++++++++++++++++++++= +++++++++ >>=20 >> I don't know if this is the correct place, the code looks much more like >> a sample, so why not place it there instead? > > I don't feel particularly strongly either way --- I defaulted to `lib/` > since that's where the `percpu_test.c` I was working off of is located. > Happy to change for v3 Since we don't have Rust stuff in lib/ yet (and that the code looks much more like the samples we already have) I think putting it in samples/rust is better. >> > rust/helpers/percpu.c | 11 +++++ >> > 4 files changed, 141 insertions(+) >> > diff --git a/lib/percpu_test_rust.rs b/lib/percpu_test_rust.rs >> > new file mode 100644 >> > index 000000000000..a9652e6ece08 >> > --- /dev/null >> > +++ b/lib/percpu_test_rust.rs >> > @@ -0,0 +1,120 @@ >> > +// SPDX-License-Identifier: GPL-2.0 >> > +//! A simple self test for the rust per-CPU API. >> > + >> > +use core::ffi::c_void; >> > + >> > +use kernel::{ >> > + bindings::{on_each_cpu, smp_processor_id}, >> > + define_per_cpu, >> > + percpu::{cpu_guard::*, *}, >> > + pr_info, >> > + prelude::*, >> > + unsafe_get_per_cpu, >> > +}; >> > + >> > +module! { >> > + type: PerCpuTestModule, >> > + name: "percpu_test_rust", >> > + author: "Mitchell Levy", >> > + description: "Test code to exercise the Rust Per CPU variable API= ", >> > + license: "GPL v2", >> > +} >> > + >> > +struct PerCpuTestModule; >> > + >> > +define_per_cpu!(PERCPU: i64 =3D 0); >> > +define_per_cpu!(UPERCPU: u64 =3D 0); >> > + >> > +impl kernel::Module for PerCpuTestModule { >> > + fn init(_module: &'static ThisModule) -> Result { >> > + pr_info!("rust percpu test start\n"); >> > + >> > + let mut native: i64 =3D 0; >> > + // SAFETY: PERCPU is properly defined >> > + let mut pcpu: StaticPerCpu =3D unsafe { unsafe_get_per_c= pu!(PERCPU) }; >>=20 >> I don't understand why we need unsafe here, can't we just create >> something specially in the `define_per_cpu` macro that is then confirmed >> by the `get_per_cpu!` macro and thus it can be safe? > > As is, something like > define_per_cpu!(PERCPU: i32 =3D 0); > > fn func() { > let mut pcpu: StaticPerCpu =3D unsafe { unsafe_get_per_cpu!(= PERCPU) }; > } > will compile, but any usage of `pcpu` will be UB. This is because > `unsafe_get_per_cpu!` is just blindly casting pointers and, as far as I > know, the compiler does not do any checking of pointer casts. If you > have thoughts/ideas on how to get around this problem, I'd certainly > *like* to provide a safe API here :) I haven't taken a look at your implementation, but you do have the type declared in `define_per_cpu!`, so it's a bit of a mystery to me why you can't get that out in `unsafe_get_per_cpu!`... Maybe in a few weeks I'll be able to take a closer look. >> > + // SAFETY: We only have one PerCpu that points at PERCPU >> > + unsafe { pcpu.get(CpuGuard::new()) }.with(|val: &mut i64| { >>=20 >> Hmm I also don't like the unsafe part here... >>=20 >> Can't we use the same API that `thread_local!` in the standard library >> has: >>=20 >> https://doc.rust-lang.org/std/macro.thread_local.html >>=20 >> So in this example you would store a `Cell` instead. >>=20 >> 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. >>=20 >> 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 >> 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? > I would definitely like to avoid *requiring* the use of `RefCell` since, > as you mention, it does have a runtime overhead. Per-CPU variables can > be used for "logical" reasons rather than just as a performance > optimization, so there might be some cases where paying the runtime > overhead is ok. But that's certainly not true in all cases. That said, > perhaps there could be a safely obtainable token type that only passes a > `&T` (rather than a `&mut T`) to its closure, and then if a user doesn't > mind the runtime overhead, they can choose `T` to be a `RefCell`. > Thoughts? So I think using an API similar to `thread_local!` will allow us to have multiple other APIs that slot into that. `Cell` for `T: Copy`, `RefCell` for cases where you don't care about the runtime overhead, plain `T` for cases where you only need `&T`. For the case where you need `&mut T`, we could have something like a `TokenCell` that gives out a token that you need to mutably borrow in order to get `&mut T`. Finally for anything else that is too restricted by this, users can also use `UnsafeCell` although that requires `unsafe`. I think the advantage of this is that the common cases are all safe and very idiomatic. In the current design, you *always* have to use unsafe. > For `UnsafeCell`, if a user of the API were to have something like a > `PerCpu>` that safely spits out a `&UnsafeCell`, my > understanding is that mutating the underlying `T` would require the > exact same safety guarantees as what's here, except now it'd need a much > bigger unsafe block and would have to do all of its manipulations via > pointers. That seems like a pretty big ergonomics burden without a clear > (to me) benefit. It would require the same amount of unsafe & safety comments, but it wouldn't be bigger comments, since you can just as well create `&mut T` to the value. --- Cheers, Benno