All of lore.kernel.org
 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 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.