Re: [PATCH v9 01/10] rust: module: move module types into `module.rs`

[Petr Pavlu <petr.pavlu@suse.com>]

On 7/23/26 4:10 AM, Alvin Sun wrote:
> Move `Module`, `InPlaceModule`, `ModuleMetadata` and `ThisModule` from
> `lib.rs` into a new `rust/kernel/module.rs`. Re-export them from `lib.rs`
> to avoid tree-wide changes.
> 
> Switch six bus driver registrations from `module.0` to the public
> `ThisModule::as_ptr()` accessor, since the field is no longer visible
> outside the new `module` submodule.
> 
> No functional change.
> 
> Assisted-by: opencode:glm-5.2
> Suggested-by: Gary Guo <gary@garyguo.net>
> Link: https://lore.kernel.org/all/DJFIQPLOVO4T.1K8T0VZM30LDA@garyguo.net/
> Reviewed-by: Gary Guo <gary@garyguo.net>
> Acked-by: Danilo Krummrich <dakr@kernel.org>
> Reviewed-by: Alice Ryhl <aliceryhl@google.com>
> Signed-off-by: Alvin Sun <alvin.sun@linux.dev>

Acked-by: Petr Pavlu <petr.pavlu@suse.com>

> ---
>  rust/kernel/auxiliary.rs |  2 +-
>  rust/kernel/i2c.rs       |  2 +-
>  rust/kernel/lib.rs       | 75 +++++-------------------------------------------
>  rust/kernel/module.rs    | 71 +++++++++++++++++++++++++++++++++++++++++++++
>  rust/kernel/net/phy.rs   |  6 +++-
>  rust/kernel/pci.rs       |  2 +-
>  rust/kernel/platform.rs  |  2 +-
>  rust/kernel/usb.rs       |  2 +-
>  8 files changed, 88 insertions(+), 74 deletions(-)
> 
> diff --git a/rust/kernel/auxiliary.rs b/rust/kernel/auxiliary.rs
> index 93c0db1f66555..4a02f83240be3 100644
> --- a/rust/kernel/auxiliary.rs
> +++ b/rust/kernel/auxiliary.rs
> @@ -63,7 +63,7 @@ unsafe fn register(
>  
>          // SAFETY: `adrv` is guaranteed to be a valid `DriverType`.
>          to_result(unsafe {
> -            bindings::__auxiliary_driver_register(adrv.get(), module.0, name.as_char_ptr())
> +            bindings::__auxiliary_driver_register(adrv.get(), module.as_ptr(), name.as_char_ptr())
>          })
>      }
>  
> diff --git a/rust/kernel/i2c.rs b/rust/kernel/i2c.rs
> index 7b908f0c5a58d..24eff08f47123 100644
> --- a/rust/kernel/i2c.rs
> +++ b/rust/kernel/i2c.rs
> @@ -142,7 +142,7 @@ unsafe fn register(
>          }
>  
>          // SAFETY: `idrv` is guaranteed to be a valid `DriverType`.
> -        to_result(unsafe { bindings::i2c_register_driver(module.0, idrv.get()) })
> +        to_result(unsafe { bindings::i2c_register_driver(module.as_ptr(), idrv.get()) })
>      }
>  
>      unsafe fn unregister(idrv: &Opaque<Self::DriverType>) {
> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> index b72b2fbe046d6..040ae85056509 100644
> --- a/rust/kernel/lib.rs
> +++ b/rust/kernel/lib.rs
> @@ -93,6 +93,7 @@
>  pub mod maple_tree;
>  pub mod miscdevice;
>  pub mod mm;
> +pub mod module;
>  pub mod module_param;
>  #[cfg(CONFIG_NET)]
>  pub mod net;
> @@ -139,79 +140,17 @@
>  #[doc(hidden)]
>  pub use bindings;
>  pub use macros;
> +pub use module::{
> +    InPlaceModule,
> +    Module,
> +    ModuleMetadata,
> +    ThisModule, //
> +};
>  pub use uapi;
>  
>  /// Prefix to appear before log messages printed from within the `kernel` crate.
>  const __LOG_PREFIX: &[u8] = b"rust_kernel\0";
>  
> -/// The top level entrypoint to implementing a kernel module.
> -///
> -/// For any teardown or cleanup operations, your type may implement [`Drop`].
> -pub trait Module: Sized + Sync + Send {
> -    /// Called at module initialization time.
> -    ///
> -    /// Use this method to perform whatever setup or registration your module
> -    /// should do.
> -    ///
> -    /// Equivalent to the `module_init` macro in the C API.
> -    fn init(module: &'static ThisModule) -> error::Result<Self>;
> -}
> -
> -/// A module that is pinned and initialised in-place.
> -pub trait InPlaceModule: Sync + Send {
> -    /// Creates an initialiser for the module.
> -    ///
> -    /// It is called when the module is loaded.
> -    fn init(module: &'static ThisModule) -> impl pin_init::PinInit<Self, error::Error>;
> -}
> -
> -impl<T: Module> InPlaceModule for T {
> -    fn init(module: &'static ThisModule) -> impl pin_init::PinInit<Self, error::Error> {
> -        let initer = move |slot: *mut Self| {
> -            let m = <Self as Module>::init(module)?;
> -
> -            // SAFETY: `slot` is valid for write per the contract with `pin_init_from_closure`.
> -            unsafe { slot.write(m) };
> -            Ok(())
> -        };
> -
> -        // SAFETY: On success, `initer` always fully initialises an instance of `Self`.
> -        unsafe { pin_init::pin_init_from_closure(initer) }
> -    }
> -}
> -
> -/// Metadata attached to a [`Module`] or [`InPlaceModule`].
> -pub trait ModuleMetadata {
> -    /// The name of the module as specified in the `module!` macro.
> -    const NAME: &'static crate::str::CStr;
> -}
> -
> -/// Equivalent to `THIS_MODULE` in the C API.
> -///
> -/// C header: [`include/linux/init.h`](srctree/include/linux/init.h)
> -pub struct ThisModule(*mut bindings::module);
> -
> -// SAFETY: `THIS_MODULE` may be used from all threads within a module.
> -unsafe impl Sync for ThisModule {}
> -
> -impl ThisModule {
> -    /// Creates a [`ThisModule`] given the `THIS_MODULE` pointer.
> -    ///
> -    /// # Safety
> -    ///
> -    /// The pointer must be equal to the right `THIS_MODULE`.
> -    pub const unsafe fn from_ptr(ptr: *mut bindings::module) -> ThisModule {
> -        ThisModule(ptr)
> -    }
> -
> -    /// Access the raw pointer for this module.
> -    ///
> -    /// It is up to the user to use it correctly.
> -    pub const fn as_ptr(&self) -> *mut bindings::module {
> -        self.0
> -    }
> -}
> -
>  #[cfg(not(testlib))]
>  #[panic_handler]
>  fn panic(info: &core::panic::PanicInfo<'_>) -> ! {
> diff --git a/rust/kernel/module.rs b/rust/kernel/module.rs
> new file mode 100644
> index 0000000000000..be242a82e86d2
> --- /dev/null
> +++ b/rust/kernel/module.rs
> @@ -0,0 +1,71 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! Module-related types and helpers.
> +
> +/// The entrypoint to implementing a kernel module.
> +///
> +/// For any teardown or cleanup operations, your type may implement [`Drop`].
> +pub trait Module: Sized + Sync + Send {
> +    /// Called at module initialization time.
> +    ///
> +    /// Use this method to perform whatever setup or registration your module
> +    /// should do.
> +    ///
> +    /// Equivalent to the `module_init` macro in the C API.
> +    fn init(module: &'static ThisModule) -> crate::error::Result<Self>;
> +}
> +
> +/// A module that is pinned and initialised in-place.
> +pub trait InPlaceModule: Sync + Send {
> +    /// Creates an initialiser for the module.
> +    ///
> +    /// It is called when the module is loaded.
> +    fn init(module: &'static ThisModule) -> impl pin_init::PinInit<Self, crate::error::Error>;
> +}
> +
> +impl<T: Module> InPlaceModule for T {
> +    fn init(module: &'static ThisModule) -> impl pin_init::PinInit<Self, crate::error::Error> {
> +        let initer = move |slot: *mut Self| {
> +            let m = <Self as Module>::init(module)?;
> +
> +            // SAFETY: `slot` is valid for write per the contract with `pin_init_from_closure`.
> +            unsafe { slot.write(m) };
> +            Ok(())
> +        };
> +
> +        // SAFETY: On success, `initer` always fully initialises an instance of `Self`.
> +        unsafe { pin_init::pin_init_from_closure(initer) }
> +    }
> +}
> +
> +/// Metadata attached to a [`Module`] or [`InPlaceModule`].
> +pub trait ModuleMetadata {
> +    /// The name of the module as specified in the `module!` macro.
> +    const NAME: &'static crate::str::CStr;
> +}
> +
> +/// Equivalent to `THIS_MODULE` in the C API.
> +///
> +/// C header: [`include/linux/init.h`](srctree/include/linux/init.h)
> +pub struct ThisModule(*mut crate::bindings::module);
> +
> +// SAFETY: `THIS_MODULE` may be used from all threads within a module.
> +unsafe impl Sync for ThisModule {}
> +
> +impl ThisModule {
> +    /// Creates a [`ThisModule`] given the `THIS_MODULE` pointer.
> +    ///
> +    /// # Safety
> +    ///
> +    /// The pointer must be equal to the right `THIS_MODULE`.
> +    pub const unsafe fn from_ptr(ptr: *mut crate::bindings::module) -> ThisModule {
> +        ThisModule(ptr)
> +    }
> +
> +    /// Access the raw pointer for this module.
> +    ///
> +    /// It is up to the user to use it correctly.
> +    pub const fn as_ptr(&self) -> *mut crate::bindings::module {
> +        self.0
> +    }
> +}
> diff --git a/rust/kernel/net/phy.rs b/rust/kernel/net/phy.rs
> index 3ca99db5cccf2..8b7036b8fe480 100644
> --- a/rust/kernel/net/phy.rs
> +++ b/rust/kernel/net/phy.rs
> @@ -659,7 +659,11 @@ pub fn register(
>          // the `drivers` slice are initialized properly. `drivers` will not be moved.
>          // So it's just an FFI call.
>          to_result(unsafe {
> -            bindings::phy_drivers_register(drivers[0].0.get(), drivers.len().try_into()?, module.0)
> +            bindings::phy_drivers_register(
> +                drivers[0].0.get(),
> +                drivers.len().try_into()?,
> +                module.as_ptr(),
> +            )
>          })?;
>          // INVARIANT: The `drivers` slice is successfully registered to the kernel via `phy_drivers_register`.
>          Ok(Registration { drivers })
> diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
> index af74ddff6114d..916ed2cb6b70b 100644
> --- a/rust/kernel/pci.rs
> +++ b/rust/kernel/pci.rs
> @@ -86,7 +86,7 @@ unsafe fn register(
>  
>          // SAFETY: `pdrv` is guaranteed to be a valid `DriverType`.
>          to_result(unsafe {
> -            bindings::__pci_register_driver(pdrv.get(), module.0, name.as_char_ptr())
> +            bindings::__pci_register_driver(pdrv.get(), module.as_ptr(), name.as_char_ptr())
>          })
>      }
>  
> diff --git a/rust/kernel/platform.rs b/rust/kernel/platform.rs
> index 8917d4ee499fb..9fdbafd53bc21 100644
> --- a/rust/kernel/platform.rs
> +++ b/rust/kernel/platform.rs
> @@ -82,7 +82,7 @@ unsafe fn register(
>          }
>  
>          // SAFETY: `pdrv` is guaranteed to be a valid `DriverType`.
> -        to_result(unsafe { bindings::__platform_driver_register(pdrv.get(), module.0) })
> +        to_result(unsafe { bindings::__platform_driver_register(pdrv.get(), module.as_ptr()) })
>      }
>  
>      unsafe fn unregister(pdrv: &Opaque<Self::DriverType>) {
> diff --git a/rust/kernel/usb.rs b/rust/kernel/usb.rs
> index 9c17a672cd275..213db32727c17 100644
> --- a/rust/kernel/usb.rs
> +++ b/rust/kernel/usb.rs
> @@ -63,7 +63,7 @@ unsafe fn register(
>  
>          // SAFETY: `udrv` is guaranteed to be a valid `DriverType`.
>          to_result(unsafe {
> -            bindings::usb_register_driver(udrv.get(), module.0, name.as_char_ptr())
> +            bindings::usb_register_driver(udrv.get(), module.as_ptr(), name.as_char_ptr())
>          })
>      }
>  
>