Messages in this thread | | | Date | Wed, 25 Mar 2009 13:35:43 -0400 (EDT) | From | Steven Rostedt <> | Subject | Re: [PATCH 5/5][RFC] function-graph: add option to calculate graph time or not |
| |
On Wed, 25 Mar 2009, Ingo Molnar wrote:
> > * Steven Rostedt <rostedt@goodmis.org> wrote: > > > --- a/kernel/trace/trace_functions_graph.c > > +++ b/kernel/trace/trace_functions_graph.c > > @@ -104,9 +105,6 @@ ftrace_pop_return_trace(struct ftrace_graph_ret *trace, unsigned long *ret) > > trace->calltime = current->ret_stack[index].calltime; > > trace->overrun = atomic_read(¤t->trace_overrun); > > trace->depth = index; > > - barrier(); > > - current->curr_ret_stack--; > > - > > } > > > > /* > > @@ -121,6 +119,8 @@ unsigned long ftrace_return_to_handler(void) > > ftrace_pop_return_trace(&trace, &ret); > > trace.rettime = trace_clock_local(); > > ftrace_graph_return(&trace); > > + barrier(); > > + current->curr_ret_stack--; > > this will modify the regular graph tracer too. No discussion of this > change in the changelog - is it a fix?
Ah, sorry, I should have commented that.
This is to prevent a race in the profiler code. This is not a problem with current function graph code.
What happens is that the profiler references the ret_stack from the ftrace_graph_return function. If we update the curr_ret_stack and we take an interrupt then we run the risk of reading incorrect data. Once we decrement the curr_ret_stack, the ret_stack for the function is fair game to be used again by an interrupt.
Perhaps I should pull this change out and add it for core. This will let any function graph user safely reference the ret_stack.
-- Steve
| |