All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/5] In-place module initialisation
@ 2024-03-28 19:54 Wedson Almeida Filho
  2024-03-28 19:54 ` [PATCH v2 1/5] rust: phy: implement `Send` for `Registration` Wedson Almeida Filho
                   ` (6 more replies)
  0 siblings, 7 replies; 20+ messages in thread
From: Wedson Almeida Filho @ 2024-03-28 19:54 UTC (permalink / raw)
  To: rust-for-linux
  Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	linux-kernel, 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


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH v2 1/5] rust: phy: implement `Send` for `Registration`
  2024-03-28 19:54 [PATCH v2 0/5] In-place module initialisation Wedson Almeida Filho
@ 2024-03-28 19:54 ` Wedson Almeida Filho
  2024-03-29  0:53   ` FUJITA Tomonori
  2024-04-04 12:46   ` Alice Ryhl
  2024-03-28 19:54 ` [PATCH v2 2/5] rust: kernel: require `Send` for `Module` implementations Wedson Almeida Filho
                   ` (5 subsequent siblings)
  6 siblings, 2 replies; 20+ messages in thread
From: Wedson Almeida Filho @ 2024-03-28 19:54 UTC (permalink / raw)
  To: rust-for-linux
  Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	linux-kernel, Wedson Almeida Filho, FUJITA Tomonori,
	Trevor Gross, netdev

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


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v2 2/5] rust: kernel: require `Send` for `Module` implementations
  2024-03-28 19:54 [PATCH v2 0/5] In-place module initialisation Wedson Almeida Filho
  2024-03-28 19:54 ` [PATCH v2 1/5] rust: phy: implement `Send` for `Registration` Wedson Almeida Filho
@ 2024-03-28 19:54 ` Wedson Almeida Filho
  2024-03-30 11:58   ` Benno Lossin
  2024-04-04 12:47   ` Alice Ryhl
  2024-03-28 19:54 ` [PATCH v2 3/5] rust: module: prefix all module paths with `::` Wedson Almeida Filho
                   ` (4 subsequent siblings)
  6 siblings, 2 replies; 20+ messages in thread
From: Wedson Almeida Filho @ 2024-03-28 19:54 UTC (permalink / raw)
  To: rust-for-linux
  Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	linux-kernel, 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


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v2 3/5] rust: module: prefix all module paths with `::`
  2024-03-28 19:54 [PATCH v2 0/5] In-place module initialisation Wedson Almeida Filho
  2024-03-28 19:54 ` [PATCH v2 1/5] rust: phy: implement `Send` for `Registration` Wedson Almeida Filho
  2024-03-28 19:54 ` [PATCH v2 2/5] rust: kernel: require `Send` for `Module` implementations Wedson Almeida Filho
@ 2024-03-28 19:54 ` Wedson Almeida Filho
  2024-03-30 11:46   ` Benno Lossin
  2024-04-04 12:47   ` Alice Ryhl
  2024-03-28 19:54 ` [PATCH v2 4/5] rust: introduce `InPlaceModule` Wedson Almeida Filho
                   ` (3 subsequent siblings)
  6 siblings, 2 replies; 20+ messages in thread
From: Wedson Almeida Filho @ 2024-03-28 19:54 UTC (permalink / raw)
  To: rust-for-linux
  Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	linux-kernel, 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


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v2 4/5] rust: introduce `InPlaceModule`
  2024-03-28 19:54 [PATCH v2 0/5] In-place module initialisation Wedson Almeida Filho
                   ` (2 preceding siblings ...)
  2024-03-28 19:54 ` [PATCH v2 3/5] rust: module: prefix all module paths with `::` Wedson Almeida Filho
