lkml.org 
[lkml]   [2022]   [Jun]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Subject[RFC[ Alloc in vsprintf
From
Date
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 = {
>

\
 
 \ /
  Last update: 2022-06-26 21:54    [W:0.131 / U:0.072 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site