ftrace: Remove ftrace_start/stop() and friends

From: "Steven Rostedt (Red Hat)" <[email protected]>

ftrace_stop() is used to stop function tracing during suspend and resume which removes a lot of possible debugging opportunities with tracing. The reason was that some function in the resume path was causing a triple fault if it were to be traced. The issue I found was that doing something as simple as calling smp_processor_id() would reboot the box!

When function tracing was first created I didn't have a good way to figure out what function was having issues, or it looked to be multiple ones. To fix it, we just created a big hammer approach to the problem which was to add a flag in the mcount trampoline that could be checked and not call the traced functions.

Lately I developed better ways to find problem functions and I can bisect down to see what function is causing the issue. I removed the flag that stopped tracing and proceeded to find the problem function and it ended up being restore_processor_state(). This function makes sense as when the CPU comes back online from a suspend it calls this function to set up registers, amongst them the GS register, which stores things such as what CPU the processor is (if you call smp_processor_id() without this set up properly, it would fault).

By making restore_processor_state() notrace, the system can suspend and resume without the need of the big hammer tracing to stop.

Signed-off-by Steven Rostedt <[email protected]>

---
 arch/x86/power/cpu.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

arch/x86/power/cpu.c +2 -2

index 424f4c97a44d..6ec7910f59bf 100644
@@ -165,7 +165,7 @@ static void fix_processor_context(void)
  *		by __save_processor_state()
  *	@ctxt - structure to load the registers contents from
  */
-static void __restore_processor_state(struct saved_context *ctxt)
+static void notrace __restore_processor_state(struct saved_context *ctxt)
 {
 	if (ctxt->misc_enable_saved)
 		wrmsrl(MSR_IA32_MISC_ENABLE, ctxt->misc_enable);

@@ -239,7 +239,7 @@ static void __restore_processor_state(struct saved_context *ctxt)
 }
 
 /* Needed by apm.c */
-void restore_processor_state(void)
+void notrace restore_processor_state(void)
 {
 	__restore_processor_state(&saved_context);
 }

From: "Steven Rostedt (Red Hat)" <[email protected]>

ftrace_stop() and ftrace_start() were added to the suspend and hibernate process because there was some function within the work flow that caused the system to reboot if it was traced. This function has recently been found (restore_processor_state()). Now there's no reason to disable function tracing while we are going into suspend or hibernate, which means that being able to trace this will help tremendously in debugging any issues with suspend or hibernate.

This also means that the ftrace_stop/start() functions can be removed and simplify the function tracing code a bit.

Signed-off-by Steven Rostedt <[email protected]>

---
 kernel/power/hibernate.c | 6 ------
 kernel/power/suspend.c   | 2 --
 2 files changed, 8 deletions(-)

kernel/power/hibernate.c +0 -6

index 49e0a20fd010..ca7b1906c6c8 100644
@@ -365,7 +365,6 @@ int hibernation_snapshot(int platform_mode)
 	}
 
 	suspend_console();
-	ftrace_stop();
 	pm_restrict_gfp_mask();
 
 	error = dpm_suspend(PMSG_FREEZE);

@@ -391,7 +390,6 @@ int hibernation_snapshot(int platform_mode)
 	if (error || !in_suspend)
 		pm_restore_gfp_mask();
 
-	ftrace_start();
 	resume_console();
 	dpm_complete(msg);
 

@@ -494,7 +492,6 @@ int hibernation_restore(int platform_mode)
 
 	pm_prepare_console();
 	suspend_console();
-	ftrace_stop();
 	pm_restrict_gfp_mask();
 	error = dpm_suspend_start(PMSG_QUIESCE);
 	if (!error) {

@@ -502,7 +499,6 @@ int hibernation_restore(int platform_mode)
 		dpm_resume_end(PMSG_RECOVER);
 	}
 	pm_restore_gfp_mask();
-	ftrace_start();
 	resume_console();
 	pm_restore_console();
 	return error;

@@ -529,7 +525,6 @@ int hibernation_platform_enter(void)
 
 	entering_platform_hibernation = true;
 	suspend_console();
-	ftrace_stop();
 	error = dpm_suspend_start(PMSG_HIBERNATE);
 	if (error) {
 		if (hibernation_ops->recover)

@@ -573,7 +568,6 @@ int hibernation_platform_enter(void)
  Resume_devices:
 	entering_platform_hibernation = false;
 	dpm_resume_end(PMSG_RESTORE);
-	ftrace_start();
 	resume_console();
 
  Close:

kernel/power/suspend.c +0 -2

index 4dd8822f732a..f6623da034d8 100644
@@ -248,7 +248,6 @@ static int suspend_enter(suspend_state_t state, bool *wakeup)
 		goto Platform_wake;
 	}
 
-	ftrace_stop();
 	error = disable_nonboot_cpus();
 	if (error || suspend_test(TEST_CPUS))
 		goto Enable_cpus;

@@ -275,7 +274,6 @@ static int suspend_enter(suspend_state_t state, bool *wakeup)
 
  Enable_cpus:
 	enable_nonboot_cpus();
-	ftrace_start();
 
  Platform_wake:
 	if (need_suspend_ops(state) && suspend_ops->wake)

From: "Steven Rostedt (Red Hat)" <[email protected]>

ftrace_stop() is going away as it disables parts of function tracing that affects users that should not be affected. But ftrace_graph_stop() is built on ftrace_stop(). Here's another example of killing all of function tracing because something went wrong with function graph tracing.

Instead of disabling all users of function tracing on function graph error, disable only function graph tracing. To do this, the arch code must call ftrace_graph_is_dead() before it implements function graph.

Cc H. Peter Anvin <[email protected]>

Signed-off-by Steven Rostedt <[email protected]>

From: "Steven Rostedt (Red Hat)" <[email protected]>

ftrace_stop() is going away as it disables parts of function tracing that affects users that should not be affected. But ftrace_graph_stop() is built on ftrace_stop(). Here's another example of killing all of function tracing because something went wrong with function graph tracing.

Instead of disabling all users of function tracing on function graph error, disable only function graph tracing. To do this, the arch code must call ftrace_graph_is_dead() before it implements function graph.

Cc Michal Simek <[email protected]>

Signed-off-by Steven Rostedt <[email protected]>

---
 arch/microblaze/kernel/ftrace.c | 3 +++
 1 file changed, 3 insertions(+)

arch/microblaze/kernel/ftrace.c +3 -0

index bbcd2533766c..fc7b48a52cd5 100644
@@ -27,6 +27,9 @@ void prepare_ftrace_return(unsigned long *parent, unsigned long self_addr)
 	unsigned long return_hooker = (unsigned long)
 				&return_to_handler;
 
+	if (unlikely(ftrace_graph_is_dead()))
+		return;
+
 	if (unlikely(atomic_read(&current->tracing_graph_pause)))
 		return;
 

From: "Steven Rostedt (Red Hat)" <[email protected]>

ftrace_stop() is going away as it disables parts of function tracing that affects users that should not be affected. But ftrace_graph_stop() is built on ftrace_stop(). Here's another example of killing all of function tracing because something went wrong with function graph tracing.

Instead of disabling all users of function tracing on function graph error, disable only function graph tracing. To do this, the arch code must call ftrace_graph_is_dead() before it implements function graph.

Cc Michal Simek <[email protected]>

Signed-off-by Steven Rostedt <[email protected]>

---
 arch/microblaze/kernel/ftrace.c | 3 +++
 1 file changed, 3 insertions(+)

arch/microblaze/kernel/ftrace.c +3 -0

index bbcd2533766c..fc7b48a52cd5 100644
@@ -27,6 +27,9 @@ void prepare_ftrace_return(unsigned long *parent, unsigned long self_addr)
 	unsigned long return_hooker = (unsigned long)
 				&return_to_handler;
 
