Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1448171 > unrolled thread

[PATCH 00/19] x86/dumpstack: rewrite x86 stack dump code

Started byJosh Poimboeuf <jpoimboe@redhat.com>
First post2016-07-21 23:30 +0200
Last post2016-07-23 15:00 +0200
Articles 3 on this page of 43 — 4 participants

Back to article view | Back to linux.kernel

Contents

  [PATCH 00/19] x86/dumpstack: rewrite x86 stack dump code Josh Poimboeuf <jpoimboe@redhat.com> - 2016-07-21 23:30 +0200
    [PATCH 05/19] x86/dumpstack: fix function graph tracing stack dump reliability issues Josh Poimboeuf <jpoimboe@redhat.com> - 2016-07-21 23:30 +0200
    [PATCH 04/19] x86/dumpstack: make printk_stack_address() more generally useful Josh Poimboeuf <jpoimboe@redhat.com> - 2016-07-21 23:30 +0200
    [PATCH 18/19] x86/dumpstack: print stack identifier on its own line Josh Poimboeuf <jpoimboe@redhat.com> - 2016-07-21 23:30 +0200
    [PATCH 13/19] x86/stacktrace: convert save_stack_trace_*() to the new unwinder Josh Poimboeuf <jpoimboe@redhat.com> - 2016-07-21 23:30 +0200
    [PATCH 19/19] x86/dumpstack: print any pt_regs found on the stack Josh Poimboeuf <jpoimboe@redhat.com> - 2016-07-21 23:30 +0200
      Re: [PATCH 19/19] x86/dumpstack: print any pt_regs found on the stack Andy Lutomirski <luto@amacapital.net> - 2016-07-22 00:40 +0200
        Re: [PATCH 19/19] x86/dumpstack: print any pt_regs found on the stack Josh Poimboeuf <jpoimboe@redhat.com> - 2016-07-22 05:40 +0200
          Re: [PATCH 19/19] x86/dumpstack: print any pt_regs found on the stack Andy Lutomirski <luto@amacapital.net> - 2016-07-22 07:20 +0200
            Re: [PATCH 19/19] x86/dumpstack: print any pt_regs found on the stack Josh Poimboeuf <jpoimboe@redhat.com> - 2016-07-22 18:00 +0200
              Re: [PATCH 19/19] x86/dumpstack: print any pt_regs found on the stack Andy Lutomirski <luto@amacapital.net> - 2016-07-22 23:50 +0200
                Re: [PATCH 19/19] x86/dumpstack: print any pt_regs found on the stack Josh Poimboeuf <jpoimboe@redhat.com> - 2016-07-23 00:30 +0200
                  Re: [PATCH 19/19] x86/dumpstack: print any pt_regs found on the stack Andy Lutomirski <luto@amacapital.net> - 2016-07-23 01:20 +0200
                    Re: [PATCH 19/19] x86/dumpstack: print any pt_regs found on the stack Andy Lutomirski <luto@amacapital.net> - 2016-07-23 01:40 +0200
                      Re: [PATCH 19/19] x86/dumpstack: print any pt_regs found on the stack Josh Poimboeuf <jpoimboe@redhat.com> - 2016-07-23 02:10 +0200
                    Re: [PATCH 19/19] x86/dumpstack: print any pt_regs found on the stack Josh Poimboeuf <jpoimboe@redhat.com> - 2016-07-23 01:40 +0200
    [PATCH 08/19] x86/dumpstack: don't disable preemption in show_stack_log_lvl() and dump_trace() Josh Poimboeuf <jpoimboe@redhat.com> - 2016-07-21 23:30 +0200
    [PATCH 14/19] oprofile/x86: convert x86_backtrace() to the new unwinder Josh Poimboeuf <jpoimboe@redhat.com> - 2016-07-21 23:30 +0200
    [PATCH 06/19] x86/dumpstack: remove extra brackets around "EOE" Josh Poimboeuf <jpoimboe@redhat.com> - 2016-07-21 23:30 +0200
    [PATCH 10/19] x86/dumpstack: add get_stack_info() interface Josh Poimboeuf <jpoimboe@redhat.com> - 2016-07-21 23:30 +0200
      Re: [PATCH 10/19] x86/dumpstack: add get_stack_info() interface Andy Lutomirski <luto@amacapital.net> - 2016-07-23 01:30 +0200
        Re: [PATCH 10/19] x86/dumpstack: add get_stack_info() interface Andy Lutomirski <luto@amacapital.net> - 2016-07-23 02:00 +0200
          Re: [PATCH 10/19] x86/dumpstack: add get_stack_info() interface Josh Poimboeuf <jpoimboe@redhat.com> - 2016-07-23 15:10 +0200
        Re: [PATCH 10/19] x86/dumpstack: add get_stack_info() interface Josh Poimboeuf <jpoimboe@redhat.com> - 2016-07-23 02:00 +0200
          Re: [PATCH 10/19] x86/dumpstack: add get_stack_info() interface Andy Lutomirski <luto@amacapital.net> - 2016-07-23 02:20 +0200
            Re: [PATCH 10/19] x86/dumpstack: add get_stack_info() interface Josh Poimboeuf <jpoimboe@redhat.com> - 2016-07-23 16:10 +0200
    [PATCH 01/19] x86/dumpstack: remove show_trace() Josh Poimboeuf <jpoimboe@redhat.com> - 2016-07-21 23:30 +0200
      Re: [PATCH 01/19] x86/dumpstack: remove show_trace() Andy Lutomirski <luto@amacapital.net> - 2016-07-22 00:00 +0200
    [PATCH 12/19] perf/x86: convert perf_callchain_kernel() to the new unwinder Josh Poimboeuf <jpoimboe@redhat.com> - 2016-07-21 23:30 +0200
    [PATCH 02/19] x86/dumpstack: add get_stack_pointer() and get_frame_pointer() Josh Poimboeuf <jpoimboe@redhat.com> - 2016-07-21 23:30 +0200
      Re: [PATCH 02/19] x86/dumpstack: add get_stack_pointer() and get_frame_pointer() Andy Lutomirski <luto@amacapital.net> - 2016-07-22 00:00 +0200
    [PATCH 11/19] x86/dumptrace: add new unwind interface and implementations Josh Poimboeuf <jpoimboe@redhat.com> - 2016-07-21 23:30 +0200
    [PATCH 09/19] x86/dumpstack: simplify in_exception_stack() Josh Poimboeuf <jpoimboe@redhat.com> - 2016-07-21 23:30 +0200
      Re: [PATCH 09/19] x86/dumpstack: simplify in_exception_stack() Andy Lutomirski <luto@amacapital.net> - 2016-07-22 00:10 +0200
    [PATCH 07/19] x86/dumpstack: add IRQ_USABLE_STACK_SIZE define Josh Poimboeuf <jpoimboe@redhat.com> - 2016-07-21 23:30 +0200
      Re: [PATCH 07/19] x86/dumpstack: add IRQ_USABLE_STACK_SIZE define Andy Lutomirski <luto@amacapital.net> - 2016-07-22 00:10 +0200
        Re: [PATCH 07/19] x86/dumpstack: add IRQ_USABLE_STACK_SIZE define Josh Poimboeuf <jpoimboe@redhat.com> - 2016-07-22 03:50 +0200
          Re: [PATCH 07/19] x86/dumpstack: add IRQ_USABLE_STACK_SIZE define Ingo Molnar <mingo@kernel.org> - 2016-07-22 10:30 +0200
    Re: [PATCH 00/19] x86/dumpstack: rewrite x86 stack dump code Linus Torvalds <torvalds@linux-foundation.org> - 2016-07-23 02:30 +0200
      Re: [PATCH 00/19] x86/dumpstack: rewrite x86 stack dump code Andy Lutomirski <luto@amacapital.net> - 2016-07-23 02:40 +0200
        Re: [PATCH 00/19] x86/dumpstack: rewrite x86 stack dump code Josh Poimboeuf <jpoimboe@redhat.com> - 2016-07-23 07:40 +0200
          Re: [PATCH 00/19] x86/dumpstack: rewrite x86 stack dump code Linus Torvalds <torvalds@linux-foundation.org> - 2016-07-23 07:50 +0200
            Re: [PATCH 00/19] x86/dumpstack: rewrite x86 stack dump code Josh Poimboeuf <jpoimboe@redhat.com> - 2016-07-23 15:00 +0200

