From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-4316.protonmail.ch (mail-4316.protonmail.ch [185.70.43.16]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 8D0F47A715; Wed, 19 Jun 2024 09:30:23 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=185.70.43.16 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1718789425; cv=none; b=DZEByS6vcgfq6EkmvJTjHhdmSGG50jHHjJRvaLKk8G9ES8Q35T5T8Di08sgaM8Q872F6wqooYJl98vMe/pfjirUfITDJLegkm0630NmsGoduMWy+MsLB3WVseBNHvzA24Y9VD73hs6AQmsJ2htmQZUIobz3l2kY82eDFuqBtYzI= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1718789425; c=relaxed/simple; bh=7ne4ekeJPSiu9Wa/xWvMdy+2iJetKy2FB72LpR8ncLY=; h=Date:To:From:Cc:Subject:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=YBK3A/xHARiogUJ1Cv+fiFSpnX6YGa2WtEVTGTMVb7ZNJHgH7+ZiR6dI2PebGoS7IgDRQXPIwrFWphW4bTY3fqvEXelj09LCotFB2dBz/+HI2Y3LJr3jRp2NoNksYKnzbA4QofP873GxHlpDHOWw5fWtC/gkxe0w9Qc2RohrUmM= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=proton.me; spf=pass smtp.mailfrom=proton.me; dkim=pass (2048-bit key) header.d=proton.me header.i=@proton.me header.b=cSbHonGn; arc=none smtp.client-ip=185.70.43.16 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=proton.me Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=proton.me Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=proton.me header.i=@proton.me header.b="cSbHonGn" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=proton.me; s=protonmail; t=1718789420; x=1719048620; bh=zV1Qzzj1i+vpiKHlTH9zT9EGlrl+KcXNNmM1imA2OVs=; h=Date:To:From:Cc:Subject:Message-ID:In-Reply-To:References: Feedback-ID:From:To:Cc:Date:Subject:Reply-To:Feedback-ID: Message-ID:BIMI-Selector; b=cSbHonGnWaJH37f1v+pSaKUQwYqlPPs+VtxX+8Sx2EZa4gfkG6EbZpEFL1ac6AJSJ +zm/edwto6ej3rtZoDCxktTyvEy3KwzMLuGps4gixWJId4+eIiKjM/YMAhuD6RbdTa j2RiXtPvBlCrR2Ag+uGa4Vgwq0/I7AWttYOmTjke/x5q7y7xM99zlCRi47Or+OqD4B BNKE8/RAyZk2+tGreTkAu5UwsC5NQJ1h9HMGsX3AIPaP33fOeRgCQa+6byjK08Ys5d EoE5c2uLBWYZcaqYKWiyoYziz+AMut+hOdm0vJt8NWQ+16u6mtF0WYN3hqgQ6OQor7 e5Z3mWOu+o96A== Date: Wed, 19 Jun 2024 09:30:15 +0000 To: Boqun Feng , Gary Guo From: Benno Lossin Cc: John Hubbard , Miguel Ojeda , rust-for-linux@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arch@vger.kernel.org, llvm@lists.linux.dev, Miguel Ojeda , Alex Gaynor , Wedson Almeida Filho , =?utf-8?Q?Bj=C3=B6rn_Roy_Baron?= , 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 Subject: Re: [RFC 2/2] rust: sync: Add atomic support Message-ID: <8f23cb56-91a9-4515-a14f-4b7de70f6852@proton.me> In-Reply-To: References: <79239550-dd6e-4738-acea-e7df50176487@nvidia.com> <20240616155145.54371240.gary@garyguo.net> Feedback-ID: 71780778:user:proton X-Pm-Message-ID: 11698e3de8f944b2217c782e5cf4abe605702990 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=utf-8 Content-Transfer-Encoding: quoted-printable On 17.06.24 07:36, Boqun Feng wrote: > On Sun, Jun 16, 2024 at 08:06:05AM -0700, Boqun Feng wrote: > [...] >>> >>> Note that crossbeam's AtomicCell is also generic, and crossbeam is used >>> by tons of crates. As Miguel mentioned, I think it's very likely that i= n >>> the future we want be able to do atomics on new types (e.g. for >>> seqlocks perhaps). We probably don't need the non-lock-free fallback of >> >> Good, another design bit, thank you! >> >> What's our overall idea on sub-word types, like Atomic and >> Atomic, do we plan to say no to them, or they could have a limited >> APIs? IIUC, some operations on them are relatively sub-optimal on some >> architectures, supporting the same set of API as i32 and i64 is probably >> a bad idea. >> >> Another thing in my mind is making `Atomic` >> >> =09pub struct Atomic { ... } >> >> so that `Atomic` will always be `Sync`, because quite frankly, an >> atomic type that cannot `Sync` is pointless. That is true, but adding semantically "unnecessary" bounds can be bad. This is because they infect everything that wants to use `Atomic`, since they also need to add that bound. > Also, how do we avoid this issue [1] in kernel? I think that we can first go the way of my second approach (ie adding a private trait as a bound on `Atomic` to prevent generic usage). And only allow primitives. If we then see that people would like to put their own (u8, u16) tuple structs into `Atomic`, we have multiple options: 1. Field projection: Only primitives can be `load`ed and `store`ed, to access the values of the tuple, one would need to project to each field and read them. 2. Disallow padding: We add an `unsafe` trait that asserts there are no padding bytes in there (like `NoUinit` from below) and also add a macro that implements the trait safely. 3. Use `MaybeUninit` under the hood: I don't know if this would fix the issue entirely, since that is what crossbeam currently uses (but the issue remains open). But I don't think that we should encourage large structs to be put into `Atomic`, since that would be bad for perf, right? So I think that going the way of 1 would be great (if we had FP, otherwise 2 seems fine). > `atomic_load()` in C is implemented as READ_ONCE() and it's, at most > time, a volatile read, so the eventual code is: >=20 > let a: (u8, u16) =3D (1, 2); > let b =3D unsafe { core::ptr::read_volatile::(&a as *const _ as = *const i32) }; >=20 > I know we probably ignore data race here and treat `read_volatile` as a > dependency read per LKMM [2]. But this is an using of uninitialized > data, so it's a bit different. But would we implement it this way? Or would it go through a C function? If we entirely do it in Rust, then yes this is bad. --- Cheers, Benno > We can do what https://crates.io/crates/atomic does: >=20 > =09pub struct Atomic { ... } >=20 > , where `NoUinit` means no internal padding bytes, but it loses the > ability to put a >=20 > =09#[repr(u32)] > =09pub enum Foo { .. } >=20 > into `Atomic`, right? Which is probably a case you want to support? >=20 > Regards, > Boqun >=20 > [1]: https://github.com/crossbeam-rs/crossbeam/issues/748#issuecomment-11= 33926617 > [2]: tools/memory-model/Documentation/access-marking.txt