On Mon, May 20, 2024 at 07:25:38PM +0200, Danilo Krummrich wrote:
> Add an (always) reference counted abstraction for a generic struct
> device. This abstraction encapsulates existing struct device instances
> and manages its reference count.
>
> Subsystems may use this abstraction as a base to abstract subsystem
> specific device instances based on a generic struct device.
>
> Co-developed-by: Wedson Almeida Filho <wedsonaf@xxxxxxxxx>
> Signed-off-by: Wedson Almeida Filho <wedsonaf@xxxxxxxxx>
> Signed-off-by: Danilo Krummrich <dakr@xxxxxxxxxx>
> ---
> rust/helpers.c | 1 +
> rust/kernel/device.rs | 76 +++++++++++++++++++++++++++++++++++++++++++
What's the status of moving .rs files next to their respective .c files
in the build system? Keeping them separate like this just isn't going
to work, sorry.
> --- /dev/null
> +++ b/rust/kernel/device.rs
> @@ -0,0 +1,76 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! Generic devices that are part of the kernel's driver model.
> +//!
> +//! C header: [`include/linux/device.h`](../../../../include/linux/device.h)
relative paths for a common "include <linux/device.h" type of thing?
Rust can't handle include paths from directories?
> +
> +use crate::{
> + bindings,
> + types::{ARef, Opaque},
> +};
> +use core::ptr;
> +
> +/// A ref-counted device.
> +///
> +/// # Invariants
> +///
> +/// The pointer stored in `Self` is non-null and valid for the lifetime of the ARef instance. In
> +/// particular, the ARef instance owns an increment on underlying object’s reference count.
> +#[repr(transparent)]
> +pub struct Device(Opaque<bindings::device>);
> +
> +impl Device {
> + /// Creates a new ref-counted instance of an existing device pointer.
> + ///
> + /// # Safety
> + ///
> + /// Callers must ensure that `ptr` is valid, non-null, and has a non-zero reference count.
Callers NEVER care about the reference count of a struct device, anyone
poking in that is asking for trouble.
And why non-NULL? Can't you check for that here? Shouldn't you check
for that here? Many driver core functions can handle a NULL pointer
just fine (i.e. get/put_device() can), why should Rust code assume that
a pointer passed to it from the C layer is going to have stricter rules
than the C layer can provide?
> + pub unsafe fn from_raw(ptr: *mut bindings::device) -> ARef<Self> {
> + // SAFETY: By the safety requirements, ptr is valid.
> + // Initially increase the reference count by one to compensate for the final decrement once
> + // this newly created `ARef<Device>` instance is dropped.
> + unsafe { bindings::get_device(ptr) };
> +
> + // CAST: `Self` is a `repr(transparent)` wrapper around `bindings::device`.
> + let ptr = ptr.cast::<Self>();
> +
> + // SAFETY: By the safety requirements, ptr is valid.
> + unsafe { ARef::from_raw(ptr::NonNull::new_unchecked(ptr)) }
> + }
> +
> + /// Obtain the raw `struct device *`.
> + pub(crate) fn as_raw(&self) -> *mut bindings::device {
> + self.0.get()
> + }
> +
> + /// Convert a raw `struct device` pointer to a `&Device`.
> + ///
> + /// # Safety
> + ///
> + /// Callers must ensure that `ptr` is valid, non-null, and has a non-zero reference count for
> + /// the entire duration when the returned reference exists.
Again, non-NULL might not be true, and reference counts are never
tracked by any user EXCEPT to increment/decrement it, you never know if
it is 0 or not, all you know is that if a pointer is given to you by the
driver core to a 'struct device' for a function that it is a valid
reference at that point in time, or maybe NULL, until your function
returns. Anything after that can not be counted on.
thanks,
greg k-h