+	if (unlikely(ftrace_graph_is_dead()))
+		return;
+
 	if (unlikely(atomic_read(&current->tracing_graph_pause)))
 		return;
 

Michal,

Can you ack this please?

-- Steve

Quoted 26 lines · On Thu, 26 Jun 2014 12:52:27 -0400 Steven Rostedt <[email protected]> wrote:

From: "Steven Rostedt (Red Hat)" <[email protected]>

ftrace_stop() is going away as it disables parts of function tracing that affects users that should not be affected. But ftrace_graph_stop() is built on ftrace_stop(). Here's another example of killing all of function tracing because something went wrong with function graph tracing.

Instead of disabling all users of function tracing on function graph error, disable only function graph tracing. To do this, the arch code must call ftrace_graph_is_dead() before it implements function graph.

Cc Michal Simek <[email protected]>

Signed-off-by Steven Rostedt <[email protected]>

---
 arch/microblaze/kernel/ftrace.c | 3 +++
 1 file changed, 3 insertions(+)

arch/microblaze/kernel/ftrace.c +0 -0

index bbcd2533766c..fc7b48a52cd5 100644
@@ -27,6 +27,9 @@ void prepare_ftrace_return(unsigned long *parent, unsigned long self_addr)
 	unsigned long return_hooker = (unsigned long)
 				&return_to_handler;
 

arch/microblaze/kernel/ftrace.c void prepare_ftrace_return(unsigned long *parent, unsigned long self_addr) · line 35

Quoted 6 lines

arch/microblaze/kernel/ftrace.c +3 -0

+	if (unlikely(ftrace_graph_is_dead()))
+		return;
+
 	if (unlikely(atomic_read(&current->tracing_graph_pause)))
 		return;
 

From: "Steven Rostedt (Red Hat)" <[email protected]>

ftrace_stop() is going away as it disables parts of function tracing that affects users that should not be affected. But ftrace_graph_stop() is built on ftrace_stop(). Here's another example of killing all of function tracing because something went wrong with function graph tracing.

Instead of disabling all users of function tracing on function graph error, disable only function graph tracing. To do this, the arch code must call ftrace_graph_is_dead() before it implements function graph.

Cc Ralf Baechle <[email protected]>

Signed-off-by Steven Rostedt <[email protected]>

---
 arch/mips/kernel/ftrace.c | 3 +++
 1 file changed, 3 insertions(+)

arch/mips/kernel/ftrace.c +3 -0

