diff options
author | Steven Rostedt (VMware) <rostedt@goodmis.org> | 2018-11-19 08:07:12 -0500 |
---|---|---|
committer | Steven Rostedt (VMware) <rostedt@goodmis.org> | 2018-11-27 20:31:54 -0500 |
commit | 39eb456dacb543de90d3bc6a8e0ac5cf51ac475e (patch) | |
tree | 790493fbeb31636acf0149ea92b146dc03f7c90d /kernel/trace/ftrace.c | |
parent | d125f3f866df88da5a85df00291f88f0baa89f7c (diff) |
function_graph: Use new curr_ret_depth to manage depth instead of curr_ret_stack
Currently, the depth of the ret_stack is determined by curr_ret_stack index.
The issue is that there's a race between setting of the curr_ret_stack and
calling of the callback attached to the return of the function.
Commit 03274a3ffb44 ("tracing/fgraph: Adjust fgraph depth before calling
trace return callback") moved the calling of the callback to after the
setting of the curr_ret_stack, even stating that it was safe to do so, when
in fact, it was the reason there was a barrier() there (yes, I should have
commented that barrier()).
Not only does the curr_ret_stack keep track of the current call graph depth,
it also keeps the ret_stack content from being overwritten by new data.
The function profiler, uses the "subtime" variable of ret_stack structure
and by moving the curr_ret_stack, it allows for interrupts to use the same
structure it was using, corrupting the data, and breaking the profiler.
To fix this, there needs to be two variables to handle the call stack depth
and the pointer to where the ret_stack is being used, as they need to change
at two different locations.
Cc: stable@kernel.org
Fixes: 03274a3ffb449 ("tracing/fgraph: Adjust fgraph depth before calling trace return callback")
Reviewed-by: Masami Hiramatsu <mhiramat@kernel.org>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
Diffstat (limited to 'kernel/trace/ftrace.c')
-rw-r--r-- | kernel/trace/ftrace.c | 3 |
1 files changed, 3 insertions, 0 deletions
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c index f536f601bd46..48513954713c 100644 --- a/kernel/trace/ftrace.c +++ b/kernel/trace/ftrace.c | |||
@@ -6814,6 +6814,7 @@ static int alloc_retstack_tasklist(struct ftrace_ret_stack **ret_stack_list) | |||
6814 | atomic_set(&t->tracing_graph_pause, 0); | 6814 | atomic_set(&t->tracing_graph_pause, 0); |
6815 | atomic_set(&t->trace_overrun, 0); | 6815 | atomic_set(&t->trace_overrun, 0); |
6816 | t->curr_ret_stack = -1; | 6816 | t->curr_ret_stack = -1; |
6817 | t->curr_ret_depth = -1; | ||
6817 | /* Make sure the tasks see the -1 first: */ | 6818 | /* Make sure the tasks see the -1 first: */ |
6818 | smp_wmb(); | 6819 | smp_wmb(); |
6819 | t->ret_stack = ret_stack_list[start++]; | 6820 | t->ret_stack = ret_stack_list[start++]; |
@@ -7038,6 +7039,7 @@ graph_init_task(struct task_struct *t, struct ftrace_ret_stack *ret_stack) | |||
7038 | void ftrace_graph_init_idle_task(struct task_struct *t, int cpu) | 7039 | void ftrace_graph_init_idle_task(struct task_struct *t, int cpu) |
7039 | { | 7040 | { |
7040 | t->curr_ret_stack = -1; | 7041 | t->curr_ret_stack = -1; |
7042 | t->curr_ret_depth = -1; | ||
7041 | /* | 7043 | /* |
7042 | * The idle task has no parent, it either has its own | 7044 | * The idle task has no parent, it either has its own |
7043 | * stack or no stack at all. | 7045 | * stack or no stack at all. |
@@ -7068,6 +7070,7 @@ void ftrace_graph_init_task(struct task_struct *t) | |||
7068 | /* Make sure we do not use the parent ret_stack */ | 7070 | /* Make sure we do not use the parent ret_stack */ |
7069 | t->ret_stack = NULL; | 7071 | t->ret_stack = NULL; |
7070 | t->curr_ret_stack = -1; | 7072 | t->curr_ret_stack = -1; |
7073 | t->curr_ret_depth = -1; | ||
7071 | 7074 | ||
7072 | if (ftrace_graph_active) { | 7075 | if (ftrace_graph_active) { |
7073 | struct ftrace_ret_stack *ret_stack; | 7076 | struct ftrace_ret_stack *ret_stack; |