diff mbox series

[v3,02/16] rust: introduce `InPlaceModule`

Message ID 20241022213221.2383-3-dakr@kernel.org
State New
Headers show
Series Device / Driver PCI / Platform Rust abstractions | expand

Commit Message

Danilo Krummrich Oct. 22, 2024, 9:31 p.m. UTC
From: Wedson Almeida Filho <walmeida@microsoft.com>

This allows modules to be initialised in-place in pinned memory, which
enables the usage of pinned types (e.g., mutexes, spinlocks, driver
registrations, etc.) in modules without any extra allocations.

Signed-off-by: Wedson Almeida Filho <walmeida@microsoft.com>
Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
 rust/kernel/lib.rs    | 23 +++++++++++++++++++++++
 rust/macros/module.rs | 28 ++++++++++++----------------
 2 files changed, 35 insertions(+), 16 deletions(-)

Comments

Alice Ryhl Oct. 29, 2024, 12:45 p.m. UTC | #1
On Tue, Oct 22, 2024 at 11:32 PM Danilo Krummrich <dakr@kernel.org> wrote:
>
> From: Wedson Almeida Filho <walmeida@microsoft.com>
>
> This allows modules to be initialised in-place in pinned memory, which
> enables the usage of pinned types (e.g., mutexes, spinlocks, driver
> registrations, etc.) in modules without any extra allocations.
>
> Signed-off-by: Wedson Almeida Filho <walmeida@microsoft.com>
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>

Reviewed-by: Alice Ryhl <aliceryhl@google.com>

Miguel: I think this patch would be good to land in 6.13. I'd like to
submit a miscdevice sample once 6.14 comes around, and having this
already in will make the miscdevice sample nicer.

Alice
Greg KH Nov. 4, 2024, 12:15 a.m. UTC | #2
On Tue, Oct 29, 2024 at 01:45:36PM +0100, Alice Ryhl wrote:
> On Tue, Oct 22, 2024 at 11:32 PM Danilo Krummrich <dakr@kernel.org> wrote:
> >
> > From: Wedson Almeida Filho <walmeida@microsoft.com>
> >
> > This allows modules to be initialised in-place in pinned memory, which
> > enables the usage of pinned types (e.g., mutexes, spinlocks, driver
> > registrations, etc.) in modules without any extra allocations.
> >
> > Signed-off-by: Wedson Almeida Filho <walmeida@microsoft.com>
> > Signed-off-by: Danilo Krummrich <dakr@kernel.org>
> 
> Reviewed-by: Alice Ryhl <aliceryhl@google.com>
> 
> Miguel: I think this patch would be good to land in 6.13. I'd like to
> submit a miscdevice sample once 6.14 comes around, and having this
> already in will make the miscdevice sample nicer.

If no one objects, I'll just add this to the char/misc tree that already
has the other misc device rust code in it.

thanks,

greg k-h
Miguel Ojeda Nov. 4, 2024, 5:36 p.m. UTC | #3
On Mon, Nov 4, 2024 at 6:48 AM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> If no one objects, I'll just add this to the char/misc tree that already
> has the other misc device rust code in it.

Sounds good, thanks! I gave it a quick spin (on top of `rust-next`)
just in case it triggered lints etc.:

Acked-by: Miguel Ojeda <ojeda@kernel.org>

Cheers,
Miguel
diff mbox series

Patch

diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index b62451f64f6e..4fdb0d91f2ad 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -83,6 +83,29 @@  pub trait Module: Sized + Sync + Send {
     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 init::PinInit<Self, error::Error>;
+}
+
+impl<T: Module> InPlaceModule for T {
+    fn init(module: &'static ThisModule) -> impl 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 { init::pin_init_from_closure(initer) }
+    }
+}
+
 /// Equivalent to `THIS_MODULE` in the C API.
 ///
 /// C header: [`include/linux/init.h`](srctree/include/linux/init.h)
diff --git a/rust/macros/module.rs b/rust/macros/module.rs
index aef3b132f32b..a03266a78cfb 100644
--- a/rust/macros/module.rs
+++ b/rust/macros/module.rs
@@ -232,6 +232,7 @@  pub(crate) fn module(ts: TokenStream) -> TokenStream {
             mod __module_init {{
                 mod __module_init {{
                     use super::super::{type_};
+                    use kernel::init::PinInit;
 
                     /// The \"Rust loadable module\" mark.
                     //
@@ -242,7 +243,8 @@  mod __module_init {{
                     #[used]
                     static __IS_RUST_MODULE: () = ();
 
-                    static mut __MOD: Option<{type_}> = None;
+                    static mut __MOD: core::mem::MaybeUninit<{type_}> =
+                        core::mem::MaybeUninit::uninit();
 
                     // Loadable modules need to export the `{{init,cleanup}}_module` identifiers.
                     /// # Safety
@@ -331,20 +333,14 @@  mod __module_init {{
                     ///
                     /// This function must only be called once.
                     unsafe fn __init() -> core::ffi::c_int {{
-                        match <{type_} as kernel::Module>::init(&super::super::THIS_MODULE) {{
-                            Ok(m) => {{
-                                // SAFETY: No data race, since `__MOD` can only be accessed by this
-                                // module and there only `__init` and `__exit` access it. These
-                                // functions are only called once and `__exit` cannot be called
-                                // before or during `__init`.
-                                unsafe {{
-                                    __MOD = Some(m);
-                                }}
-                                return 0;
-                            }}
-                            Err(e) => {{
-                                return e.to_errno();
-                            }}
+                        let initer =
+                            <{type_} as kernel::InPlaceModule>::init(&super::super::THIS_MODULE);
+                        // SAFETY: No data race, since `__MOD` can only be accessed by this module
+                        // and there only `__init` and `__exit` access it. These functions are only
+                        // called once and `__exit` cannot be called before or during `__init`.
+                        match unsafe {{ initer.__pinned_init(__MOD.as_mut_ptr()) }} {{
+                            Ok(m) => 0,
+                            Err(e) => e.to_errno(),
                         }}
                     }}
 
@@ -359,7 +355,7 @@  unsafe fn __exit() {{
                         // called once and `__init` was already called.
                         unsafe {{
                             // Invokes `drop()` on `__MOD`, which should be used for cleanup.
-                            __MOD = None;
+                            __MOD.assume_init_drop();
                         }}
                     }}