@ 2024-03-28 19:54 ` Wedson Almeida Filho
  2024-04-04 12:48   ` Alice Ryhl
  2024-03-28 19:54 ` [PATCH v2 5/5] samples: rust: add in-place initialisation sample Wedson Almeida Filho
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 20+ messages in thread
From: Wedson Almeida Filho @ 2024-03-28 19:54 UTC (permalink / raw)
  To: rust-for-linux
  Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	linux-kernel, Wedson Almeida Filho, Valentin Obst

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


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v2 5/5] samples: rust: add in-place initialisation sample
  2024-03-28 19:54 [PATCH v2 0/5] In-place module initialisation Wedson Almeida Filho
                   ` (3 preceding siblings ...)
  2024-03-28 19:54 ` [PATCH v2 4/5] rust: introduce `InPlaceModule` Wedson Almeida Filho
@ 2024-03-28 19:54 ` Wedson Almeida Filho
  2024-03-30 11:49   ` Benno Lossin
  2024-04-04 12:48   ` Alice Ryhl
  2024-03-29 12:10 ` [PATCH v2 0/5] In-place module initialisation Valentin Obst
  2024-04-23  0:42 ` Miguel Ojeda
  6 siblings, 2 replies; 20+ messages in thread
From: Wedson Almeida Filho @ 2024-03-28 19:54 UTC (permalink / raw)
  To: rust-for-linux
  Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	linux-kernel, 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


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 1/5] rust: phy: implement `Send` for `Registration`
  2024-03-28 19:54 ` [PATCH v2 1/5] rust: phy: implement `Send` for `Registration` Wedson Almeida Filho
@ 2024-03-29  0:53   ` FUJITA Tomonori
  2024-04-04 12:46   ` Alice Ryhl
  1 sibling, 0 replies; 20+ messages in thread
From: FUJITA Tomonori @ 2024-03-29  0:53 UTC (permalink / raw)
  To: wedsonaf
  Cc: rust-for-linux, ojeda, alex.gaynor, boqun.feng, gary, bjorn3_gh,
	benno.lossin, a.hindborg, aliceryhl, linux-kernel, walmeida,
	fujita.tomonori, tmgross, netdev

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`.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 0/5] In-place module initialisation
  2024-03-28 19:54 [PATCH v2 0/5] In-place module initialisation Wedson Almeida Filho
                   ` (4 preceding siblings ...)
  2024-03-28 19:54 ` [PATCH v2 5/5] samples: rust: add in-place initialisation sample Wedson Almeida Filho
@ 2024-03-29 12:10 ` Valentin Obst
  2024-03-29 13:03   ` Miguel Ojeda
  2024-04-23  0:42 ` Miguel Ojeda
  6 siblings, 1 reply; 20+ messages in thread
From: Valentin Obst @ 2024-03-29 12:10 UTC (permalink / raw)
  To: wedsonaf
  Cc: a.hindborg, alex.gaynor, aliceryhl, benno.lossin, bjorn3_gh,
	boqun.feng, gary, linux-kernel, ojeda, rust-for-linux, walmeida

> 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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 0/5] In-place module initialisation
  2024-03-29 12:10 ` [PATCH v2 0/5] In-place module initialisation Valentin Obst
@ 2024-03-29 13:03   ` Miguel Ojeda
  2024-03-29 14:00     ` Valentin Obst
  0 siblings, 1 reply; 20+ messages in thread
From: Miguel Ojeda @ 2024-03-29 13:03 UTC (permalink / raw)
  To: Valentin Obst
  Cc: wedsonaf, a.hindborg, alex.gaynor, aliceryhl, benno.lossin,
	bjorn3_gh, boqun.feng, gary, linux-kernel, ojeda, rust-for-linux,
	walmeida

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 0/5] In-place module initialisation
  2024-03-29 13:03   ` Miguel Ojeda
@ 2024-03-29 14:00     ` Valentin Obst
  2024-03-29 14:26       ` Miguel Ojeda
  0 siblings, 1 reply; 20+ messages in thread
From: Valentin Obst @ 2024-03-29 14:00 UTC (permalink / raw)
  To: miguel.ojeda.sandonis
  Cc: a.hindborg, alex.gaynor, aliceryhl, benno.lossin, bjorn3_gh,
	boqun.feng, gary, kernel, linux-kernel, ojeda, rust-for-linux,
	walmeida, wedsonaf

> > 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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 0/5] In-place module initialisation
  2024-03-29 14:00     ` Valentin Obst
