[PATCH v2 0/3] Ftrace fixes

[PATCH v2 0/3] Ftrace fixes

[Julien Thierry]

Hi,

>From what Steve Rostedt said, we shouldn't rely on ftrace_trace_function
not being ftrace_stub when using the function graph tracer. As x86 does,
always check whether function graph tracing is requested in static tracing.

While testing that I didn't break things, I discovered that the perf ftrace
builtin doesn't handle kernels built without function graph tracer
(regardless of whether tracing is dynamic or not).

First and second patches fixe the ftrace issue for arm64 and arm
respectively.
Third patch fixes the perf front-end.

Changes since v1:
* Added fix for arm suggested by Akashi Takahiro
* Reset graph depth file when it exists

Cheers,

Julien Thierry (3):
  arm64: Fix static use of function graph
  arm: ftrace: function_graph with DYNAMIC_FTRACE
  perf: Fix ftrace builtin when kernel doesn't have function_graph

 arch/arm/kernel/entry-ftrace.S   | 14 +++++++-------
 arch/arm64/kernel/entry-ftrace.S | 12 +++---------
 tools/perf/builtin-ftrace.c      | 14 +++++++++++++-
 3 files changed, 23 insertions(+), 17 deletions(-)

--
1.9.1

[PATCH v2 1/3] arm64: Fix static use of function graph

[Julien Thierry]

Function graph does not work currently when CONFIG_DYNAMIC_TRACE is not
set. This is because ftrace_function_trace is not always set to ftrace_stub
when function_graph is in use.

Do not skip checking of graph tracer functions when ftrace_function_trace
is set.

Signed-off-by: Julien Thierry <julien.thierry@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Acked-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
Reviewed-by: AKASHI Takahiro <takahiro.akashi@linaro.org>

---
 arch/arm64/kernel/entry-ftrace.S | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/arch/arm64/kernel/entry-ftrace.S b/arch/arm64/kernel/entry-ftrace.S
index e1be42e..1175f58 100644
--- a/arch/arm64/kernel/entry-ftrace.S
+++ b/arch/arm64/kernel/entry-ftrace.S
@@ -108,13 +108,8 @@ ENTRY(_mcount)
 	mcount_get_lr	x1		//       function's lr (= parent's pc)
 	blr	x2			//   (*ftrace_trace_function)(pc, lr);

-#ifndef CONFIG_FUNCTION_GRAPH_TRACER
-skip_ftrace_call:			//   return;
-	mcount_exit			// }
-#else
-	mcount_exit			//   return;
-					// }
-skip_ftrace_call:
+skip_ftrace_call:			// }
+#ifdef CONFIG_FUNCTION_GRAPH_TRACER
 	ldr_l	x2, ftrace_graph_return
 	cmp	x0, x2			//   if ((ftrace_graph_return
 	b.ne	ftrace_graph_caller	//        != ftrace_stub)
@@ -123,9 +118,8 @@ skip_ftrace_call:
 	adr_l	x0, ftrace_graph_entry_stub //     != ftrace_graph_entry_stub))
 	cmp	x0, x2
 	b.ne	ftrace_graph_caller	//     ftrace_graph_caller();
-
-	mcount_exit
 #endif /* CONFIG_FUNCTION_GRAPH_TRACER */
+	mcount_exit
 ENDPROC(_mcount)

 #else /* CONFIG_DYNAMIC_FTRACE */
--
1.9.1

[PATCH v2 1/3] arm64: Fix static use of function graph

[Will Deacon]

On Fri, Nov 03, 2017 at 11:44:16AM +0000, Julien Thierry wrote:
> Function graph does not work currently when CONFIG_DYNAMIC_TRACE is not
> set. This is because ftrace_function_trace is not always set to ftrace_stub
> when function_graph is in use.
> 
> Do not skip checking of graph tracer functions when ftrace_function_trace
> is set.
> 
> Signed-off-by: Julien Thierry <julien.thierry@arm.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Acked-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
> Reviewed-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> 
> ---
>  arch/arm64/kernel/entry-ftrace.S | 12 +++---------
>  1 file changed, 3 insertions(+), 9 deletions(-)

Thanks, applied. I assume you'll route the other two patches via their
respective trees.

Will

[PATCH v2 2/3] arm: ftrace: function_graph with DYNAMIC_FTRACE

[Julien Thierry]

Function graph does not work currently when CONFIG_DYNAMIC_TRACE is not
set. This is because ftrace_function_trace is not always set to ftrace_stub
when function_graph is in use.

Do not skip checking of graph tracer functions when ftrace_function_trace
is set.

Suggested-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
Signed-off-by: Julien Thierry <julien.thierry@arm.com>
Cc: Russell King <linux@armlinux.org.uk>
---
 arch/arm/kernel/entry-ftrace.S | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

Takahiro's patch was not signed-off, let me know if I need to change the
Suggested-by into a Signed-off-by.

