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 2B3D5C83F17 for ; Tue, 15 Jul 2025 15:55:22 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id C257F6B0098; Tue, 15 Jul 2025 11:55:21 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id BFD926B0099; Tue, 15 Jul 2025 11:55:21 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id B3A816B009A; Tue, 15 Jul 2025 11:55:21 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0013.hostedemail.com [216.40.44.13]) by kanga.kvack.org (Postfix) with ESMTP id A14BA6B0098 for ; Tue, 15 Jul 2025 11:55:21 -0400 (EDT) Received: from smtpin09.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id 5A607C04BD for ; Tue, 15 Jul 2025 15:55:21 +0000 (UTC) X-FDA: 83666948442.09.8911914 Received: from nyc.source.kernel.org (nyc.source.kernel.org [147.75.193.91]) by imf26.hostedemail.com (Postfix) with ESMTP id A5F86140008 for ; Tue, 15 Jul 2025 15:55:19 +0000 (UTC) Authentication-Results: imf26.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=NAgAN9Q7; dmarc=pass (policy=quarantine) header.from=kernel.org; spf=pass (imf26.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=1752594919; 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=2+liLkFJyP3XRD5nl984VMzgguVWkiTAk6YxZEKqg8o=; b=st2EJuW7CHX3aMw6DPQjAAyyXREiPmDIzKf9wHepB+9a0tm8RJ/GA2ZkkQXh9RWQj934IT sTVzNhhwkfhOT983kaM7oUSeYhUhfHy8om8IYIpN+0N+kpAXV/AfWvcXUAK9uRFq8OTFZQ sxmAy66O7FimDHUTPxFY4OitN4UKJK4= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1752594919; a=rsa-sha256; cv=none; b=WWXUdogmnBCmfBhylwYj6DOwGWSvf2OvDaqtq3YXBHViU7WWkWnqn5NZPXyybihR6DZnX6 aCZwET/xblC6uus4763CfGuyQcQWmPbSpP/BOBdueuqN8/j3iWRtNUJbFVVeC9/k/wdSlS naJ3738aMllw3/V2ua47UR0xRyDfu74= ARC-Authentication-Results: i=1; imf26.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=NAgAN9Q7; dmarc=pass (policy=quarantine) header.from=kernel.org; spf=pass (imf26.hostedemail.com: domain of lossin@kernel.org designates 147.75.193.91 as permitted sender) smtp.mailfrom=lossin@kernel.org Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by nyc.source.kernel.org (Postfix) with ESMTP id DED06A57180; Tue, 15 Jul 2025 15:55:18 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 4BBA5C4CEE3; Tue, 15 Jul 2025 15:55:15 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1752594918; bh=lM5npeo5SVpxom38WImUYkut32bVHi3FnoTSjwhrrIo=; h=Date:Cc:Subject:From:To:References:In-Reply-To:From; b=NAgAN9Q7wevmKgtEDKjHWtJnopUlZdW+nC4ywK6db4c/JpCOOHAfv4++Lkkkfv8NL VbFaJp5BhaxwEIv+pTU6pQ8StQgSa/HprpOzMYpt5W1Gz+i4THEGrsLwVagW10OFjC HKw4MyjMb6/EMQQKWIdW5EgCSyyqfsRYCPdbhf1b5bx9PeyLl397FXJPKj+WdoppRS Fd446Pxb3I/mvoWawUB26gG226rVTR8n3t0JhAQI8Dhl2nOKsB1dznrhqGIXxDmeRv IPq8U4ULsUMZW7wxxpPzBjb0ryPn6OnlAvogPg/T7PqC1FdGCjjjCDvsTBi3eSHNb6 LyGpSyxsLGaaA== Mime-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=UTF-8 Date: Tue, 15 Jul 2025 17:55:13 +0200 Message-Id: Cc: "Mitchell Levy" , "Miguel Ojeda" , "Alex Gaynor" , "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: "Boqun Feng" 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: X-Stat-Signature: nkg1q4bwyd8ks9b91bowp6gk1grjxcdh X-Rspamd-Queue-Id: A5F86140008 X-Rspamd-Server: rspam10 X-Rspam-User: X-HE-Tag: 1752594919-333796 X-HE-Meta: U2FsdGVkX1+7TI54GlWkOmxbCXcrg9ScGvX3tNSEnbWdkxyJj+yjFfN1yqGz0ErHHTtMLbCFhqj/RWpRgFC/jEbUiCE2pCnq6bniAVns3QzzLlehkLoSE7DLTRSgDxEfKX/djZBu8Bk/8ZluCLko4GkJMtm0/wNXD3jBY569Mf1PdZ5rDsPx5Jo/G0TnMj6R5atHvw1fp6DK8scaQMFOPhgBTATV0AzrFRMRiOQiOvVPWS9a+kWQoOsYEEPXo9KtnPABeZLV6rC0KYTb+PRVcGO9k8QZqD8r8enN2CM1cHbJuExdSAwRxBKwS9uYiIpOYksPnccCHaf6Mt/DAJKX6duWLqFyb4tfqOJvuZpxq2zdzBD5VYd6z9dL2nQpUH0T1xYqppbgDa4ZajIX2rrS689mMp0b8o5RbZ8zRglsvEgD5978RSsRTreG4bkpveRn1EM4gwExYGT+tTSL7XSkrv9U9VEt/PIsSLTBlLAW6KapHam/nLpS95FapW+x2tYIJU1ev0sQBDwZ3+iVFwods2KFqagufClZ5tntzPLVwEWctoqUrTpkCfv25SehmamuLLPwLXpgApNA1zOg9y6TwLrD3UkqcvaSJRIvLKUhKJP09Zo8Ga9yjhngl4Z28a7BULH418JjGMPSp2CFMM7XXjlYhUFQZ/bFDlLIUVPCCEqL3oVN3QUWG9gkuA6fUj3bjzjCrxwhfy4jIx+7quv9YUrLKJ4kGFtMqzz7UIX1qqUj1XGuIUmu2cOfY+px89L8H5DE+Ghkb9o/O8pnN7ps7DcaGu1MwO77xLxOpLby35MxetE10rs4TTGJyZa6Q1QncJMZ7ShBurkNObWH6wSggXbUhzlfGwhfslXQ00vxk6cxGlCSGqf/nORBLFSJpfGeGxaiMQuZaBUqa9jC6EtJhY8IrOelq1vohZ5wKuJNgK50PJUGAC/zBSYHrvHyr1xe5atpLoSqxE4SbTI6ahA 09upgD7X h4ErH7xUOuQapfu4H2PSj3N0uVtuVsxfFM120xy2bp01K6bubTjX6FdYdyNV+akxBwycDo97XrdoAhnjHtykEUDYn341gE7d+6nfJtzGB9XtvtlAxNJ98NWO5kURBhJ2+sb16a+PAS4S7M/s6CxArFRignSAPrQ6nihfgdajMXl7XpQ5/z9f5X2cMsi3114wtK+ffZrsLsCyrOdtd5QMMK0PlJiZ085gQZ8sSZIdkmPKstSjrqhjtnlopIpPrjzwXHQOPKZ5eYz+6SSofugpCD4HwWvYsRnINTZsqFAFFGDv69QGvONl4sfmbsm0R1iSyyCbyGCXvQbltKF+/wv41lD1DeTNellDciwrAQDkNuE8cHHcU4vH1JScO/UPXd+yeP/vzq5KSgSox4T5TOB6OwJjDZf2WyuNBhvbvuRrXR2UnYSWTLp0cMl4ipCLfjLzZhO2G 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 4:10 PM CEST, Boqun Feng wrote: > On Tue, Jul 15, 2025 at 01:31:06PM +0200, Benno Lossin wrote: > [...] >> >> > +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_pe= r_cpu!(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 confir= med >> >> 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_cp= u!(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 :) >>=20 >> 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!`... >>=20 >> Maybe in a few weeks I'll be able to take a closer look. >>=20 >> >> > + // 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 librar= y > > 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_cr= eate.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 do our usual due diligence and work out a safe abstraction. Only fall back to unsafe if it isn't possible. 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 to detect context switches. >> >> 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 storin= g >> >> `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 > > 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 i= s >> > 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.) >>=20 >> What kinds of types would you like to store? Allocations? Just integers >> in bigger structs? Mutexes? >>=20 > > 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 :) >> > I would definitely like to avoid *requiring* the use of `RefCell` sinc= e, >> > 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? >>=20 >> 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`. >>=20 >> 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. >>=20 > > I agree, but like I said, we need to figure out the unsafe interface > that C already uses and build API upon it. I think focusing on the > unsafe mechanism may be the way to start: you cannot implement something > that cannot be implemented, and we don't have the magic pthread_key here > ;-) Sure we can do some experimentation, but I don't think we should put unsafe abstractions upstream that we intend to replace with a safe abstraction later. Otherwise people are going to depend on it and it's going to be a mess. Do the experimenting out of tree and learn there. --- Cheers, Benno