rust-for-linux.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: david laight <david.laight@runbox.com>
To: Miguel Ojeda <ojeda@kernel.org>
Cc: "Nathan Chancellor" <nathan@kernel.org>,
	"Nicolas Schier" <nicolas@fjasle.eu>,
	linux-kbuild@vger.kernel.org, "Boqun Feng" <boqun.feng@gmail.com>,
	"Gary Guo" <gary@garyguo.net>,
	"Björn Roy Baron" <bjorn3_gh@protonmail.com>,
	"Benno Lossin" <lossin@kernel.org>,
	"Andreas Hindborg" <a.hindborg@kernel.org>,
	"Alice Ryhl" <aliceryhl@google.com>,
	"Trevor Gross" <tmgross@umich.edu>,
	"Danilo Krummrich" <dakr@kernel.org>,
	"Sergio González Collado" <sergio.collado@gmail.com>,
	rust-for-linux@vger.kernel.org, linux-kernel@vger.kernel.org,
	patches@lists.linux.dev
Subject: Re: `KSYM_NAME_LEN` strikes again
Date: Mon, 1 Dec 2025 09:15:14 +0000	[thread overview]
Message-ID: <20251201091514.0bea544b@pumpkin> (raw)
In-Reply-To: <20251201021659.1729308-1-ojeda@kernel.org>

On Mon,  1 Dec 2025 03:16:59 +0100
Miguel Ojeda <ojeda@kernel.org> wrote:

