From patchwork Thu Sep 21 21:34:40 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Wedson Almeida Filho X-Patchwork-Id: 143045 Return-Path: Delivered-To: ouuuleilei@gmail.com Received: by 2002:a05:612c:172:b0:3f2:4152:657d with SMTP id h50csp5239793vqi; Thu, 21 Sep 2023 17:52:40 -0700 (PDT) X-Google-Smtp-Source: AGHT+IEQ4jn8E/DNX5RGUNoT/XUGywhq7n0NkPmNeZUBFgqG7rZl26xVbpcq0DglShqwRCM5dvZ8 X-Received: by 2002:a05:6a00:2288:b0:690:cae9:714d with SMTP id f8-20020a056a00228800b00690cae9714dmr7844267pfe.13.1695343960010; Thu, 21 Sep 2023 17:52:40 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1695343959; cv=none; d=google.com; s=arc-20160816; b=JG6RIykCVqRHo/EBAcmHxxzCF1OHmqolUxUg0oAGiVDx87Lw3qfTwlYcP4gbc6ya/I NHtpE2Wntgjp9IaLUoLmzgooRxiYcbLoEdKfhLvyM8GvL54GVUI87SNzylLjLdSv51tT Lz4v14pS/PYkPHMCRfe/rcXPGaOfYTOd7BqsC/PQ40kAYHkLJxtN/+LFVpDvpXhfgP7E ECxYBmCsGjyJvVHv5hLeQt+T0/Aw+98sPbj1uhwJFi4l3bssFVTMHM9waCv27p2xYnX5 7xnJliSweiwnFD/HNsZ1ea1gU9HEt+UJzbLIA7rq6PYsDB/L5qqva/aYXcShHx/m5MFD Dxfg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from :dkim-signature; bh=KaMXIYRdgSMU0GyDTGJxyylDd+HthrxvkGhhTMkKEDY=; fh=BpeH/FUAN0wxLMH3ClwVqRp0rua8Kbn4MIwmp4atozc=; b=nQmL/Imfo1e8ovGMchManvCmLRbivM0EIRhjohpEIfbKO8adUZmh8bsF4ntn85xQY2 ruaz5hd7AJ5PQFVWiCu3oAskyeuQXaI9+9mpmRs7QXFFi6u41UvyJD8zCWdDnYQN7MiC djBuS9KK9HOVFvFv+50VVVQs9/uO08UfpFHfl3o4rtkramLWhlbIJ8u+6/WqgTPVUGT3 W4C+zCK9Cwvh06wqd4Li4IySJ9jqY8DVb+ZpMU2zzof6NW4EEaxJw700iS2hJscxAjf7 xWBZgpUhg7HDqz8Za8AwwuyDb3hNSLKxQ+wMI/GPHQj9Q6u0eaA1nBeA9KLzYDcw61hw YDKw== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@gmail.com header.s=20230601 header.b=RDG7jR7V; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.35 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Received: from groat.vger.email (groat.vger.email. [23.128.96.35]) by mx.google.com with ESMTPS id h12-20020a056a00230c00b0068ff741579fsi2769854pfh.318.2023.09.21.17.52.39 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 21 Sep 2023 17:52:39 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.35 as permitted sender) client-ip=23.128.96.35; Authentication-Results: mx.google.com; dkim=pass header.i=@gmail.com header.s=20230601 header.b=RDG7jR7V; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.35 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Received: from out1.vger.email (depot.vger.email [IPv6:2620:137:e000::3:0]) by groat.vger.email (Postfix) with ESMTP id 8D17E8347B8F; Thu, 21 Sep 2023 15:09:28 -0700 (PDT) X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.103.10 at groat.vger.email Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231865AbjIUWJG (ORCPT + 29 others); Thu, 21 Sep 2023 18:09:06 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:43876 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232946AbjIUWIw (ORCPT ); Thu, 21 Sep 2023 18:08:52 -0400 Received: from mail-pl1-x62e.google.com (mail-pl1-x62e.google.com [IPv6:2607:f8b0:4864:20::62e]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id D2D4329794; Thu, 21 Sep 2023 14:35:06 -0700 (PDT) Received: by mail-pl1-x62e.google.com with SMTP id d9443c01a7336-1c5c91bece9so12423485ad.3; Thu, 21 Sep 2023 14:35:06 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1695332106; x=1695936906; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=KaMXIYRdgSMU0GyDTGJxyylDd+HthrxvkGhhTMkKEDY=; b=RDG7jR7VxvJKGkh7G4iEvNDHLvwwjkzxce57DPRV2DWNSIx64KW79inq/zs7Og3EfR Qe/43M3RCqbhuGPldCs5+gc3WzR+Fcr83FBHraAmnW8ROs4ZyWNOqs20BabOv88rPxlU GyYHtTt572n45l2HNnd35EP2oSVx359va8QZCHe6QtcPFlYxRM/aVp8LZvdBucx0xEmD QEy0O71YdvkfdrMzwjkf67gKd21yOf3Sbn9bEsib5e8VWzvmlHNcq+iU1pAuleQsXZEg 05Thg+dcTTTvlG3+pznr8RWghvIX4jAkXq/oq2j3mBbN0c9Y1jBH6BYzH0uKXfUyU3eb WazA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1695332106; x=1695936906; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=KaMXIYRdgSMU0GyDTGJxyylDd+HthrxvkGhhTMkKEDY=; b=wSOmwj4W5XtShFm0/NwuQQfha487A9OG6PchsQX5jFandGNgHTQLBAQL4Pa4UnwRB1 yX6Os9JxT3cl5nyIPNjzBIpctftrZceqSz13mP+ueEFLwmfMrq+pgomDC1ckwmZbbA5f jns3XZYyBr1zWUpH1pffln8iDjU35Ck4A7zKUue2Q27NczplgTj9Uiozb4NLo8xsLMtQ dN9Okqrd3GJAan1Jk3OAYKPbzzZwKa6IX9BJj5CX0Phbm4rI6pU8yDAtJhqWAnXRoOIH 1Yf2YKxfYu7Qt8ryw20n4mOau0DFx1/+OhrBXV5PLrcs20LOx2UWBapvNaP0io3a4vTx jZEw== X-Gm-Message-State: AOJu0Yz/08QUh9B/v8q1X+sQXT+GWkMBeuT3P3SwPLWCvbjw3XUEHo1s Eaqtv1LwCeo2N3fxSlNkWzEMhcz+ELI= X-Received: by 2002:a17:902:eccb:b0:1c4:62a:e4a with SMTP id a11-20020a170902eccb00b001c4062a0e4amr7503915plh.64.1695332106072; Thu, 21 Sep 2023 14:35:06 -0700 (PDT) Received: from wedsonaf-dev.home.lan ([189.124.190.154]) by smtp.googlemail.com with ESMTPSA id jc6-20020a17090325c600b001c567dcf22dsm2007981plb.281.2023.09.21.14.35.02 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 21 Sep 2023 14:35:05 -0700 (PDT) From: Wedson Almeida Filho To: rust-for-linux@vger.kernel.org Cc: Miguel Ojeda , Alex Gaynor , Boqun Feng , Gary Guo , =?utf-8?q?B?= =?utf-8?q?j=C3=B6rn_Roy_Baron?= , Benno Lossin , Andreas Hindborg , Alice Ryhl , linux-kernel@vger.kernel.org, Wedson Almeida Filho Subject: [PATCH 2/2] rust: arc: remove `ArcBorrow` in favour of `WithRef` Date: Thu, 21 Sep 2023 18:34:40 -0300 Message-Id: <20230921213440.202017-3-wedsonaf@gmail.com> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20230921213440.202017-1-wedsonaf@gmail.com> References: <20230921213440.202017-1-wedsonaf@gmail.com> MIME-Version: 1.0 X-Spam-Status: No, score=-0.6 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS,URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on groat.vger.email Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (groat.vger.email [0.0.0.0]); Thu, 21 Sep 2023 15:09:28 -0700 (PDT) X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1777696987986017021 X-GMAIL-MSGID: 1777696987986017021 From: Wedson Almeida Filho With GATs, we don't need a separate type to represent a borrowed object with a refcount, we can just use Rust's regular shared borrowing. In this case, we use `&WithRef` instead of `ArcBorrow<'_, T>`. Co-developed-by: Boqun Feng Signed-off-by: Boqun Feng Signed-off-by: Wedson Almeida Filho Reviewed-by: Benno Lossin Reviewed-by: Alice Ryhl Reviewed-by: Martin Rodriguez Reboredo Reviewed-by: Gary Guo --- rust/kernel/sync.rs | 2 +- rust/kernel/sync/arc.rs | 180 ++++++++++++++-------------------------- 2 files changed, 62 insertions(+), 120 deletions(-) diff --git a/rust/kernel/sync.rs b/rust/kernel/sync.rs index d219ee518eff..083494884500 100644 --- a/rust/kernel/sync.rs +++ b/rust/kernel/sync.rs @@ -12,7 +12,7 @@ pub mod lock; mod locked_by; -pub use arc::{Arc, ArcBorrow, UniqueArc}; +pub use arc::{Arc, UniqueArc, WithRef}; pub use condvar::CondVar; pub use lock::{mutex::Mutex, spinlock::SpinLock}; pub use locked_by::LockedBy; diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs index 86bff1e0002c..5948e42b9c8f 100644 --- a/rust/kernel/sync/arc.rs +++ b/rust/kernel/sync/arc.rs @@ -105,14 +105,14 @@ /// Coercion from `Arc` to `Arc`: /// /// ``` -/// use kernel::sync::{Arc, ArcBorrow}; +/// use kernel::sync::{Arc, WithRef}; /// /// trait MyTrait { /// // Trait has a function whose `self` type is `Arc`. /// fn example1(self: Arc) {} /// -/// // Trait has a function whose `self` type is `ArcBorrow<'_, Self>`. -/// fn example2(self: ArcBorrow<'_, Self>) {} +/// // Trait has a function whose `self` type is `&WithRef`. +/// fn example2(self: &WithRef) {} /// } /// /// struct Example; @@ -130,9 +130,48 @@ pub struct Arc { _p: PhantomData>, } +/// An instance of `T` with an attached reference count. +/// +/// # Examples +/// +/// ``` +/// use kernel::sync::{Arc, WithRef}; +/// +/// struct Example; +/// +/// fn do_something(e: &WithRef) -> Arc { +/// e.into() +/// } +/// +/// let obj = Arc::try_new(Example)?; +/// let cloned = do_something(obj.as_with_ref()); +/// +/// // Assert that both `obj` and `cloned` point to the same underlying object. +/// assert!(core::ptr::eq(&*obj, &*cloned)); +/// ``` +/// +/// Using `WithRef` as the type of `self`: +/// +/// ``` +/// use kernel::sync::{Arc, WithRef}; +/// +/// struct Example { +/// _a: u32, +/// _b: u32, +/// } +/// +/// impl Example { +/// fn use_reference(self: &WithRef) { +/// // ... +/// } +/// } +/// +/// let obj = Arc::try_new(Example { _a: 10, _b: 20 })?; +/// obj.as_with_ref().use_reference(); +/// ``` #[pin_data] #[repr(C)] -struct WithRef { +pub struct WithRef { refcount: Opaque, data: T, } @@ -215,16 +254,16 @@ unsafe fn from_inner(inner: NonNull>) -> Self { } } - /// Returns an [`ArcBorrow`] from the given [`Arc`]. + /// Returns a [`WithRef`] from the given [`Arc`]. /// - /// This is useful when the argument of a function call is an [`ArcBorrow`] (e.g., in a method - /// receiver), but we have an [`Arc`] instead. Getting an [`ArcBorrow`] is free when optimised. + /// This is useful when the argument of a function call is a [`WithRef`] (e.g., in a method + /// receiver), but we have an [`Arc`] instead. Getting a [`WithRef`] is free when optimised. #[inline] - pub fn as_arc_borrow(&self) -> ArcBorrow<'_, T> { + pub fn as_with_ref(&self) -> &WithRef { // SAFETY: The constraint that the lifetime of the shared reference must outlive that of - // the returned `ArcBorrow` ensures that the object remains alive and that no mutable + // the returned `WithRef` ensures that the object remains alive and that no mutable // reference can be created. - unsafe { ArcBorrow::new(self.ptr) } + unsafe { self.ptr.as_ref() } } /// Compare whether two [`Arc`] pointers reference the same underlying object. @@ -234,20 +273,17 @@ pub fn ptr_eq(this: &Self, other: &Self) -> bool { } impl ForeignOwnable for Arc { - type Borrowed<'a> = ArcBorrow<'a, T>; + type Borrowed<'a> = &'a WithRef; fn into_foreign(self) -> *const core::ffi::c_void { ManuallyDrop::new(self).ptr.as_ptr() as _ } - unsafe fn borrow<'a>(ptr: *const core::ffi::c_void) -> ArcBorrow<'a, T> { + unsafe fn borrow<'a>(ptr: *const core::ffi::c_void) -> &'a WithRef { // SAFETY: By the safety requirement of this function, we know that `ptr` came from - // a previous call to `Arc::into_foreign`. - let inner = NonNull::new(ptr as *mut WithRef).unwrap(); - - // SAFETY: The safety requirements of `from_foreign` ensure that the object remains alive - // for the lifetime of the returned value. - unsafe { ArcBorrow::new(inner) } + // a previous call to `Arc::into_foreign`. The safety requirements of `from_foreign` ensure + // that the object remains alive for the lifetime of the returned value. + unsafe { &*(ptr.cast::>()) } } unsafe fn from_foreign(ptr: *const core::ffi::c_void) -> Self { @@ -320,119 +356,25 @@ fn from(item: Pin>) -> Self { } } -/// A borrowed reference to an [`Arc`] instance. -/// -/// For cases when one doesn't ever need to increment the refcount on the allocation, it is simpler -/// to use just `&T`, which we can trivially get from an `Arc` instance. -/// -/// However, when one may need to increment the refcount, it is preferable to use an `ArcBorrow` -/// over `&Arc` because the latter results in a double-indirection: a pointer (shared reference) -/// to a pointer (`Arc`) to the object (`T`). An [`ArcBorrow`] eliminates this double -/// indirection while still allowing one to increment the refcount and getting an `Arc` when/if -/// needed. -/// -/// # Invariants -/// -/// There are no mutable references to the underlying [`Arc`], and it remains valid for the -/// lifetime of the [`ArcBorrow`] instance. -/// -/// # Example -/// -/// ``` -/// use kernel::sync::{Arc, ArcBorrow}; -/// -/// struct Example; -/// -/// fn do_something(e: ArcBorrow<'_, Example>) -> Arc { -/// e.into() -/// } -/// -/// let obj = Arc::try_new(Example)?; -/// let cloned = do_something(obj.as_arc_borrow()); -/// -/// // Assert that both `obj` and `cloned` point to the same underlying object. -/// assert!(core::ptr::eq(&*obj, &*cloned)); -/// # Ok::<(), Error>(()) -/// ``` -/// -/// Using `ArcBorrow` as the type of `self`: -/// -/// ``` -/// use kernel::sync::{Arc, ArcBorrow}; -/// -/// struct Example { -/// a: u32, -/// b: u32, -/// } -/// -/// impl Example { -/// fn use_reference(self: ArcBorrow<'_, Self>) { -/// // ... -/// } -/// } -/// -/// let obj = Arc::try_new(Example { a: 10, b: 20 })?; -/// obj.as_arc_borrow().use_reference(); -/// # Ok::<(), Error>(()) -/// ``` -pub struct ArcBorrow<'a, T: ?Sized + 'a> { - inner: NonNull>, - _p: PhantomData<&'a ()>, -} - -// This is to allow [`ArcBorrow`] (and variants) to be used as the type of `self`. -impl core::ops::Receiver for ArcBorrow<'_, T> {} - -// This is to allow `ArcBorrow` to be dispatched on when `ArcBorrow` can be coerced into -// `ArcBorrow`. -impl, U: ?Sized> core::ops::DispatchFromDyn> - for ArcBorrow<'_, T> -{ -} - -impl Clone for ArcBorrow<'_, T> { - fn clone(&self) -> Self { - *self - } -} - -impl Copy for ArcBorrow<'_, T> {} - -impl ArcBorrow<'_, T> { - /// Creates a new [`ArcBorrow`] instance. - /// - /// # Safety - /// - /// Callers must ensure the following for the lifetime of the returned [`ArcBorrow`] instance: - /// 1. That `inner` remains valid; - /// 2. That no mutable references to `inner` are created. - unsafe fn new(inner: NonNull>) -> Self { - // INVARIANT: The safety requirements guarantee the invariants. - Self { - inner, - _p: PhantomData, - } - } -} +// This is to allow [`WithRef`] (and variants) to be used as the type of `self`. +impl core::ops::Receiver for WithRef {} -impl From> for Arc { - fn from(b: ArcBorrow<'_, T>) -> Self { +impl From<&WithRef> for Arc { + fn from(b: &WithRef) -> Self { // SAFETY: The existence of `b` guarantees that the refcount is non-zero. `ManuallyDrop` // guarantees that `drop` isn't called, so it's ok that the temporary `Arc` doesn't own the // increment. - ManuallyDrop::new(unsafe { Arc::from_inner(b.inner) }) + ManuallyDrop::new(unsafe { Arc::from_inner(b.into()) }) .deref() .clone() } } -impl Deref for ArcBorrow<'_, T> { +impl Deref for WithRef { type Target = T; fn deref(&self) -> &Self::Target { - // SAFETY: By the type invariant, the underlying object is still alive with no mutable - // references to it, so it is safe to create a shared reference. - unsafe { &self.inner.as_ref().data } + &self.data } }