Re: [PATCH 8/8] rust: add device::Data

[Greg KH <gregkh@linuxfoundation.org>]

On Mon, Mar 25, 2024 at 06:49:12PM +0100, Danilo Krummrich wrote:
> From: Wedson Almeida Filho <wedsonaf@gmail.com>
> 
> This allows access to registrations and io resources to be automatically
> revoked when devices are removed, even if the ref count to their state
> is non-zero.

So it's re-implementing the devm_*() calls?  Why?

And this will trigger only on remove, but which remove?  The bus remove?
Or the unbinding of the driver to the device (two totally different
things, be specific and very careful here.)

If this is implementing the devm_() calls, why not call them the same
thing?


> 
> Co-developed-by: Andreas Hindborg <a.hindborg@samsung.com>
> Signed-off-by: Andreas Hindborg <a.hindborg@samsung.com>
> Signed-off-by: Wedson Almeida Filho <wedsonaf@gmail.com>
> Signed-off-by: Danilo Krummrich <dakr@redhat.com>
> ---
>  rust/kernel/device.rs | 120 +++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 119 insertions(+), 1 deletion(-)
> 
> diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs
> index 7309a236f512..707c699c3090 100644
> --- a/rust/kernel/device.rs
> +++ b/rust/kernel/device.rs
> @@ -4,7 +4,22 @@
>  //!
>  //! C header: [`include/linux/device.h`](../../../../include/linux/device.h)
>  
> -use crate::{bindings, str::CStr};
> +use macros::pin_data;
> +
> +use crate::{
> +    bindings,
> +    error::Result,
> +    init::InPlaceInit,
> +    init::PinInit,
> +    pin_init,
> +    revocable::{Revocable, RevocableGuard},
> +    str::CStr,
> +    sync::{LockClassKey, RevocableMutex, UniqueArc},
> +};
> +use core::{
> +    ops::{Deref, DerefMut},
> +    pin::Pin,
> +};
>  
>  /// A raw device.
>  ///
> @@ -95,3 +110,106 @@ fn clone(&self) -> Self {
>          Self::from_dev(self)
>      }
>  }
> +
> +/// Device data.
> +///
> +/// When a device is removed (for whatever reason, for example, because the device was unplugged or
> +/// because the user decided to unbind the driver), the driver is given a chance to clean its state
> +/// up, and all io resources should ideally not be used anymore.

Wait, unplugging a device and unbinding a device from a driver are two
totally different things, do NOT get them mixed up or assume that they
are the same thing at all please.  They have different lifetime rules
and different patterns of what happens.

So this needs to be taken out and rewritten from the beginning please.
If the comments describe something that is incorrect, I can't trust that
the code is correct...

thanks,

greg k-h