In a reply to the printbufs thread, I wrote a proposal to use an
alloc to reduce stack in vsprintf when CONFIG_KALLSYMS is enabled.
No one has replied to this but I think it's somewhat sensible.
Thoughts?
On Mon, 2022-06-20 at 19:10 -0700, Joe Perches wrote:
> In a brief looking around at stack uses in vsprintf, I believe
> this is the largest stack declaration there.
>
> Especially since KSYM_NAME_LEN was increased to 512 by
> commit 394dffa6680c ("kallsyms: increase maximum kernel symbol length to 512")
>
> Perhaps this stack declaration should instead be an alloc/free
> as it can be quite large.
>
> I suppose one could quibble about the kzalloc vs kmalloc or the nominally
> unnecessary initialization of sym.
>
> I think this makes sense though and it reduces the #ifdef uses too.
> ---
> lib/vsprintf.c | 41 ++++++++++++++++++++++++-----------------
> 1 file changed, 24 insertions(+), 17 deletions(-)
>
> diff --git a/lib/vsprintf.c b/lib/vsprintf.c
> index c414a8d9f1ea9..30113a30fd88a 100644
> --- a/lib/vsprintf.c
> +++ b/lib/vsprintf.c
> @@ -980,30 +980,37 @@ char *symbol_string(char *buf, char *end, void *ptr,
> struct printf_spec spec, const char *fmt)
> {
> unsigned long value;
> -#ifdef CONFIG_KALLSYMS
> - char sym[KSYM_SYMBOL_LEN];
> -#endif
> + char *sym = NULL;
>
> if (fmt[1] == 'R')
> ptr = __builtin_extract_return_addr(ptr);
> value = (unsigned long)ptr;
>
> -#ifdef CONFIG_KALLSYMS
> - if (*fmt == 'B' && fmt[1] == 'b')
> - sprint_backtrace_build_id(sym, value);
> - else if (*fmt == 'B')
> - sprint_backtrace(sym, value);
> - else if (*fmt == 'S' && (fmt[1] == 'b' || (fmt[1] == 'R' && fmt[2] == 'b')))
> - sprint_symbol_build_id(sym, value);
> - else if (*fmt != 's')
> - sprint_symbol(sym, value);
> - else
> - sprint_symbol_no_offset(sym, value);
> + if (IS_ENABLED(CONFIG_KALLSYMS) &&
> + (sym = kzalloc(KSYM_SYMBOL_LEN, GFP_NOWAIT | __GFP_NOWARN))) {
> + char *rtn;
> +
> + if (*fmt == 'B' && fmt[1] == 'b')
> + sprint_backtrace_build_id(sym, value);
> + else if (*fmt == 'B')
> + sprint_backtrace(sym, value);
> + else if (*fmt == 'S' &&
> + (fmt[1] == 'b' ||
> + (fmt[1] == 'R' && fmt[2] == 'b')))
> + sprint_symbol_build_id(sym, value);
> + else if (*fmt != 's')
> + sprint_symbol(sym, value);
> + else
> + sprint_symbol_no_offset(sym, value);
> +
> + rtn = string_nocheck(buf, end, sym, spec);
> +
> + kfree(sym);
> +
> + return rtn;
> + }
>
> - return string_nocheck(buf, end, sym, spec);
> -#else
> return special_hex_number(buf, end, value, sizeof(void *));
> -#endif
> }
>
> static const struct printf_spec default_str_spec = {
>