@ 2024-03-29 14:26       ` Miguel Ojeda
  0 siblings, 0 replies; 20+ messages in thread
From: Miguel Ojeda @ 2024-03-29 14:26 UTC (permalink / raw)
  To: Valentin Obst
  Cc: a.hindborg, alex.gaynor, aliceryhl, benno.lossin, bjorn3_gh,
	boqun.feng, gary, linux-kernel, ojeda, rust-for-linux, walmeida,
	wedsonaf

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 3/5] rust: module: prefix all module paths with `::`
  2024-03-28 19:54 ` [PATCH v2 3/5] rust: module: prefix all module paths with `::` Wedson Almeida Filho
@ 2024-03-30 11:46   ` Benno Lossin
  2024-04-04 12:47   ` Alice Ryhl
  1 sibling, 0 replies; 20+ messages in thread
From: Benno Lossin @ 2024-03-30 11:46 UTC (permalink / raw)
  To: Wedson Almeida Filho, rust-for-linux
  Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Andreas Hindborg, Alice Ryhl, linux-kernel,
	Wedson Almeida Filho

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


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 5/5] samples: rust: add in-place initialisation sample
  2024-03-28 19:54 ` [PATCH v2 5/5] samples: rust: add in-place initialisation sample Wedson Almeida Filho
@ 2024-03-30 11:49   ` Benno Lossin
  2024-04-04 12:48   ` Alice Ryhl
  1 sibling, 0 replies; 20+ messages in thread
From: Benno Lossin @ 2024-03-30 11:49 UTC (permalink / raw)
  To: Wedson Almeida Filho, rust-for-linux
  Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Andreas Hindborg, Alice Ryhl, linux-kernel,
	Wedson Almeida Filho

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


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 2/5] rust: kernel: require `Send` for `Module` implementations
  2024-03-28 19:54 ` [PATCH v2 2/5] rust: kernel: require `Send` for `Module` implementations Wedson Almeida Filho
@ 2024-03-30 11:58   ` Benno Lossin
  2024-04-04 12:47   ` Alice Ryhl
  1 sibling, 0 replies; 20+ messages in thread
From: Benno Lossin @ 2024-03-30 11:58 UTC (permalink / raw)
  To: Wedson Almeida Filho, rust-for-linux
  Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Andreas Hindborg, Alice Ryhl, linux-kernel,
	Wedson Almeida Filho

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


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 1/5] rust: phy: implement `Send` for `Registration`
  2024-03-28 19:54 ` [PATCH v2 1/5] rust: phy: implement `Send` for `Registration` Wedson Almeida Filho
  2024-03-29  0:53   ` FUJITA Tomonori
@ 2024-04-04 12:46   ` Alice Ryhl
  1 sibling, 0 replies; 20+ messages in thread
From: Alice Ryhl @ 2024-04-04 12:46 UTC (permalink / raw)
  To: Wedson Almeida Filho
  Cc: rust-for-linux, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg,
	linux-kernel, Wedson Almeida Filho, FUJITA Tomonori,
	Trevor Gross, netdev

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>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 2/5] rust: kernel: require `Send` for `Module` implementations
  2024-03-28 19:54 ` [PATCH v2 2/5] rust: kernel: require `Send` for `Module` implementations Wedson Almeida Filho
  2024-03-30 11:58   ` Benno Lossin
@ 2024-04-04 12:47   ` Alice Ryhl
  1 sibling, 0 replies; 20+ messages in thread
From: Alice Ryhl @ 2024-04-04 12:47 UTC (permalink / raw)
  To: Wedson Almeida Filho
  Cc: rust-for-linux, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg,
	linux-kernel, Wedson Almeida Filho

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>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 3/5] rust: module: prefix all module paths with `::`
  2024-03-28 19:54 ` [PATCH v2 3/5] rust: module: prefix all module paths with `::` Wedson Almeida Filho
  2024-03-30 11:46   ` Benno Lossin