index 60e7e5e45af1..8b6538750fe1 100644
@@ -302,6 +302,9 @@ void prepare_ftrace_return(unsigned long *parent_ra_addr, unsigned long self_ra,
 	    &return_to_handler;
 	int faulted, insns;
 
+	if (unlikely(ftrace_graph_is_dead()))
+		return;
+
 	if (unlikely(atomic_read(&current->tracing_graph_pause)))
 		return;
 

Adding [email protected].

-- Steve

Quoted 23 lines · On Thu, 26 Jun 2014 12:52:28 -0400 Steven Rostedt <[email protected]> wrote:

From: "Steven Rostedt (Red Hat)" <[email protected]>

ftrace_stop() is going away as it disables parts of function tracing that affects users that should not be affected. But ftrace_graph_stop() is built on ftrace_stop(). Here's another example of killing all of function tracing because something went wrong with function graph tracing.

Instead of disabling all users of function tracing on function graph error, disable only function graph tracing. To do this, the arch code must call ftrace_graph_is_dead() before it implements function graph.

Cc Ralf Baechle <[email protected]>

Signed-off-by Steven Rostedt <[email protected]>

---
 arch/mips/kernel/ftrace.c | 3 +++
 1 file changed, 3 insertions(+)

arch/mips/kernel/ftrace.c +0 -0

index 60e7e5e45af1..8b6538750fe1 100644

arch/mips/kernel/ftrace.c void prepare_ftrace_return(unsigned long *parent_ra_addr, unsigned long self_ra, · line 310

Quoted 9 lines

arch/mips/kernel/ftrace.c +3 -0

@@ -302,6 +302,9 @@ void prepare_ftrace_return(unsigned long *parent_ra_addr, unsigned long self_ra,
 	    &return_to_handler;
 	int faulted, insns;
 
+	if (unlikely(ftrace_graph_is_dead()))
+		return;
+
 	if (unlikely(atomic_read(&current->tracing_graph_pause)))
 		return;
 

--
To unsubscribe from this list: send the line "unsubscribe linux-arch" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

From: "Steven Rostedt (Red Hat)" <[email protected]>

ftrace_stop() is going away as it disables parts of function tracing that affects users that should not be affected. But ftrace_graph_stop() is built on ftrace_stop(). Here's another example of killing all of function tracing because something went wrong with function graph tracing.

Instead of disabling all users of function tracing on function graph error, disable only function graph tracing. To do this, the arch code must call ftrace_graph_is_dead() before it implements function graph.

Cc Ralf Baechle <[email protected]>

Signed-off-by Steven Rostedt <[email protected]>

---
 arch/mips/kernel/ftrace.c | 3 +++
 1 file changed, 3 insertions(+)

arch/mips/kernel/ftrace.c +3 -0

index 60e7e5e45af1..8b6538750fe1 100644
@@ -302,6 +302,9 @@ void prepare_ftrace_return(unsigned long *parent_ra_addr, unsigned long self_ra,
 	    &return_to_handler;
 	int faulted, insns;
 
+	if (unlikely(ftrace_graph_is_dead()))
+		return;
+
 	if (unlikely(atomic_read(&current->tracing_graph_pause)))
 		return;
 

From: "Steven Rostedt (Red Hat)" <[email protected]>

ftrace_stop() is going away as it disables parts of function tracing that affects users that should not be affected. But ftrace_graph_stop() is built on ftrace_stop(). Here's another example of killing all of function tracing because something went wrong with function graph tracing.

Instead of disabling all users of function tracing on function graph error, disable only function graph tracing. To do this, the arch code must call ftrace_graph_is_dead() before it implements function graph.

Cc Anton Blanchard <[email protected]>

Cc Benjamin Herrenschmidt <[email protected]>

Signed-off-by Steven Rostedt <[email protected]>

---
 arch/powerpc/kernel/ftrace.c | 3 +++
 1 file changed, 3 insertions(+)

arch/powerpc/kernel/ftrace.c +3 -0

index f202d0731b06..f8c0ce5523a5 100644
@@ -527,6 +527,9 @@ void prepare_ftrace_return(unsigned long *parent, unsigned long self_addr)
 	struct ftrace_graph_ent trace;
 	unsigned long return_hooker = (unsigned long)&return_to_handler;
 
+	if (unlikely(ftrace_graph_is_dead()))
+		return;
+
 	if (unlikely(atomic_read(&current->tracing_graph_pause)))
 		return;
 

From: "Steven Rostedt (Red Hat)" <[email protected]>

Nothing sets function_trace_stop to disable function tracing anymore. Remove the check for it in the arch code.

Cc H. Peter Anvin <[email protected]>

Signed-off-by Steven Rostedt <[email protected]>

From: "Steven Rostedt (Red Hat)" <[email protected]>

Nothing sets function_trace_stop to disable function tracing anymore. Remove the check for it in the arch code.

[ Please test this on your arch ]

Cc Chris Metcalf <[email protected]>

Signed-off-by Steven Rostedt <[email protected]>

---
 arch/tile/Kconfig            |  1 -
 arch/tile/kernel/mcount_64.S | 18 ------------------
 2 files changed, 19 deletions(-)

arch/tile/Kconfig +0 -1

index 4f3006b600e3..7fcd492adbfc 100644
@@ -128,7 +128,6 @@ config TILEGX
 	select SPARSE_IRQ
 	select GENERIC_IRQ_LEGACY_ALLOC_HWIRQ
 	select HAVE_FUNCTION_TRACER
-	select HAVE_FUNCTION_TRACE_MCOUNT_TEST
 	select HAVE_FUNCTION_GRAPH_TRACER
 	select HAVE_DYNAMIC_FTRACE
 	select HAVE_FTRACE_MCOUNT_RECORD

arch/tile/kernel/mcount_64.S +0 -18

index 70d7bb0c4d8f..3c2b8d5e1d1a 100644
@@ -77,15 +77,6 @@ STD_ENDPROC(__mcount)
 
 	.align	64
 STD_ENTRY(ftrace_caller)
-	moveli	r11, hw2_last(function_trace_stop)
-	{ shl16insli	r11, r11, hw1(function_trace_stop); move r12, lr }
-	{ shl16insli	r11, r11, hw0(function_trace_stop); move lr, r10 }
-	ld	r11, r11
-	beqz	r11, 1f
-	jrp	r12
-
-1:
-	{ move	r10, lr; move	lr, r12 }
 	MCOUNT_SAVE_REGS
 
 	/* arg1: self return address */

@@ -119,15 +110,6 @@ STD_ENDPROC(ftrace_caller)
 
 	.align	64
 STD_ENTRY(__mcount)
-	moveli	r11, hw2_last(function_trace_stop)
-	{ shl16insli	r11, r11, hw1(function_trace_stop); move r12, lr }
-	{ shl16insli	r11, r11, hw0(function_trace_stop); move lr, r10 }
-	ld	r11, r11
-	beqz	r11, 1f
-	jrp	r12
-
-1:
-	{ move	r10, lr; move	lr, r12 }
 	{
 	 moveli	r11, hw2_last(ftrace_trace_function)
 	 moveli	r13, hw2_last(ftrace_stub)

From: "Steven Rostedt (Red Hat)"<[email protected]>

Nothing sets function_trace_stop to disable function tracing anymore. Remove the check for it in the arch code.

[ Please test this on your arch ]

Cc Chris Metcalf<[email protected]>

Signed-off-by Steven Rostedt<[email protected]>

Quoted 5 lines · Steven Rostedt

From: "Steven Rostedt (Red Hat)"<[email protected]>

Nothing sets function_trace_stop to disable function tracing anymore. Remove the check for it in the arch code.

Quoted 4 lines

[ Please test this on your arch ]

Cc Chris Metcalf<[email protected]>

Signed-off-by Steven Rostedt<[email protected]>

I forwarded this internally to the guy who did the work at Tilera and he's happy with the change.

Acked-by Zhigang Lu<[email protected]>

From: "Steven Rostedt (Red Hat)" <[email protected]>

Nothing sets function_trace_stop to disable function tracing anymore. Remove the check for it in the arch code.

[ Please test this on your arch ]

Cc David S. Miller <[email protected]>

Signed-off-by Steven Rostedt <[email protected]>

---
 arch/sparc/Kconfig      |  1 -
 arch/sparc/lib/mcount.S | 10 ++--------
 2 files changed, 2 insertions(+), 9 deletions(-)

arch/sparc/Kconfig +0 -1

index 29f2e988c56a..abd7d5575a7d 100644
@@ -55,7 +55,6 @@ config SPARC64
 	select HAVE_FUNCTION_TRACER
 	select HAVE_FUNCTION_GRAPH_TRACER
 	select HAVE_FUNCTION_GRAPH_FP_TEST
-	select HAVE_FUNCTION_TRACE_MCOUNT_TEST
 	select HAVE_KRETPROBES
 	select HAVE_KPROBES
 	select HAVE_RCU_TABLE_FREE if SMP

arch/sparc/lib/mcount.S +2 -8

index 3ad6cbdc2163..0b0ed4d34219 100644
@@ -24,10 +24,7 @@ mcount:
 #ifdef CONFIG_DYNAMIC_FTRACE
 	/* Do nothing, the retl/nop below is all we need.  */
 #else
-	sethi		%hi(function_trace_stop), %g1
-	lduw		[%g1 + %lo(function_trace_stop)], %g2
-	brnz,pn		%g2, 2f
-	 sethi		%hi(ftrace_trace_function), %g1
+	sethi		%hi(ftrace_trace_function), %g1
 	sethi		%hi(ftrace_stub), %g2
 	ldx		[%g1 + %lo(ftrace_trace_function)], %g1
 	or		%g2, %lo(ftrace_stub), %g2

@@ -80,11 +77,8 @@ ftrace_stub:
 	.globl		ftrace_caller
 	.type		ftrace_caller,#function
 ftrace_caller:
-	sethi		%hi(function_trace_stop), %g1
 	mov		%i7, %g2
-	lduw		[%g1 + %lo(function_trace_stop)], %g1
-	brnz,pn		%g1, ftrace_stub
-	 mov		%fp, %g3
+	mov		%fp, %g3
 	save		%sp, -176, %sp
 	mov		%g2, %o1
 	mov		%g2, %l0

From: "Steven Rostedt (Red Hat)" <[email protected]>

Nothing sets function_trace_stop to disable function tracing anymore. Remove the check for it in the arch code.

[ Please test this on your arch ]

Cc David S. Miller <[email protected]>

Signed-off-by Steven Rostedt <[email protected]>

---
 arch/sparc/Kconfig      |  1 -
 arch/sparc/lib/mcount.S | 10 ++--------
 2 files changed, 2 insertions(+), 9 deletions(-)

arch/sparc/Kconfig +0 -1

index 29f2e988c56a..abd7d5575a7d 100644
@@ -55,7 +55,6 @@ config SPARC64
 	select HAVE_FUNCTION_TRACER
 	select HAVE_FUNCTION_GRAPH_TRACER
 	select HAVE_FUNCTION_GRAPH_FP_TEST
-	select HAVE_FUNCTION_TRACE_MCOUNT_TEST
 	select HAVE_KRETPROBES
 	select HAVE_KPROBES
 	select HAVE_RCU_TABLE_FREE if SMP

arch/sparc/lib/mcount.S +2 -8

index 3ad6cbdc2163..0b0ed4d34219 100644
@@ -24,10 +24,7 @@ mcount:
 #ifdef CONFIG_DYNAMIC_FTRACE
 	/* Do nothing, the retl/nop below is all we need.  */
 #else
-	sethi		%hi(function_trace_stop), %g1
-	lduw		[%g1 + %lo(function_trace_stop)], %g2
-	brnz,pn		%g2, 2f
-	 sethi		%hi(ftrace_trace_function), %g1
+	sethi		%hi(ftrace_trace_function), %g1
 	sethi		%hi(ftrace_stub), %g2
 	ldx		[%g1 + %lo(ftrace_trace_function)], %g1
 	or		%g2, %lo(ftrace_stub), %g2

@@ -80,11 +77,8 @@ ftrace_stub:
 	.globl		ftrace_caller
 	.type		ftrace_caller,#function
 ftrace_caller:
-	sethi		%hi(function_trace_stop), %g1
 	mov		%i7, %g2
-	lduw		[%g1 + %lo(function_trace_stop)], %g1
-	brnz,pn		%g1, ftrace_stub
-	 mov		%fp, %g3
+	mov		%fp, %g3
 	save		%sp, -176, %sp
 	mov		%g2, %o1
 	mov		%g2, %l0

Do you have any problem with this patch going through my tree? It compiles, but I do not have any hardware to test it. If you can verify that this patch works and ack it, it would be much appreciated.

This code is at:

git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace.git

branch: rfc/remove-function-trace-stop

From: "Steven Rostedt (Red Hat)" <[email protected]>

Nothing sets function_trace_stop to disable function tracing anymore. Remove the check for it in the arch code.

[ Please test this on your arch ]

Cc Matt Fleming <[email protected]>

Cc Paul Mundt <[email protected]>

Signed-off-by Steven Rostedt <[email protected]>

---
 arch/sh/Kconfig      |  1 -
 arch/sh/lib/mcount.S | 24 ++----------------------
 2 files changed, 2 insertions(+), 23 deletions(-)

arch/sh/Kconfig +0 -1

index 834b67c4db5a..aa2df3eaeb29 100644
@@ -57,7 +57,6 @@ config SUPERH32
 	select HAVE_FUNCTION_TRACER
 	select HAVE_FTRACE_MCOUNT_RECORD
 	select HAVE_DYNAMIC_FTRACE
-	select HAVE_FUNCTION_TRACE_MCOUNT_TEST
 	select HAVE_FTRACE_NMI_ENTER if DYNAMIC_FTRACE
 	select ARCH_WANT_IPC_PARSE_VERSION
 	select HAVE_FUNCTION_GRAPH_TRACER

arch/sh/lib/mcount.S +2 -22

index 52aa2011d753..7a8572f9d58b 100644
@@ -92,13 +92,6 @@ mcount:
 	rts
 	 nop
 #else
-#ifndef CONFIG_DYNAMIC_FTRACE
-	mov.l	.Lfunction_trace_stop, r0
-	mov.l	@r0, r0
-	tst	r0, r0
-	bf	ftrace_stub
-#endif
-
 	MCOUNT_ENTER()
 
 #ifdef CONFIG_DYNAMIC_FTRACE

@@ -174,11 +167,6 @@ ftrace_graph_call:
 
 	.globl ftrace_caller
 ftrace_caller:
-	mov.l	.Lfunction_trace_stop, r0
-	mov.l	@r0, r0
-	tst	r0, r0
-	bf	ftrace_stub
-
 	MCOUNT_ENTER()
 
 	.globl ftrace_call

@@ -196,8 +184,6 @@ ftrace_call:
 #endif /* CONFIG_DYNAMIC_FTRACE */
 
 	.align 2
-.Lfunction_trace_stop:
-	.long	function_trace_stop
 
 /*
  * NOTE: From here on the locations of the .Lftrace_stub label and

@@ -217,12 +203,7 @@ ftrace_stub:
 #ifdef CONFIG_FUNCTION_GRAPH_TRACER
 	.globl	ftrace_graph_caller
 ftrace_graph_caller:
-	mov.l	2f, r0
-	mov.l	@r0, r0
-	tst	r0, r0
-	bt	1f
-
-	mov.l	3f, r1
+	mov.l	2f, r1
 	jmp	@r1
 	 nop
 1:

@@ -242,8 +223,7 @@ ftrace_graph_caller:
 	MCOUNT_LEAVE()
 
 	.align 2
-2:	.long	function_trace_stop
-3:	.long	skip_trace
+2:	.long	skip_trace
 .Lprepare_ftrace_return:
 	.long	prepare_ftrace_return
 

From: "Steven Rostedt (Red Hat)" <[email protected]>

Nothing sets function_trace_stop to disable function tracing anymore. Remove the check for it in the arch code.

Cc Helge Deller <[email protected]>

Cc Kyle McMartin <[email protected]>

Signed-off-by Steven Rostedt <[email protected]>

---
 arch/parisc/Kconfig         | 1 -
 arch/parisc/kernel/ftrace.c | 3 ---
 2 files changed, 4 deletions(-)

arch/parisc/Kconfig +0 -1

index 108d48e652af..6e75e2030927 100644
@@ -6,7 +6,6 @@ config PARISC
 	select HAVE_OPROFILE
 	select HAVE_FUNCTION_TRACER if 64BIT
 	select HAVE_FUNCTION_GRAPH_TRACER if 64BIT
-	select HAVE_FUNCTION_TRACE_MCOUNT_TEST if 64BIT
 	select ARCH_WANT_FRAME_POINTERS
 	select RTC_CLASS
 	select RTC_DRV_GENERIC

arch/parisc/kernel/ftrace.c +0 -3

index 5512ab32c5e0..559d400f9385 100644
@@ -155,9 +155,6 @@ void ftrace_function_trampoline(unsigned long parent,
 {
 	extern ftrace_func_t ftrace_trace_function;
 
-	if (function_trace_stop)
-		return;
-
 	if (ftrace_trace_function != ftrace_stub) {
 		ftrace_trace_function(parent, self_addr);
 		return;

Quoted 4 lines · On Thu, 26 Jun 2014 12:52:41 -0400 Steven Rostedt <[email protected]> wrote:

From: "Steven Rostedt (Red Hat)" <[email protected]>

Nothing sets function_trace_stop to disable function tracing anymore. Remove the check for it in the arch code.

From the cover letter, you were not Cc'd on.

Anyway, as there is no more reason to set function_trace_stop it is time to remove it. Unfortunately it's in several archs in assembly. Most of the assembly looks rather straight forward and I removed them myself. But I was only able to compile test them (for archs: arm64, metag, and microblaze I do not have my cross tools set up for them and did not even compile test it). But I would really love it if people can download their patch and test it out. You only need the patches that go against your arch and to really test it, also include the patch titled:

ftrace: Remove check for HAVE_FUNCTION_TRACE_MCOUNT_TEST

Otherwise your arch patch will call the list op that still does the check. That is, if you want to test suspend and resume on your arch.

As you may see, there are patches to the ftrace infrastructure that depend on the arch patches being implemented. I removed the functionality from the infrastructure, then removed it from the archs, and then finally removed the existence of the function_trace_stop variable, which would cause the archs to fail to compile if that were to go first.

If you can test your arch and give me your acked-by, I would appreciate it. Otherwise, if you need this to go through your tree, I would ask you to set up a dedicated branch that I can pull from to keep this order intact.

Sadly the arch-related tracing code in parisc is really broken.
It doesn't even compile cleanly on parisc (at least on 64bit), and as I wrote you in another mail I really need to fix this soon (which I already started on).

But your patches look clean, so to get the basics right, I'm happy to give a

Acked-by Helge Deller <[email protected]>

for the two patches which affect parisc, which are:
[PATCH 08/27] parisc: ftrace: Add call to ftrace_graph_is_dead() in function graph code [PATCH 20/27] parisc: ftrace: Remove check of obsolete variable function_trace_stop

It would be good if you can push them through your tree. I assume your tree is:
git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace.git
with branch ftrace/next.
Is this correct?
If yes, I can build my patches upon this tree...

From: "Steven Rostedt (Red Hat)" <[email protected]>

Nothing sets function_trace_stop to disable function tracing anymore. Remove the check for it in the arch code.

[ Please test this on your arch ]

Signed-off-by Steven Rostedt <[email protected]>

---
 arch/mips/Kconfig         | 1 -
 arch/mips/kernel/mcount.S | 7 -------
 2 files changed, 8 deletions(-)

arch/mips/Kconfig +0 -1

index 7a469acee33c..9ca52987fcd5 100644
@@ -15,7 +15,6 @@ config MIPS
 	select HAVE_BPF_JIT if !CPU_MICROMIPS
 	select ARCH_HAVE_CUSTOM_GPIO_H
 	select HAVE_FUNCTION_TRACER
-	select HAVE_FUNCTION_TRACE_MCOUNT_TEST
 	select HAVE_DYNAMIC_FTRACE
 	select HAVE_FTRACE_MCOUNT_RECORD
 	select HAVE_C_RECORDMCOUNT

arch/mips/kernel/mcount.S +0 -7

index 539b6294b613..00940d1d5c4f 100644
@@ -74,10 +74,6 @@ _mcount:
 #endif
 
 	/* When tracing is activated, it calls ftrace_caller+8 (aka here) */
-	lw	t1, function_trace_stop
-	bnez	t1, ftrace_stub
-	 nop
-
 	MCOUNT_SAVE_REGS
 #ifdef KBUILD_MCOUNT_RA_ADDRESS
 	PTR_S	MCOUNT_RA_ADDRESS_REG, PT_R12(sp)

@@ -105,9 +101,6 @@ ftrace_stub:
 #else	/* ! CONFIG_DYNAMIC_FTRACE */
 
 NESTED(_mcount, PT_SIZE, ra)
-	lw	t1, function_trace_stop
-	bnez	t1, ftrace_stub
-	 nop
 	PTR_LA	t1, ftrace_stub
 	PTR_L	t2, ftrace_trace_function /* Prepare t2 for (1) */
 	bne	t1, t2, static_trace

From: "Steven Rostedt (Red Hat)" <[email protected]>

Nothing sets function_trace_stop to disable function tracing anymore. Remove the check for it in the arch code.

[ Please test this on your arch ]

Signed-off-by Steven Rostedt <[email protected]>

---
 arch/mips/Kconfig         | 1 -
 arch/mips/kernel/mcount.S | 7 -------
 2 files changed, 8 deletions(-)

arch/mips/Kconfig +0 -1

index 7a469acee33c..9ca52987fcd5 100644
@@ -15,7 +15,6 @@ config MIPS
 	select HAVE_BPF_JIT if !CPU_MICROMIPS
 	select ARCH_HAVE_CUSTOM_GPIO_H
 	select HAVE_FUNCTION_TRACER
-	select HAVE_FUNCTION_TRACE_MCOUNT_TEST
 	select HAVE_DYNAMIC_FTRACE
 	select HAVE_FTRACE_MCOUNT_RECORD
 	select HAVE_C_RECORDMCOUNT

arch/mips/kernel/mcount.S +0 -7

index 539b6294b613..00940d1d5c4f 100644
@@ -74,10 +74,6 @@ _mcount:
 #endif
 
 	/* When tracing is activated, it calls ftrace_caller+8 (aka here) */
-	lw	t1, function_trace_stop
-	bnez	t1, ftrace_stub
-	 nop
-
 	MCOUNT_SAVE_REGS
 #ifdef KBUILD_MCOUNT_RA_ADDRESS
 	PTR_S	MCOUNT_RA_ADDRESS_REG, PT_R12(sp)

@@ -105,9 +101,6 @@ ftrace_stub:
 #else	/* ! CONFIG_DYNAMIC_FTRACE */
 
 NESTED(_mcount, PT_SIZE, ra)
-	lw	t1, function_trace_stop
-	bnez	t1, ftrace_stub
-	 nop
 	PTR_LA	t1, ftrace_stub
 	PTR_L	t2, ftrace_trace_function /* Prepare t2 for (1) */
 	bne	t1, t2, static_trace

From: "Steven Rostedt (Red Hat)" <[email protected]>

Nothing sets function_trace_stop to disable function tracing anymore. Remove the check for it in the arch code.

[ Please test this on your arch ]

Signed-off-by Steven Rostedt <[email protected]>

---
 arch/mips/Kconfig         | 1 -
 arch/mips/kernel/mcount.S | 7 -------
 2 files changed, 8 deletions(-)

arch/mips/Kconfig +0 -1

index 7a469acee33c..9ca52987fcd5 100644
@@ -15,7 +15,6 @@ config MIPS
 	select HAVE_BPF_JIT if !CPU_MICROMIPS
 	select ARCH_HAVE_CUSTOM_GPIO_H
 	select HAVE_FUNCTION_TRACER
-	select HAVE_FUNCTION_TRACE_MCOUNT_TEST
 	select HAVE_DYNAMIC_FTRACE
 	select HAVE_FTRACE_MCOUNT_RECORD
 	select HAVE_C_RECORDMCOUNT

arch/mips/kernel/mcount.S +0 -7

index 539b6294b613..00940d1d5c4f 100644
@@ -74,10 +74,6 @@ _mcount:
 #endif
 
 	/* When tracing is activated, it calls ftrace_caller+8 (aka here) */
-	lw	t1, function_trace_stop
-	bnez	t1, ftrace_stub
-	 nop
-
 	MCOUNT_SAVE_REGS
 #ifdef KBUILD_MCOUNT_RA_ADDRESS
 	PTR_S	MCOUNT_RA_ADDRESS_REG, PT_R12(sp)

@@ -105,9 +101,6 @@ ftrace_stub:
 #else	/* ! CONFIG_DYNAMIC_FTRACE */
 
 NESTED(_mcount, PT_SIZE, ra)
-	lw	t1, function_trace_stop
-	bnez	t1, ftrace_stub
-	 nop
 	PTR_LA	t1, ftrace_stub
 	PTR_L	t2, ftrace_trace_function /* Prepare t2 for (1) */
 	bne	t1, t2, static_trace

From: "Steven Rostedt (Red Hat)" <[email protected]>

Nothing sets function_trace_stop to disable function tracing anymore. Remove the check for it in the arch code.

[ Please test this on your arch ]

Signed-off-by Steven Rostedt <[email protected]>

---
 arch/mips/Kconfig         | 1 -
 arch/mips/kernel/mcount.S | 7 -------
 2 files changed, 8 deletions(-)

arch/mips/Kconfig +0 -1

index 7a469acee33c..9ca52987fcd5 100644
@@ -15,7 +15,6 @@ config MIPS
 	select HAVE_BPF_JIT if !CPU_MICROMIPS
 	select ARCH_HAVE_CUSTOM_GPIO_H
 	select HAVE_FUNCTION_TRACER
-	select HAVE_FUNCTION_TRACE_MCOUNT_TEST
 	select HAVE_DYNAMIC_FTRACE
 	select HAVE_FTRACE_MCOUNT_RECORD
 	select HAVE_C_RECORDMCOUNT

arch/mips/kernel/mcount.S +0 -7

index 539b6294b613..00940d1d5c4f 100644
@@ -74,10 +74,6 @@ _mcount:
 #endif
 
 	/* When tracing is activated, it calls ftrace_caller+8 (aka here) */
-	lw	t1, function_trace_stop
-	bnez	t1, ftrace_stub
-	 nop
-
 	MCOUNT_SAVE_REGS
 #ifdef KBUILD_MCOUNT_RA_ADDRESS
 	PTR_S	MCOUNT_RA_ADDRESS_REG, PT_R12(sp)

@@ -105,9 +101,6 @@ ftrace_stub:
 #else	/* ! CONFIG_DYNAMIC_FTRACE */
 
 NESTED(_mcount, PT_SIZE, ra)
-	lw	t1, function_trace_stop
-	bnez	t1, ftrace_stub
-	 nop
 	PTR_LA	t1, ftrace_stub
 	PTR_L	t2, ftrace_trace_function /* Prepare t2 for (1) */
 	bne	t1, t2, static_trace

From: "Steven Rostedt (Red Hat)" <[email protected]>

Nothing sets function_trace_stop to disable function tracing anymore. Remove the check for it in the arch code.

[ Please test this on your arch ]

Cc Michal Simek <[email protected]>

Signed-off-by Steven Rostedt <[email protected]>

---
 arch/microblaze/Kconfig         | 1 -
 arch/microblaze/kernel/mcount.S | 5 -----
 2 files changed, 6 deletions(-)

arch/microblaze/Kconfig +0 -1

index 9ae08541e30d..40e1c1dd0e24 100644
@@ -22,7 +22,6 @@ config MICROBLAZE
 	select HAVE_DYNAMIC_FTRACE
 	select HAVE_FTRACE_MCOUNT_RECORD
 	select HAVE_FUNCTION_GRAPH_TRACER
-	select HAVE_FUNCTION_TRACE_MCOUNT_TEST
 	select HAVE_FUNCTION_TRACER
 	select HAVE_MEMBLOCK
 	select HAVE_MEMBLOCK_NODE_MAP

arch/microblaze/kernel/mcount.S +0 -5

index fc1e1322ce4c..fed9da5de8c4 100644
@@ -91,11 +91,6 @@ ENTRY(ftrace_caller)
 #endif /* CONFIG_DYNAMIC_FTRACE */
 	SAVE_REGS
 	swi	r15, r1, 0;
-	/* MS: HAVE_FUNCTION_TRACE_MCOUNT_TEST begin of checking */
-	lwi	r5, r0, function_trace_stop;
-	bneid	r5, end;
-	nop;
-	/* MS: HAVE_FUNCTION_TRACE_MCOUNT_TEST end of checking */
 #ifdef CONFIG_FUNCTION_GRAPH_TRACER
 #ifndef CONFIG_DYNAMIC_FTRACE
 	lwi	r5, r0, ftrace_graph_return;

From: "Steven Rostedt (Red Hat)" <[email protected]>

Nothing sets function_trace_stop to disable function tracing anymore. Remove the check for it in the arch code.

[ Please test this on your arch ]

Cc Michal Simek <[email protected]>

Signed-off-by Steven Rostedt <[email protected]>

---
 arch/microblaze/Kconfig         | 1 -
 arch/microblaze/kernel/mcount.S | 5 -----
 2 files changed, 6 deletions(-)

arch/microblaze/Kconfig +0 -1

index 9ae08541e30d..40e1c1dd0e24 100644
@@ -22,7 +22,6 @@ config MICROBLAZE
 	select HAVE_DYNAMIC_FTRACE
 	select HAVE_FTRACE_MCOUNT_RECORD
 	select HAVE_FUNCTION_GRAPH_TRACER
-	select HAVE_FUNCTION_TRACE_MCOUNT_TEST
 	select HAVE_FUNCTION_TRACER
 	select HAVE_MEMBLOCK
 	select HAVE_MEMBLOCK_NODE_MAP

arch/microblaze/kernel/mcount.S +0 -5

index fc1e1322ce4c..fed9da5de8c4 100644
@@ -91,11 +91,6 @@ ENTRY(ftrace_caller)
 #endif /* CONFIG_DYNAMIC_FTRACE */
 	SAVE_REGS
 	swi	r15, r1, 0;
-	/* MS: HAVE_FUNCTION_TRACE_MCOUNT_TEST begin of checking */
-	lwi	r5, r0, function_trace_stop;
-	bneid	r5, end;
-	nop;
-	/* MS: HAVE_FUNCTION_TRACE_MCOUNT_TEST end of checking */
 #ifdef CONFIG_FUNCTION_GRAPH_TRACER
 #ifndef CONFIG_DYNAMIC_FTRACE
 	lwi	r5, r0, ftrace_graph_return;

From: "Steven Rostedt (Red Hat)" <[email protected]>

Nothing sets function_trace_stop to disable function tracing anymore. Remove the check for it in the arch code.

[ Please test this on your arch ]

Cc Michal Simek <[email protected]>

Signed-off-by Steven Rostedt <[email protected]>

---
 arch/microblaze/Kconfig         | 1 -
 arch/microblaze/kernel/mcount.S | 5 -----
 2 files changed, 6 deletions(-)

arch/microblaze/Kconfig +0 -1

index 9ae08541e30d..40e1c1dd0e24 100644
@@ -22,7 +22,6 @@ config MICROBLAZE
 	select HAVE_DYNAMIC_FTRACE
 	select HAVE_FTRACE_MCOUNT_RECORD
 	select HAVE_FUNCTION_GRAPH_TRACER
-	select HAVE_FUNCTION_TRACE_MCOUNT_TEST
 	select HAVE_FUNCTION_TRACER
 	select HAVE_MEMBLOCK
 	select HAVE_MEMBLOCK_NODE_MAP

arch/microblaze/kernel/mcount.S +0 -5

index fc1e1322ce4c..fed9da5de8c4 100644
@@ -91,11 +91,6 @@ ENTRY(ftrace_caller)
 #endif /* CONFIG_DYNAMIC_FTRACE */
 	SAVE_REGS
 	swi	r15, r1, 0;
-	/* MS: HAVE_FUNCTION_TRACE_MCOUNT_TEST begin of checking */
-	lwi	r5, r0, function_trace_stop;
-	bneid	r5, end;
-	nop;
-	/* MS: HAVE_FUNCTION_TRACE_MCOUNT_TEST end of checking */
 #ifdef CONFIG_FUNCTION_GRAPH_TRACER
 #ifndef CONFIG_DYNAMIC_FTRACE
 	lwi	r5, r0, ftrace_graph_return;

Michal,

Can you please test and ack this and patch 6 of this series. Otherwise I may have to stip it out of the series which will break the build on microblaze.

You can test my branch at:

git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace.git
rfc/remove-function-trace-stop

From: "Steven Rostedt (Red Hat)" <[email protected]>

Nothing sets function_trace_stop to disable function tracing anymore. Remove the check for it in the arch code.

[ Please test this on your arch ]

Cc James Hogan <[email protected]>

Signed-off-by Steven Rostedt <[email protected]>

---
 arch/metag/Kconfig              |  1 -
 arch/metag/kernel/ftrace_stub.S | 14 --------------
 2 files changed, 15 deletions(-)

arch/metag/Kconfig +0 -1

index 499b7610eaaf..0b389a81c43a 100644
@@ -13,7 +13,6 @@ config METAG
 	select HAVE_DYNAMIC_FTRACE
 	select HAVE_FTRACE_MCOUNT_RECORD
 	select HAVE_FUNCTION_TRACER
-	select HAVE_FUNCTION_TRACE_MCOUNT_TEST
 	select HAVE_KERNEL_BZIP2
 	select HAVE_KERNEL_GZIP
 	select HAVE_KERNEL_LZO

arch/metag/kernel/ftrace_stub.S +0 -14

index e70bff745bdd..3acc288217c0 100644
@@ -16,13 +16,6 @@ _mcount_wrapper:
 	.global _ftrace_caller
 	.type	_ftrace_caller,function
 _ftrace_caller:
-	MOVT    D0Re0,#HI(_function_trace_stop)
-	ADD	D0Re0,D0Re0,#LO(_function_trace_stop)
-	GETD	D0Re0,[D0Re0]
-	CMP	D0Re0,#0
-	BEQ	$Lcall_stub
-	MOV	PC,D0.4
-$Lcall_stub:
 	MSETL   [A0StP], D0Ar6, D0Ar4, D0Ar2, D0.4
 	MOV     D1Ar1, D0.4
 	MOV     D0Ar2, D1RtP

@@ -42,13 +35,6 @@ _ftrace_call:
 	.global	_mcount_wrapper
 	.type	_mcount_wrapper,function
 _mcount_wrapper:
-	MOVT    D0Re0,#HI(_function_trace_stop)
-	ADD	D0Re0,D0Re0,#LO(_function_trace_stop)
-	GETD	D0Re0,[D0Re0]
-	CMP	D0Re0,#0
-	BEQ	$Lcall_mcount
-	MOV	PC,D0.4
-$Lcall_mcount:
 	MSETL   [A0StP], D0Ar6, D0Ar4, D0Ar2, D0.4
 	MOV     D1Ar1, D0.4
 	MOV     D0Ar2, D1RtP

Quoted 6 lines · On Thu, 26 Jun 2014 12:52:44 -0400 Steven Rostedt <[email protected]> wrote:

From: "Steven Rostedt (Red Hat)" <[email protected]>

Nothing sets function_trace_stop to disable function tracing anymore. Remove the check for it in the arch code.

[ Please test this on your arch ]

From the cover letter, you were not Cc'd on.

Anyway, as there is no more reason to set function_trace_stop it is time to remove it. Unfortunately it's in several archs in assembly. Most of the assembly looks rather straight forward and I removed them myself. But I was only able to compile test them (for archs: arm64, metag, and microblaze I do not have my cross tools set up for them and did not even compile test it). But I would really love it if people can download their patch and test it out. You only need the patches that go against your arch and to really test it, also include the patch titled:

ftrace: Remove check for HAVE_FUNCTION_TRACE_MCOUNT_TEST

Otherwise your arch patch will call the list op that still does the check. That is, if you want to test suspend and resume on your arch.

As you may see, there are patches to the ftrace infrastructure that depend on the arch patches being implemented. I removed the functionality from the infrastructure, then removed it from the archs, and then finally removed the existence of the function_trace_stop variable, which would cause the archs to fail to compile if that were to go first.

If you can test your arch and give me your acked-by, I would appreciate it. Otherwise, if you need this to go through your tree, I would ask you to set up a dedicated branch that I can pull from to keep this order intact.

-- Steve

Quoted 52 lines

Cc James Hogan <[email protected]>

Signed-off-by Steven Rostedt <[email protected]>

---
 arch/metag/Kconfig              |  1 -
 arch/metag/kernel/ftrace_stub.S | 14 --------------
 2 files changed, 15 deletions(-)

arch/metag/Kconfig +0 -1

index 499b7610eaaf..0b389a81c43a 100644
@@ -13,7 +13,6 @@ config METAG
 	select HAVE_DYNAMIC_FTRACE
 	select HAVE_FTRACE_MCOUNT_RECORD
 	select HAVE_FUNCTION_TRACER
-	select HAVE_FUNCTION_TRACE_MCOUNT_TEST
 	select HAVE_KERNEL_BZIP2
 	select HAVE_KERNEL_GZIP
 	select HAVE_KERNEL_LZO

arch/metag/kernel/ftrace_stub.S +0 -14

index e70bff745bdd..3acc288217c0 100644
@@ -16,13 +16,6 @@ _mcount_wrapper:
 	.global _ftrace_caller
 	.type	_ftrace_caller,function
 _ftrace_caller:
-	MOVT    D0Re0,#HI(_function_trace_stop)
-	ADD	D0Re0,D0Re0,#LO(_function_trace_stop)
-	GETD	D0Re0,[D0Re0]
-	CMP	D0Re0,#0
-	BEQ	$Lcall_stub
-	MOV	PC,D0.4
-$Lcall_stub:
 	MSETL   [A0StP], D0Ar6, D0Ar4, D0Ar2, D0.4
 	MOV     D1Ar1, D0.4
 	MOV     D0Ar2, D1RtP

@@ -42,13 +35,6 @@ _ftrace_call:
 	.global	_mcount_wrapper
 	.type	_mcount_wrapper,function
 _mcount_wrapper:
-	MOVT    D0Re0,#HI(_function_trace_stop)
-	ADD	D0Re0,D0Re0,#LO(_function_trace_stop)
-	GETD	D0Re0,[D0Re0]
-	CMP	D0Re0,#0
-	BEQ	$Lcall_mcount
-	MOV	PC,D0.4
-$Lcall_mcount:
 	MSETL   [A0StP], D0Ar6, D0Ar4, D0Ar2, D0.4
 	MOV     D1Ar1, D0.4
 	MOV     D0Ar2, D1RtP

This patch is pretty much the inverse of the original patch that added it, so looks good to me (and it didn't seem to break anything).

So, for both this patch and the "ftrace: Remove check for HAVE_FUNCTION_TRACE_MCOUNT_TEST" one:

Acked-by James Hogan <[email protected]>

From: "Steven Rostedt (Red Hat)" <[email protected]>

Nothing sets function_trace_stop to disable function tracing anymore. Remove the check for it in the arch code.

[ Please test this on your arch ]

Cc Mike Frysinger <[email protected]>

Signed-off-by Steven Rostedt <[email protected]>

---
 arch/blackfin/Kconfig               |  1 -
 arch/blackfin/kernel/ftrace-entry.S | 18 ------------------
 2 files changed, 19 deletions(-)

arch/blackfin/Kconfig +0 -1

index f81e7b989fff..ed30699cc635 100644
@@ -18,7 +18,6 @@ config BLACKFIN
 	select HAVE_FTRACE_MCOUNT_RECORD
 	select HAVE_FUNCTION_GRAPH_TRACER
 	select HAVE_FUNCTION_TRACER
-	select HAVE_FUNCTION_TRACE_MCOUNT_TEST
 	select HAVE_IDE
 	select HAVE_KERNEL_GZIP if RAMKERNEL
 	select HAVE_KERNEL_BZIP2 if RAMKERNEL

arch/blackfin/kernel/ftrace-entry.S +0 -18

index 7eed00bbd26d..28d059540424 100644
@@ -33,15 +33,6 @@ ENDPROC(__mcount)
  * function will be waiting there.  mmmm pie.
  */
 ENTRY(_ftrace_caller)
-# ifdef CONFIG_HAVE_FUNCTION_TRACE_MCOUNT_TEST
-	/* optional micro optimization: return if stopped */
-	p1.l = _function_trace_stop;
-	p1.h = _function_trace_stop;
-	r3 = [p1];
-	cc = r3 == 0;
-	if ! cc jump _ftrace_stub (bp);
-# endif
-
 	/* save first/second/third function arg and the return register */
 	[--sp] = r2;
 	[--sp] = r0;

@@ -83,15 +74,6 @@ ENDPROC(_ftrace_caller)
 
 /* See documentation for _ftrace_caller */
 ENTRY(__mcount)
-# ifdef CONFIG_HAVE_FUNCTION_TRACE_MCOUNT_TEST
-	/* optional micro optimization: return if stopped */
-	p1.l = _function_trace_stop;
-	p1.h = _function_trace_stop;
-	r3 = [p1];
-	cc = r3 == 0;
-	if ! cc jump _ftrace_stub (bp);
-# endif
-
 	/* save third function arg early so we can do testing below */
 	[--sp] = r2;
 

From: "Steven Rostedt (Red Hat)" <[email protected]>

Nothing sets function_trace_stop to disable function tracing anymore. Remove the check for it in the arch code.

[ Please test this on your arch ]

Quoted 5 lines · On Thu 26 Jun 2014 12:52:45 Steven Rostedt wrote:

From: "Steven Rostedt (Red Hat)" <[email protected]>

Nothing sets function_trace_stop to disable function tracing anymore. Remove the check for it in the arch code.

Quoted 1 line

[ Please test this on your arch ]

i haven't tested, but the asm looks correct -- it's only using scratch registers, and the rest of the code doesn't rely on the state in that ifdef (by design).

Acked-by Mike Frysinger <[email protected]>

-mike

From: "Steven Rostedt (Red Hat)" <[email protected]>

Nothing sets function_trace_stop to disable function tracing anymore. Remove the check for it in the arch code.

arm64 was broken anyway, as it had an ifdef testing
CONFIG_HAVE_FUNCTION_TRACE_MCOUNT_TEST which is only set if
the arch supports the code (which it obviously did not), and it was testing a non existent ftrace_trace_stop instead of function_trace_stop.

Cc AKASHI Takahiro <[email protected]>

Cc Will Deacon <[email protected]>

Signed-off-by Steven Rostedt <[email protected]>

---
 arch/arm64/kernel/entry-ftrace.S | 5 -----
 1 file changed, 5 deletions(-)

arch/arm64/kernel/entry-ftrace.S +0 -5

index b051871f2965..1b1e330e6849 100644
@@ -96,11 +96,6 @@
  *     - ftrace_graph_caller to set up an exit hook
  */
 ENTRY(_mcount)
-#ifdef CONFIG_HAVE_FUNCTION_TRACE_MCOUNT_TEST
-	ldr	x0, =ftrace_trace_stop
-	ldr	x0, [x0]		// if ftrace_trace_stop
-	ret				//   return;
-#endif
 	mcount_enter
 
 	ldr	x0, =ftrace_trace_function

From: "Steven Rostedt (Red Hat)" <[email protected]>

Nothing sets function_trace_stop to disable function tracing anymore. Remove the check for it in the arch code.

arm64 was broken anyway, as it had an ifdef testing
CONFIG_HAVE_FUNCTION_TRACE_MCOUNT_TEST which is only set if
the arch supports the code (which it obviously did not), and it was testing a non existent ftrace_trace_stop instead of function_trace_stop.

Cc AKASHI Takahiro <[email protected]>

Cc Will Deacon <[email protected]>

Signed-off-by Steven Rostedt <[email protected]>

---
 arch/arm64/kernel/entry-ftrace.S | 5 -----
 1 file changed, 5 deletions(-)

arch/arm64/kernel/entry-ftrace.S +0 -5

index b051871f2965..1b1e330e6849 100644
@@ -96,11 +96,6 @@
  *     - ftrace_graph_caller to set up an exit hook
  */
 ENTRY(_mcount)
-#ifdef CONFIG_HAVE_FUNCTION_TRACE_MCOUNT_TEST
-	ldr	x0, =ftrace_trace_stop
-	ldr	x0, [x0]		// if ftrace_trace_stop
-	ret				//   return;
-#endif
 	mcount_enter
 
 	ldr	x0, =ftrace_trace_function

Signature · 1 line

2.0.0

Hi Steve,

Quoted 5 lines · Steven Rostedt

From: "Steven Rostedt (Red Hat)" <[email protected]>

Nothing sets function_trace_stop to disable function tracing anymore. Remove the check for it in the arch code.

Quoted 5 lines

arm64 was broken anyway, as it had an ifdef testing
CONFIG_HAVE_FUNCTION_TRACE_MCOUNT_TEST which is only set if
the arch supports the code (which it obviously did not), and it was testing a non existent ftrace_trace_stop instead of function_trace_stop.

Yeah, the current code is total crap. Well spotted.

Quoted 7 lines

Cc AKASHI Takahiro <[email protected]>

Cc Will Deacon <[email protected]>

Signed-off-by Steven Rostedt <[email protected]>

---
  arch/arm64/kernel/entry-ftrace.S | 5 -----
  1 file changed, 5 deletions(-)

Acked-by Will Deacon <[email protected]>

I'm happy for you to take this via the ftrace tree, along with the rest of the series.

I successfully ran the ftrace self tests with this applied (on top of "ftrace: Remove check for HAVE_FUNCTION_TRACE_MCOUNT_TEST"), but note that I don't have a platform with working suspend/resume right now.

Cheers,

Will

Quoted 21 lines

arch/arm64/kernel/entry-ftrace.S +0 -6

index b051871f2965..1b1e330e6849 100644
@@ -96,11 +96,6 @@
   *     - ftrace_graph_caller to set up an exit hook
   */
  ENTRY(_mcount)
-#ifdef CONFIG_HAVE_FUNCTION_TRACE_MCOUNT_TEST
-	ldr	x0, =ftrace_trace_stop
-	ldr	x0, [x0]		// if ftrace_trace_stop
-	ret				//   return;
-#endif
  	mcount_enter
 
  	ldr	x0, =ftrace_trace_function
--

2.0.0

From: Heiko Carstens <[email protected]>

Remove check of obsolete variable function_trace_stop as requested by Steven Rostedt.

Signed-off-by Heiko Carstens <[email protected]>

Signed-off-by Steven Rostedt <[email protected]>

---
 arch/s390/Kconfig           |  1 -
 arch/s390/kernel/mcount.S   | 10 +++-------
 arch/s390/kernel/mcount64.S |  3 ---
 3 files changed, 3 insertions(+), 11 deletions(-)

arch/s390/Kconfig +0 -1

index bb63499fc5d3..f5af5f6ef0f4 100644
@@ -116,7 +116,6 @@ config S390
 	select HAVE_FTRACE_MCOUNT_RECORD
 	select HAVE_FUNCTION_GRAPH_TRACER
 	select HAVE_FUNCTION_TRACER
-	select HAVE_FUNCTION_TRACE_MCOUNT_TEST
 	select HAVE_FUTEX_CMPXCHG if FUTEX
 	select HAVE_KERNEL_BZIP2
 	select HAVE_KERNEL_GZIP

arch/s390/kernel/mcount.S +3 -7

index 08dcf21cb8df..433c6dbfa442 100644
@@ -21,13 +21,9 @@ ENTRY(_mcount)
 ENTRY(ftrace_caller)
 #endif
 	stm	%r2,%r5,16(%r15)
-	bras	%r1,2f
+	bras	%r1,1f
 0:	.long	ftrace_trace_function
-1:	.long	function_trace_stop
-2:	l	%r2,1b-0b(%r1)
-	icm	%r2,0xf,0(%r2)
-	jnz	3f
-	st	%r14,56(%r15)
+1:	st	%r14,56(%r15)
 	lr	%r0,%r15
 	ahi	%r15,-96
 	l	%r3,100(%r15)

@@ -50,7 +46,7 @@ ENTRY(ftrace_graph_caller)
 #endif
 	ahi	%r15,96
 	l	%r14,56(%r15)
-3:	lm	%r2,%r5,16(%r15)
+	lm	%r2,%r5,16(%r15)
 	br	%r14
 
 #ifdef CONFIG_FUNCTION_GRAPH_TRACER

arch/s390/kernel/mcount64.S +0 -3

index 1c52eae3396a..c67a8bf0fd9a 100644
@@ -20,9 +20,6 @@ ENTRY(_mcount)
 
 ENTRY(ftrace_caller)
 #endif
-	larl	%r1,function_trace_stop
-	icm	%r1,0xf,0(%r1)
-	bnzr	%r14
 	stmg	%r2,%r5,32(%r15)
 	stg	%r14,112(%r15)
 	lgr	%r1,%r15

I know you wrote this patch, but you may want to test it out with the ftrace: Remove check for HAVE_FUNCTION_TRACE_MCOUNT_TEST patch applied too.

* Request for Acks *

Quoted 1 line

From the time I created the use of function_trace_stop, I hated it.

There was two reasons to create this, one was to try to lower the function tracing overhead when debugfs file tracing_enable was set to zero (note, tracing_enable no longer exists), the other was a way to stop function tracing when going down into suspend and resume.

Some function was causing suspend and resume to reboot the kernel. In debugging this I found that an empty callback from mcount would work. That is, instead of running the tracing code, I would just have the function trace callback be a nop:

void function_trace_call(unsigned long ip, unsigned long parent_ip) {
}

This worked. That means the code in mcount wasn't an issue. I started bisecting the contents of the function_trace_call and found that this would cause the system to reboot!

void function_trace_call(unsigned long ip, unsigned long parent_ip)

 {
	smp_processor_id();

}

That is, just calling smp_processor_id() was enough to trigger a triple fault on resume of the system.

Today, this big hammer approach of disabling the function tracer is starting to show its issues. It can't help out in debugging suspend and resume, and there's other function trace callbacks that should still work.

I also have learned ways to bisect functions that cause bugs in function tracing. I finally got some time to do so with a box that would reboot on suspend and resume. This led me down to a single function:

restore_processor_state()

This made perfect sense, as this function is called from assembly on a CPU startup. One of the jobs of this function was to set up the GS register. That register is also the register that is used by smp_processor_id() to find what CPU the task is running on. With it not set up it will offset into some random location and fault. As the fault handlers can also be traced, those will fault too and finally the system will reset due to a triple fault.

Uh, from the same reason, I must list it in the kprobe blacklist too...

BTW, as far as I can review, x86 and generic parts of the series seems OK to me. :)

Quoted 1 line · On Mon, 30 Jun 2014 12:13:08 +0900 Masami Hiramatsu…

Uh, from the same reason, I must list it in the kprobe blacklist too...

Ah yes!

Quoted 4 lines

BTW, as far as I can review, x86 and generic parts of the series seems OK to me. :)

Can you post a Acked-by or Reviewed-by then?