Page 3 of 3 — ← Prev page 1 2 [3]

#1448875

FromJosh Poimboeuf <jpoimboe@redhat.com>
Date2016-07-23 07:40 +0200
Message-ID<rXXPk-7o1-1@gated-at.bofh.it>
In reply to#1448852
On Fri, Jul 22, 2016 at 05:31:47PM -0700, Andy Lutomirski wrote:
> On Fri, Jul 22, 2016 at 5:22 PM, Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
> >
> > 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.
> 
> I think that Josh's code has the potential to be extremely robust
> *and* give more correct results when possible.  One thing I intend to
> review when v2 shows up is that it's as conservative as it needs to be
> to avoid ever dereferencing an out-of-bounds pointer.  And Josh's oops
> printer carefully walks and prints out all addresses on the stack
> (complete with question marks) even if the unwinder doesn't find them.

I should add that while the show_trace_log_lvl() code (which is used for
oopses) looks different on the surface, the algorithm is fundamentally
the same as before: traverse the stacks, scanning and printing any
kernel text addresses.

While doing the scanning and printing, it does call the frame pointer
unwinder in parallel, but like before, that's *only* used to determine
whether a found address should be printed without a question mark.  If
the unwinder goes off the rails, the scanning and printing of text
addresses goes on, undisturbed.