@ 2024-04-04 12:47   ` Alice Ryhl
  1 sibling, 0 replies; 20+ messages in thread
From: Alice Ryhl @ 2024-04-04 12:47 UTC (permalink / raw)
  To: Wedson Almeida Filho
  Cc: rust-for-linux, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg,
	linux-kernel, Wedson Almeida Filho

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>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 4/5] rust: introduce `InPlaceModule`
  2024-03-28 19:54 ` [PATCH v2 4/5] rust: introduce `InPlaceModule` Wedson Almeida Filho
@ 2024-04-04 12:48   ` Alice Ryhl
  0 siblings, 0 replies; 20+ messages in thread
From: Alice Ryhl @ 2024-04-04 12:48 UTC (permalink / raw)
  To: Wedson Almeida Filho
  Cc: rust-for-linux, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg,
	linux-kernel, Wedson Almeida Filho, Valentin Obst

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>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 5/5] samples: rust: add in-place initialisation sample
  2024-03-28 19:54 ` [PATCH v2 5/5] samples: rust: add in-place initialisation sample Wedson Almeida Filho
  2024-03-30 11:49   ` Benno Lossin
@ 2024-04-04 12:48   ` Alice Ryhl
  1 sibling, 0 replies; 20+ messages in thread
From: Alice Ryhl @ 2024-04-04 12:48 UTC (permalink / raw)
  To: Wedson Almeida Filho
  Cc: rust-for-linux, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg,
	linux-kernel, Wedson Almeida Filho

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>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 0/5] In-place module initialisation
  2024-03-28 19:54 [PATCH v2 0/5] In-place module initialisation Wedson Almeida Filho
                   ` (5 preceding siblings ...)
  2024-03-29 12:10 ` [PATCH v2 0/5] In-place module initialisation Valentin Obst
@ 2024-04-23  0:42 ` Miguel Ojeda
  6 siblings, 0 replies; 20+ messages in thread
From: Miguel Ojeda @ 2024-04-23  0:42 UTC (permalink / raw)
  To: Wedson Almeida Filho
  Cc: rust-for-linux, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	linux-kernel, Wedson Almeida Filho

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2024-04-23  0:43 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-03-28 19:54 [PATCH v2 0/5] In-place module initialisation Wedson Almeida Filho
2024-03-28 19:54 ` [PATCH v2 1/5] rust: phy: implement `Send` for `Registration` Wedson Almeida Filho
2024-03-29  0:53   ` FUJITA Tomonori
2024-04-04 12:46   ` Alice Ryhl
2024-03-28 19:54 ` [PATCH v2 2/5] rust: kernel: require `Send` for `Module` implementations Wedson Almeida Filho
2024-03-30 11:58   ` Benno Lossin
2024-04-04 12:47   ` Alice Ryhl
2024-03-28 19:54 ` [PATCH v2 3/5] rust: module: prefix all module paths with `::` Wedson Almeida Filho
2024-03-30 11:46   ` Benno Lossin
2024-04-04 12:47   ` Alice Ryhl
2024-03-28 19:54 ` [PATCH v2 4/5] rust: introduce `InPlaceModule` Wedson Almeida Filho
2024-04-04 12:48   ` Alice Ryhl
2024-03-28 19:54 ` [PATCH v2 5/5] samples: rust: add in-place initialisation sample Wedson Almeida Filho
2024-03-30 11:49   ` Benno Lossin
2024-04-04 12:48   ` Alice Ryhl
2024-03-29 12:10 ` [PATCH v2 0/5] In-place module initialisation Valentin Obst
2024-03-29 13:03   ` Miguel Ojeda
2024-03-29 14:00     ` Valentin Obst
2024-03-29 14:26       ` Miguel Ojeda
2024-04-23  0:42 ` Miguel Ojeda

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.