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

[Petr Pavlu]

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 <ga...@garyguo.net>
> Link: https://lore.kernel.org/all/DJFIQPLOVO4T....@garyguo.net/
> Reviewed-by: Gary Guo <ga...@garyguo.net>
> Acked-by: Danilo Krummrich <da...@kernel.org>
> Reviewed-by: Alice Ryhl <alic...@google.com>
> Signed-off-by: Alvin Sun <alvi...@linux.dev>

Acked-by: Petr Pavlu <petr....@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())
> })
> }
>
>