diff --git a/arch/arm/kernel/entry-ftrace.S b/arch/arm/kernel/entry-ftrace.S
index efcd9f2..e3555f8 100644
--- a/arch/arm/kernel/entry-ftrace.S
+++ b/arch/arm/kernel/entry-ftrace.S
@@ -68,7 +68,13 @@
 	ldr	r2, [r0]
 	adr	r0, .Lftrace_stub
 	cmp	r0, r2
-	bne	1f
+	beq	1f
+
+	mcount_get_lr	r1			@ lr of instrumented func
+	mcount_adjust_addr	r0, lr		@ instrumented function
+	badr	lr, 1f
+	mov	pc, r2
+1:

 #ifdef CONFIG_FUNCTION_GRAPH_TRACER
 	ldr     r1, =ftrace_graph_return
@@ -84,12 +90,6 @@
 #endif

 	mcount_exit
-
-1: 	mcount_get_lr	r1			@ lr of instrumented func
-	mcount_adjust_addr	r0, lr		@ instrumented function
-	badr	lr, 2f
-	mov	pc, r2
-2:	mcount_exit
 .endm

 #ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
--
1.9.1

[PATCH v2 3/3] perf: Fix ftrace builtin when kernel doesn't have function_graph

[Julien Thierry]

When linux is built without support for function graph tracer, the ftrace
builtin of perf will fail when trying to reset max_graph_depth because the
file does not exist. This prevents the use of function tracer from perf.

Only try to write the file max_graph_depth file when it exists.

Signed-off-by: Julien Thierry <julien.thierry@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: linux-kernel at vger.kernel.org
---
 tools/perf/builtin-ftrace.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/tools/perf/builtin-ftrace.c b/tools/perf/builtin-ftrace.c
index 25a42ac..1abc066 100644
--- a/tools/perf/builtin-ftrace.c
+++ b/tools/perf/builtin-ftrace.c
@@ -14,6 +14,7 @@
 #include <signal.h>
 #include <fcntl.h>
 #include <poll.h>
+#include <sys/stat.h>

 #include "debug.h"
 #include <subcmd/parse-options.h>
@@ -116,6 +117,13 @@ static int append_tracing_file(const char *name, const char *val)
 static int reset_tracing_cpu(void);
 static void reset_tracing_filters(void);

+static inline bool tracing_file_accessible(const char *name)
+{
+	struct stat s;
+
+	return stat(get_tracing_file(name), &s) == 0;
+}
+
 static int reset_tracing_files(struct perf_ftrace *ftrace __maybe_unused)
 {
 	if (write_tracing_file("tracing_on", "0") < 0)
@@ -130,8 +138,12 @@ static int reset_tracing_files(struct perf_ftrace *ftrace __maybe_unused)
 	if (reset_tracing_cpu() < 0)
 		return -1;

-	if (write_tracing_file("max_graph_depth", "0") < 0)
+	if (!tracing_file_accessible("max_graph_depth")) {
+		if (!strcmp(ftrace->tracer, "function_graph"))
+			return -1;
+	} else if (write_tracing_file("max_graph_depth", "0") < 0) {
 		return -1;
+	}

 	reset_tracing_filters();
 	return 0;
--
1.9.1

[PATCH v2 3/3] perf: Fix ftrace builtin when kernel doesn't have function_graph

[Julien Thierry]

When linux is built without support for function graph tracer, the ftrace
builtin of perf will fail when trying to reset max_graph_depth because the
file does not exist. This prevents the use of function tracer from perf.

Only try to write the file max_graph_depth file when it exists.

Signed-off-by: Julien Thierry <julien.thierry@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: linux-kernel@vger.kernel.org
---
 tools/perf/builtin-ftrace.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/tools/perf/builtin-ftrace.c b/tools/perf/builtin-ftrace.c
index 25a42ac..1abc066 100644
--- a/tools/perf/builtin-ftrace.c
+++ b/tools/perf/builtin-ftrace.c
@@ -14,6 +14,7 @@
 #include <signal.h>
 #include <fcntl.h>
 #include <poll.h>
+#include <sys/stat.h>

 #include "debug.h"
 #include <subcmd/parse-options.h>
@@ -116,6 +117,13 @@ static int append_tracing_file(const char *name, const char *val)
 static int reset_tracing_cpu(void);
 static void reset_tracing_filters(void);

+static inline bool tracing_file_accessible(const char *name)
+{
+	struct stat s;
+
+	return stat(get_tracing_file(name), &s) == 0;
+}
+
 static int reset_tracing_files(struct perf_ftrace *ftrace __maybe_unused)
 {
 	if (write_tracing_file("tracing_on", "0") < 0)
@@ -130,8 +138,12 @@ static int reset_tracing_files(struct perf_ftrace *ftrace __maybe_unused)
 	if (reset_tracing_cpu() < 0)
 		return -1;

-	if (write_tracing_file("max_graph_depth", "0") < 0)
+	if (!tracing_file_accessible("max_graph_depth")) {
+		if (!strcmp(ftrace->tracer, "function_graph"))
+			return -1;
+	} else if (write_tracing_file("max_graph_depth", "0") < 0) {
 		return -1;
+	}

 	reset_tracing_filters();
 	return 0;
--
1.9.1