[PATCH v2 0/5] In-place module initialisation

[PATCH v2 0/5] In-place module initialisation

[Wedson Almeida Filho]

From: Wedson Almeida Filho <walmeida@microsoft.com>

Introduce `InPlaceModule`, which allows modules to be initialised
inplace, as opposed to the current state where modules must return an
instance which is moved to its final memory location.

This allows us to have modules whose state hold objects that must be
initialised inplace like locks. It also allows us to implement
registrations (e.g., driver registration) inplace and have them similar
to their C counterparts where no new allocations are needed.

This is built on top of the allocation APIs because the sample module is
a modified version of rust_minimal, which would be incompatible with the
allocation API series because it uses vectors.

---

Changes in v2:

- Rebased to latest rust-next 
- Split off adding `Send` requirement to `Module` into a separate patch
- Prefixed all `core` and `kernel` references with `::` in
  `module!` macro-generated code.
- Calling `__pinned_init` with explicit full path.
- Add `Mutex` to `rust_inplace`  sample.
- Added `Send` implementation for `Registration` in net/phy.
- Link to v1: https://lore.kernel.org/rust-for-linux/20240327032337.188938-1-wedsonaf@gmail.com/T/#t

---
Wedson Almeida Filho (5):
  rust: phy: implement `Send` for `Registration`
  rust: kernel: require `Send` for `Module` implementations
  rust: module: prefix all module paths with `::`
  rust: introduce `InPlaceModule`
  samples: rust: add in-place initialisation sample

 rust/kernel/lib.rs           | 25 +++++++++++++++++++-
 rust/kernel/net/phy.rs       |  4 ++++
 rust/macros/module.rs        | 36 ++++++++++++-----------------
 samples/rust/Kconfig         | 11 +++++++++
 samples/rust/Makefile        |  1 +
 samples/rust/rust_inplace.rs | 44 ++++++++++++++++++++++++++++++++++++
 6 files changed, 99 insertions(+), 22 deletions(-)
 create mode 100644 samples/rust/rust_inplace.rs


base-commit: 453275c66aa4d87c73c5152140b3573ab2435bb7
-- 
2.34.1

[PATCH v2 1/5] rust: phy: implement `Send` for `Registration`

[Wedson Almeida Filho]

From: Wedson Almeida Filho <walmeida@microsoft.com>

In preparation for requiring `Send` for `Module` implementations in the
next patch.

Cc: FUJITA Tomonori <fujita.tomonori@gmail.com>
Cc: Trevor Gross <tmgross@umich.edu>
Cc: netdev@vger.kernel.org
Signed-off-by: Wedson Almeida Filho <walmeida@microsoft.com>
---
 rust/kernel/net/phy.rs | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/rust/kernel/net/phy.rs b/rust/kernel/net/phy.rs
index 96e09c6e8530..265d0e1c1371 100644
--- a/rust/kernel/net/phy.rs
+++ b/rust/kernel/net/phy.rs
@@ -640,6 +640,10 @@ pub struct Registration {
     drivers: Pin<&'static mut [DriverVTable]>,
 }
 