> Hi all,
> 
> In some Rust configs with -Os and doctests enabled, we are hitting
> KSYM_NAME_LEN again:
> 
>     Symbol _RINvMs3_NvNtNtCs1ewLyjEZ7Le_6kernel4sync4lock1__INtB6_12___ThePinData
>     NtNvNvNvCs1RN9EnoqSlC_25doctests_kernel_generated43rust_doctest_kernel_sync_lock_spinlock_rs_0
>     4main52__doctest_main_rust_kernel_sync_lock_spinlock_rs_34_05InnerNtNtB8_8spinlock
>     15SpinLockBackendE5stateNtNtCsb7ts3l0a5c3_4core7convert10InfallibleINtNtCs5hDniGXnib3_
>     8pin_init10___internal11InitClosureNCINvMs5_NtBc_5typesINtB61_6OpaqueNtNtCse4H1uDrGHP9_
>     8bindings12bindings_raw8spinlockE8ffi_initNCNCNvMs0_B8_INtB8_4LockB15_B3F_E3new0s_0E0B6c_B4j_EEB1d_
>     too long for kallsyms (518 >= 512).
>     Please increase KSYM_NAME_LEN both in kernel and kallsyms.c
> 
> Which expands to e.g.
> 
>     <kernel::sync::lock::_::__ThePinData<doctests_kernel_generated::rust_doctest_kernel_sync_lock_spinlock_rs_0::main::_doctest_main_rust_kernel_sync_lock_spinlock_rs_34_0::Inner,
>     kernel::sync::lock::spinlock::SpinLockBackend>>::state::<core::convert::Infallible,  
>     pin_init::__internal::InitClosure<<kernel::types::Opaque<bindings::bindings_raw::spinlock>>::ffi_init
>     <<kernel::sync::lock::Lock<doctests_kernel_generated::rust_doctest_kernel_sync_lock_spinlock_rs_0::main::_doctest_main_rust_kernel_sync_lock_spinlock_rs_34_0::Inner,
>     kernel::sync::lock::spinlock::SpinLockBackend>>::new::{closure#0}::{closure#1}>::{closure#0},  
>     kernel::types::Opaque<bindings::bindings_raw::spinlock>, core::convert::Infallible>>

Given that is even more unreadable than a typical C++ name is it actually worth printing?
Looking at it, perhaps some of the inner <...> could be replaced with a hash
(possibly with the last real name added) with the ability to look up the hash
in a separate table if needed.
I think that is equivalent to using a short name for a type?

If rust is using the same ELF symbol resolution as C uses in shared libraries
you really don't want long symbols at all.
Clearly no one thought about the performance of dynamic linking.

	David


> 
> If we decide to increase KSYM_NAME_LEN again, we need to use a number
> that is a multiple of 64:
> 
>     BUILD_BUG_ON(KSYM_NAME_LEN % sizeof(u64));
> 
> I first tried with 512+128, but I still saw things symbols slightly over
> that:
> 
>     too long for kallsyms (645 >= 640).
> 
> So I picked 512+256 and it builds clean with a patch like the one below.
> Hopefully nobody relies on it being a power of 2.
> 
> Other options:
> 
>   - Conditionally doing it only for those builds that need it, to avoid
>     penalizing other builds (including normal Rust builds).
> 
>   - Building the Rust side with -O2 for now, or at least tweaking some
>     of the inlining behavior under -Os.
> 
>   - Not supporting Rust for `CC_OPTIMIZE_FOR_SIZE` for now (e.g. under
>     `BROKEN`).
> 
>   - Stop all this and go with a hashing approach like Willy/Linus
>     discussed back then:
> 
>         https://lore.kernel.org/rust-for-linux/CAHk-=whzXv=Fu7dQshSTyd9H1-JS5=gyKwW-GMNGccAKs4Mwpg@mail.gmail.com/
> 
> Cheers,
> Miguel
> 
> From: Miguel Ojeda <ojeda@kernel.org>
> Date: Sun, 30 Nov 2025 17:46:29 +0100
> Subject: [PATCH] kallsyms: increase KSYM_NAME_LEN to 768 (512+256)
> 
> Untested!
> ---
>  include/linux/kallsyms.h            | 2 +-
>  kernel/livepatch/core.c             | 4 ++--
>  lib/tests/longest_symbol_kunit.c    | 7 +++++--
>  scripts/kallsyms.c                  | 2 +-
>  tools/include/linux/kallsyms.h      | 2 +-
>  tools/lib/perf/include/perf/event.h | 2 +-
>  tools/lib/symbol/kallsyms.h         | 2 +-
>  7 files changed, 12 insertions(+), 9 deletions(-)
> 
> diff --git a/include/linux/kallsyms.h b/include/linux/kallsyms.h
> index d5dd54c53ace..f17d335f36b2 100644
> --- a/include/linux/kallsyms.h
> +++ b/include/linux/kallsyms.h
> @@ -15,7 +15,7 @@
> 
>  #include <asm/sections.h>
> 
> -#define KSYM_NAME_LEN 512
> +#define KSYM_NAME_LEN 768
>  #define KSYM_SYMBOL_LEN (sizeof("%s+%#lx/%#lx [%s %s]") + \
>  			(KSYM_NAME_LEN - 1) + \
>  			2*(BITS_PER_LONG*3/10) + (MODULE_NAME_LEN - 1) + \
> diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
> index 0e73fac55f8e..cf7f2d561386 100644
> --- a/kernel/livepatch/core.c
> +++ b/kernel/livepatch/core.c
> @@ -210,7 +210,7 @@ static int klp_resolve_symbols(Elf_Shdr *sechdrs, const char *strtab,
>  	 * we use the smallest/strictest upper bound possible (56, based on
>  	 * the current definition of MODULE_NAME_LEN) to prevent overflows.
>  	 */
> -	BUILD_BUG_ON(MODULE_NAME_LEN < 56 || KSYM_NAME_LEN != 512);
> +	BUILD_BUG_ON(MODULE_NAME_LEN < 56 || KSYM_NAME_LEN != 768);
> 
>  	relas = (Elf_Rela *) relasec->sh_addr;
>  	/* For each rela in this klp relocation section */
> @@ -224,7 +224,7 @@ static int klp_resolve_symbols(Elf_Shdr *sechdrs, const char *strtab,
> 
>  		/* Format: .klp.sym.sym_objname.sym_name,sympos */
>  		cnt = sscanf(strtab + sym->st_name,
> -			     ".klp.sym.%55[^.].%511[^,],%lu",
> +			     ".klp.sym.%55[^.].%767[^,],%lu",
>  			     sym_objname, sym_name, &sympos);
>  		if (cnt != 3) {
>  			pr_err("symbol %s has an incorrectly formatted name\n",
> diff --git a/lib/tests/longest_symbol_kunit.c b/lib/tests/longest_symbol_kunit.c
> index 9b4de3050ba7..ff8eed2cb4bd 100644
> --- a/lib/tests/longest_symbol_kunit.c
> +++ b/lib/tests/longest_symbol_kunit.c
> @@ -19,8 +19,11 @@
>  #define DDDDI(name) DDDI(n##name##name)
>  #define DDDDDI(name) DDDDI(n##name##name)
> 
> -/*Generate a symbol whose name length is 511 */
> -#define LONGEST_SYM_NAME  DDDDDI(g1h2i3j4k5l6m7n)
> +#define __SUM(A, B) s##A##B
> +#define SUM(A, B) __SUM(A, B)
> +
> +/* Generate a symbol whose name length is KSYM_NAME_LEN (including null) */
> +#define LONGEST_SYM_NAME SUM(DDDDDI(g1h2i3j4k5l6m7n),DDDDI(g1h2i3j4k5l6m7n))
> 
>  #define RETURN_LONGEST_SYM 0xAAAAA
> 
> diff --git a/scripts/kallsyms.c b/scripts/kallsyms.c
> index 4b0234e4b12f..c90bb90c874e 100644
> --- a/scripts/kallsyms.c
> +++ b/scripts/kallsyms.c
> @@ -31,7 +31,7 @@
> 
>  #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof(arr[0]))
> 
> -#define KSYM_NAME_LEN		512
> +#define KSYM_NAME_LEN		768
> 
>  struct sym_entry {
>  	unsigned long long addr;
> diff --git a/tools/include/linux/kallsyms.h b/tools/include/linux/kallsyms.h
> index f61a01dd7eb7..089b59906641 100644
> --- a/tools/include/linux/kallsyms.h
> +++ b/tools/include/linux/kallsyms.h
> @@ -6,7 +6,7 @@
>  #include <stdio.h>
>  #include <unistd.h>
> 
> -#define KSYM_NAME_LEN 512
> +#define KSYM_NAME_LEN 768
> 
>  struct module;
> 
> diff --git a/tools/lib/perf/include/perf/event.h b/tools/lib/perf/include/perf/event.h
> index aa1e91c97a22..ea3debf97db9 100644
> --- a/tools/lib/perf/include/perf/event.h
> +++ b/tools/lib/perf/include/perf/event.h
> @@ -104,7 +104,7 @@ struct perf_record_throttle {
>  };
> 
>  #ifndef KSYM_NAME_LEN
> -#define KSYM_NAME_LEN 512
> +#define KSYM_NAME_LEN 768
>  #endif
> 
>  struct perf_record_ksymbol {
> diff --git a/tools/lib/symbol/kallsyms.h b/tools/lib/symbol/kallsyms.h
> index 542f9b059c3b..435f626c92d1 100644
> --- a/tools/lib/symbol/kallsyms.h
> +++ b/tools/lib/symbol/kallsyms.h
> @@ -7,7 +7,7 @@
>  #include <linux/types.h>
> 
>  #ifndef KSYM_NAME_LEN
> -#define KSYM_NAME_LEN 512
> +#define KSYM_NAME_LEN 768
>  #endif
> 
>  static inline u8 kallsyms2elf_binding(char type)
> 
> base-commit: 7d0a66e4bb9081d75c82ec4957c50034cb0ea449
> --
> 2.52.0
> 


  parent reply	other threads:[~2025-12-01  9:15 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-12-01  2:16 `KSYM_NAME_LEN` strikes again Miguel Ojeda
2025-12-01  4:56 ` John Hubbard
2025-12-01 16:30   ` Miguel Ojeda
2025-12-01  9:15 ` david laight [this message]
2025-12-01 16:32   ` Miguel Ojeda

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20251201091514.0bea544b@pumpkin \
    --to=david.laight@runbox.com \
    --cc=a.hindborg@kernel.org \
    --cc=aliceryhl@google.com \
    --cc=bjorn3_gh@protonmail.com \
    --cc=boqun.feng@gmail.com \
    --cc=dakr@kernel.org \
    --cc=gary@garyguo.net \
    --cc=linux-kbuild@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lossin@kernel.org \
    --cc=nathan@kernel.org \
    --cc=nicolas@fjasle.eu \
    --cc=ojeda@kernel.org \
    --cc=patches@lists.linux.dev \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=sergio.collado@gmail.com \
    --cc=tmgross@umich.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).