From: Nick Desaulniers <ndesaulniers@google.com>
To: Steven Rostedt <rostedt@goodmis.org>
Cc: Arnd Bergmann <arnd@kernel.org>, Ingo Molnar <mingo@redhat.com>,
Nathan Chancellor <nathan@kernel.org>,
Arnd Bergmann <arnd@arndb.de>,
"Steven Rostedt (VMware)" <rostedt@godmis.org>,
Tom Zanussi <zanussi@kernel.org>,
Masami Hiramatsu <mhiramat@kernel.org>,
Qiujun Huang <hqjagain@gmail.com>, Tom Rix <trix@redhat.com>,
LKML <linux-kernel@vger.kernel.org>,
clang-built-linux <clang-built-linux@googlegroups.com>
Subject: Re: [PATCH] tracing: events_hist: avoid using excessive stack space
Date: Fri, 14 May 2021 13:19:24 -0700 [thread overview]
Message-ID: <CAKwvOdmJfm+PKxjR-N=YfFcjrXi4N4xUiRCLbSNMNJ+5g_wX+g@mail.gmail.com> (raw)
In-Reply-To: <20210514101610.4392adbc@gandalf.local.home>
On Fri, May 14, 2021 at 7:16 AM Steven Rostedt <rostedt@goodmis.org> wrote:
>
> On Fri, 14 May 2021 16:04:25 +0200
> Arnd Bergmann <arnd@kernel.org> wrote:
>
> > From: Arnd Bergmann <arnd@arndb.de>
> >
> > In some configurations, clang produces a warning about an overly large
> > amount of stack space used in hist_trigger_print_key():
> >
> > kernel/trace/trace_events_hist.c:4594:13: error: stack frame size of 1248 bytes in function 'hist_trigger_print_key' [-Werror,-Wframe-larger-than=]
> > static void hist_trigger_print_key(struct seq_file *m,
> >
> > Moving the 'str' variable into a more local scope in the two places
> > where it gets used actually reduces the the used stack space here
> > and gets it below the warning limit, because the compiler can now
> > assume that it is safe to use the same stack slot that it has for
> > the stack of any inline function.
>
> Thanks Arnd for the nice explanation of the rationale for this change.
>
> But I still find it too subtle to my liking that we need to move the
> declaration like this (and duplicate it twice) for internal behavior of the
> compiler (where it can't figure out itself by the use cases if it can
> optimize the stack).
Under which configurations this warning was observed wasn't specified,
but I'd bet it's one of the sanitizers splitting this in two in order
to tell which branch may have overflowed the buffer.
--
Thanks,
~Nick Desaulniers
next prev parent reply other threads:[~2021-05-14 20:19 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-05-14 14:04 [PATCH] tracing: events_hist: avoid using excessive stack space Arnd Bergmann
2021-05-14 14:16 ` Steven Rostedt
2021-05-14 18:33 ` Arnd Bergmann
2021-05-14 20:19 ` Nick Desaulniers [this message]
2021-05-14 21:14 ` Arnd Bergmann
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='CAKwvOdmJfm+PKxjR-N=YfFcjrXi4N4xUiRCLbSNMNJ+5g_wX+g@mail.gmail.com' \
--to=ndesaulniers@google.com \
--cc=arnd@arndb.de \
--cc=arnd@kernel.org \
--cc=clang-built-linux@googlegroups.com \
--cc=hqjagain@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mhiramat@kernel.org \
--cc=mingo@redhat.com \
--cc=nathan@kernel.org \
--cc=rostedt@godmis.org \
--cc=rostedt@goodmis.org \
--cc=trix@redhat.com \
--cc=zanussi@kernel.org \
/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.