+// SAFETY: The only action allowed in a `Registration` instance is dropping it, which is safe to do
+// from any thread because `phy_drivers_unregister` can be called from any thread context.
+unsafe impl Send for Registration {}
+
 impl Registration {
     /// Registers a PHY driver.
     pub fn register(
-- 
2.34.1

Re: [PATCH v2 1/5] rust: phy: implement `Send` for `Registration`

[FUJITA Tomonori]

Hi,

On Thu, 28 Mar 2024 16:54:53 -0300
Wedson Almeida Filho <wedsonaf@gmail.com> wrote:

> From: Wedson Almeida Filho <walmeida@microsoft.com>
> 
> In preparation for requiring `Send` for `Module` implementations in the
> next patch.
> 
> Cc: FUJITA Tomonori <fujita.tomonori@gmail.com>
> Cc: Trevor Gross <tmgross@umich.edu>
> Cc: netdev@vger.kernel.org
> Signed-off-by: Wedson Almeida Filho <walmeida@microsoft.com>
> ---
>  rust/kernel/net/phy.rs | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/rust/kernel/net/phy.rs b/rust/kernel/net/phy.rs
> index 96e09c6e8530..265d0e1c1371 100644
> --- a/rust/kernel/net/phy.rs
> +++ b/rust/kernel/net/phy.rs
> @@ -640,6 +640,10 @@ pub struct Registration {
>      drivers: Pin<&'static mut [DriverVTable]>,
>  }
>  
> +// SAFETY: The only action allowed in a `Registration` instance is dropping it, which is safe to do
> +// from any thread because `phy_drivers_unregister` can be called from any thread context.
> +unsafe impl Send for Registration {}
> +
>  impl Registration {
>      /// Registers a PHY driver.
>      pub fn register(

After the following discussion, I dropped Send for Registration:

https://lore.kernel.org/netdev/8f476b7c-4647-457b-ab45-d6a979da4e78@lunn.ch/T/

If you guys think that Send can be added here, it's fine by me.


Once this In-place module series are merged, I'll revisit the phy
module initialization to remove `static mut DRIVERS`.

Re: [PATCH v2 1/5] rust: phy: implement `Send` for `Registration`

[Alice Ryhl]

On Thu, Mar 28, 2024 at 8:55 PM Wedson Almeida Filho <wedsonaf@gmail.com> wrote:
>
> From: Wedson Almeida Filho <walmeida@microsoft.com>
>
> In preparation for requiring `Send` for `Module` implementations in the
> next patch.
>
> Cc: FUJITA Tomonori <fujita.tomonori@gmail.com>
> Cc: Trevor Gross <tmgross@umich.edu>
> Cc: netdev@vger.kernel.org
> Signed-off-by: Wedson Almeida Filho <walmeida@microsoft.com>

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

[PATCH v2 2/5] rust: kernel: require `Send` for `Module` implementations

[Wedson Almeida Filho]

From: Wedson Almeida Filho <walmeida@microsoft.com>

The thread that calls the module initialisation code when a module is
loaded is not guaranteed [in fact, it is unlikely] to be the same one
that calls the module cleanup code on module unload, therefore, `Module`
implementations must be `Send` to account for them moving from one
thread to another implicitly.

Signed-off-by: Wedson Almeida Filho <walmeida@microsoft.com>
---
 rust/kernel/lib.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index 5c641233e26d..9141a95efb25 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -62,7 +62,7 @@
 /// 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 {
+pub trait Module: Sized + Sync + Send {
     /// Called at module initialization time.
     ///
     /// Use this method to perform whatever setup or registration your module
-- 
2.34.1

Re: [PATCH v2 2/5] rust: kernel: require `Send` for `Module` implementations

[Benno Lossin]

On 28.03.24 20:54, Wedson Almeida Filho wrote:
> From: Wedson Almeida Filho <walmeida@microsoft.com>
> 
> The thread that calls the module initialisation code when a module is
> loaded is not guaranteed [in fact, it is unlikely] to be the same one
> that calls the module cleanup code on module unload, therefore, `Module`
> implementations must be `Send` to account for them moving from one
> thread to another implicitly.
> 
> Signed-off-by: Wedson Almeida Filho <walmeida@microsoft.com>
> ---
>   rust/kernel/lib.rs | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)

As already said by Valentin, this should go to the stable tree. (and
patch 1 should also go to stable 6.8.2, since IIRC the phy-driver is in
that tree)

Reviewed-by: Benno Lossin <benno.lossin@proton.me>

-- 
Cheers,
Benno

Re: [PATCH v2 2/5] rust: kernel: require `Send` for `Module` implementations

[Alice Ryhl]

On Thu, Mar 28, 2024 at 8:55 PM Wedson Almeida Filho <wedsonaf@gmail.com> wrote:
>
> From: Wedson Almeida Filho <walmeida@microsoft.com>
>
> The thread that calls the module initialisation code when a module is
> loaded is not guaranteed [in fact, it is unlikely] to be the same one
> that calls the module cleanup code on module unload, therefore, `Module`
> implementations must be `Send` to account for them moving from one
> thread to another implicitly.
>
> Signed-off-by: Wedson Almeida Filho <walmeida@microsoft.com>

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

[PATCH v2 3/5] rust: module: prefix all module paths with `::`

[Wedson Almeida Filho]

From: Wedson Almeida Filho <walmeida@microsoft.com>

This prevents the macro-generated code from accidentally referring to
the wrong symbol if the caller's code happens to have submodules called
`core` or `kernel`.

Signed-off-by: Wedson Almeida Filho <walmeida@microsoft.com>
---
 rust/macros/module.rs | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/rust/macros/module.rs b/rust/macros/module.rs
index 27979e582e4b..6da1246742a5 100644
--- a/rust/macros/module.rs
+++ b/rust/macros/module.rs
@@ -213,12 +213,12 @@ pub(crate) fn module(ts: TokenStream) -> TokenStream {
             // SAFETY: `__this_module` is constructed by the kernel at load time and will not be
             // freed until the module is unloaded.
             #[cfg(MODULE)]
-            static THIS_MODULE: kernel::ThisModule = unsafe {{
-                kernel::ThisModule::from_ptr(&kernel::bindings::__this_module as *const _ as *mut _)
+            static THIS_MODULE: ::kernel::ThisModule = unsafe {{
+                ::kernel::ThisModule::from_ptr(&::kernel::bindings::__this_module as *const _ as *mut _)
             }};
             #[cfg(not(MODULE))]
-            static THIS_MODULE: kernel::ThisModule = unsafe {{
-                kernel::ThisModule::from_ptr(core::ptr::null_mut())
+            static THIS_MODULE: ::kernel::ThisModule = unsafe {{
+                ::kernel::ThisModule::from_ptr(::core::ptr::null_mut())
             }};
 
             // Loadable modules need to export the `{{init,cleanup}}_module` identifiers.
@@ -230,7 +230,7 @@ pub(crate) fn module(ts: TokenStream) -> TokenStream {
             #[doc(hidden)]
             #[no_mangle]
             #[link_section = \".init.text\"]
-            pub unsafe extern \"C\" fn init_module() -> core::ffi::c_int {{
+            pub unsafe extern \"C\" fn init_module() -> ::core::ffi::c_int {{
                 __init()
             }}
 
@@ -248,11 +248,11 @@ pub(crate) fn module(ts: TokenStream) -> TokenStream {
             #[doc(hidden)]
             #[link_section = \"{initcall_section}\"]
             #[used]
-            pub static __{name}_initcall: extern \"C\" fn() -> core::ffi::c_int = __{name}_init;
+            pub static __{name}_initcall: extern \"C\" fn() -> ::core::ffi::c_int = __{name}_init;
 
             #[cfg(not(MODULE))]
             #[cfg(CONFIG_HAVE_ARCH_PREL32_RELOCATIONS)]
-            core::arch::global_asm!(
+            ::core::arch::global_asm!(
                 r#\".section \"{initcall_section}\", \"a\"
                 __{name}_initcall:
                     .long   __{name}_init - .
@@ -263,7 +263,7 @@ pub(crate) fn module(ts: TokenStream) -> TokenStream {
             #[cfg(not(MODULE))]
             #[doc(hidden)]
             #[no_mangle]
-            pub extern \"C\" fn __{name}_init() -> core::ffi::c_int {{
+            pub extern \"C\" fn __{name}_init() -> ::core::ffi::c_int {{
                 __init()
             }}
 
@@ -274,8 +274,8 @@ pub(crate) fn module(ts: TokenStream) -> TokenStream {
                 __exit()
             }}
 
-            fn __init() -> core::ffi::c_int {{
-                match <{type_} as kernel::Module>::init(&THIS_MODULE) {{
+            fn __init() -> ::core::ffi::c_int {{
+                match <{type_} as ::kernel::Module>::init(&THIS_MODULE) {{
                     Ok(m) => {{
                         unsafe {{
                             __MOD = Some(m);
-- 
2.34.1

Re: [PATCH v2 3/5] rust: module: prefix all module paths with `::`

[Benno Lossin]

On 28.03.24 20:54, Wedson Almeida Filho wrote:
> From: Wedson Almeida Filho <walmeida@microsoft.com>
> 
> This prevents the macro-generated code from accidentally referring to
> the wrong symbol if the caller's code happens to have submodules called
> `core` or `kernel`.
> 
> Signed-off-by: Wedson Almeida Filho <walmeida@microsoft.com>
> ---
>   rust/macros/module.rs | 20 ++++++++++----------
>   1 file changed, 10 insertions(+), 10 deletions(-)

Reviewed-by: Benno Lossin <benno.lossin@proton.me>

-- 
Cheers,
Benno

Re: [PATCH v2 3/5] rust: module: prefix all module paths with `::`

[Alice Ryhl]

On Thu, Mar 28, 2024 at 8:55 PM Wedson Almeida Filho <wedsonaf@gmail.com> wrote:
>
> From: Wedson Almeida Filho <walmeida@microsoft.com>
>
> This prevents the macro-generated code from accidentally referring to
> the wrong symbol if the caller's code happens to have submodules called
> `core` or `kernel`.
>
> Signed-off-by: Wedson Almeida Filho <walmeida@microsoft.com>

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

[PATCH v2 4/5] rust: introduce `InPlaceModule`

[Wedson Almeida Filho]

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.

Drivers that don't need this may continue to implement `Module` without
any changes.

Reviewed-by: Benno Lossin <benno.lossin@proton.me>
Tested-by: Valentin Obst <kernel@valentinobst.de>
Signed-off-by: Wedson Almeida Filho <walmeida@microsoft.com>
---
 rust/kernel/lib.rs    | 23 +++++++++++++++++++++++
 rust/macros/module.rs | 18 ++++++------------
 2 files changed, 29 insertions(+), 12 deletions(-)

diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index 9141a95efb25..64aee4fbc53b 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -72,6 +72,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/export.h`](srctree/include/linux/export.h)
diff --git a/rust/macros/module.rs b/rust/macros/module.rs
index 6da1246742a5..4e5b5a68c3af 100644
--- a/rust/macros/module.rs
+++ b/rust/macros/module.rs
@@ -208,7 +208,7 @@ pub(crate) fn module(ts: TokenStream) -> TokenStream {
             #[used]
             static __IS_RUST_MODULE: () = ();
 
-            static mut __MOD: Option<{type_}> = None;
+            static mut __MOD: ::core::mem::MaybeUninit<{type_}> = ::core::mem::MaybeUninit::uninit();
 
             // SAFETY: `__this_module` is constructed by the kernel at load time and will not be
             // freed until the module is unloaded.
@@ -275,23 +275,17 @@ pub(crate) fn module(ts: TokenStream) -> TokenStream {
             }}
 
             fn __init() -> ::core::ffi::c_int {{
-                match <{type_} as ::kernel::Module>::init(&THIS_MODULE) {{
-                    Ok(m) => {{
-                        unsafe {{
-                            __MOD = Some(m);
-                        }}
-                        return 0;
-                    }}
-                    Err(e) => {{
-                        return e.to_errno();
-                    }}
+                let initer = <{type_} as ::kernel::InPlaceModule>::init(&THIS_MODULE);
+                match unsafe {{ ::kernel::init::PinInit::__pinned_init(initer, __MOD.as_mut_ptr()) }} {{
+                    Ok(m) => 0,
+                    Err(e) => e.to_errno(),
                 }}
             }}
 
             fn __exit() {{
                 unsafe {{
                     // Invokes `drop()` on `__MOD`, which should be used for cleanup.
-                    __MOD = None;
+                    __MOD.assume_init_drop();
                 }}
             }}
 
-- 
2.34.1

Re: [PATCH v2 4/5] rust: introduce `InPlaceModule`

[Alice Ryhl]

On Thu, Mar 28, 2024 at 8:55 PM Wedson Almeida Filho <wedsonaf@gmail.com> 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.
>
> Drivers that don't need this may continue to implement `Module` without
> any changes.
>
> Reviewed-by: Benno Lossin <benno.lossin@proton.me>
> Tested-by: Valentin Obst <kernel@valentinobst.de>
> Signed-off-by: Wedson Almeida Filho <walmeida@microsoft.com>

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

[PATCH v2 5/5] samples: rust: add in-place initialisation sample

[Wedson Almeida Filho]

From: Wedson Almeida Filho <walmeida@microsoft.com>

This is a modified version of rust_minimal that is initialised in-place
and has a mutex.

Signed-off-by: Wedson Almeida Filho <walmeida@microsoft.com>
---
 samples/rust/Kconfig         | 11 +++++++++
 samples/rust/Makefile        |  1 +
 samples/rust/rust_inplace.rs | 44 ++++++++++++++++++++++++++++++++++++
 3 files changed, 56 insertions(+)
 create mode 100644 samples/rust/rust_inplace.rs

diff --git a/samples/rust/Kconfig b/samples/rust/Kconfig
index b0f74a81c8f9..59f44a8b6958 100644
--- a/samples/rust/Kconfig
+++ b/samples/rust/Kconfig
@@ -20,6 +20,17 @@ config SAMPLE_RUST_MINIMAL
 
 	  If unsure, say N.
 
+config SAMPLE_RUST_INPLACE
+	tristate "Minimal in-place"
+	help
+	  This option builds the Rust minimal module with in-place
+	  initialisation.
+
+	  To compile this as a module, choose M here:
+	  the module will be called rust_inplace.
+
+	  If unsure, say N.
+
 config SAMPLE_RUST_PRINT
 	tristate "Printing macros"
 	help
diff --git a/samples/rust/Makefile b/samples/rust/Makefile
index 03086dabbea4..791fc18180e9 100644
--- a/samples/rust/Makefile
+++ b/samples/rust/Makefile
@@ -1,6 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0
 
 obj-$(CONFIG_SAMPLE_RUST_MINIMAL)		+= rust_minimal.o
+obj-$(CONFIG_SAMPLE_RUST_INPLACE)		+= rust_inplace.o
 obj-$(CONFIG_SAMPLE_RUST_PRINT)			+= rust_print.o
 
 subdir-$(CONFIG_SAMPLE_RUST_HOSTPROGS)		+= hostprogs
diff --git a/samples/rust/rust_inplace.rs b/samples/rust/rust_inplace.rs
new file mode 100644
index 000000000000..0410e97a689f
--- /dev/null
+++ b/samples/rust/rust_inplace.rs
@@ -0,0 +1,44 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Rust minimal in-place sample.
+
+use kernel::prelude::*;
+use kernel::{new_mutex, sync::Mutex};
+
+module! {
+    type: RustInPlace,
+    name: "rust_inplace",
+    author: "Rust for Linux Contributors",
+    description: "Rust minimal in-place sample",
+    license: "GPL",
+}
+
+#[pin_data(PinnedDrop)]
+struct RustInPlace {
+    #[pin]
+    numbers: Mutex<Vec<i32>>,
+}
+
+impl kernel::InPlaceModule for RustInPlace {
+    fn init(_module: &'static ThisModule) -> impl PinInit<Self, Error> {
+        pr_info!("Rust in-place minimal sample (init)\n");
+        pr_info!("Am I built-in? {}\n", !cfg!(MODULE));
+        try_pin_init!(Self {
+            numbers <- {
+                let mut numbers = Vec::new();
+                numbers.push(72, GFP_KERNEL)?;
+                numbers.push(108, GFP_KERNEL)?;
+                numbers.push(200, GFP_KERNEL)?;
+                new_mutex!(numbers)
+            },
+        })
+    }
+}
+
+#[pinned_drop]
+impl PinnedDrop for RustInPlace {
+    fn drop(self: Pin<&mut Self>) {
+        pr_info!("My numbers are {:?}\n", *self.numbers.lock());
+        pr_info!("Rust minimal inplace sample (exit)\n");
+    }
+}
-- 
2.34.1

Re: [PATCH v2 5/5] samples: rust: add in-place initialisation sample

[Benno Lossin]

On 28.03.24 20:54, Wedson Almeida Filho wrote:
> From: Wedson Almeida Filho <walmeida@microsoft.com>
> 
> This is a modified version of rust_minimal that is initialised in-place
> and has a mutex.
> 
> Signed-off-by: Wedson Almeida Filho <walmeida@microsoft.com>
> ---
>   samples/rust/Kconfig         | 11 +++++++++
>   samples/rust/Makefile        |  1 +
>   samples/rust/rust_inplace.rs | 44 ++++++++++++++++++++++++++++++++++++
>   3 files changed, 56 insertions(+)
>   create mode 100644 samples/rust/rust_inplace.rs

Reviewed-by: Benno Lossin <benno.lossin@proton.me>

-- 
Cheers,
Benno

Re: [PATCH v2 5/5] samples: rust: add in-place initialisation sample

[Alice Ryhl]

On Thu, Mar 28, 2024 at 8:55 PM Wedson Almeida Filho <wedsonaf@gmail.com> wrote:
>
> From: Wedson Almeida Filho <walmeida@microsoft.com>
>
> This is a modified version of rust_minimal that is initialised in-place
> and has a mutex.
>
> Signed-off-by: Wedson Almeida Filho <walmeida@microsoft.com>

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

Re: [PATCH v2 0/5] In-place module initialisation

[Valentin Obst]

> Introduce `InPlaceModule`, which allows modules to be initialised
> inplace, as opposed to the current state where modules must return an
> instance which is moved to its final memory location.
>
> This allows us to have modules whose state hold objects that must be
> initialised inplace like locks. It also allows us to implement
> registrations (e.g., driver registration) inplace and have them similar
> to their C counterparts where no new allocations are needed.
>
> This is built on top of the allocation APIs because the sample module is
> a modified version of rust_minimal, which would be incompatible with the
> allocation API series because it uses vectors.
>
> ---
>
> Changes in v2:
>
> - Rebased to latest rust-next
> - Split off adding `Send` requirement to `Module` into a separate patch

I think the idea in [1] was to have this patch being included in the
stable trees. I got little experience with stable trees but wouldn't the
easiest way be that you add:

	Cc: stable@vger.kernel.org # 6.8.x: 715dd8950d4e rust: phy: implement `Send` for `Registration`
	Cc: stable@vger.kernel.org
	Fixes: 247b365dc8dc ("rust: add `kernel` crate")

in the sign-off section for this patch? (Or mark the first one for stable
inclusion as well, [2] has more information on that).

	- Best Valentin

[1]: https://lore.kernel.org/rust-for-linux/20240327032337.188938-1-wedsonaf@gmail.com/T/#m4b4daf27038f920a0c6fafff453efb3c8da72067
[2]: https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html#option-1

> - Prefixed all `core` and `kernel` references with `::` in
>   `module!` macro-generated code.
> - Calling `__pinned_init` with explicit full path.
> - Add `Mutex` to `rust_inplace`  sample.
> - Added `Send` implementation for `Registration` in net/phy.
> - Link to v1: https://lore.kernel.org/rust-for-linux/20240327032337.188938-1-wedsonaf@gmail.com/T/#t
>
> ---
> Wedson Almeida Filho (5):
>   rust: phy: implement `Send` for `Registration`
>   rust: kernel: require `Send` for `Module` implementations
>   rust: module: prefix all module paths with `::`
>   rust: introduce `InPlaceModule`
>   samples: rust: add in-place initialisation sample
>
>  rust/kernel/lib.rs           | 25 +++++++++++++++++++-
>  rust/kernel/net/phy.rs       |  4 ++++
>  rust/macros/module.rs        | 36 ++++++++++++-----------------
>  samples/rust/Kconfig         | 11 +++++++++
>  samples/rust/Makefile        |  1 +
>  samples/rust/rust_inplace.rs | 44 ++++++++++++++++++++++++++++++++++++
>  6 files changed, 99 insertions(+), 22 deletions(-)
>  create mode 100644 samples/rust/rust_inplace.rs
>
>
> base-commit: 453275c66aa4d87c73c5152140b3573ab2435bb7
> --
> 2.34.1

Re: [PATCH v2 0/5] In-place module initialisation

[Miguel Ojeda]

On Fri, Mar 29, 2024 at 1:11 PM Valentin Obst <kernel@valentinobst.de> wrote:
>
> I think the idea in [1] was to have this patch being included in the
> stable trees. I got little experience with stable trees but wouldn't the
> easiest way be that you add:
>
>         Cc: stable@vger.kernel.org # 6.8.x: 715dd8950d4e rust: phy: implement `Send` for `Registration`
>         Cc: stable@vger.kernel.org
>         Fixes: 247b365dc8dc ("rust: add `kernel` crate")
>
> in the sign-off section for this patch? (Or mark the first one for stable
> inclusion as well, [2] has more information on that).

715dd8950d4e is your local hash for 1/5, right? So I would drop the
hash, because it may be confusing.

It may be possible to remove the first line (since 1/5 will only apply
to 6.8.x and it is already the previous patch in the series, while the
`Fixes` tag here may make it clear that 2/5 should still go everywhere
regardless of 1/5), but I guess it does not hurt to be extra clear.

What about:

    Cc: stable@vger.kernel.org # 6.8.x: rust: phy: implement `Send`
for `Registration`
    Cc: stable@vger.kernel.org # 6.1+
    Fixes: 247b365dc8dc ("rust: add `kernel` crate")

Cheers,
Miguel

Re: [PATCH v2 0/5] In-place module initialisation

[Valentin Obst]

> > I think the idea in [1] was to have this patch being included in the
> > stable trees. I got little experience with stable trees but wouldn't the
> > easiest way be that you add:
> >
> >         Cc: stable@vger.kernel.org # 6.8.x: 715dd8950d4e rust: phy: implement `Send` for `Registration`
> >         Cc: stable@vger.kernel.org
> >         Fixes: 247b365dc8dc ("rust: add `kernel` crate")
> >
> > in the sign-off section for this patch? (Or mark the first one for stable
> > inclusion as well, [2] has more information on that).
>
> 715dd8950d4e is your local hash for 1/5, right? So I would drop the
> hash, because it may be confusing.

Ah, right, of course this won't be the hash of the commit in mainline;

>
> It may be possible to remove the first line (since 1/5 will only apply
> to 6.8.x and it is already the previous patch in the series, while the

If I interpret the docs correctly, previous patches in the same series are
only implicitly considered as prerequisites for the marked patch if they
are marked themselves:

    "[...] you do not have to list patch1 as prerequisite of patch2
    if you have already marked patch1 for stable inclusion."

So I guess it is important to be explicit.

> `Fixes` tag here may make it clear that 2/5 should still go everywhere
> regardless of 1/5), but I guess it does not hurt to be extra clear.
>
> What about:
>
>     Cc: stable@vger.kernel.org # 6.8.x: rust: phy: implement `Send`
> for `Registration`
>     Cc: stable@vger.kernel.org # 6.1+
>     Fixes: 247b365dc8dc ("rust: add `kernel` crate")

Looks reasonable to me; Also think that the 6.1+ is not striclty necessary
due to the `Fixes` tag though.

    - Best Valentin

>
> Cheers,
> Miguel

Re: [PATCH v2 0/5] In-place module initialisation

[Miguel Ojeda]

On Fri, Mar 29, 2024 at 3:00 PM Valentin Obst <kernel@valentinobst.de> wrote:
>
> If I interpret the docs correctly, previous patches in the same series are
> only implicitly considered as prerequisites for the marked patch if they
> are marked themselves:
>
>     "[...] you do not have to list patch1 as prerequisite of patch2
>     if you have already marked patch1 for stable inclusion."

Right, "Cc: stable" would be needed in both 1/5 and 2/5 (but one could
remove the "# ..." comments in that case, i.e. it seems simpler).

Cheers,
Miguel

Re: [PATCH v2 0/5] In-place module initialisation

[Miguel Ojeda]

On Thu, Mar 28, 2024 at 8:55 PM Wedson Almeida Filho <wedsonaf@gmail.com> wrote:
>
> From: Wedson Almeida Filho <walmeida@microsoft.com>
>
> Introduce `InPlaceModule`, which allows modules to be initialised
> inplace, as opposed to the current state where modules must return an
> instance which is moved to its final memory location.
>
> This allows us to have modules whose state hold objects that must be
> initialised inplace like locks. It also allows us to implement
> registrations (e.g., driver registration) inplace and have them similar
> to their C counterparts where no new allocations are needed.
>
> This is built on top of the allocation APIs because the sample module is
> a modified version of rust_minimal, which would be incompatible with the
> allocation API series because it uses vectors.

For the moment, applied 1/5 and 2/5 to `rust-fixes` (tagged for stable
too) -- thanks everyone!

Cheers,
Miguel