[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20160722013836.2fkqu4wk5hix2wqn@treble>
Date: Thu, 21 Jul 2016 20:38:36 -0500
From: Josh Poimboeuf <jpoimboe@...hat.com>
To: Byungchul Park <byungchul.park@....com>
Cc: Thomas Gleixner <tglx@...utronix.de>,
Ingo Molnar <mingo@...nel.org>,
"H . Peter Anvin" <hpa@...or.com>, x86@...nel.org,
linux-kernel@...r.kernel.org,
Andy Lutomirski <luto@...capital.net>,
Linus Torvalds <torvalds@...ux-foundation.org>,
Steven Rostedt <rostedt@...dmis.org>,
Brian Gerst <brgerst@...il.com>,
Kees Cook <keescook@...omium.org>,
Peter Zijlstra <peterz@...radead.org>,
Frederic Weisbecker <fweisbec@...il.com>
Subject: Re: [PATCH 15/19] x86/dumpstack: convert show_trace_log_lvl() to the
new unwinder
On Fri, Jul 22, 2016 at 06:49:01AM +0900, Byungchul Park wrote:
> On Thu, Jul 21, 2016 at 04:21:52PM -0500, Josh Poimboeuf wrote:
> > Convert show_trace_log_lvl() to the new unwinder. dump_trace() has been
> > deprecated.
> >
> > show_trace_log_lvl() is special compared to other users of the unwinder.
> > It's the only place where both reliable *and* unreliable addresses are
> > needed. With frame pointers enabled, most stack walking code doesn't
> > want to know about unreliable addresses. But in this case, when we're
> > dumping the stack to the console because something presumably went
> > wrong, the unreliable addresses are useful:
> >
> > - They show stale data on the stack which can provide useful clues.
> >
> > - If something goes wrong with the unwinder, or if frame pointers are
> > corrupt or missing, all the stack addresses still get shown.
> >
> > So in order to show all addresses on the stack, and at the same time
> > figure out which addresses are reliable, we have to do the scanning and
> > the unwinding in parallel.
> >
> > The scanning is done with the help of get_stack_info() to traverse the
> > stacks. The unwinding is done separately by the new unwinder.
> >
> > In theory we could simplify show_trace_log_lvl() by instead pushing some
> > of this logic into the unwind code. But then we would need some kind of
> > "fake" frame logic in the unwinder which would add a lot of complexity
> > and wouldn't be worth it in order to support only one user.
> >
> > Another benefit of this approach is that once we have a DWARF unwinder,
> > we should be able to just plug it in with minimal impact to this code.
> >
> > Another change here is that callers of show_trace_log_lvl() don't need
> > to provide the 'bp' argument. The unwinder already finds the relevant
> > frame pointer by unwinding until it reaches the first frame after the
> > provided stack pointer.
>
> Hello,
>
> You seem to have changed a lot of code with which I dealt in another patch.
> I might be supposed to wait until yours will be done. I need to check yours
> at first anyway.
Yeah, that's why I added you to cc :-) I think this obsoletes your
patches.
--
Josh
Powered by blists - more mailing lists