Currently, the Devres<T> container uses devm_add_action() to register a
devres callback.
devm_add_action() allocates a struct action_devres, which on top of
struct devres_node, just keeps a data pointer and release function
pointer.
This is an unnecessary indirection, given that analogous to struct
devres, the Devres<T> container can just embed a struct devres_node
directly without an additional allocation.
In contrast to struct devres, we don't need to force an alignment of
ARCH_DMA_MINALIGN (as struct devres does to account for the worst case)
since we have generics in Rust. I.e. the compiler already ensures
correct alignment of the embedded T in Devres<T>.
Thus, get rid of devm_add_action() and instead embed a struct
devres_node directly.
Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
rust/kernel/devres.rs | 182 +++++++++++++++++++++++++++++++-----------
1 file changed, 135 insertions(+), 47 deletions(-)
diff --git a/rust/kernel/devres.rs b/rust/kernel/devres.rs
index 6afe196be42c..4c1efad758c7 100644
--- a/rust/kernel/devres.rs
+++ b/rust/kernel/devres.rs
@@ -23,9 +23,22 @@
rcu,
Arc, //
},
- types::ForeignOwnable,
+ types::{
+ ForeignOwnable,
+ Opaque, //
+ },
};
+/// Inner type that embeds a `struct devres_node` and the `Revocable<T>`.
+#[repr(C)]
+#[pin_data]
+struct Inner<T> {
+ #[pin]
+ node: Opaque<bindings::devres_node>,
+ #[pin]
+ data: Revocable<T>,
+}
+
/// This abstraction is meant to be used by subsystems to containerize [`Device`] bound resources to
/// manage their lifetime.
///
@@ -111,12 +124,63 @@
/// ```
pub struct Devres<T: Send> {
dev: ARef<Device>,
- /// Pointer to [`Self::devres_callback`].
- ///
- /// Has to be stored, since Rust does not guarantee to always return the same address for a
- /// function. However, the C API uses the address as a key.
- callback: unsafe extern "C" fn(*mut c_void),
- data: Arc<Revocable<T>>,
+ inner: Arc<Inner<T>>,
+}
+
+// Calling the FFI functions from the `base` module directly from the `Devres<T>` impl may result in
+// them being called directly from driver modules. This happens since the Rust compiler will use
+// monomorphisation, so it might happen that functions are instantiated within the calling driver
+// module. For now, work around this with `#[inline(never)]` helpers.
+//
+// TODO: Remove once a more generic solution has been implemented. For instance, we may be able to
+// leverage `bindgen` to take care of this depending on whether a symbol is (already) exported.
+mod base {
+ use kernel::{
+ bindings,
+ prelude::*, //
+ };
+
+ #[inline(never)]
+ #[allow(clippy::missing_safety_doc)]
+ pub(super) unsafe fn devres_node_init(
+ node: *mut bindings::devres_node,
+ release: bindings::dr_node_release_t,
+ free: bindings::dr_node_free_t,
+ ) {
+ // SAFETY: Safety requirements are the same as `bindings::devres_node_init`.
+ unsafe { bindings::devres_node_init(node, release, free) }
+ }
+
+ #[inline(never)]
+ #[allow(clippy::missing_safety_doc)]
+ pub(super) unsafe fn devres_set_node_dbginfo(
+ node: *mut bindings::devres_node,
+ name: *const c_char,
+ size: usize,
+ ) {
+ // SAFETY: Safety requirements are the same as `bindings::devres_set_node_dbginfo`.
+ unsafe { bindings::devres_set_node_dbginfo(node, name, size) }
+ }
+
+ #[inline(never)]
+ #[allow(clippy::missing_safety_doc)]
+ pub(super) unsafe fn devres_node_add(
+ dev: *mut bindings::device,
+ node: *mut bindings::devres_node,
+ ) {
+ // SAFETY: Safety requirements are the same as `bindings::devres_node_add`.
+ unsafe { bindings::devres_node_add(dev, node) }
+ }
+
+ #[inline(never)]
+ #[allow(clippy::missing_safety_doc)]
+ pub(super) unsafe fn devres_node_remove(
+ dev: *mut bindings::device,
+ node: *mut bindings::devres_node,
+ ) -> bool {
+ // SAFETY: Safety requirements are the same as `bindings::devres_node_remove`.
+ unsafe { bindings::devres_node_remove(dev, node) }
+ }
}
impl<T: Send> Devres<T> {
@@ -128,58 +192,82 @@ pub fn new<E>(dev: &Device<Bound>, data: impl PinInit<T, E>) -> Result<Self>
where
Error: From<E>,
{
- let callback = Self::devres_callback;
- let data = Arc::pin_init(Revocable::new(data), GFP_KERNEL)?;
- let devres_data = data.clone();
+ let inner = Arc::pin_init::<Error>(
+ try_pin_init!(Inner {
+ node <- Opaque::ffi_init(|node: *mut bindings::devres_node| {
+ // SAFETY: `node` is a valid pointer to an uninitialized `struct devres_node`.
+ unsafe {
+ base::devres_node_init(
+ node,
+ Some(Self::devres_node_release),
+ Some(Self::devres_node_free_node),
+ )
+ };
+
+ // SAFETY: `node` is a valid pointer to an uninitialized `struct devres_node`.
+ unsafe {
+ base::devres_set_node_dbginfo(
+ node,
+ // TODO: Use `core::any::type_name::<T>()` once we are able to convert
+ // the `&str` to a NULL terminated `&CStr` without additional
+ // allocation.
+ c"Devres<T>".as_char_ptr(),
+ core::mem::size_of::<Revocable<T>>(),
+ )
+ };
+ }),
+ data <- Revocable::new(data),
+ }),
+ GFP_KERNEL,
+ )?;
// SAFETY:
- // - `dev.as_raw()` is a pointer to a valid bound device.
- // - `data` is guaranteed to be a valid for the duration of the lifetime of `Self`.
- // - `devm_add_action()` is guaranteed not to call `callback` for the entire lifetime of
- // `dev`.
- to_result(unsafe {
- bindings::devm_add_action(
- dev.as_raw(),
- Some(callback),
- Arc::as_ptr(&data).cast_mut().cast(),
- )
- })?;
-
- // `devm_add_action()` was successful and has consumed the reference count.
- core::mem::forget(devres_data);
+ // - `dev` is a valid pointer to a bound `struct device`.
+ // - `node` is a valid pointer to a `struct devres_node`.
+ // - `devres_node_add()` is guaranteed not to call `devres_node_release()` for the entire
+ // lifetime of `dev`.
+ unsafe { base::devres_node_add(dev.as_raw(), inner.node.get()) };
+
+ // Take additional reference count for `devres_node_add()`.
+ core::mem::forget(inner.clone());
Ok(Self {
dev: dev.into(),
- callback,
- data,
+ inner,
})
}
fn data(&self) -> &Revocable<T> {
- &self.data
+ &self.inner.data
}
#[allow(clippy::missing_safety_doc)]
- unsafe extern "C" fn devres_callback(ptr: *mut kernel::ffi::c_void) {
- // SAFETY: In `Self::new` we've passed a valid pointer of `Revocable<T>` to
- // `devm_add_action()`, hence `ptr` must be a valid pointer to `Revocable<T>`.
- let data = unsafe { Arc::from_raw(ptr.cast::<Revocable<T>>()) };
+ unsafe extern "C" fn devres_node_release(
+ _dev: *mut bindings::device,
+ node: *mut bindings::devres_node,
+ ) {
+ let node = Opaque::cast_from(node);
+
+ // SAFETY: `node` is in the same allocation as its container.
+ let inner = unsafe { kernel::container_of!(node, Inner<T>, node) };
+
+ // SAFETY: `inner` is a valid `Inner<T>` pointer.
+ let inner = unsafe { &*inner };
- data.revoke();
+ inner.data.revoke();
+ }
+
+ #[allow(clippy::missing_safety_doc)]
+ unsafe extern "C" fn devres_node_free_node(node: *mut bindings::devres_node) {
+ // SAFETY: `node` points to the entire `Inner<T>` allocation.
+ drop(unsafe { Arc::from_raw(node.cast::<Inner<T>>()) });
}
- fn remove_action(&self) -> bool {
+ fn remove_node(&self) -> bool {
// SAFETY:
- // - `self.dev` is a valid `Device`,
- // - the `action` and `data` pointers are the exact same ones as given to
- // `devm_add_action()` previously,
- (unsafe {
- bindings::devm_remove_action_nowarn(
- self.dev.as_raw(),
- Some(self.callback),
- core::ptr::from_ref(self.data()).cast_mut().cast(),
- )
- } == 0)
+ // - `self.device().as_raw()` is a valid pointer to a bound `struct device`.
+ // - `self.inner.node.get()` is a valid pointer to a `struct devres_node`.
+ unsafe { base::devres_node_remove(self.device().as_raw(), self.inner.node.get()) }
}
/// Return a reference of the [`Device`] this [`Devres`] instance has been created with.
@@ -261,12 +349,12 @@ fn drop(&mut self) {
// SAFETY: When `drop` runs, it is guaranteed that nobody is accessing the revocable data
// anymore, hence it is safe not to wait for the grace period to finish.
if unsafe { self.data().revoke_nosync() } {
- // We revoked `self.data` before the devres action did, hence try to remove it.
- if self.remove_action() {
+ // We revoked `self.data` before devres did, hence try to remove it.
+ if self.remove_node() {
// SAFETY: In `Self::new` we have taken an additional reference count of `self.data`
- // for `devm_add_action()`. Since `remove_action()` was successful, we have to drop
+ // for `devres_node_add()`. Since `remove_node()` was successful, we have to drop
// this additional reference count.
- drop(unsafe { Arc::from_raw(Arc::as_ptr(&self.data)) });
+ drop(unsafe { Arc::from_raw(Arc::as_ptr(&self.inner)) });
}
}
}
--
2.53.0On Fri, Feb 13, 2026 at 11:07:15PM +0100, Danilo Krummrich wrote:
> Currently, the Devres<T> container uses devm_add_action() to register a
> devres callback.
>
> devm_add_action() allocates a struct action_devres, which on top of
> struct devres_node, just keeps a data pointer and release function
> pointer.
>
> This is an unnecessary indirection, given that analogous to struct
> devres, the Devres<T> container can just embed a struct devres_node
> directly without an additional allocation.
>
> In contrast to struct devres, we don't need to force an alignment of
> ARCH_DMA_MINALIGN (as struct devres does to account for the worst case)
> since we have generics in Rust. I.e. the compiler already ensures
> correct alignment of the embedded T in Devres<T>.
>
> Thus, get rid of devm_add_action() and instead embed a struct
> devres_node directly.
>
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
> /// This abstraction is meant to be used by subsystems to containerize [`Device`] bound resources to
> /// manage their lifetime.
> ///
> @@ -111,12 +124,63 @@
> /// ```
> pub struct Devres<T: Send> {
> dev: ARef<Device>,
Wouldn't it be nicer to move this into the Arc? Most of the time, it
would probably fit in the kmalloc bucket without enlarging it, so it
seems like that would be better for most scenarios.
> +// Calling the FFI functions from the `base` module directly from the `Devres<T>` impl may result in
> +// them being called directly from driver modules. This happens since the Rust compiler will use
> +// monomorphisation, so it might happen that functions are instantiated within the calling driver
> +// module. For now, work around this with `#[inline(never)]` helpers.
> +//
> +// TODO: Remove once a more generic solution has been implemented. For instance, we may be able to
> +// leverage `bindgen` to take care of this depending on whether a symbol is (already) exported.
I'm not sure what a generic solution would look like. There is an
assumption that if A can call B and B can call C, then A can call C.
Other than just exporting the C methods, possibly for Rust only?
What happens in C when you turn on LTO and core kernel methods get
inlined into modules?
> + #[inline(never)]
> + #[allow(clippy::missing_safety_doc)]
> + pub(super) unsafe fn devres_node_remove(
> + dev: *mut bindings::device,
> + node: *mut bindings::devres_node,
> + ) -> bool {
Since you're taking the opportunity to wrap these, I'd consider adding
#[must_use] here.
> + // SAFETY: `node` is a valid pointer to an uninitialized `struct devres_node`.
> + unsafe {
> + base::devres_set_node_dbginfo(
> + node,
> + // TODO: Use `core::any::type_name::<T>()` once we are able to convert
> + // the `&str` to a NULL terminated `&CStr` without additional
> + // allocation.
> + c"Devres<T>".as_char_ptr(),
The problem is that type_name() is not const. We can convert a const
&str to &CStr with no problems already.
Alice
© 2016 - 2026 Red Hat, Inc.