On Thu, 19 Dec 2024 18:04:10 +0100
Danilo Krummrich <dakr@xxxxxxxxxx> wrote:
> Add a Rust abstraction for the kernel's devres (device resource
> management) implementation.
>
> The Devres type acts as a container to manage the lifetime and
> accessibility of device bound resources. Therefore it registers a
> devres callback and revokes access to the resource on invocation.
>
> Users of the Devres abstraction can simply free the corresponding
> resources in their Drop implementation, which is invoked when either the
> Devres instance goes out of scope or the devres callback leads to the
> resource being revoked, which implies a call to drop_in_place().
>
> Signed-off-by: Danilo Krummrich <dakr@xxxxxxxxxx>
> ---
> MAINTAINERS | 1 +
> rust/helpers/device.c | 10 +++
> rust/helpers/helpers.c | 1 +
> rust/kernel/devres.rs | 178 +++++++++++++++++++++++++++++++++++++++++
> rust/kernel/lib.rs | 1 +
> 5 files changed, 191 insertions(+)
> create mode 100644 rust/helpers/device.c
> create mode 100644 rust/kernel/devres.rs
>
> <snip>
>
> +pub struct Devres<T>(Arc<DevresInner<T>>);
> +
> +impl<T> DevresInner<T> {
> + fn new(dev: &Device, data: T, flags: Flags) -> Result<Arc<DevresInner<T>>> {
> + let inner = Arc::pin_init(
> + pin_init!( DevresInner {
> + data <- Revocable::new(data),
> + }),
> + flags,
> + )?;
> +
> + // Convert `Arc<DevresInner>` into a raw pointer and make devres own this reference until
> + // `Self::devres_callback` is called.
> + let data = inner.clone().into_raw();
> +
> + // SAFETY: `devm_add_action` guarantees to call `Self::devres_callback` once `dev` is
> + // detached.
> + let ret = unsafe {
> + bindings::devm_add_action(dev.as_raw(), Some(Self::devres_callback), data as _)
> + };
> +
> + if ret != 0 {
> + // SAFETY: We just created another reference to `inner` in order to pass it to
> + // `bindings::devm_add_action`. If `bindings::devm_add_action` fails, we have to drop
> + // this reference accordingly.
> + let _ = unsafe { Arc::from_raw(data) };
> + return Err(Error::from_errno(ret));
> + }
> +
> + Ok(inner)
> + }
> +
> + #[allow(clippy::missing_safety_doc)]
> + unsafe extern "C" fn devres_callback(ptr: *mut kernel::ffi::c_void) {
> + let ptr = ptr as *mut DevresInner<T>;
> + // Devres owned this memory; now that we received the callback, drop the `Arc` and hence the
> + // reference.
> + // SAFETY: Safe, since we leaked an `Arc` reference to devm_add_action() in
> + // `DevresInner::new`.
> + let inner = unsafe { Arc::from_raw(ptr) };
> +
> + inner.data.revoke();
> + }
> +}
> +
> +impl<T> Devres<T> {
> + /// Creates a new [`Devres`] instance of the given `data`. The `data` encapsulated within the
> + /// returned `Devres` instance' `data` will be revoked once the device is detached.
> + pub fn new(dev: &Device, data: T, flags: Flags) -> Result<Self> {
> + let inner = DevresInner::new(dev, data, flags)?;
> +
> + Ok(Devres(inner))
> + }
> +
> + /// 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 _ = DevresInner::new(dev, data, flags)?;
> +
> + Ok(())
> + }
> +}
> +
> +impl<T> Deref for Devres<T> {
> + type Target = Revocable<T>;
> +
> + fn deref(&self) -> &Self::Target {
> + &self.0.data
> + }
> +}
> +
> +impl<T> Drop for Devres<T> {
> + fn drop(&mut self) {
> + // Revoke the data, such that it gets dropped already and the actual resource is freed.
> + //
> + // `DevresInner` has to stay alive until the devres callback has been called. This is
> + // necessary since we don't know when `Devres` is dropped and calling
> + // `devm_remove_action()` instead could race with `devres_release_all()`.
IIUC, the outcome of that race is the `WARN` if
devres_release_all takes the spinlock first and has already remvoed the
action?
Could you do a custom devres_release here that mimick
`devm_remove_action` but omit the `WARN`? This way it allows the memory
behind DevresInner to be freed early without keeping it allocated until
the end of device lifetime.
> + //
> + // SAFETY: When `drop` runs, it's guaranteed that nobody is accessing the revocable data
> + // anymore, hence it is safe not to wait for the grace period to finish.
> + unsafe { self.revoke_nosync() };
> + }
> +}
> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> index 6c836ab73771..2b61bf99d1ee 100644
> --- a/rust/kernel/lib.rs
> +++ b/rust/kernel/lib.rs
> @@ -41,6 +41,7 @@
> pub mod cred;
> pub mod device;
> pub mod device_id;
> +pub mod devres;
> pub mod driver;
> pub mod error;
> #[cfg(CONFIG_RUST_FW_LOADER_ABSTRACTIONS)]