The frame pointer unwinder code itself is quite careful not to
dereference anything it shouldn't (though of course I welcome any review
comments that find otherwise).

> > 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.
> >
> 
> I think you may be giving the current tracer slightly more credit than
> it's due.  In my stack guard page patchset, I fixed two separate
> issues, one of which caused recursive faults and one of which caused
> it to output nothing at all.  So maybe *now* it's very robust :)  But
> it's still an umaintainable mess IMO, and Josh's patchset helps a
> *lot*.

-- 
Josh

[toc] | [prev] | [next] | [standalone]

#1448878

FromLinus Torvalds <torvalds@linux-foundation.org>
Date2016-07-23 07:50 +0200
Message-ID<rXXYZ-7rg-3@gated-at.bofh.it>
In reply to#1448875
On Sat, Jul 23, 2016 at 2:35 PM, Josh Poimboeuf <jpoimboe@redhat.com> wrote:
>
> While doing the scanning and printing, it does call the frame pointer
> unwinder in parallel, but like before, that's *only* used to determine
> whether a found address should be printed without a question mark.  If
> the unwinder goes off the rails, the scanning and printing of text
> addresses goes on, undisturbed.
>
> The frame pointer unwinder code itself is quite careful not to
> dereference anything it shouldn't (though of course I welcome any review
> comments that find otherwise).

So this was the bug the last time around we did unwinders - the code
would dereference the unwind tables, and the tables would be
corrupted. End result: recursive oops.

And they were corrupted not even because of memory corruption, but
simply because they contained incorrect data, due to compiler bugs and
other issues.

I have really bad memories from that time. Several years after the
fact. It took months to finally revert the crap, because the author
continued to insist that "this was the last bug" for several passes
through that thing.

As they say, "Once burned, twice shy". But in this case, it's more
like "Four times burned, sixteen times as shy".

            Linus

[toc] | [prev] | [next] | [standalone]

#1448944

FromJosh Poimboeuf <jpoimboe@redhat.com>
Date2016-07-23 15:00 +0200
Message-ID<rY4H7-2Tb-11@gated-at.bofh.it>
In reply to#1448878
On Sat, Jul 23, 2016 at 02:39:52PM +0900, Linus Torvalds wrote:
> On Sat, Jul 23, 2016 at 2:35 PM, Josh Poimboeuf <jpoimboe@redhat.com> wrote:
> >
> > While doing the scanning and printing, it does call the frame pointer
> > unwinder in parallel, but like before, that's *only* used to determine
> > whether a found address should be printed without a question mark.  If
> > the unwinder goes off the rails, the scanning and printing of text
> > addresses goes on, undisturbed.
> >
> > The frame pointer unwinder code itself is quite careful not to
> > dereference anything it shouldn't (though of course I welcome any review
> > comments that find otherwise).
> 
> So this was the bug the last time around we did unwinders - the code
> would dereference the unwind tables, and the tables would be
> corrupted. End result: recursive oops.
> 
> And they were corrupted not even because of memory corruption, but
> simply because they contained incorrect data, due to compiler bugs and
> other issues.
> 
> I have really bad memories from that time. Several years after the
> fact. It took months to finally revert the crap, because the author
> continued to insist that "this was the last bug" for several passes
> through that thing.
> 
> As they say, "Once burned, twice shy". But in this case, it's more
> like "Four times burned, sixteen times as shy".

But that was DWARF, right?  This is still just simple frame pointers.

Don't think of it as a new unwinder.  Think of it instead as a "gentle
reshuffling of the existing code to vastly improve readability and
maintenance."

Yes, I would like to eventually propose a DWARF unwinder, which
hopefully learns from the mistakes of previous attempts.  But either
way, I think this patch set stands on its own as a big improvement.

-- 
Josh

[toc] | [prev] | [standalone]

Page 3 of 3 — ← Prev page 1 2 [3]

Back to top | Article view | linux.kernel

csiph-web