[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <355e912490dbaef8fe4e12df0201c3f5b439565d.camel@perches.com>
Date: Sun, 26 Jun 2022 12:53:26 -0700
From: Joe Perches <joe@...ches.com>
To: Andrew Morton <akpm@...ux-foundation.org>,
Linus Torvalds <torvalds@...ux-foundation.org>
Cc: David Laight <David.Laight@...LAB.COM>,
Petr Mladek <pmladek@...e.com>,
Steven Rostedt <rostedt@...dmis.org>,
Sergey Senozhatsky <senozhatsky@...omium.org>,
Rasmus Villemoes <linux@...musvillemoes.dk>,
Matthew Wilcox <willy@...radead.org>,
Miguel Ojeda <ojeda@...nel.org>,
Kent Overstreet <kent.overstreet@...il.com>,
Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
LKML <linux-kernel@...r.kernel.org>,
linux-mm <linux-mm@...ck.org>
Subject: [RFC[ Alloc in vsprintf
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 = {
>
Powered by blists - more mailing lists