lkml.org 
[lkml]   [2016]   [Aug]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 05/19] x86/dumpstack: fix function graph tracing stack dump reliability issues
On Mon, Aug 01, 2016 at 10:28:21AM -0400, Steven Rostedt wrote:
> On Sat, 30 Jul 2016 08:51:25 -0500
> Josh Poimboeuf <jpoimboe@redhat.com> wrote:
>
> > On Fri, Jul 29, 2016 at 10:20:36PM -0400, Steven Rostedt wrote:
> > > On Fri, 29 Jul 2016 19:50:59 -0500
> > > Josh Poimboeuf <jpoimboe@redhat.com> wrote:
> > >
> > > > BTW, it would be really nice if ftrace_graph_ret_addr() were idempotent
> > > > so we could get the "real" return address without having to pass in a
> > > > state variable.
> > > >
> > > > For example we could add an "unsigned long *retp" pointer to
> > > > ftrace_ret_stack, which points to the return address on the stack. Then
> > > > we could get rid of the index state variable in ftrace_graph_ret_addr,
> > > > and also then there would never be a chance of the stack dump getting
> > > > out of sync with the ret_stack.
> > > >
> > > > What do you think?
> > > >
> > >
> > > I don't want to extend ret_stack as that is allocated 50 of these
> > > structures for every task. That said, we have the "fp" field that's
> > > used to check for frame pointer corruption when mcount is used. With
> > > CC_USING_FENTRY, that field is ignored. Perhaps we could overload that
> > > field for this.
> >
> > In that case, I guess we would need two versions of
> > ftrace_graph_ret_addr(), with the current implementation still needed
> > for mcount+HAVE_FUNCTION_GRAPH_FP_TEST.
>
> How hard would it be in that case?

Well, it would be easy enough, but then the caller would still need to
pass in the state variable. So maybe it's not worth the trouble.

> > Or would you want to get rid of HAVE_FUNCTION_GRAPH_FP_TEST for x86?
>
> No, because there's gcc versions that we still support that mess up
> mcount, and could still cause issues with function graph.
>
>
> >
> > BTW, on a different note, should I put ftrace_graph_ret_addr() to
> > kernel/trace/trace_functions_graph.c so other arches can use it?
> >
>
> I guess you could. There doesn't seem to be any x86 specific code in
> that right?

Right. And I noticed that several arches implement this same
functionality in a slightly different way to fit with their stack dump
code. So it would be nice to make it common.

--
Josh

\
 
 \ /
  Last update: 2016-08-01 18:21    [W:0.100 / U:0.120 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site