From: Dan Carpenter <dan.carpenter@linaro.org>
To: Steven Rostedt <rostedt@goodmis.org>
Cc: linux-trace-kernel@vger.kernel.org
Subject: Re: [bug report] x86/ftrace: Make function graph use ftrace directly
Date: Fri, 7 Jul 2023 08:37:29 +0300 [thread overview]
Message-ID: <2344e517-cdce-42ef-868e-6b9ae8b4ea2c@kadam.mountain> (raw)
In-Reply-To: <20230706133734.499e9cbe@gandalf.local.home>
On Thu, Jul 06, 2023 at 01:37:34PM -0400, Steven Rostedt wrote:
> On Thu, 6 Jul 2023 08:50:31 +0300
> Dan Carpenter <dan.carpenter@linaro.org> wrote:
>
> > Hello Steven Rostedt (VMware),
> >
> > The patch 0c0593b45c9b: "x86/ftrace: Make function graph use ftrace
> > directly" from Oct 8, 2021, leads to the following Smatch static
> > checker warning:
> >
> > kernel/trace/trace_selftest.c:769 trace_graph_entry_watchdog()
> > warn: sleeping in atomic context
> >
> > kernel/trace/trace_selftest.c
> > 765 static int trace_graph_entry_watchdog(struct ftrace_graph_ent *trace)
> > 766 {
> > 767 /* This is harmlessly racy, we want to approximately detect a hang */
> > 768 if (unlikely(++graph_hang_thresh > GRAPH_MAX_FUNC_TEST)) {
> > --> 769 ftrace_graph_stop();
> >
> > This is a sleeping function.
>
> Hmm, this is an interesting scenario. If this triggers, it means that the
> system is likely locked up by the function graph tracer. The only way to
> stop the hang, is via calling ftrace_graph_stop(). But you are correct,
> that's calling something that can crash the system as well.
>
> If anything, it should be called after the dump_on_oops output, with a
> warning to reboot the machine.
>
> IOW, yes, it's doing something buggy, but pretty much the only other
> alternative is to call panic(). Not sure that's better :-/
>
> Perhaps the solution is simply to move it to after the dump, with a warning
> saying: "Dazed and confused, and trying to continue, but please reboot the machine!"
>
> ??
I feel like sleeping in atomic bugs used to be more of a big deal back
in the day when systems only had one CPU. In those days it was way more
common for it to lead to a hang, but these days we quite often
re-schedule the sleeping process on a different CPU and recover. (I
haven't actually looked at how processes are moved to different CPUs
but this is just my theory of why we see fewer real life hangs from this
bug today).
regards,
dan carpenter
next prev parent reply other threads:[~2023-07-07 5:37 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-07-06 5:50 [bug report] x86/ftrace: Make function graph use ftrace directly Dan Carpenter
2023-07-06 17:37 ` Steven Rostedt
2023-07-07 5:37 ` Dan Carpenter [this message]
2023-07-08 13:48 ` Steven Rostedt
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=2344e517-cdce-42ef-868e-6b9ae8b4ea2c@kadam.mountain \
--to=dan.carpenter@linaro.org \
--cc=linux-trace-kernel@vger.kernel.org \
--cc=rostedt@goodmis.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox