Thread (12 messages) 12 messages, 3 authors, 2025-01-17

Re: [PATCH v13 5/5] rust: Use gendwarfksyms + extended modversions for CONFIG_MODVERSIONS

From: Sami Tolvanen <samitolvanen@google.com>
Date: 2025-01-13 20:04:29
Also in: linux-doc, linux-kbuild, linux-modules, lkml, rust-for-linux

Hi Masahiro,

On Fri, Jan 10, 2025 at 6:26 PM Masahiro Yamada [off-list ref] wrote:
On Sat, Jan 4, 2025 at 2:37 AM Matthew Maurer [off-list ref] wrote:
quoted
From: Sami Tolvanen <samitolvanen@google.com>

Previously, two things stopped Rust from using MODVERSIONS:
1. Rust symbols are occasionally too long to be represented in the
   original versions table
2. Rust types cannot be properly hashed by the existing genksyms
   approach because:
        * Looking up type definitions in Rust is more complex than C
        * Type layout is potentially dependent on the compiler in Rust,
          not just the source type declaration.

CONFIG_EXTENDED_MODVERSIONS addresses the first point, and
CONFIG_GENDWARFKSYMS the second. If Rust wants to use MODVERSIONS, allow
it to do so by selecting both features.

Signed-off-by: Sami Tolvanen <samitolvanen@google.com>
Co-developed-by: Matthew Maurer <redacted>
Signed-off-by: Matthew Maurer <redacted>
---
 init/Kconfig  |  3 ++-
 rust/Makefile | 34 ++++++++++++++++++++++++++++++++--
 2 files changed, 34 insertions(+), 3 deletions(-)
diff --git a/init/Kconfig b/init/Kconfig
index c1f9eb3d5f2e892e977ba1425599502dc830f552..b60acfd9431e0ac2bf401ecb6523b5104ad31150 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -1959,7 +1959,8 @@ config RUST
        bool "Rust support"
        depends on HAVE_RUST
        depends on RUST_IS_AVAILABLE
-       depends on !MODVERSIONS
+       select EXTENDED_MODVERSIONS if MODVERSIONS
+       depends on !MODVERSIONS || GENDWARFKSYMS
        depends on !GCC_PLUGIN_RANDSTRUCT
        depends on !RANDSTRUCT
        depends on !DEBUG_INFO_BTF || PAHOLE_HAS_LANG_EXCLUDE
diff --git a/rust/Makefile b/rust/Makefile
index a40a3936126d603836e0ec9b42a1285916b60e45..80f970ad81f7989afe5ff2b5f633f50feb7f6006 100644
--- a/rust/Makefile
+++ b/rust/Makefile
@@ -329,10 +329,11 @@ $(obj)/bindings/bindings_helpers_generated.rs: private bindgen_target_extra = ;
 $(obj)/bindings/bindings_helpers_generated.rs: $(src)/helpers/helpers.c FORCE
        $(call if_changed_dep,bindgen)

+rust_exports = $(NM) -p --defined-only $(1) | awk '$$2~/(T|R|D|B)/ && $$3!~/__cfi/ { printf $(2),$(3) }'
+
 quiet_cmd_exports = EXPORTS $@
       cmd_exports = \
-       $(NM) -p --defined-only $< \
-               | awk '$$2~/(T|R|D|B)/ && $$3!~/__cfi/ {printf "EXPORT_SYMBOL_RUST_GPL(%s);\n",$$3}' > $@
+       $(call rust_exports,$<,"EXPORT_SYMBOL_RUST_GPL(%s);\n",$$3) > $@
I noticed a nit:

Both of the two callsites of rust_exports pass
'$$3' to the last parameter instead of hardcoding it.

Is it a flexibility for future extensions?

I cannot think of any other use except for printing
the third column, i.e. symbol name.
Good catch, the last parameter isn't necessary anymore. It was used in
early versions of the series to also pass symbol addresses to
gendwarfksyms, but that's not needed since we read the symbol table
directly now.

Sami
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help