From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-qk1-f180.google.com (mail-qk1-f180.google.com [209.85.222.180]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 56C5D2F3E; Mon, 21 Apr 2025 16:27:38 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.222.180 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1745252860; cv=none; b=AotM3KF3ZNiHwAbOS6xiEpbhCnx7HW3lVbwbf0KgnuKJLENzStupEoSVvRhVbHpXczZPQqNppfeILYTIT6lwytO1v+Wg/S4e/EYEzUix9CqICsLPVRgi1S83k42P4eAK3znpqmuWP/wKeInV7A3Pe4KPggoLDXIDKE+X7PAkQWk= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1745252860; c=relaxed/simple; bh=d29f1CbXH/K+cVH+CEkYm8rZ6C6Sgeo7I7Z1B49VYSs=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=LBKZvHAlgJEB+BZTSSi7zjPYLECKk+Xy7x2hDNuIcI0oEkrTC13Gi8L/SEh6ks3kbok4U2hJsDTHFWYbxpJ0bs+KHwd/PQ2tEBan+7slQAuYB+2UCvHZZWqGelz9qjcOA0Z3pDjwQHXJcCtcTwS2HM92YqZAhSWB1TW1FFAbKqU= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=mfktyoxR; arc=none smtp.client-ip=209.85.222.180 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="mfktyoxR" Received: by mail-qk1-f180.google.com with SMTP id af79cd13be357-7c5568355ffso334962385a.0; Mon, 21 Apr 2025 09:27:38 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1745252857; x=1745857657; darn=vger.kernel.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:feedback-id:from:to:cc:subject:date :message-id:reply-to; bh=oLHYuRchr7pwRfQW4u682e1RScHliKjKzEJvvrT1wG0=; b=mfktyoxRgLaOqRHlq2BACASEJ1xUe5ywvqnPYzuFFn1FnMHEjOqD5/YvfIhhSfdQgh EH6J8Q6kUGua8Yi0umt3U9od2eq2Toly8z9aMfH0hkLPgQ5xX9tZpSTR5fARxWc/OodB UfmTVCld07DUaHww36g1RxSCjrdj1zsVHhfUnMIj3LUaKNvyIOkiMbQMvRpemnmizQp+ z7d+PwQ8TvueikFfT8suy3YPIuCeCGccZ7gq+zOkPHrAsnOM7K5Ynp7Isj//q9wLnoYp 4flwYCAoxtJkbRrKcQzmoKL+ldGnDVkPkduUamFft7H0kLWXhdyxiKlX4N+q6BmfcBsC QJ0w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1745252857; x=1745857657; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:feedback-id:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=oLHYuRchr7pwRfQW4u682e1RScHliKjKzEJvvrT1wG0=; b=VMQmRFvn4ukhAasUd6m79vDPmAUfAyDwGUGqdVSkIUccLrEX5uTkNjeGZK3aFgoEF6 Cycp7SwmrasxgSd9S669tLp4qB2368E7HS1WPNYXnfQYiLT4uVvDogbU7B2RphX/Ff0G 8bNv25wBLInX+nWcvQG0YFIlPosI8NuBgXdP0RBl4OT32kzU8qZP0r7ARDpMDGZOHaDk E/goNdEF67U3NQaoK9TXMrgmqYzITk4VFJYP18cGY0qadJPzJUVStRigwgWs1e/b4sJR yQAmIHX2+eYjt2BjYMTgro8OJLeXq5J3vJDn6RFFbKddQsXBMc/hJYVQt2dYjMKWNLNR aZZw== X-Forwarded-Encrypted: i=1; AJvYcCU51y/5Kyivb5xmMIwnJ3soN2dyzlh3gZptber4B5OpzrnQfb6yU65RJWrpJa41mZiilWRatwMKPYLSrOeaRw==@vger.kernel.org, AJvYcCUU7s9w28bJHkOe6F0jGHihOzuqQWpHRH45wUfBvlw54um4AB1VoYmFvvKf0DAVuYAefS3avJb/1H96MSag@vger.kernel.org, AJvYcCUxQ4Gdc1PN3G3blMTMAY9wICRUSppfL2oTCdCbwcSiVS3obnySeoblTOoNETXKR+sBMrnprdNEwhNH@vger.kernel.org, AJvYcCXsNy0w9JeS1I2HuJlbOsBKZKfAbCwN+oQ5vjTAdLgH+LO3a5H/4z1VENNYLjGCfetWIhRB@vger.kernel.org X-Gm-Message-State: AOJu0YwUnwJA1kHioFcoc/5gtLEwNBVqa97Y+D5jdAHmZMWz8rE8vaZQ FrecFTryUCSDraFFM5WTB3A9h9RWO5+UjFLnUoyiYFF+wzBhLTbB X-Gm-Gg: ASbGncvzJzxMHrS4fwgtZGxEcoVgL2ZLfalG9KcMu4nHBZqlX1DH92M7o3jbTQSoA+I mscHqmXqYvnfLoRx4/bvqFV+DtuD3BiegD48A6orpt0vZXy/K/U4vwbgaeHJcCd5iJgEfECPvrJ krsJGzMZO/tgvtLL8cNucO6qIPomrcvJyhYWdGZOySqmvCvinrX+xoAgctoxvcyXfp+Tb+pMvel ZamuMMCcAAHhb0eNyrGINS+hjyP5IikTDihUbieiZkONcIbnmZGMNfl5jYQR+E9tvN91dFmaMNI p4B4qVoqbkJHsMnEcR+INowfJzlDqEKrDDJo67MRXCWUpQQBZ03Zmzr+I46TQN1M2WzQKnMWEu5 0ETCz6hhywpW3dmHqdd2kgGMZ5RMkIAo= X-Google-Smtp-Source: AGHT+IHKXIDlBPVp3gyYgxOjFyUzKte/+Px4RacQpXr3c5UOtQ5sJmTvggKv28JeiPdTZfVVuzZ2eg== X-Received: by 2002:a05:620a:17aa:b0:7c5:a41a:b1a with SMTP id af79cd13be357-7c927f6b4e5mr1830134985a.10.1745252857057; Mon, 21 Apr 2025 09:27:37 -0700 (PDT) Received: from fauth-a2-smtp.messagingengine.com (fauth-a2-smtp.messagingengine.com. [103.168.172.201]) by smtp.gmail.com with ESMTPSA id af79cd13be357-7c925a6eb31sm438947785a.3.2025.04.21.09.27.36 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 21 Apr 2025 09:27:36 -0700 (PDT) Received: from phl-compute-08.internal (phl-compute-08.phl.internal [10.202.2.48]) by mailfauth.phl.internal (Postfix) with ESMTP id D68B31200043; Mon, 21 Apr 2025 12:27:35 -0400 (EDT) Received: from phl-mailfrontend-01 ([10.202.2.162]) by phl-compute-08.internal (MEProxy); Mon, 21 Apr 2025 12:27:35 -0400 X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeefvddrtddtgddvgedufedvucetufdoteggodetrf dotffvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdggtfgfnhhsuhgsshgtrhhisggv pdfurfetoffkrfgpnffqhgenuceurghilhhouhhtmecufedttdenucesvcftvggtihhpih gvnhhtshculddquddttddmnecujfgurhepfffhvfevuffkfhggtggujgesthdtredttddt vdenucfhrhhomhepuehoqhhunhcuhfgvnhhguceosghoqhhunhdrfhgvnhhgsehgmhgrih hlrdgtohhmqeenucggtffrrghtthgvrhhnpeffvdehfeejjeeliefgtdduuddtjeejveeu veeugeefleefkeekuefgudeuhfefgeenucffohhmrghinhepkhgvrhhnvghlrdhorhhgpd hlfihnrdhnvghtpdhgihhthhhusgdrtghomhenucevlhhushhtvghrufhiiigvpedtnecu rfgrrhgrmhepmhgrihhlfhhrohhmpegsohhquhhnodhmvghsmhhtphgruhhthhhpvghrsh honhgrlhhithihqdeiledvgeehtdeigedqudejjeekheehhedvqdgsohhquhhnrdhfvghn gheppehgmhgrihhlrdgtohhmsehfihigmhgvrdhnrghmvgdpnhgspghrtghpthhtohephe ekpdhmohguvgepshhmthhpohhuthdprhgtphhtthhopegurghvihgughhofiesghhoohhg lhgvrdgtohhmpdhrtghpthhtoheprhhushhtqdhfohhrqdhlihhnuhigsehvghgvrhdrkh gvrhhnvghlrdhorhhgpdhrtghpthhtoheprhgtuhesvhhgvghrrdhkvghrnhgvlhdrohhr ghdprhgtphhtthhopehlihhnuhigqdhkvghrnhgvlhesvhhgvghrrdhkvghrnhgvlhdroh hrghdprhgtphhtthhopehlihhnuhigqdgrrhgthhesvhhgvghrrdhkvghrnhgvlhdrohhr ghdprhgtphhtthhopehllhhvmheslhhishhtshdrlhhinhhugidruggvvhdprhgtphhtth hopehlkhhmmheslhhishhtshdrlhhinhhugidruggvvhdprhgtphhtthhopehojhgvuggr sehkvghrnhgvlhdrohhrghdprhgtphhtthhopegrlhgvgidrghgrhihnohhrsehgmhgrih hlrdgtohhm X-ME-Proxy: Feedback-ID: iad51458e:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Mon, 21 Apr 2025 12:27:34 -0400 (EDT) Date: Mon, 21 Apr 2025 09:27:28 -0700 From: Boqun Feng To: David Gow Cc: rust-for-linux@vger.kernel.org, rcu@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arch@vger.kernel.org, llvm@lists.linux.dev, lkmm@lists.linux.dev, Miguel Ojeda , Alex Gaynor , Wedson Almeida Filho , Gary Guo , =?iso-8859-1?Q?Bj=F6rn?= Roy Baron , Benno Lossin , Andreas Hindborg , Alice Ryhl , Alan Stern , Andrea Parri , Will Deacon , Peter Zijlstra , Nicholas Piggin , David Howells , Jade Alglave , Luc Maranget , "Paul E. McKenney" , Akira Yokosawa , Daniel Lustig , Joel Fernandes , Nathan Chancellor , Nick Desaulniers , kent.overstreet@gmail.com, Greg Kroah-Hartman , elver@google.com, Mark Rutland , Thomas Gleixner , Ingo Molnar , Borislav Petkov , Dave Hansen , x86@kernel.org, "H. Peter Anvin" , Catalin Marinas , torvalds@linux-foundation.org, linux-arm-kernel@lists.infradead.org, linux-fsdevel@vger.kernel.org, Trevor Gross , dakr@redhat.com, Frederic Weisbecker , Neeraj Upadhyay , Josh Triplett , Uladzislau Rezki , Steven Rostedt , Mathieu Desnoyers , Lai Jiangshan , Zqiang , Paul Walmsley , Palmer Dabbelt , Albert Ou , linux-riscv@lists.infradead.org Subject: Re: [RFC v2 00/13] LKMM *generic* atomics in Rust Message-ID: References: <20241101060237.1185533-1-boqun.feng@gmail.com> Precedence: bulk X-Mailing-List: rust-for-linux@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: On Sat, Nov 02, 2024 at 03:35:36PM +0800, David Gow wrote: > On Fri, 1 Nov 2024 at 14:04, Boqun Feng wrote: > > > > Hi, > > > > This is another RFC version of LKMM atomics in Rust, you can find the > > previous versions: > > > > v1: https://lore.kernel.org/rust-for-linux/20240612223025.1158537-1-boqun.feng@gmail.com/ > > wip: https://lore.kernel.org/rust-for-linux/20240322233838.868874-1-boqun.feng@gmail.com/ > > > > I add a few more people Cced this time, so if you're curious about why > > we choose to implement LKMM atomics instead of using the Rust ones, you > > can find some explanation: > > > > * In my Kangrejos talk: https://lwn.net/Articles/993785/ > > * In Linus' email: https://lore.kernel.org/rust-for-linux/CAHk-=whkQk=zq5XiMcaU3xj4v69+jyoP-y6Sywhq-TvxSSvfEA@mail.gmail.com/ > > > > This time, I try implementing a generic atomic type `Atomic`, since > > Benno and Gary suggested last time, and also Rust standard library is > > also going to that direction [1]. > > > > Honestly, a generic atomic type is still not quite necessary for myself, > > but here are a few reasons that it's useful: > > > > * It's useful for type alias, for example, if you have: > > > > type c_long = isize; > > > > Then `Atomic` and `Atomic` is the same type, > > this may make FFI code (mapping a C type to a Rust type or vice > > versa) more readable. > > > > * In kernel, we sometimes switch atomic to percpu for better > > scalabity, percpu is naturally a generic type, because it can > > have data that is larger than machine word size. Making atomic > > generic ease the potential switching/refactoring. > > > > * Generic atomics provide all the functionalities that non-generic > > atomics could have. > > > > That said, currently "generic" is limited to a few types: the type must > > be the same size as atomic_t or atomic64_t, other than basic types, only > > #[repr()] struct can be used in a `Atomic`. > > > > Also this is not a full feature set patchset, things like different > > arithmetic operations and bit operations are still missing, they can be > > either future work or for future versions. > > > > I included an RCU pointer implementation as one example of the usage, so > > a patch from Danilo is added, but I will drop it once his patch merged. > > > > This is based on today's rust-next, and I've run all kunit tests from > > the doc test on x86, arm64 and riscv. > > > > Feedbacks and comments are welcome! Thanks. > > > > Regards, > > Boqun > > > > [1]: https://github.com/rust-lang/rust/issues/130539 > > > > Thanks, Boqun. > Hi David, > I played around a bit with porting the blk-mq atomic code to this. As > neither an expert in Rust nor an expert in atomics, this is probably > both non-idiomatic and wrong, but unlike the `core` atomics, it > provides an Atomic:: on 32-bit systems, which gets UML's 32-bit > build working again. > > Diff below -- I'm not likely to have much time to work on this again > soon, so feel free to pick it up/fix it if it suits. > Thanks. These look good to me, however, I think I prefer Gary's patch for this: https://lore.kernel.org/lkml/20250219201602.1898383-4-gary@garyguo.net/ therefore, I won't take this into the next version. But thank you for taking a look! Regards, Boqun > Thanks, > -- David > > --- > diff --git a/rust/kernel/block/mq/operations.rs > b/rust/kernel/block/mq/operations.rs > index 9ba7fdfeb4b2..822d64230e11 100644 > --- a/rust/kernel/block/mq/operations.rs > +++ b/rust/kernel/block/mq/operations.rs > @@ -11,7 +11,8 @@ > error::{from_result, Result}, > types::ARef, > }; > -use core::{marker::PhantomData, sync::atomic::AtomicU64, > sync::atomic::Ordering}; > +use core::marker::PhantomData; > +use kernel::sync::atomic::{Atomic, Relaxed}; > > /// Implement this trait to interface blk-mq as block devices. > /// > @@ -77,7 +78,7 @@ impl OperationsVTable { > let request = unsafe { &*(*bd).rq.cast::>() }; > > // One refcount for the ARef, one for being in flight > - request.wrapper_ref().refcount().store(2, Ordering::Relaxed); > + request.wrapper_ref().refcount().store(2, Relaxed); > > // SAFETY: > // - We own a refcount that we took above. We pass that to `ARef`. > @@ -186,7 +187,7 @@ impl OperationsVTable { > > // SAFETY: The refcount field is allocated but not initialized, so > // it is valid for writes. > - unsafe { > RequestDataWrapper::refcount_ptr(pdu.as_ptr()).write(AtomicU64::new(0)) > }; > + unsafe { > RequestDataWrapper::refcount_ptr(pdu.as_ptr()).write(Atomic::::new(0)) > }; > > Ok(0) > }) > diff --git a/rust/kernel/block/mq/request.rs b/rust/kernel/block/mq/request.rs > index 7943f43b9575..8d4060d65159 100644 > --- a/rust/kernel/block/mq/request.rs > +++ b/rust/kernel/block/mq/request.rs > @@ -13,8 +13,8 @@ > use core::{ > marker::PhantomData, > ptr::{addr_of_mut, NonNull}, > - sync::atomic::{AtomicU64, Ordering}, > }; > +use kernel::sync::atomic::{Atomic, Relaxed}; > > /// A wrapper around a blk-mq [`struct request`]. This represents an > IO request. > /// > @@ -102,8 +102,7 @@ fn try_set_end(this: ARef) -> Result<*mut > bindings::request, ARef> { > if let Err(_old) = this.wrapper_ref().refcount().compare_exchange( > 2, > 0, > - Ordering::Relaxed, > - Ordering::Relaxed, > + Relaxed > ) { > return Err(this); > } > @@ -168,13 +167,13 @@ pub(crate) struct RequestDataWrapper { > /// - 0: The request is owned by C block layer. > /// - 1: The request is owned by Rust abstractions but there are > no [`ARef`] references to it. > /// - 2+: There are [`ARef`] references to the request. > - refcount: AtomicU64, > + refcount: Atomic::, > } > > impl RequestDataWrapper { > /// Return a reference to the refcount of the request that is embedding > /// `self`. > - pub(crate) fn refcount(&self) -> &AtomicU64 { > + pub(crate) fn refcount(&self) -> &Atomic:: { > &self.refcount > } > > @@ -184,7 +183,7 @@ pub(crate) fn refcount(&self) -> &AtomicU64 { > /// # Safety > /// > /// - `this` must point to a live allocation of at least the size > of `Self`. > - pub(crate) unsafe fn refcount_ptr(this: *mut Self) -> *mut AtomicU64 { > + pub(crate) unsafe fn refcount_ptr(this: *mut Self) -> *mut Atomic:: { > // SAFETY: Because of the safety requirements of this function, the > // field projection is safe. > unsafe { addr_of_mut!((*this).refcount) } > @@ -202,28 +201,22 @@ unsafe impl Sync for Request {} > > /// Store the result of `op(target.load())` in target, returning new value of > /// target. > -fn atomic_relaxed_op_return(target: &AtomicU64, op: impl Fn(u64) -> > u64) -> u64 { > - let old = target.fetch_update(Ordering::Relaxed, > Ordering::Relaxed, |x| Some(op(x))); > - > - // SAFETY: Because the operation passed to `fetch_update` above always > - // return `Some`, `old` will always be `Ok`. > - let old = unsafe { old.unwrap_unchecked() }; > - > - op(old) > +fn atomic_relaxed_op_return(target: &Atomic::, op: impl Fn(u64) > -> u64) -> u64 { > + let old = target.load(Relaxed); > + let new_val = op(old); > + target.compare_exchange(old, new_val, Relaxed); > + old > } > > /// Store the result of `op(target.load)` in `target` if `target.load() != > /// pred`, returning [`true`] if the target was updated. > -fn atomic_relaxed_op_unless(target: &AtomicU64, op: impl Fn(u64) -> > u64, pred: u64) -> bool { > - target > - .fetch_update(Ordering::Relaxed, Ordering::Relaxed, |x| { > - if x == pred { > - None > - } else { > - Some(op(x)) > - } > - }) > - .is_ok() > +fn atomic_relaxed_op_unless(target: &Atomic::, op: impl Fn(u64) > -> u64, pred: u64) -> bool { > + let old = target.load(Relaxed); > + if old == pred { > + false > + } else { > + target.compare_exchange(old, op(old), Relaxed).is_ok() > + } > } > > // SAFETY: All instances of `Request` are reference counted. This