Date:	Fri,  6 Mar 2015 23:58:06 +0530
From:	Pratyush Anand <panand@...hat.com>
To:	linux-kernel@...r.kernel.org
Cc:	Pratyush Anand <panand@...hat.com>,
	Steven Rostedt <rostedt@...dmis.org>
Subject: [PATCH] ftrace: fix en(dis)able graph caller when en(dis)abling record via sysctl

When ftrace is enabled globally through proc interface, we must check if
ftrace_graph_active is set. If it is set, then we should also pass
FTRACE_START_FUNC_RET command to ftrace_run_update_code. Similarly, when
ftrace is disabled globally through proc interface, we must check if
ftrace_graph_active is set. If it is set, then we should also pass
FTRACE_STOP_FUNC_RET command to ftrace_run_update_code.

Consider following situation.

 # echo 0 > /proc/sys/kernel/ftrace_enabled

After this ftrace_enabled = 0.

 # echo function_graph > /sys/kernel/debug/tracing/current_tracer

Since ftrace_enabled = 0. Therefore, ftrace_enable_ftrace_graph_caller
is never called.

 # echo 1 > /proc/sys/kernel/ftrace_enabled

Now ftrace_enabled will be set to true, but still
ftrace_enable_ftrace_graph_caller will not be called, which is not
desired.

Further if we execute following after this:
  # echo nop > /sys/kernel/debug/tracing/current_tracer

Now since ftrace_enabled is set, so it will call
ftrace_disable_ftrace_graph_caller, which causes a kernel warning on ARM
platform.

On ARM platform, when ftrace_enable_ftrace_graph_caller is called, it
checks whether old instruction is nop or not. If its not nop, then it
returns error. If its nop then it replaces instruction at that address
with a branch to ftrace_graph_caller. ftrace_disable_ftrace_graph_caller
behaves just opposite. Therefore if generic ftrace code ever calls
either ftrace_enable_ftrace_graph_caller or
ftrace_disable_ftrace_graph_caller consecutively two times, then it will
return error, which will cause generic ftrace code to raise a warn.

This patch will fixes the issue described here.

Signed-off-by: Pratyush Anand <panand@...hat.com>
Cc: Steven Rostedt <rostedt@...dmis.org>
---
 kernel/trace/ftrace.c | 21 +++++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 010ab93660ec..bdd63154e724 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -1058,6 +1058,7 @@ static __init void ftrace_profile_debugfs(struct dentry *d_tracer)
 #endif /* CONFIG_FUNCTION_PROFILER */
 
 static struct pid * const ftrace_swapper_pid = &init_struct_pid;
+static int ftrace_graph_active;
 
 #ifdef CONFIG_DYNAMIC_FTRACE
 
@@ -2692,24 +2693,38 @@ static int ftrace_shutdown(struct ftrace_ops *ops, int command)
 
 static void ftrace_startup_sysctl(void)
 {
+	int command;
+
 	if (unlikely(ftrace_disabled))
 		return;
 
 	/* Force update next time */
 	saved_ftrace_func = NULL;
 	/* ftrace_start_up is true if we want ftrace running */
+	if (ftrace_start_up) {
+		command = FTRACE_UPDATE_CALLS;
+		if (ftrace_graph_active)
+			command |= FTRACE_START_FUNC_RET;
+		ftrace_run_update_code(command);
+	}
 	if (ftrace_start_up)
 		ftrace_run_update_code(FTRACE_UPDATE_CALLS);
 }
 
 static void ftrace_shutdown_sysctl(void)
 {
+	int command;
+
 	if (unlikely(ftrace_disabled))
 		return;
 
 	/* ftrace_start_up is true if ftrace is running */
-	if (ftrace_start_up)
-		ftrace_run_update_code(FTRACE_DISABLE_CALLS);
+	if (ftrace_start_up) {
+		command = FTRACE_DISABLE_CALLS;
+		if (ftrace_graph_active)
+			command |= FTRACE_STOP_FUNC_RET;
+		ftrace_run_update_code(command);
+	}
 }
 
 static cycle_t		ftrace_update_time;
@@ -5594,8 +5609,6 @@ static struct ftrace_ops graph_ops = {
 	ASSIGN_OPS_HASH(graph_ops, &global_ops.local_hash)
 };
 
-static int ftrace_graph_active;
-
 int ftrace_graph_entry_stub(struct ftrace_graph_ent *trace)
 {
 	return 0;
-- 
2.1.0

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists