linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Linus Torvalds <torvalds@linux-foundation.org>
To: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@kernel.org>, "H . Peter Anvin" <hpa@zytor.com>,
	"the arch/x86 maintainers" <x86@kernel.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Andy Lutomirski <luto@amacapital.net>,
	Steven Rostedt <rostedt@goodmis.org>,
	Brian Gerst <brgerst@gmail.com>,
	Kees Cook <keescook@chromium.org>,
	Peter Zijlstra <peterz@infradead.org>,
	Frederic Weisbecker <fweisbec@gmail.com>,
	Byungchul Park <byungchul.park@lge.com>
Subject: Re: [PATCH 00/19] x86/dumpstack: rewrite x86 stack dump code
Date: Sat, 23 Jul 2016 09:22:25 +0900	[thread overview]
Message-ID: <CA+55aFxsUjKO=agNj8-p0yo_3=VJiCk75pWioMDhHHwZNB8fbQ@mail.gmail.com> (raw)
In-Reply-To: <cover.1469136008.git.jpoimboe@redhat.com>

On Fri, Jul 22, 2016 at 6:21 AM, Josh Poimboeuf <jpoimboe@redhat.com> wrote:
>
> Some if its advantages:
>
> - simplicity: no more callback sprawl and less code duplication.
>
> - flexibility: allows the caller to stop and inspect the stack state at
>   each step in the unwinding process.
>
> - modularity: the unwinder code, console stack dump code, and stack
>   metadata analysis code are all better separated so that changing one
>   of them shouldn't have much of an impact on any of the others.

I've been without internet for the last week, so I have a ton pending,
and not good enough internet even now to take a good look.

However, I want to make one thing really really clear: the absolute
NUMBER ONE requirement for the stack tracing code is none of the
above.

The #1 requirement is that it works, and not have a chance in hell of
ever breaking. We had that happen once before when people wanted to
make it fancy and add Dwarf info, and it was such a f*cking disaster
that I am not sure I ever want to do that again. Seriously.

It does not matter if the stack tracing gives the wrong answers.

It does not matter if the stack tracing is complicated and odd old code.

It does not matter one whit if some new user is inconvenienced, and in
fact it is possible that new users should write their *own* stack
tracer code.

The ONLY thing that matters (to a very high degree) is that the code
is stable, and if an Oops happens, the stack tracer never *ever*
cause even more problems than we already have.

If the stack tracer *ever* takes a recursive fault and kills the
machine, the stack tracer is worse than bad - we'd be better off
*without* a stack tracer at all.

And yes, we had exactly that situation, where bugs in the stack tracer
meant that other bugs ended up being much harder to debug, because
instead of a nice logged oops message that would have been trivial to
figure out, we very occasionally ended up with a dead machine instead.

So without having yet looked at the code, I want people to understand
that to a very real degree, the stack tracer that the *oopsing* code
(ie what all the usual kernel fault handlers use) is very very special
code and needs to be handled very carefully, and needs to be extra
robust, even in the presence of stack corruption, and even in the
presence of the dwarf info being totally corrupted. Because we've very
much had both things happen.

It is very possible that we should have two different stack tracers -
the stupid "for oopses only" code that doesn't necessarily give the
perfect trace, but is very anal and happily gives old stale addresses
(which can be very useful for seeing what happened just before the
"real" stack trace), and then a separate stack trace engine that is
clever and gets things right, and if that one faults it can depend on
the normal kernel fault handling picking up the pieces.

Yes, the current stack tracer is  crufty. No, it's not perfect. But it
is very well tested, and has held up. That should not be dismissed.

                 Linus

  parent reply	other threads:[~2016-07-23  0:22 UTC|newest]

Thread overview: 91+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-07-21 21:21 [PATCH 00/19] x86/dumpstack: rewrite x86 stack dump code Josh Poimboeuf
2016-07-21 21:21 ` [PATCH 01/19] x86/dumpstack: remove show_trace() Josh Poimboeuf
2016-07-21 21:49   ` Andy Lutomirski
2016-07-21 21:21 ` [PATCH 02/19] x86/dumpstack: add get_stack_pointer() and get_frame_pointer() Josh Poimboeuf
2016-07-21 21:53   ` Andy Lutomirski
2016-07-21 21:21 ` [PATCH 03/19] x86/dumpstack: remove unnecessary stack pointer arguments Josh Poimboeuf
2016-07-21 21:56   ` Andy Lutomirski
2016-07-22  1:41     ` Josh Poimboeuf
2016-07-22  2:29       ` Andy Lutomirski
2016-07-22  3:08       ` Brian Gerst
2016-07-21 21:21 ` [PATCH 04/19] x86/dumpstack: make printk_stack_address() more generally useful Josh Poimboeuf
2016-07-21 21:21 ` [PATCH 05/19] x86/dumpstack: fix function graph tracing stack dump reliability issues Josh Poimboeuf
2016-07-29 22:55   ` Steven Rostedt
2016-07-30  0:50     ` Josh Poimboeuf
2016-07-30  2:20       ` Steven Rostedt
2016-07-30 13:51         ` Josh Poimboeuf
2016-08-01 14:28           ` Steven Rostedt
2016-08-01 15:36             ` Josh Poimboeuf
2016-08-02 21:00               ` Josh Poimboeuf
2016-08-02 21:16                 ` Steven Rostedt
2016-08-02 22:13                   ` Josh Poimboeuf
2016-08-02 23:16                     ` Steven Rostedt
2016-08-03  1:56                       ` Josh Poimboeuf
2016-08-03  2:30                         ` Steven Rostedt
2016-08-03  2:50                           ` Josh Poimboeuf
2016-08-03  2:59                             ` Steven Rostedt
2016-08-03  3:12                               ` Josh Poimboeuf
2016-08-03  3:18                                 ` Steven Rostedt
2016-08-03  3:21                                   ` Steven Rostedt
2016-08-03  3:31                                     ` Josh Poimboeuf
2016-08-03  3:45                                       ` Steven Rostedt
2016-08-03 14:13                                         ` Josh Poimboeuf
2016-08-03  3:30                                   ` Josh Poimboeuf
2016-08-01 15:59     ` Josh Poimboeuf
2016-08-01 16:05       ` Steven Rostedt
2016-08-01 16:19         ` Josh Poimboeuf
2016-08-01 16:24     ` Josh Poimboeuf
2016-08-01 16:56       ` Steven Rostedt
2016-07-21 21:21 ` [PATCH 06/19] x86/dumpstack: remove extra brackets around "EOE" Josh Poimboeuf
2016-07-21 21:21 ` [PATCH 07/19] x86/dumpstack: add IRQ_USABLE_STACK_SIZE define Josh Poimboeuf
2016-07-21 22:01   ` Andy Lutomirski
2016-07-22  1:48     ` Josh Poimboeuf
2016-07-22  8:24       ` Ingo Molnar
2016-07-21 21:21 ` [PATCH 08/19] x86/dumpstack: don't disable preemption in show_stack_log_lvl() and dump_trace() Josh Poimboeuf
2016-07-21 21:21 ` [PATCH 09/19] x86/dumpstack: simplify in_exception_stack() Josh Poimboeuf
2016-07-21 22:05   ` Andy Lutomirski
2016-07-21 21:21 ` [PATCH 10/19] x86/dumpstack: add get_stack_info() interface Josh Poimboeuf
2016-07-22 23:26   ` Andy Lutomirski
2016-07-22 23:52     ` Andy Lutomirski
2016-07-23 13:09       ` Josh Poimboeuf
2016-07-22 23:54     ` Josh Poimboeuf
2016-07-23  0:15       ` Andy Lutomirski
2016-07-23 14:04         ` Josh Poimboeuf
2016-07-26  0:09           ` Andy Lutomirski
2016-07-26 16:26             ` Josh Poimboeuf
2016-07-26 17:51               ` Steven Rostedt
2016-07-26 18:56                 ` Josh Poimboeuf
2016-07-26 20:59               ` Andy Lutomirski
2016-07-26 22:24                 ` Josh Poimboeuf
2016-07-26 22:31                   ` Steven Rostedt
2016-07-26 22:37                   ` Andy Lutomirski
2016-07-26 16:47             ` Josh Poimboeuf
2016-07-26 17:49               ` Brian Gerst
2016-07-26 18:59                 ` Josh Poimboeuf
2016-07-21 21:21 ` [PATCH 11/19] x86/dumptrace: add new unwind interface and implementations Josh Poimboeuf
2016-07-21 21:21 ` [PATCH 12/19] perf/x86: convert perf_callchain_kernel() to the new unwinder Josh Poimboeuf
2016-07-21 21:21 ` [PATCH 13/19] x86/stacktrace: convert save_stack_trace_*() " Josh Poimboeuf
2016-07-21 21:21 ` [PATCH 14/19] oprofile/x86: convert x86_backtrace() " Josh Poimboeuf
2016-07-21 21:21 ` [PATCH 15/19] x86/dumpstack: convert show_trace_log_lvl() " Josh Poimboeuf
2016-07-21 21:49   ` Byungchul Park
2016-07-22  1:38     ` Josh Poimboeuf
2016-07-21 21:21 ` [PATCH 16/19] x86/dumpstack: remove dump_trace() Josh Poimboeuf
2016-07-21 21:21 ` [PATCH 17/19] x86/entry/dumpstack: encode pt_regs pointer in frame pointer Josh Poimboeuf
2016-07-21 22:27   ` Andy Lutomirski
2016-07-21 21:21 ` [PATCH 18/19] x86/dumpstack: print stack identifier on its own line Josh Poimboeuf
2016-07-21 21:21 ` [PATCH 19/19] x86/dumpstack: print any pt_regs found on the stack Josh Poimboeuf
2016-07-21 22:32   ` Andy Lutomirski
2016-07-22  3:30     ` Josh Poimboeuf
2016-07-22  5:13       ` Andy Lutomirski
2016-07-22 15:57         ` Josh Poimboeuf
2016-07-22 21:46           ` Andy Lutomirski
2016-07-22 22:20             ` Josh Poimboeuf
2016-07-22 23:18               ` Andy Lutomirski
2016-07-22 23:30                 ` Josh Poimboeuf
2016-07-22 23:39                   ` Andy Lutomirski
2016-07-23  0:00                     ` Josh Poimboeuf
2016-07-23  0:22 ` Linus Torvalds [this message]
2016-07-23  0:31   ` [PATCH 00/19] x86/dumpstack: rewrite x86 stack dump code Andy Lutomirski
2016-07-23  5:35     ` Josh Poimboeuf
2016-07-23  5:39       ` Linus Torvalds
2016-07-23 12:53         ` Josh Poimboeuf

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='CA+55aFxsUjKO=agNj8-p0yo_3=VJiCk75pWioMDhHHwZNB8fbQ@mail.gmail.com' \
    --to=torvalds@linux-foundation.org \
    --cc=brgerst@gmail.com \
    --cc=byungchul.park@lge.com \
    --cc=fweisbec@gmail.com \
    --cc=hpa@zytor.com \
    --cc=jpoimboe@redhat.com \
    --cc=keescook@chromium.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luto@amacapital.net \
    --cc=mingo@kernel.org \
    --cc=peterz@infradead.org \
    --cc=rostedt@goodmis.org \
    --cc=tglx@linutronix.de \
    --cc=x86@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).