[PATCH v1] rust: module: place cleanup_module() in .exit.text section

[PATCH v1] rust: module: place cleanup_module() in .exit.text section

[FUJITA Tomonori]

Place cleanup_module() in .exit.text section. Currently,
cleanup_module() is likely placed in the .text section. It's
inconsistent with the layout of C modules, where cleanup_module() is
placed in .exit.text.

Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
---
 rust/macros/module.rs | 1 +
 1 file changed, 1 insertion(+)

diff --git a/rust/macros/module.rs b/rust/macros/module.rs
index cdf94f4982df..36daa149a691 100644
--- a/rust/macros/module.rs
+++ b/rust/macros/module.rs
@@ -275,6 +275,7 @@ mod __module_init {{
                     #[cfg(MODULE)]
                     #[doc(hidden)]
                     #[no_mangle]
+                    #[link_section = \".exit.text\"]
                     pub extern \"C\" fn cleanup_module() {{
                         // SAFETY:
                         // - This function is inaccessible to the outside due to the double

base-commit: 6ad64bf91728502fe8a4d1419c0a3e4fd323f503
-- 
2.43.0

Re: [PATCH v1] rust: module: place cleanup_module() in .exit.text section

[Jarkko Sakkinen]

On Sat, Mar 08, 2025 at 01:45:06PM +0900, FUJITA Tomonori wrote:
> Place cleanup_module() in .exit.text section. Currently,
> cleanup_module() is likely placed in the .text section. It's
> inconsistent with the layout of C modules, where cleanup_module() is
> placed in .exit.text.
> 
> Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
> ---
>  rust/macros/module.rs | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/rust/macros/module.rs b/rust/macros/module.rs
> index cdf94f4982df..36daa149a691 100644
> --- a/rust/macros/module.rs
> +++ b/rust/macros/module.rs
> @@ -275,6 +275,7 @@ mod __module_init {{
>                      #[cfg(MODULE)]
>                      #[doc(hidden)]
>                      #[no_mangle]
> +                    #[link_section = \".exit.text\"]
>                      pub extern \"C\" fn cleanup_module() {{
>                          // SAFETY:
>                          // - This function is inaccessible to the outside due to the double
> 
> base-commit: 6ad64bf91728502fe8a4d1419c0a3e4fd323f503
> -- 
> 2.43.0
> 
> 

Acked-by: Jarkko Sakkinen <jarkko@kernel.org>

BR, Jarkko

Re: [PATCH v1] rust: module: place cleanup_module() in .exit.text section

[Boqun Feng]

On Sat, Mar 08, 2025 at 01:45:06PM +0900, FUJITA Tomonori wrote:
> Place cleanup_module() in .exit.text section. Currently,
> cleanup_module() is likely placed in the .text section. It's

Do you mind compiling a module and using readelf or other tools to show
which section the symbol cleanup_module() is? Putting that information
in the changelog would be nice, and it also shows the actual effect of
your patch. Thanks!

Regards,
Boqun

> inconsistent with the layout of C modules, where cleanup_module() is
> placed in .exit.text.
> 
> Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
> ---
>  rust/macros/module.rs | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/rust/macros/module.rs b/rust/macros/module.rs
> index cdf94f4982df..36daa149a691 100644
> --- a/rust/macros/module.rs
> +++ b/rust/macros/module.rs
> @@ -275,6 +275,7 @@ mod __module_init {{
>                      #[cfg(MODULE)]
>                      #[doc(hidden)]
>                      #[no_mangle]
> +                    #[link_section = \".exit.text\"]
>                      pub extern \"C\" fn cleanup_module() {{
>                          // SAFETY:
>                          // - This function is inaccessible to the outside due to the double
> 
> base-commit: 6ad64bf91728502fe8a4d1419c0a3e4fd323f503
> -- 
> 2.43.0
> 

Re: [PATCH v1] rust: module: place cleanup_module() in .exit.text section

[FUJITA Tomonori]

On Fri, 7 Mar 2025 21:32:31 -0800
Boqun Feng <boqun.feng@gmail.com> wrote:

> On Sat, Mar 08, 2025 at 01:45:06PM +0900, FUJITA Tomonori wrote:
>> Place cleanup_module() in .exit.text section. Currently,
>> cleanup_module() is likely placed in the .text section. It's
> 
> Do you mind compiling a module and using readelf or other tools to show
> which section the symbol cleanup_module() is? Putting that information
> in the changelog would be nice, and it also shows the actual effect of
> your patch. Thanks!

Of course!

o layout of C module example

$ objdump -t ~/build/x86/drivers/block/loop.o|grep clean
0000000000000000 l     O .exit.data	0000000000000008 __UNIQUE_ID___addressable_cleanup_module412
0000000000000000 g     F .exit.text	000000000000009c cleanup_module

o layout of Rust module example without this patch

$ objdump -t ~/build/x86/samples/rust/rust_minimal.o|grep clean
00000000000002b0 g     F .text	00000000000000c6 cleanup_module
0000000000000000 g     O .exit.data	0000000000000008 _RNvNtNtCskuib3KEDsuT_12rust_minimal13___module_init13___module_init40___UNIQUE_ID___addressable_cleanup_module

o layout of Rust module example with this patch

$ objdump -t ~/build/x86/samples/rust/rust_minimal.o|grep clean
0000000000000000 g     F .exit.text	00000000000000c6 cleanup_module
0000000000000000 g     O .exit.data	0000000000000008 _RNvNtNtCskuib3KEDsuT_12rust_minimal13___module_init13___module_init40___UNIQUE_ID___addressable_cleanup_module

Re: [PATCH v1] rust: module: place cleanup_module() in .exit.text section

[Miguel Ojeda]

On Sat, Mar 8, 2025 at 5:45 AM FUJITA Tomonori
<fujita.tomonori@gmail.com> wrote:
>
> Place cleanup_module() in .exit.text section. Currently,
> cleanup_module() is likely placed in the .text section. It's
> inconsistent with the layout of C modules, where cleanup_module() is
> placed in .exit.text.
>
> Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>

I think this is OK -- it is only under `MODULE`, and `__exit` is OK to
place in other functions (`module_cleanup` gets it by copy).

To be extra careful, I will pick it early in the next cycle, unless
someone sees something wrong with it.

Cheers,
Miguel

Re: [PATCH v1] rust: module: place cleanup_module() in .exit.text section

[FUJITA Tomonori]

On Mon, 24 Mar 2025 20:18:13 +0100
Miguel Ojeda <miguel.ojeda.sandonis@gmail.com> wrote:

>> Place cleanup_module() in .exit.text section. Currently,
>> cleanup_module() is likely placed in the .text section. It's
>> inconsistent with the layout of C modules, where cleanup_module() is
>> placed in .exit.text.
>>
>> Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
> 
> I think this is OK -- it is only under `MODULE`, and `__exit` is OK to
> place in other functions (`module_cleanup` gets it by copy).
> 
> To be extra careful, I will pick it early in the next cycle, unless
> someone sees something wrong with it.

Do I need to update the commit message and submit a v2 of this patch?

Re: [PATCH v1] rust: module: place cleanup_module() in .exit.text section

[Miguel Ojeda]

On Fri, May 9, 2025 at 6:16 AM FUJITA Tomonori
<fujita.tomonori@gmail.com> wrote:
>
> Do I need to update the commit message and submit a v2 of this patch?

I think it is OK, i.e. we can add what you wrote above when applying
(though it always helps to do it on your side when it is a non-trivial
change).

By the way, Cc'ing MODULE SUPPORT in case they want to say something
(or even take it themselves).

Thanks!

Cheers,
Miguel

Re: [PATCH v1] rust: module: place cleanup_module() in .exit.text section

[Miguel Ojeda]

On Sat, Mar 8, 2025 at 5:45 AM FUJITA Tomonori
<fujita.tomonori@gmail.com> wrote:
>
> Place cleanup_module() in .exit.text section. Currently,
> cleanup_module() is likely placed in the .text section. It's
> inconsistent with the layout of C modules, where cleanup_module() is
> placed in .exit.text.
>
> Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>

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

    [ Boqun asked for an example of how the section changed to be
      put in the log. Tomonori provided the following examples:

        C module:

          $ objdump -t ~/build/x86/drivers/block/loop.o|grep clean
          0000000000000000 l     O .exit.data    0000000000000008
__UNIQUE_ID___addressable_cleanup_module412
          0000000000000000 g     F .exit.text    000000000000009c cleanup_module

        Rust module without this patch:

          $ objdump -t ~/build/x86/samples/rust/rust_minimal.o|grep clean
          00000000000002b0 g     F .text         00000000000000c6 cleanup_module
          0000000000000000 g     O .exit.data    0000000000000008
_R...___UNIQUE_ID___addressable_cleanup_module

        Rust module with this patch:

          $ objdump -t ~/build/x86/samples/rust/rust_minimal.o|grep clean
          0000000000000000 g     F .exit.text    00000000000000c6 cleanup_module
          0000000000000000 g     O .exit.data    0000000000000008
_R...___UNIQUE_ID___addressable_cleanup_module

      - Miguel ]

Cheers,
Miguel