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 bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 66038C27C6E for ; Fri, 14 Jun 2024 10:41:12 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:List-Subscribe:List-Help :List-Post:List-Archive:List-Unsubscribe:List-Id:In-Reply-To:Content-Type: MIME-Version:References:Message-ID:Subject:Cc:To:From:Date:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=szOmQCYztl4J9m4j/Llg2C/BpBkQvvulyERZmGvdSRs=; b=d0cB3QvoC/Zkj1QMKrpq8C7Pn7 ToSw1Eev0lCoup/3cvX76bxX1EJTgo+N8C4LOkrBTRrOePJqn9gUvHZt9EUPKhFYDh7QYJwcaLeKE ybGYx3cOkH4fEup1i0XuOLCnShVfXkJaJih8yPmdJvbvyrqxoneBnX83Z8o8RdxoZb0Y1GRnvivrz T/w9N9WaQUoxepAZZP3rJwJlAoTpoinHEGzblnhDvFu8sl4YUquyeMYSbMZhU/brhoYlLdgNE8GoF JEaRZb6EWlRhjWBJ/6EoSCDYMP/CCwLPxPbYflY8TUn1oIWTZEGD+YM4xmcJKefrWwbVe+Gf0U51q ceYgc9Iw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1sI4Md-00000002UOO-2khG; Fri, 14 Jun 2024 10:40:59 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1sI4Ma-00000002UNj-2UDc for linux-arm-kernel@lists.infradead.org; Fri, 14 Jun 2024 10:40:58 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 1476B1480; Fri, 14 Jun 2024 03:41:20 -0700 (PDT) Received: from J2N7QTR9R3.cambridge.arm.com (usa-sjc-imap-foss1.foss.arm.com [10.121.207.14]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id B4E003F5A1; Fri, 14 Jun 2024 03:40:49 -0700 (PDT) Date: Fri, 14 Jun 2024 11:40:47 +0100 From: Mark Rutland To: Boqun Feng Cc: 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 , Gary Guo , =?utf-8?B?QmrDtnJu?= 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, 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: References: <20240612223025.1158537-1-boqun.feng@gmail.com> <20240612223025.1158537-3-boqun.feng@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20240612223025.1158537-3-boqun.feng@gmail.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240614_034056_750967_C62167CC X-CRM114-Status: GOOD ( 27.66 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Wed, Jun 12, 2024 at 03:30:25PM -0700, Boqun Feng wrote: > Provide two atomic types: AtomicI32 and AtomicI64 with the existing > implemenation of C atomics. These atomics have the same semantics of the > corresponding LKMM C atomics, and using one memory (ordering) model > certainly reduces the reasoning difficulty and potential bugs from the > interaction of two different memory models. > > Also bump my role to the maintainer of ATOMIC INFRASTRUCTURE to reflect > my responsiblity on these Rust APIs. > > Note that `Atomic*::new()`s are implemented vi open coding on struct > atomic*_t. This allows `new()` being a `const` function, so that it can > be used in constant contexts. > > Signed-off-by: Boqun Feng I have a few minor comments below. > --- > MAINTAINERS | 4 +- > arch/arm64/kernel/cpufeature.c | 2 + > rust/kernel/sync.rs | 1 + > rust/kernel/sync/atomic.rs | 63 ++ > rust/kernel/sync/atomic/impl.rs | 1375 +++++++++++++++++++++++++++++ > scripts/atomic/gen-atomics.sh | 1 + > scripts/atomic/gen-rust-atomic.sh | 136 +++ > 7 files changed, 1581 insertions(+), 1 deletion(-) > create mode 100644 rust/kernel/sync/atomic.rs > create mode 100644 rust/kernel/sync/atomic/impl.rs > create mode 100755 scripts/atomic/gen-rust-atomic.sh > > diff --git a/MAINTAINERS b/MAINTAINERS > index d6c90161c7bf..a8528d27b260 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -3458,7 +3458,7 @@ F: drivers/input/touchscreen/atmel_mxt_ts.c > ATOMIC INFRASTRUCTURE > M: Will Deacon > M: Peter Zijlstra > -R: Boqun Feng > +M: Boqun Feng > R: Mark Rutland > L: linux-kernel@vger.kernel.org > S: Maintained > @@ -3467,6 +3467,8 @@ F: arch/*/include/asm/atomic*.h > F: include/*/atomic*.h > F: include/linux/refcount.h > F: scripts/atomic/ > +F: rust/kernel/sync/atomic.rs > +F: rust/kernel/sync/atomic/ > > ATTO EXPRESSSAS SAS/SATA RAID SCSI DRIVER > M: Bradley Grove > diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c > index 48e7029f1054..99e6e2b2867f 100644 > --- a/arch/arm64/kernel/cpufeature.c > +++ b/arch/arm64/kernel/cpufeature.c > @@ -1601,6 +1601,8 @@ static bool > has_cpuid_feature(const struct arm64_cpu_capabilities *entry, int scope) > { > u64 val = read_scoped_sysreg(entry, scope); > + if (entry->capability == ARM64_HAS_LSE_ATOMICS) > + return false; > return feature_matches(val, entry); > } As per other replies, this'll obviously need to go. > diff --git a/rust/kernel/sync/atomic.rs b/rust/kernel/sync/atomic.rs > new file mode 100644 > index 000000000000..b0f852cf1741 > --- /dev/null > +++ b/rust/kernel/sync/atomic.rs > @@ -0,0 +1,63 @@ > +// SPDX-License-Identifier: GPL-2.0 > + > +//! Atomic primitives. > +//! > +//! These primitives have the same semantics as their C counterparts, for precise definitions of > +//! the semantics, please refer to tools/memory-model. Note that Linux Kernel Memory (Consistency) > +//! Model is the only model for Rust development in kernel right now, please avoid to use Rust's > +//! own atomics. > + > +use crate::bindings::{atomic64_t, atomic_t}; As with the last patch, why no atomic_long_t? [...] > +#gen_proto_order_variant(meta, pfx, name, sfx, order, atomic, ty, int, raw, arg...) > +gen_proto_order_variant() > +{ > + local meta="$1"; shift > + local pfx="$1"; shift > + local name="$1"; shift > + local sfx="$1"; shift > + local order="$1"; shift > + local atomic="$1"; shift > + local ty="$1"; shift > + local int="$1"; shift > + local raw="$1"; shift > + > + local fn_name="${raw}${pfx}${name}${sfx}${order}" > + local atomicname="${raw}${atomic}_${pfx}${name}${sfx}${order}" > + > + local ret="$(gen_ret_type "${meta}" "${int}")" > + local params="$(gen_params "${int}" $@)" > + local args="$(gen_args "$@")" > + local retstmt="$(gen_ret_stmt "${meta}")" > + > +cat < + /// See \`${atomicname}\`. > + #[inline(always)] > + pub fn ${fn_name}(&self${params}) ${ret}{ > + // SAFETY:\`self.0.get()\` is a valid pointer. > + unsafe { > + ${retstmt}${atomicname}(${args}); > + } > + } > +EOF > +} AFAICT the 'ty' argument (AtomicI32/AtomicI64) isn't used and can be removed. Likewise for 'raw'. > + > +cat << EOF > +// SPDX-License-Identifier: GPL-2.0 > + > +//! Generated by $0 > +//! DO NOT MODIFY THIS FILE DIRECTLY > + > +use super::*; > +use crate::bindings::*; > + > +impl AtomicI32 { > +EOF > + > +grep '^[a-z]' "$1" | while read name meta args; do > + gen_proto "${meta}" "${name}" "atomic" "AtomicI32" "i32" "" ${args} With 'ty' and 'raw' gone, this'd be: gen_proto "${meta}" "${name}" "atomic" "i32" ${args} > +done > + > +cat << EOF > +} > + > +impl AtomicI64 { > +EOF > + > +grep '^[a-z]' "$1" | while read name meta args; do > + gen_proto "${meta}" "${name}" "atomic64" "AtomicI64" "i64" "" ${args} With 'ty' and 'raw' gone, this'd be: gen_proto "${meta}" "${name}" "atomic64" "i64" ${args} Mark. > +done > + > +cat << EOF > +} > + > +EOF > -- > 2.45.2 >