From nobody Thu Oct 9 02:15:29 2025 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (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 94CDA2AE8E; Sun, 22 Jun 2025 16:41:30 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1750610490; cv=none; b=K+ZWBpG1CcXp25zPM/a4CYlSt8FiHGnuEHAICNwOCpDKI5qWgL/fY0RkUT3hK0Ye52gQXfqERXwgJ0NGeoCZQGeQYAyEyuESUmKITspalZmH6diRTdGgNQnWRi0uh+1ihR0tF7CxHBuszeMJGE0q69B2zfLwXBaBEd9sVip6R/I= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1750610490; c=relaxed/simple; bh=qd4m+pRuwBT7Lu3gw5UGth4bTaVLrbANlY/hRPsaidQ=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=DV/MfXj7rDlGLSL/O7Hr7vPmJvySa/gxnz08zTpUpZ8A1VBsXRP/bJyhc5q933cr3qfrJ3v9uvlvOYMOVhxP1NBw5o+nJwgjI0z1bF9TrDn+3CWCRmE9eolFKEapIMGIVlI60eDGIRK9gYekI2NQHbrAcRE/0iTd3hQAFxndEV8= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=W+62q8nQ; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="W+62q8nQ" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 9EAFBC4CEF1; Sun, 22 Jun 2025 16:41:25 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1750610490; bh=qd4m+pRuwBT7Lu3gw5UGth4bTaVLrbANlY/hRPsaidQ=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=W+62q8nQUI17F5oF98Yvcj/CC+uM199E6m8Lox7oQ/CfndPAiOX+QGiZDDeimjCNj qzhp0lOjOf/TABucF8NCeWSuvzNoQw8cAro1OtrMespJNYBNmbInbFe8RLPaklTAbD D0plLnCSR+wUeEKGODnQI+2M/bMN2JfNlgU0EytUXw6CrWP8p7mw5RX1MbgwzsLo8/ btrfti0glsvg6XgrT+kbG+ETODdyFgR54IDlnWRKSFej82Ppvvm7U8viOMotHPBc1a yYjn5F6dcCZrKpX0qoQPqbUCnJGGD49uDvYAZxOZ8mPuWZ/4Z55GTDN3d+qQQpQ+Ef iAQNgb3rqXlKw== From: Danilo Krummrich To: gregkh@linuxfoundation.org, rafael@kernel.org, ojeda@kernel.org, alex.gaynor@gmail.com, boqun.feng@gmail.com, gary@garyguo.net, bjorn3_gh@protonmail.com, lossin@kernel.org, a.hindborg@kernel.org, aliceryhl@google.com, tmgross@umich.edu, david.m.ertman@intel.com, ira.weiny@intel.com, leon@kernel.org, kwilczynski@kernel.org, bhelgaas@google.com Cc: rust-for-linux@vger.kernel.org, linux-kernel@vger.kernel.org, linux-pci@vger.kernel.org, Danilo Krummrich , Dave Airlie , Simona Vetter , Viresh Kumar Subject: [PATCH v2 2/4] rust: devres: replace Devres::new_foreign_owned() Date: Sun, 22 Jun 2025 18:40:39 +0200 Message-ID: <20250622164050.20358-3-dakr@kernel.org> X-Mailer: git-send-email 2.49.0 In-Reply-To: <20250622164050.20358-1-dakr@kernel.org> References: <20250622164050.20358-1-dakr@kernel.org> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="utf-8" Replace Devres::new_foreign_owned() with devres::register(). The current implementation of Devres::new_foreign_owned() creates a full Devres container instance, including the internal Revocable and completion. However, none of that is necessary for the intended use of giving full ownership of an object to devres and getting it dropped once the given device is unbound. Hence, implement devres::register(), which is limited to consume the given data, wrap it in a KBox and drop the KBox once the given device is unbound, without any other synchronization. Cc: Dave Airlie Cc: Simona Vetter Cc: Viresh Kumar Acked-by: Viresh Kumar Signed-off-by: Danilo Krummrich Reviewed-by: Alice Ryhl Reviewed-by: Benno Lossin --- rust/helpers/device.c | 7 ++++ rust/kernel/cpufreq.rs | 11 +++--- rust/kernel/devres.rs | 70 +++++++++++++++++++++++++++++++++------ rust/kernel/drm/driver.rs | 14 ++++---- 4 files changed, 82 insertions(+), 20 deletions(-) diff --git a/rust/helpers/device.c b/rust/helpers/device.c index b2135c6686b0..502fef7e9ae8 100644 --- a/rust/helpers/device.c +++ b/rust/helpers/device.c @@ -8,3 +8,10 @@ int rust_helper_devm_add_action(struct device *dev, { return devm_add_action(dev, action, data); } + +int rust_helper_devm_add_action_or_reset(struct device *dev, + void (*action)(void *), + void *data) +{ + return devm_add_action_or_reset(dev, action, data); +} diff --git a/rust/kernel/cpufreq.rs b/rust/kernel/cpufreq.rs index 11b03e9d7e89..dd84e2b4d7ae 100644 --- a/rust/kernel/cpufreq.rs +++ b/rust/kernel/cpufreq.rs @@ -13,7 +13,7 @@ cpu::CpuId, cpumask, device::{Bound, Device}, - devres::Devres, + devres, error::{code::*, from_err_ptr, from_result, to_result, Result, VTABLE_= DEFAULT_ERROR}, ffi::{c_char, c_ulong}, prelude::*, @@ -1046,10 +1046,13 @@ pub fn new() -> Result { =20 /// Same as [`Registration::new`], but does not return a [`Registratio= n`] instance. /// - /// Instead the [`Registration`] is owned by [`Devres`] and will be re= voked / dropped, once the + /// Instead the [`Registration`] is owned by [`devres::register`] and = will be dropped, once the /// device is detached. - pub fn new_foreign_owned(dev: &Device) -> Result { - Devres::new_foreign_owned(dev, Self::new()?, GFP_KERNEL) + pub fn new_foreign_owned(dev: &Device) -> Result + where + T: 'static, + { + devres::register(dev, Self::new()?, GFP_KERNEL) } } =20 diff --git a/rust/kernel/devres.rs b/rust/kernel/devres.rs index 544e50efab43..250073749279 100644 --- a/rust/kernel/devres.rs +++ b/rust/kernel/devres.rs @@ -9,12 +9,12 @@ alloc::Flags, bindings, device::{Bound, Device}, - error::{Error, Result}, + error::{to_result, Error, Result}, ffi::c_void, prelude::*, revocable::{Revocable, RevocableGuard}, sync::{rcu, Arc, Completion}, - types::ARef, + types::{ARef, ForeignOwnable}, }; =20 #[pin_data] @@ -184,14 +184,6 @@ pub fn new(dev: &Device, data: T, flags: Flags)= -> Result { Ok(Devres(inner)) } =20 - /// Same as [`Devres::new`], but does not return a `Devres` instance. = Instead the given `data` - /// is owned by devres and will be revoked / dropped, once the device = is detached. - pub fn new_foreign_owned(dev: &Device, data: T, flags: Flags) -= > Result { - let _ =3D DevresInner::new(dev, data, flags)?; - - Ok(()) - } - /// Obtain `&'a T`, bypassing the [`Revocable`]. /// /// This method allows to directly obtain a `&'a T`, bypassing the [`R= evocable`], by presenting @@ -261,3 +253,61 @@ fn drop(&mut self) { } } } + +/// Consume `data` and [`Drop::drop`] `data` once `dev` is unbound. +fn register_foreign(dev: &Device, data: P) -> Re= sult { + let ptr =3D data.into_foreign(); + + #[allow(clippy::missing_safety_doc)] + unsafe extern "C" fn callback(ptr: *mut kernel::ffi= ::c_void) { + // SAFETY: `ptr` is the pointer to the `ForeignOwnable` leaked abo= ve and hence valid. + let _ =3D unsafe { P::from_foreign(ptr.cast()) }; + } + + // SAFETY: + // - `dev.as_raw()` is a pointer to a valid and bound device. + // - `ptr` is a valid pointer the `ForeignOwnable` devres takes owners= hip of. + to_result(unsafe { + // `devm_add_action_or_reset()` also calls `callback` on failure, = such that the + // `ForeignOwnable` is released eventually. + bindings::devm_add_action_or_reset(dev.as_raw(), Some(callback::), ptr.cast()) + }) +} + +/// Encapsulate `data` in a [`KBox`] and [`Drop::drop`] `data` once `dev` = is unbound. +/// +/// # Examples +/// +/// ```no_run +/// use kernel::{device::{Bound, Device}, devres}; +/// +/// /// Registration of e.g. a class device, IRQ, etc. +/// struct Registration; +/// +/// impl Registration { +/// fn new() -> Self { +/// // register +/// +/// Self +/// } +/// } +/// +/// impl Drop for Registration { +/// fn drop(&mut self) { +/// // unregister +/// } +/// } +/// +/// fn from_bound_context(dev: &Device) -> Result { +/// devres::register(dev, Registration::new(), GFP_KERNEL) +/// } +/// ``` +pub fn register(dev: &Device, data: impl PinInit, flags= : Flags) -> Result +where + T: 'static, + Error: From, +{ + let data =3D KBox::pin_init(data, flags)?; + + register_foreign(dev, data) +} diff --git a/rust/kernel/drm/driver.rs b/rust/kernel/drm/driver.rs index acb638086131..f63addaf7235 100644 --- a/rust/kernel/drm/driver.rs +++ b/rust/kernel/drm/driver.rs @@ -5,9 +5,7 @@ //! C header: [`include/linux/drm/drm_drv.h`](srctree/include/linux/drm/dr= m_drv.h) =20 use crate::{ - bindings, device, - devres::Devres, - drm, + bindings, device, devres, drm, error::{to_result, Result}, prelude::*, str::CStr, @@ -130,18 +128,22 @@ fn new(drm: &drm::Device, flags: usize) -> Result<= Self> { } =20 /// Same as [`Registration::new`}, but transfers ownership of the [`Re= gistration`] to - /// [`Devres`]. + /// [`devres::register`]. pub fn new_foreign_owned( drm: &drm::Device, dev: &device::Device, flags: usize, - ) -> Result { + ) -> Result + where + T: 'static, + { if drm.as_ref().as_raw() !=3D dev.as_raw() { return Err(EINVAL); } =20 let reg =3D Registration::::new(drm, flags)?; - Devres::new_foreign_owned(dev, reg, GFP_KERNEL) + + devres::register(dev, reg, GFP_KERNEL) } =20 /// Returns a reference to the `Device` instance for this registration. --=20 2.49.0