rust-for-linux.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3] rust: place generated init_module() function in .init.text
@ 2024-02-06 15:38 Thomas Bertschinger
  2024-02-25 20:28 ` Miguel Ojeda
  0 siblings, 1 reply; 2+ messages in thread
From: Thomas Bertschinger @ 2024-02-06 15:38 UTC (permalink / raw)
  To: ojeda, alex.gaynor, wedsonaf, rust-for-linux
  Cc: Thomas Bertschinger, Martin Rodriguez Reboredo, Alice Ryhl

Currently Rust kernel modules have their init code placed in the `.text`
section of the .ko file. I don't think this causes any real problems
for Rust modules as long as all code called during initialization lives
in `.text`.

However, if a Rust `init_module()` function (that lives in `.text`)
calls a function marked with `__init` (in C) or
`#[link_section = ".init.text"]` (in Rust), then a warning is
generated by modpost because that function lives in `.init.text`.
For example:

WARNING: modpost: fs/bcachefs/bcachefs: section mismatch in reference: init_module+0x6 (section: .text) -> _RNvXCsj7d3tFpT5JS_15bcachefs_moduleNtB2_8BcachefsNtCsjDtqRIL3JAG_6kernel6Module4init (section: .init.text)

I ran into this while experimenting with converting the bcachefs kernel
module from C to Rust. The module's `init()`, written in Rust, calls C
functions like `bch2_vfs_init()` which are placed in `.init.text`.

This patch places the macro-generated `init_module()` Rust function in
the `.init.text` section. It also marks `init_module()` as unsafe--now
it may not be called after module initialization completes because it
may be freed already.

Note that this is not enough on its own to actually get all the module
initialization code in that section. The module author must still add
the `#[link_section = ".init.text"]` attribute to the Rust `init()` in
the `impl kernel::Module` block in order to then call `__init`
functions. However, this patch enables module authors do so, when
previously it would not be possible (without warnings).

Signed-off-by: Thomas Bertschinger <tahbertschinger@gmail.com>
Reviewed-by: Martin Rodriguez Reboredo <yakoyoku@gmail.com>
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
---
 v3: adjust safety comment to be more accurate

 rust/macros/module.rs | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/rust/macros/module.rs b/rust/macros/module.rs
index d62d8710d77a..27979e582e4b 100644
--- a/rust/macros/module.rs
+++ b/rust/macros/module.rs
@@ -222,10 +222,15 @@ pub(crate) fn module(ts: TokenStream) -> TokenStream {
             }};
 
             // Loadable modules need to export the `{{init,cleanup}}_module` identifiers.
+            /// # Safety
+            ///
+            /// This function must not be called after module initialization, because it may be
+            /// freed after that completes.
             #[cfg(MODULE)]
             #[doc(hidden)]
             #[no_mangle]
-            pub extern \"C\" fn init_module() -> core::ffi::c_int {{
+            #[link_section = \".init.text\"]
+            pub unsafe extern \"C\" fn init_module() -> core::ffi::c_int {{
                 __init()
             }}
 
-- 
2.43.0


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

* Re: [PATCH v3] rust: place generated init_module() function in .init.text
  2024-02-06 15:38 [PATCH v3] rust: place generated init_module() function in .init.text Thomas Bertschinger
@ 2024-02-25 20:28 ` Miguel Ojeda
  0 siblings, 0 replies; 2+ messages in thread
From: Miguel Ojeda @ 2024-02-25 20:28 UTC (permalink / raw)
  To: Thomas Bertschinger
  Cc: ojeda, alex.gaynor, wedsonaf, rust-for-linux,
	Martin Rodriguez Reboredo, Alice Ryhl

On Tue, Feb 6, 2024 at 4:39 PM Thomas Bertschinger
<tahbertschinger@gmail.com> wrote:
>
> Currently Rust kernel modules have their init code placed in the `.text`
> section of the .ko file. I don't think this causes any real problems
> for Rust modules as long as all code called during initialization lives
> in `.text`.
>
> However, if a Rust `init_module()` function (that lives in `.text`)
> calls a function marked with `__init` (in C) or
> `#[link_section = ".init.text"]` (in Rust), then a warning is
> generated by modpost because that function lives in `.init.text`.
> For example:
>
> WARNING: modpost: fs/bcachefs/bcachefs: section mismatch in reference: init_module+0x6 (section: .text) -> _RNvXCsj7d3tFpT5JS_15bcachefs_moduleNtB2_8BcachefsNtCsjDtqRIL3JAG_6kernel6Module4init (section: .init.text)
>
> I ran into this while experimenting with converting the bcachefs kernel
> module from C to Rust. The module's `init()`, written in Rust, calls C
> functions like `bch2_vfs_init()` which are placed in `.init.text`.
>
> This patch places the macro-generated `init_module()` Rust function in
> the `.init.text` section. It also marks `init_module()` as unsafe--now
> it may not be called after module initialization completes because it
> may be freed already.
>
> Note that this is not enough on its own to actually get all the module
> initialization code in that section. The module author must still add
> the `#[link_section = ".init.text"]` attribute to the Rust `init()` in
> the `impl kernel::Module` block in order to then call `__init`
> functions. However, this patch enables module authors do so, when
> previously it would not be possible (without warnings).
>
> Signed-off-by: Thomas Bertschinger <tahbertschinger@gmail.com>
> Reviewed-by: Martin Rodriguez Reboredo <yakoyoku@gmail.com>
> Reviewed-by: Alice Ryhl <aliceryhl@google.com>

Applied to `rust-next` -- thanks everyone!

[ Reworded title to add prefix. ]

Cheers,
Miguel

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

end of thread, other threads:[~2024-02-25 20:28 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-02-06 15:38 [PATCH v3] rust: place generated init_module() function in .init.text Thomas Bertschinger
2024-02-25 20:28 ` Miguel Ojeda

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).