function_graph: Move return callback before update of curr_ret_stack

In the past, curr_ret_stack had two functions. One was to denote the depth
of the call graph, the other is to keep track of where on the ret_stack the
data is used. Although they may be slightly related, there are two cases
where they need to be used differently.

The one case is that it keeps the ret_stack data from being corrupted by an
interrupt coming in and overwriting the data still in use. The other is just
to know where the depth of the stack currently is.

The function profiler uses the ret_stack to save a "subtime" variable that
is part of the data on the ret_stack. If curr_ret_stack is modified too
early, then this variable can be corrupted.

The "max_depth" option, when set to 1, will record the first functions going
into the kernel. To see all top functions (when dealing with timings), the
depth variable needs to be lowered before calling the return hook. But by
lowering the curr_ret_stack, it makes the data on the ret_stack still being
used by the return hook susceptible to being overwritten.

Now that there's two variables to handle both cases (curr_ret_depth), we can
move them to the locations where they can handle both cases.

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>

diff --git a/kernel/trace/trace_functions_graph.c b/kernel/trace/trace_functions_graph.c
index 02d4081a7f5afdeaa13d87da0da25350d3f512a7..4f0d72ae636228e1626cc4118255afdc48593582 100644 (file)
--- a/kernel/trace/trace_functions_graph.c
+++ b/kernel/trace/trace_functions_graph.c
@@ -261,7 +261,13 @@ ftrace_pop_return_trace(struct ftrace_graph_ret *trace, unsigned long *ret,
        trace->func = current->ret_stack[index].func;
        trace->calltime = current->ret_stack[index].calltime;
        trace->overrun = atomic_read(&current->trace_overrun);
-       trace->depth = current->curr_ret_depth;
+       trace->depth = current->curr_ret_depth--;
+       /*
+        * We still want to trace interrupts coming in if
+        * max_depth is set to 1. Make sure the decrement is
+        * seen before ftrace_graph_return.
+        */
+       barrier();
 }
 
 /*
@@ -275,9 +281,14 @@ unsigned long ftrace_return_to_handler(unsigned long frame_pointer)
 
        ftrace_pop_return_trace(&trace, &ret, frame_pointer);
        trace.rettime = trace_clock_local();
+       ftrace_graph_return(&trace);
+       /*
+        * The ftrace_graph_return() may still access the current
+        * ret_stack structure, we need to make sure the update of
+        * curr_ret_stack is after that.
+        */
        barrier();
        current->curr_ret_stack--;
-       current->curr_ret_depth--;
        /*
         * The curr_ret_stack can be less than -1 only if it was
         * filtered out and it's about to return from the function.
@@ -288,13 +299,6 @@ unsigned long ftrace_return_to_handler(unsigned long frame_pointer)
                return ret;
        }
 
-       /*
-        * The trace should run after decrementing the ret counter
-        * in case an interrupt were to come in. We don't want to
-        * lose the interrupt if max_depth is set.
-        */
-       ftrace_graph_return(&trace);
-
        if (unlikely(!ret)) {
                ftrace_graph_stop();
                WARN_ON(1);