All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC PATCH 0/4] ftrace: Add use of -mfentry for x86_64
@ 2012-08-07 19:38 Steven Rostedt
  2012-08-07 19:38 ` [RFC PATCH 1/4] ftrace: Make recordmcount.c handle __fentry__ Steven Rostedt
                   ` (4 more replies)
  0 siblings, 5 replies; 25+ messages in thread
From: Steven Rostedt @ 2012-08-07 19:38 UTC (permalink / raw)
  To: linux-kernel
  Cc: Ingo Molnar, Andrew Morton, Thomas Gleixner, Frederic Weisbecker,
	Masami Hiramatsu, Linus Torvalds, Andi Kleen

[-- Attachment #1: Type: text/plain, Size: 1764 bytes --]


This is an RFC patch set that makes gcc use the -mfentry option with
-pg. This will set the ftrace 'hook' to the beginning of the function
and also remove the requirement that -pg enables frame pointers.

This has a couple of benefits (and probably more).

1) removal of the frame pointer requirement makes for smaller and faster code.

2) Having the function trace hook at the beginning of the function instead
 of after the frame is set up, gives the function tracing callbacks access
 to the parameters. This means that kprobes can take advantage of this.
 When a kprobe is set on top of a ftrace hook (nop), it will automatically
 use the function tracing callback. This makes it into an 'optimized' probe
 as there's no need to hit a breakpoint and trigger the probe that way.
 The function tracing code can do the work for it. Note, optimized probes
 are only allowed with !PREEMPT, but a ftrace optimize probe is allowed
 in any context (another benefit).

This only implements fentry for x86_64.

Steven Rostedt (4):
      ftrace: Make recordmcount.c handle __fentry__
      ftrace: Add -mfentry to Makefile on function tracer
      ftrace: Do not test frame pointers if -mfentry is used
      ftrace/x86: Add support for -mfentry to x86_64

----
 Makefile                             |    6 +++++-
 arch/x86/Kconfig                     |    1 +
 arch/x86/include/asm/ftrace.h        |    7 ++++++-
 arch/x86/kernel/entry_64.S           |   18 +++++++++++++++++-
 arch/x86/kernel/x8664_ksyms_64.c     |    6 +++++-
 kernel/trace/Kconfig                 |    5 +++++
 kernel/trace/trace_functions_graph.c |    5 ++++-
 scripts/recordmcount.h               |    4 +++-
 8 files changed, 46 insertions(+), 6 deletions(-)

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [RFC PATCH 1/4] ftrace: Make recordmcount.c handle __fentry__
  2012-08-07 19:38 [RFC PATCH 0/4] ftrace: Add use of -mfentry for x86_64 Steven Rostedt
@ 2012-08-07 19:38 ` Steven Rostedt
  2012-08-07 23:57   ` John Reiser
  2012-08-27 17:03   ` [tip:perf/core] " tip-bot for Steven Rostedt
  2012-08-07 19:38 ` [RFC PATCH 2/4] ftrace: Add -mfentry to Makefile on function tracer Steven Rostedt
                   ` (3 subsequent siblings)
  4 siblings, 2 replies; 25+ messages in thread
From: Steven Rostedt @ 2012-08-07 19:38 UTC (permalink / raw)
  To: linux-kernel
  Cc: Ingo Molnar, Andrew Morton, Thomas Gleixner, Frederic Weisbecker,
	Masami Hiramatsu, Linus Torvalds, Andi Kleen, John Reiser

[-- Attachment #1: Type: text/plain, Size: 1249 bytes --]

From: Steven Rostedt <srostedt@redhat.com>

With gcc 4.6.0 the -mfentry feature places the function profiling
call at the start of the function. When this is used, the call is
to __fentry__ and not mcount.

Change recordmcount.c to record both callers to __fentry__ and
mcount.

Cc: John Reiser <jreiser@bitwagon.com>
Cc: Andi Kleen <andi@firstfloor.org>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 scripts/recordmcount.h |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/scripts/recordmcount.h b/scripts/recordmcount.h
index 54e35c1..9d1421e 100644
--- a/scripts/recordmcount.h
+++ b/scripts/recordmcount.h
@@ -261,11 +261,13 @@ static unsigned get_mcountsym(Elf_Sym const *const sym0,
 		&sym0[Elf_r_sym(relp)];
 	char const *symname = &str0[w(symp->st_name)];
 	char const *mcount = gpfx == '_' ? "_mcount" : "mcount";
+	char const *fentry = "__fentry__";
 
 	if (symname[0] == '.')
 		++symname;  /* ppc64 hack */
 	if (strcmp(mcount, symname) == 0 ||
-	    (altmcount && strcmp(altmcount, symname) == 0))
+	    (altmcount && strcmp(altmcount, symname) == 0) ||
+	    (strcmp(fentry, symname) == 0))
 		mcountsym = Elf_r_sym(relp);
 
 	return mcountsym;
-- 
1.7.10.4



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [RFC PATCH 2/4] ftrace: Add -mfentry to Makefile on function tracer
  2012-08-07 19:38 [RFC PATCH 0/4] ftrace: Add use of -mfentry for x86_64 Steven Rostedt
  2012-08-07 19:38 ` [RFC PATCH 1/4] ftrace: Make recordmcount.c handle __fentry__ Steven Rostedt
@ 2012-08-07 19:38 ` Steven Rostedt
  2012-08-27 17:04   ` [tip:perf/core] " tip-bot for Steven Rostedt
  2012-08-07 19:38 ` [RFC PATCH 3/4] ftrace: Do not test frame pointers if -mfentry is used Steven Rostedt
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 25+ messages in thread
From: Steven Rostedt @ 2012-08-07 19:38 UTC (permalink / raw)
  To: linux-kernel
  Cc: Ingo Molnar, Andrew Morton, Thomas Gleixner, Frederic Weisbecker,
	Masami Hiramatsu, Linus Torvalds, Andi Kleen, Michal Marek

[-- Attachment #1: Type: text/plain, Size: 1914 bytes --]

From: Steven Rostedt <srostedt@redhat.com>

Thanks to Andi Kleen, gcc 4.6.0 now supports -mfentry for x86
(and hopefully soon for other archs). What this does is to have
the function profiler start at the beginning of the function
instead of after the stack is set up. As plain -pg (mcount) is
called after the stack is set up, and in some cases can have issues
with the function graph tracer. It also requires frame pointers to
be enabled.

The -mfentry now calls __fentry__ at the beginning of the function.
This allows for compiling without frame pointers and even has the
ability to access parameters if needed.

If the architecture and the compiler both support -mfentry then
use that instead.

Cc: Michal Marek <mmarek@suse.cz>
Cc: Andi Kleen <andi@firstfloor.org>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 Makefile             |    6 +++++-
 kernel/trace/Kconfig |    5 +++++
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index aa8e315..eeb634f 100644
--- a/Makefile
+++ b/Makefile
@@ -609,7 +609,11 @@ KBUILD_CFLAGS 	+= $(call cc-option, -femit-struct-debug-baseonly)
 endif
 
 ifdef CONFIG_FUNCTION_TRACER
-KBUILD_CFLAGS	+= -pg
+ifdef CONFIG_HAVE_FENTRY
+CC_USING_FENTRY	:= $(call cc-option, -mfentry -DCC_USING_FENTRY)
+endif
+KBUILD_CFLAGS	+= -pg $(CC_USING_FENTRY)
+KBUILD_AFLAGS	+= $(CC_USING_FENTRY)
 ifdef CONFIG_DYNAMIC_FTRACE
 	ifdef CONFIG_HAVE_C_RECORDMCOUNT
 		BUILD_C_RECORDMCOUNT := y
diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
index 8c4c070..9301a0e 100644
--- a/kernel/trace/Kconfig
+++ b/kernel/trace/Kconfig
@@ -49,6 +49,11 @@ config HAVE_SYSCALL_TRACEPOINTS
 	help
 	  See Documentation/trace/ftrace-design.txt
 
+config HAVE_FENTRY
+	bool
+	help
+	  Arch supports the gcc options -pg with -mfentry
+
 config HAVE_C_RECORDMCOUNT
 	bool
 	help
-- 
1.7.10.4



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [RFC PATCH 3/4] ftrace: Do not test frame pointers if -mfentry is used
  2012-08-07 19:38 [RFC PATCH 0/4] ftrace: Add use of -mfentry for x86_64 Steven Rostedt
  2012-08-07 19:38 ` [RFC PATCH 1/4] ftrace: Make recordmcount.c handle __fentry__ Steven Rostedt
  2012-08-07 19:38 ` [RFC PATCH 2/4] ftrace: Add -mfentry to Makefile on function tracer Steven Rostedt
@ 2012-08-07 19:38 ` Steven Rostedt
  2012-08-08  4:34   ` Masami Hiramatsu
  2012-08-27 17:05   ` [tip:perf/core] " tip-bot for Steven Rostedt
  2012-08-07 19:38 ` [RFC PATCH 4/4] ftrace/x86: Add support for -mfentry to x86_64 Steven Rostedt
  2012-08-07 20:23 ` [RFC PATCH 0/4] ftrace: Add use of -mfentry for x86_64 H. Peter Anvin
  4 siblings, 2 replies; 25+ messages in thread
From: Steven Rostedt @ 2012-08-07 19:38 UTC (permalink / raw)
  To: linux-kernel
  Cc: Ingo Molnar, Andrew Morton, Thomas Gleixner, Frederic Weisbecker,
	Masami Hiramatsu, Linus Torvalds, Andi Kleen

[-- Attachment #1: Type: text/plain, Size: 1511 bytes --]

From: Steven Rostedt <srostedt@redhat.com>

The function graph has a test to check if the frame pointer is
corrupted, which can happen with various options of gcc with mcount.
But this is not an issue with -mfentry as -mfentry does not need nor use
frame pointers for function graph tracing.

Cc: Andi Kleen <andi@firstfloor.org>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 kernel/trace/trace_functions_graph.c |    5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/kernel/trace/trace_functions_graph.c b/kernel/trace/trace_functions_graph.c
index ce27c8b..99b4378 100644
--- a/kernel/trace/trace_functions_graph.c
+++ b/kernel/trace/trace_functions_graph.c
@@ -143,7 +143,7 @@ ftrace_pop_return_trace(struct ftrace_graph_ret *trace, unsigned long *ret,
 		return;
 	}
 
-#ifdef CONFIG_HAVE_FUNCTION_GRAPH_FP_TEST
+#if defined(CONFIG_HAVE_FUNCTION_GRAPH_FP_TEST) && !defined(CC_USING_FENTRY)
 	/*
 	 * The arch may choose to record the frame pointer used
 	 * and check it here to make sure that it is what we expect it
@@ -154,6 +154,9 @@ ftrace_pop_return_trace(struct ftrace_graph_ret *trace, unsigned long *ret,
 	 *
 	 * Currently, x86_32 with optimize for size (-Os) makes the latest
 	 * gcc do the above.
+	 *
+	 * Note, -mfentry does not use frame pointers, and this test
+	 *  is not needed if CC_USING_FENTRY is set.
 	 */
 	if (unlikely(current->ret_stack[index].fp != frame_pointer)) {
 		ftrace_graph_stop();
-- 
1.7.10.4



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [RFC PATCH 4/4] ftrace/x86: Add support for -mfentry to x86_64
  2012-08-07 19:38 [RFC PATCH 0/4] ftrace: Add use of -mfentry for x86_64 Steven Rostedt
                   ` (2 preceding siblings ...)
  2012-08-07 19:38 ` [RFC PATCH 3/4] ftrace: Do not test frame pointers if -mfentry is used Steven Rostedt
@ 2012-08-07 19:38 ` Steven Rostedt
  2012-08-09  8:34   ` Masami Hiramatsu
                     ` (2 more replies)
  2012-08-07 20:23 ` [RFC PATCH 0/4] ftrace: Add use of -mfentry for x86_64 H. Peter Anvin
  4 siblings, 3 replies; 25+ messages in thread
From: Steven Rostedt @ 2012-08-07 19:38 UTC (permalink / raw)
  To: linux-kernel
  Cc: Ingo Molnar, Andrew Morton, Thomas Gleixner, Frederic Weisbecker,
	Masami Hiramatsu, Linus Torvalds, Andi Kleen

[-- Attachment #1: Type: text/plain, Size: 4751 bytes --]

From: Steven Rostedt <srostedt@redhat.com>

If the kernel is compiled with gcc 4.6.0 which supports -mfentry,
then use that instead of mcount.

With mcount, frame pointers are forced with the -pg option and we
get something like:

<can_vma_merge_before>:
       55                      push   %rbp
       48 89 e5                mov    %rsp,%rbp
       53                      push   %rbx
       41 51                   push   %r9
       e8 fe 6a 39 00          callq  ffffffff81483d00 <mcount>
       31 c0                   xor    %eax,%eax
       48 89 fb                mov    %rdi,%rbx
       48 89 d7                mov    %rdx,%rdi
       48 33 73 30             xor    0x30(%rbx),%rsi
       48 f7 c6 ff ff ff f7    test   $0xfffffffff7ffffff,%rsi

With -mfentry, frame pointers are no longer forced and the call looks
like this:

<can_vma_merge_before>:
       e8 33 af 37 00          callq  ffffffff81461b40 <__fentry__>
       53                      push   %rbx
       48 89 fb                mov    %rdi,%rbx
       31 c0                   xor    %eax,%eax
       48 89 d7                mov    %rdx,%rdi
       41 51                   push   %r9
       48 33 73 30             xor    0x30(%rbx),%rsi
       48 f7 c6 ff ff ff f7    test   $0xfffffffff7ffffff,%rsi

This adds the ftrace hook at the beginning of the function before a
frame is set up, and allows the function callbacks to be able to access
parameters. As kprobes now can use function tracing (at least on x86)
this speeds up the kprobe hooks that are at the beginning of the
function.

Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Andi Kleen <andi@firstfloor.org>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 arch/x86/Kconfig                 |    1 +
 arch/x86/include/asm/ftrace.h    |    7 ++++++-
 arch/x86/kernel/entry_64.S       |   18 +++++++++++++++++-
 arch/x86/kernel/x8664_ksyms_64.c |    6 +++++-
 4 files changed, 29 insertions(+), 3 deletions(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index c70684f..bbbf5d8 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -36,6 +36,7 @@ config X86
 	select HAVE_KRETPROBES
 	select HAVE_OPTPROBES
 	select HAVE_FTRACE_MCOUNT_RECORD
+	select HAVE_FENTRY if X86_64
 	select HAVE_C_RECORDMCOUNT
 	select HAVE_DYNAMIC_FTRACE
 	select HAVE_FUNCTION_TRACER
diff --git a/arch/x86/include/asm/ftrace.h b/arch/x86/include/asm/ftrace.h
index a6cae0c..9a25b52 100644
--- a/arch/x86/include/asm/ftrace.h
+++ b/arch/x86/include/asm/ftrace.h
@@ -35,7 +35,11 @@
 #endif
 
 #ifdef CONFIG_FUNCTION_TRACER
-#define MCOUNT_ADDR		((long)(mcount))
+#ifdef CC_USING_FENTRY
+# define MCOUNT_ADDR		((long)(__fentry__))
+#else
+# define MCOUNT_ADDR		((long)(mcount))
+#endif
 #define MCOUNT_INSN_SIZE	5 /* sizeof mcount call */
 
 #ifdef CONFIG_DYNAMIC_FTRACE
@@ -46,6 +50,7 @@
 #ifndef __ASSEMBLY__
 extern void mcount(void);
 extern atomic_t modifying_ftrace_code;
+extern void __fentry__(void);
 
 static inline unsigned long ftrace_call_adjust(unsigned long addr)
 {
diff --git a/arch/x86/kernel/entry_64.S b/arch/x86/kernel/entry_64.S
index 38308fa..2add3bb 100644
--- a/arch/x86/kernel/entry_64.S
+++ b/arch/x86/kernel/entry_64.S
@@ -69,9 +69,16 @@
 
 #ifdef CONFIG_FUNCTION_TRACER
 #ifdef CONFIG_DYNAMIC_FTRACE
+
+#ifdef CC_USING_FENTRY
+ENTRY(__fentry__)
+	retq
+END(__fentry__)
+#else
 ENTRY(mcount)
 	retq
 END(mcount)
+#endif
 
 /* skip is set if stack has been adjusted */
 .macro ftrace_caller_setup skip=0
@@ -84,7 +91,11 @@ END(mcount)
 	movq RIP(%rsp), %rdi
 	subq $MCOUNT_INSN_SIZE, %rdi
 	/* Load the parent_ip into the second parameter */
+#ifdef CC_USING_FENTRY
+	movq SS+16(%rsp), %rsi
+#else
 	movq 8(%rbp), %rsi
+#endif
 .endm
 
 ENTRY(ftrace_caller)
@@ -215,9 +226,14 @@ END(mcount)
 ENTRY(ftrace_graph_caller)
 	MCOUNT_SAVE_FRAME
 
+#ifdef CC_USING_FENTRY
+	leaq SS+16(%rsp), %rdi
+	movq $0, %rdx	/* No framepointers needed */
+#else
 	leaq 8(%rbp), %rdi
-	movq RIP(%rsp), %rsi
 	movq (%rbp), %rdx
+#endif
+	movq RIP(%rsp), %rsi
 	subq $MCOUNT_INSN_SIZE, %rsi
 
 	call	prepare_ftrace_return
diff --git a/arch/x86/kernel/x8664_ksyms_64.c b/arch/x86/kernel/x8664_ksyms_64.c
index 9796c2f..643b236 100644
--- a/arch/x86/kernel/x8664_ksyms_64.c
+++ b/arch/x86/kernel/x8664_ksyms_64.c
@@ -13,9 +13,13 @@
 #include <asm/ftrace.h>
 
 #ifdef CONFIG_FUNCTION_TRACER
-/* mcount is defined in assembly */
+/* mcount and __fentry__ are defined in assembly */
+#ifdef CC_USING_FENTRY
+EXPORT_SYMBOL(__fentry__);
+#else
 EXPORT_SYMBOL(mcount);
 #endif
+#endif
 
 EXPORT_SYMBOL(__get_user_1);
 EXPORT_SYMBOL(__get_user_2);
-- 
1.7.10.4



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* Re: [RFC PATCH 0/4] ftrace: Add use of -mfentry for x86_64
  2012-08-07 19:38 [RFC PATCH 0/4] ftrace: Add use of -mfentry for x86_64 Steven Rostedt
                   ` (3 preceding siblings ...)
  2012-08-07 19:38 ` [RFC PATCH 4/4] ftrace/x86: Add support for -mfentry to x86_64 Steven Rostedt
@ 2012-08-07 20:23 ` H. Peter Anvin
  2012-08-13  8:42   ` Ingo Molnar
  4 siblings, 1 reply; 25+ messages in thread
From: H. Peter Anvin @ 2012-08-07 20:23 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: linux-kernel, Ingo Molnar, Andrew Morton, Thomas Gleixner,
	Frederic Weisbecker, Masami Hiramatsu, Linus Torvalds,
	Andi Kleen

On 08/07/2012 12:38 PM, Steven Rostedt wrote:
>
> This is an RFC patch set that makes gcc use the -mfentry option with
> -pg. This will set the ftrace 'hook' to the beginning of the function
> and also remove the requirement that -pg enables frame pointers.
>
> This has a couple of benefits (and probably more).
>
> 1) removal of the frame pointer requirement makes for smaller and faster code.
>
> 2) Having the function trace hook at the beginning of the function instead
>   of after the frame is set up, gives the function tracing callbacks access
>   to the parameters. This means that kprobes can take advantage of this.
>   When a kprobe is set on top of a ftrace hook (nop), it will automatically
>   use the function tracing callback. This makes it into an 'optimized' probe
>   as there's no need to hit a breakpoint and trigger the probe that way.
>   The function tracing code can do the work for it. Note, optimized probes
>   are only allowed with !PREEMPT, but a ftrace optimize probe is allowed
>   in any context (another benefit).
>
> This only implements fentry for x86_64.
>

Acked-by: H. Peter Anvin <hpa@linux.intel.com>

	-hpa


-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [RFC PATCH 1/4] ftrace: Make recordmcount.c handle __fentry__
  2012-08-07 19:38 ` [RFC PATCH 1/4] ftrace: Make recordmcount.c handle __fentry__ Steven Rostedt
@ 2012-08-07 23:57   ` John Reiser
  2012-08-08  0:05     ` Steven Rostedt
  2012-08-27 17:03   ` [tip:perf/core] " tip-bot for Steven Rostedt
  1 sibling, 1 reply; 25+ messages in thread
From: John Reiser @ 2012-08-07 23:57 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: linux-kernel, Ingo Molnar, Andrew Morton, Thomas Gleixner,
	Frederic Weisbecker, Masami Hiramatsu, Linus Torvalds,
	Andi Kleen

On 08/07/2012 12:38 PM, Steven Rostedt wrote:
> From: Steven Rostedt <srostedt@redhat.com>
> 
> With gcc 4.6.0 the -mfentry feature places the function profiling call at the start of the function. When this is used, the call is to __fentry__ and not mcount.
> 
> Change recordmcount.c to record both callers to __fentry__ and mcount.
   [snip]
> -	    (altmcount && strcmp(altmcount, symname) == 0))
> +	    (altmcount && strcmp(altmcount, symname) == 0) ||
> +	    (strcmp(fentry, symname) == 0))


The proposed change will work as long as all the *.o use the same name.
Only one of {"__fentry__", "mcount", "_mcount", altmcount} is allowed
for all the *.o as input for a particular run.  [Modulo the hack
of ignoring a leading '.' for 64-bit PowerPC, of course.]

If the user changes compilers (or changes CFLAGS by insert/remove "-mfentry")
without doing a "make clean", then recordmcount will omit some calls.

Those restrictions are easy to guess, and recovery is easy.  Therefore,
Ack'ed by: John Reiser <jreiser@bitwagon.com>

-- 


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [RFC PATCH 1/4] ftrace: Make recordmcount.c handle __fentry__
  2012-08-07 23:57   ` John Reiser
@ 2012-08-08  0:05     ` Steven Rostedt
  0 siblings, 0 replies; 25+ messages in thread
From: Steven Rostedt @ 2012-08-08  0:05 UTC (permalink / raw)
  To: John Reiser
  Cc: linux-kernel, Ingo Molnar, Andrew Morton, Thomas Gleixner,
	Frederic Weisbecker, Masami Hiramatsu, Linus Torvalds,
	Andi Kleen

On Tue, 2012-08-07 at 16:57 -0700, John Reiser wrote:

> If the user changes compilers (or changes CFLAGS by insert/remove "-mfentry")
> without doing a "make clean", then recordmcount will omit some calls.

The make dependencies of such a change should (hopefully) cause a full
recompile of the code. Yeah, I expect that we only have "mcount" or
"fentry" as something that can be called. If we had both mcount and
fentry, it would fail to link anyway, as we only define mcount or fentry
in the assembly, never both.

> 
> Those restrictions are easy to guess, and recovery is easy.  Therefore,
> Ack'ed by: John Reiser <jreiser@bitwagon.com>
> 

Thanks!

-- Steve



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [RFC PATCH 3/4] ftrace: Do not test frame pointers if -mfentry is used
  2012-08-07 19:38 ` [RFC PATCH 3/4] ftrace: Do not test frame pointers if -mfentry is used Steven Rostedt
@ 2012-08-08  4:34   ` Masami Hiramatsu
  2012-08-08 12:49     ` Steven Rostedt
  2012-08-27 17:05   ` [tip:perf/core] " tip-bot for Steven Rostedt
  1 sibling, 1 reply; 25+ messages in thread
From: Masami Hiramatsu @ 2012-08-08  4:34 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: linux-kernel, Ingo Molnar, Andrew Morton, Thomas Gleixner,
	Frederic Weisbecker, Linus Torvalds, Andi Kleen

(2012/08/08 4:38), Steven Rostedt wrote:
> From: Steven Rostedt <srostedt@redhat.com>
> 
> The function graph has a test to check if the frame pointer is
> corrupted, which can happen with various options of gcc with mcount.
> But this is not an issue with -mfentry as -mfentry does not need nor use
> frame pointers for function graph tracing.
> 
> Cc: Andi Kleen <andi@firstfloor.org>
> Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
> ---
>  kernel/trace/trace_functions_graph.c |    5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel/trace/trace_functions_graph.c b/kernel/trace/trace_functions_graph.c
> index ce27c8b..99b4378 100644
> --- a/kernel/trace/trace_functions_graph.c
> +++ b/kernel/trace/trace_functions_graph.c
> @@ -143,7 +143,7 @@ ftrace_pop_return_trace(struct ftrace_graph_ret *trace, unsigned long *ret,
>  		return;
>  	}
>  
> -#ifdef CONFIG_HAVE_FUNCTION_GRAPH_FP_TEST
> +#if defined(CONFIG_HAVE_FUNCTION_GRAPH_FP_TEST) && !defined(CC_USING_FENTRY)

I think CONFIG_HAVE_FENTRY would better unselect
CONFIG_HAVE_FUNCTION_GRAPH_FP_TEST in arch/x86/Kconfig explicitly.

Thank you,


-- 
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [RFC PATCH 3/4] ftrace: Do not test frame pointers if -mfentry is used
  2012-08-08  4:34   ` Masami Hiramatsu
@ 2012-08-08 12:49     ` Steven Rostedt
  2012-08-09  2:58       ` Masami Hiramatsu
  2012-08-09  3:45       ` Linus Torvalds
  0 siblings, 2 replies; 25+ messages in thread
From: Steven Rostedt @ 2012-08-08 12:49 UTC (permalink / raw)
  To: Masami Hiramatsu
  Cc: linux-kernel, Ingo Molnar, Andrew Morton, Thomas Gleixner,
	Frederic Weisbecker, Linus Torvalds, Andi Kleen

On Wed, 2012-08-08 at 13:34 +0900, Masami Hiramatsu wrote:
>  
> > -#ifdef CONFIG_HAVE_FUNCTION_GRAPH_FP_TEST
> > +#if defined(CONFIG_HAVE_FUNCTION_GRAPH_FP_TEST) && !defined(CC_USING_FENTRY)
> 
> I think CONFIG_HAVE_FENTRY would better unselect
> CONFIG_HAVE_FUNCTION_GRAPH_FP_TEST in arch/x86/Kconfig explicitly.

No, CONFIG_HAVE_FENTRY just means fentry is supported, it does not mean
that it is being used. It only gets used if CC_USING_FENTRY is set,
which is set by the Makefile at time of compile.

If CONFIG_HAVE_FENTRY is defined, a test is done to see if the gcc
compiling the kernel supports -mfentry. If it does, then it defines the
CC_USING_FENTRY macro, if not, the macro is not defined and the old way
is performed.

If the old way is performed, even if CONFIG_HAVE_FENTRY is defined, then
we still need the above test. We can not have CONFIG_HAVE_FENTRY
unselect CONFIG_HAVE_FUNCTION_GRAPH_FP_TEST.

-- Steve



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [RFC PATCH 3/4] ftrace: Do not test frame pointers if -mfentry is used
  2012-08-08 12:49     ` Steven Rostedt
@ 2012-08-09  2:58       ` Masami Hiramatsu
  2012-08-09  3:45       ` Linus Torvalds
  1 sibling, 0 replies; 25+ messages in thread
From: Masami Hiramatsu @ 2012-08-09  2:58 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: linux-kernel, Ingo Molnar, Andrew Morton, Thomas Gleixner,
	Frederic Weisbecker, Linus Torvalds, Andi Kleen,
	yrl.pp-manager.tt

(2012/08/08 21:49), Steven Rostedt wrote:
> On Wed, 2012-08-08 at 13:34 +0900, Masami Hiramatsu wrote:
>>  
>>> -#ifdef CONFIG_HAVE_FUNCTION_GRAPH_FP_TEST
>>> +#if defined(CONFIG_HAVE_FUNCTION_GRAPH_FP_TEST) && !defined(CC_USING_FENTRY)
>>
>> I think CONFIG_HAVE_FENTRY would better unselect
>> CONFIG_HAVE_FUNCTION_GRAPH_FP_TEST in arch/x86/Kconfig explicitly.
> 
> No, CONFIG_HAVE_FENTRY just means fentry is supported, it does not mean
> that it is being used. It only gets used if CC_USING_FENTRY is set,
> which is set by the Makefile at time of compile.
> 
> If CONFIG_HAVE_FENTRY is defined, a test is done to see if the gcc
> compiling the kernel supports -mfentry. If it does, then it defines the
> CC_USING_FENTRY macro, if not, the macro is not defined and the old way
> is performed.
> 
> If the old way is performed, even if CONFIG_HAVE_FENTRY is defined, then
> we still need the above test. We can not have CONFIG_HAVE_FENTRY
> unselect CONFIG_HAVE_FUNCTION_GRAPH_FP_TEST.

Ah, I see. OK, I don't see any other issue.

Thank you,

-- 
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [RFC PATCH 3/4] ftrace: Do not test frame pointers if -mfentry is used
  2012-08-08 12:49     ` Steven Rostedt
  2012-08-09  2:58       ` Masami Hiramatsu
@ 2012-08-09  3:45       ` Linus Torvalds
  2012-08-09  3:57         ` Steven Rostedt
                           ` (2 more replies)
  1 sibling, 3 replies; 25+ messages in thread
From: Linus Torvalds @ 2012-08-09  3:45 UTC (permalink / raw)
  To: Steven Rostedt, Michal Marek
  Cc: Masami Hiramatsu, linux-kernel, Ingo Molnar, Andrew Morton,
	Thomas Gleixner, Frederic Weisbecker, Andi Kleen

On Wed, Aug 8, 2012 at 3:49 PM, Steven Rostedt <rostedt@goodmis.org> wrote:
>
> No, CONFIG_HAVE_FENTRY just means fentry is supported, it does not mean
> that it is being used. It only gets used if CC_USING_FENTRY is set,
> which is set by the Makefile at time of compile.

Btw, it might be lovely to consider the concept of "Kconfig variables
set by shell-scripts".

We currently have a metric sh*t-ton of Makefile magic for testing
various things like this, and integrating it into Kconfig might be a
good idea. That way you would be able to use the Kconfig logic on
these things.

Kconfig already has the "option env=XYZ" syntax for importing values
from the environment variables. Extending it to some kind of "option
shell=xyz" would allow for more complex interactions like this
(imagine testing compiler options and version dependencies in the
Kconfig files instead of the Makefiles)?

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [RFC PATCH 3/4] ftrace: Do not test frame pointers if -mfentry is used
  2012-08-09  3:45       ` Linus Torvalds
@ 2012-08-09  3:57         ` Steven Rostedt
  2012-08-09  4:15         ` H. Peter Anvin
  2012-08-09 12:37         ` Andi Kleen
  2 siblings, 0 replies; 25+ messages in thread
From: Steven Rostedt @ 2012-08-09  3:57 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Michal Marek, Masami Hiramatsu, linux-kernel, Ingo Molnar,
	Andrew Morton, Thomas Gleixner, Frederic Weisbecker, Andi Kleen

On Thu, 2012-08-09 at 06:45 +0300, Linus Torvalds wrote:
> On Wed, Aug 8, 2012 at 3:49 PM, Steven Rostedt <rostedt@goodmis.org> wrote:
> >
> > No, CONFIG_HAVE_FENTRY just means fentry is supported, it does not mean
> > that it is being used. It only gets used if CC_USING_FENTRY is set,
> > which is set by the Makefile at time of compile.
> 
> Btw, it might be lovely to consider the concept of "Kconfig variables
> set by shell-scripts".
> 
> We currently have a metric sh*t-ton of Makefile magic for testing
> various things like this, and integrating it into Kconfig might be a
> good idea. That way you would be able to use the Kconfig logic on
> these things.
> 
> Kconfig already has the "option env=XYZ" syntax for importing values
> from the environment variables. Extending it to some kind of "option
> shell=xyz" would allow for more complex interactions like this
> (imagine testing compiler options and version dependencies in the
> Kconfig files instead of the Makefiles)?

I understand your pain. I think this 'test the environment in the
makefile' is a bit of a hack.

I've worked a little with the Kconfig infrastructure, but I wouldn't
consider myself an expert. Would Kconfig be able to handle a change in
environment? That is, if you change your compiler, would a normal make
kick off Kconfig to update the configs for the new environment. Or would
the user have to do some kind of 'make oldconfig' before that.

Or is make 'oldconfig' performed at all compiles regardless now?

One nice thing of having all these tests represented in the .config
file, is that when a user produces a bug, when they send their config,
we would be able to see much more information about their environment.

-- Steve



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [RFC PATCH 3/4] ftrace: Do not test frame pointers if -mfentry is used
  2012-08-09  3:45       ` Linus Torvalds
  2012-08-09  3:57         ` Steven Rostedt
@ 2012-08-09  4:15         ` H. Peter Anvin
  2012-08-09 12:37         ` Andi Kleen
  2 siblings, 0 replies; 25+ messages in thread
From: H. Peter Anvin @ 2012-08-09  4:15 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Steven Rostedt, Michal Marek, Masami Hiramatsu, linux-kernel,
	Ingo Molnar, Andrew Morton, Thomas Gleixner, Frederic Weisbecker,
	Andi Kleen

On 08/08/2012 08:45 PM, Linus Torvalds wrote:
> 
> Btw, it might be lovely to consider the concept of "Kconfig variables
> set by shell-scripts".
> 
> We currently have a metric sh*t-ton of Makefile magic for testing
> various things like this, and integrating it into Kconfig might be a
> good idea. That way you would be able to use the Kconfig logic on
> these things.
> 
> Kconfig already has the "option env=XYZ" syntax for importing values
> from the environment variables. Extending it to some kind of "option
> shell=xyz" would allow for more complex interactions like this
> (imagine testing compiler options and version dependencies in the
> Kconfig files instead of the Makefiles)?
> 

This is a Frequently Requested Feature over on the kbuild list... and it
almost certainly would speed up the build given how many times the tests
are currently run.  Furthermore, it would let us base rules on the
support existing, which we currently can't.

	-hpa


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [RFC PATCH 4/4] ftrace/x86: Add support for -mfentry to x86_64
  2012-08-07 19:38 ` [RFC PATCH 4/4] ftrace/x86: Add support for -mfentry to x86_64 Steven Rostedt
@ 2012-08-09  8:34   ` Masami Hiramatsu
  2012-08-09 13:46   ` Steven Rostedt
  2012-08-27 17:06   ` [tip:perf/core] " tip-bot for Steven Rostedt
  2 siblings, 0 replies; 25+ messages in thread
From: Masami Hiramatsu @ 2012-08-09  8:34 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: linux-kernel, Ingo Molnar, Andrew Morton, Thomas Gleixner,
	Frederic Weisbecker, Linus Torvalds, Andi Kleen

(2012/08/08 4:38), Steven Rostedt wrote:
> From: Steven Rostedt <srostedt@redhat.com>
> 
> If the kernel is compiled with gcc 4.6.0 which supports -mfentry,
> then use that instead of mcount.
> 
> With mcount, frame pointers are forced with the -pg option and we
> get something like:
> 
> <can_vma_merge_before>:
>        55                      push   %rbp
>        48 89 e5                mov    %rsp,%rbp
>        53                      push   %rbx
>        41 51                   push   %r9
>        e8 fe 6a 39 00          callq  ffffffff81483d00 <mcount>
>        31 c0                   xor    %eax,%eax
>        48 89 fb                mov    %rdi,%rbx
>        48 89 d7                mov    %rdx,%rdi
>        48 33 73 30             xor    0x30(%rbx),%rsi
>        48 f7 c6 ff ff ff f7    test   $0xfffffffff7ffffff,%rsi
> 
> With -mfentry, frame pointers are no longer forced and the call looks
> like this:
> 
> <can_vma_merge_before>:
>        e8 33 af 37 00          callq  ffffffff81461b40 <__fentry__>
>        53                      push   %rbx
>        48 89 fb                mov    %rdi,%rbx
>        31 c0                   xor    %eax,%eax
>        48 89 d7                mov    %rdx,%rdi
>        41 51                   push   %r9
>        48 33 73 30             xor    0x30(%rbx),%rsi
>        48 f7 c6 ff ff ff f7    test   $0xfffffffff7ffffff,%rsi
> 
> This adds the ftrace hook at the beginning of the function before a
> frame is set up, and allows the function callbacks to be able to access
> parameters. As kprobes now can use function tracing (at least on x86)
> this speeds up the kprobe hooks that are at the beginning of the
> function.

This looks good for me:)

Reviewed-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>

Thanks!

> 
> Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
> Cc: Andi Kleen <andi@firstfloor.org>
> Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
> ---
>  arch/x86/Kconfig                 |    1 +
>  arch/x86/include/asm/ftrace.h    |    7 ++++++-
>  arch/x86/kernel/entry_64.S       |   18 +++++++++++++++++-
>  arch/x86/kernel/x8664_ksyms_64.c |    6 +++++-
>  4 files changed, 29 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index c70684f..bbbf5d8 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -36,6 +36,7 @@ config X86
>  	select HAVE_KRETPROBES
>  	select HAVE_OPTPROBES
>  	select HAVE_FTRACE_MCOUNT_RECORD
> +	select HAVE_FENTRY if X86_64
>  	select HAVE_C_RECORDMCOUNT
>  	select HAVE_DYNAMIC_FTRACE
>  	select HAVE_FUNCTION_TRACER
> diff --git a/arch/x86/include/asm/ftrace.h b/arch/x86/include/asm/ftrace.h
> index a6cae0c..9a25b52 100644
> --- a/arch/x86/include/asm/ftrace.h
> +++ b/arch/x86/include/asm/ftrace.h
> @@ -35,7 +35,11 @@
>  #endif
>  
>  #ifdef CONFIG_FUNCTION_TRACER
> -#define MCOUNT_ADDR		((long)(mcount))
> +#ifdef CC_USING_FENTRY
> +# define MCOUNT_ADDR		((long)(__fentry__))
> +#else
> +# define MCOUNT_ADDR		((long)(mcount))
> +#endif
>  #define MCOUNT_INSN_SIZE	5 /* sizeof mcount call */
>  
>  #ifdef CONFIG_DYNAMIC_FTRACE
> @@ -46,6 +50,7 @@
>  #ifndef __ASSEMBLY__
>  extern void mcount(void);
>  extern atomic_t modifying_ftrace_code;
> +extern void __fentry__(void);
>  
>  static inline unsigned long ftrace_call_adjust(unsigned long addr)
>  {
> diff --git a/arch/x86/kernel/entry_64.S b/arch/x86/kernel/entry_64.S
> index 38308fa..2add3bb 100644
> --- a/arch/x86/kernel/entry_64.S
> +++ b/arch/x86/kernel/entry_64.S
> @@ -69,9 +69,16 @@
>  
>  #ifdef CONFIG_FUNCTION_TRACER
>  #ifdef CONFIG_DYNAMIC_FTRACE
> +
> +#ifdef CC_USING_FENTRY
> +ENTRY(__fentry__)
> +	retq
> +END(__fentry__)
> +#else
>  ENTRY(mcount)
>  	retq
>  END(mcount)
> +#endif
>  
>  /* skip is set if stack has been adjusted */
>  .macro ftrace_caller_setup skip=0
> @@ -84,7 +91,11 @@ END(mcount)
>  	movq RIP(%rsp), %rdi
>  	subq $MCOUNT_INSN_SIZE, %rdi
>  	/* Load the parent_ip into the second parameter */
> +#ifdef CC_USING_FENTRY
> +	movq SS+16(%rsp), %rsi
> +#else
>  	movq 8(%rbp), %rsi
> +#endif
>  .endm
>  
>  ENTRY(ftrace_caller)
> @@ -215,9 +226,14 @@ END(mcount)
>  ENTRY(ftrace_graph_caller)
>  	MCOUNT_SAVE_FRAME
>  
> +#ifdef CC_USING_FENTRY
> +	leaq SS+16(%rsp), %rdi
> +	movq $0, %rdx	/* No framepointers needed */
> +#else
>  	leaq 8(%rbp), %rdi
> -	movq RIP(%rsp), %rsi
>  	movq (%rbp), %rdx
> +#endif
> +	movq RIP(%rsp), %rsi
>  	subq $MCOUNT_INSN_SIZE, %rsi
>  
>  	call	prepare_ftrace_return
> diff --git a/arch/x86/kernel/x8664_ksyms_64.c b/arch/x86/kernel/x8664_ksyms_64.c
> index 9796c2f..643b236 100644
> --- a/arch/x86/kernel/x8664_ksyms_64.c
> +++ b/arch/x86/kernel/x8664_ksyms_64.c
> @@ -13,9 +13,13 @@
>  #include <asm/ftrace.h>
>  
>  #ifdef CONFIG_FUNCTION_TRACER
> -/* mcount is defined in assembly */
> +/* mcount and __fentry__ are defined in assembly */
> +#ifdef CC_USING_FENTRY
> +EXPORT_SYMBOL(__fentry__);
> +#else
>  EXPORT_SYMBOL(mcount);
>  #endif
> +#endif
>  
>  EXPORT_SYMBOL(__get_user_1);
>  EXPORT_SYMBOL(__get_user_2);
> 


-- 
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [RFC PATCH 3/4] ftrace: Do not test frame pointers if -mfentry is used
  2012-08-09  3:45       ` Linus Torvalds
  2012-08-09  3:57         ` Steven Rostedt
  2012-08-09  4:15         ` H. Peter Anvin
@ 2012-08-09 12:37         ` Andi Kleen
  2 siblings, 0 replies; 25+ messages in thread
From: Andi Kleen @ 2012-08-09 12:37 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Steven Rostedt, Michal Marek, Masami Hiramatsu, linux-kernel,
	Ingo Molnar, Andrew Morton, Thomas Gleixner, Frederic Weisbecker,
	Andi Kleen

On Thu, Aug 09, 2012 at 06:45:37AM +0300, Linus Torvalds wrote:
> On Wed, Aug 8, 2012 at 3:49 PM, Steven Rostedt <rostedt@goodmis.org> wrote:
> >
> > No, CONFIG_HAVE_FENTRY just means fentry is supported, it does not mean
> > that it is being used. It only gets used if CC_USING_FENTRY is set,
> > which is set by the Makefile at time of compile.
> 
> Btw, it might be lovely to consider the concept of "Kconfig variables
> set by shell-scripts".

I looked into it some time ago because I needed it for something else.
But no mergeable patch produced. Will try again.

One issue was that it will break some odd cross compiling or ccache
setups where the compiler is not set correctly at config time. 
But I think it would be worth it.

> We currently have a metric sh*t-ton of Makefile magic for testing
> various things like this, and integrating it into Kconfig might be a
> good idea. That way you would be able to use the Kconfig logic on
> these things.

Also another big win would be to make the empty do nothing build faster,
because the decisions would be cached.

I measured recently and we execute 30+ gccs on a empty build.

-Andi

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [RFC PATCH 4/4] ftrace/x86: Add support for -mfentry to x86_64
  2012-08-07 19:38 ` [RFC PATCH 4/4] ftrace/x86: Add support for -mfentry to x86_64 Steven Rostedt
  2012-08-09  8:34   ` Masami Hiramatsu
@ 2012-08-09 13:46   ` Steven Rostedt
  2012-08-09 13:48     ` Steven Rostedt
  2012-08-10  7:45     ` Masami Hiramatsu
  2012-08-27 17:06   ` [tip:perf/core] " tip-bot for Steven Rostedt
  2 siblings, 2 replies; 25+ messages in thread
From: Steven Rostedt @ 2012-08-09 13:46 UTC (permalink / raw)
  To: linux-kernel, H. Peter Anvin
  Cc: Ingo Molnar, Andrew Morton, Thomas Gleixner, Frederic Weisbecker,
	Masami Hiramatsu, Linus Torvalds, Andi Kleen

Peter and Masami

During my final tests, I found that this change breaks the
!DYNAMIC_FTRACE config. That is, when we don't do the run-time updates
of mcount calls to nops, the compiler will use fentry but the code still
uses mcount.

I fixed this in the patch below. But as you two have acked and reviewed
it, I can't add your tags if I have changed the code. Can you ack/review
it again.

Thanks!

-- Steve


On Tue, 2012-08-07 at 15:38 -0400, Steven Rostedt wrote:
> From: Steven Rostedt <srostedt@redhat.com>
> 
> If the kernel is compiled with gcc 4.6.0 which supports -mfentry,
> then use that instead of mcount.
> 
> With mcount, frame pointers are forced with the -pg option and we
> get something like:
> 
> <can_vma_merge_before>:
>        55                      push   %rbp
>        48 89 e5                mov    %rsp,%rbp
>        53                      push   %rbx
>        41 51                   push   %r9
>        e8 fe 6a 39 00          callq  ffffffff81483d00 <mcount>
>        31 c0                   xor    %eax,%eax
>        48 89 fb                mov    %rdi,%rbx
>        48 89 d7                mov    %rdx,%rdi
>        48 33 73 30             xor    0x30(%rbx),%rsi
>        48 f7 c6 ff ff ff f7    test   $0xfffffffff7ffffff,%rsi
> 
> With -mfentry, frame pointers are no longer forced and the call looks
> like this:
> 
> <can_vma_merge_before>:
>        e8 33 af 37 00          callq  ffffffff81461b40 <__fentry__>
>        53                      push   %rbx
>        48 89 fb                mov    %rdi,%rbx
>        31 c0                   xor    %eax,%eax
>        48 89 d7                mov    %rdx,%rdi
>        41 51                   push   %r9
>        48 33 73 30             xor    0x30(%rbx),%rsi
>        48 f7 c6 ff ff ff f7    test   $0xfffffffff7ffffff,%rsi
> 
> This adds the ftrace hook at the beginning of the function before a
> frame is set up, and allows the function callbacks to be able to access
> parameters. As kprobes now can use function tracing (at least on x86)
> this speeds up the kprobe hooks that are at the beginning of the
> function.
> 
> Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
> Cc: Andi Kleen <andi@firstfloor.org>
> Signed-off-by: Steven Rostedt <rostedt@goodmis.org>

(change log kept the same)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index c70684f..bbbf5d8 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -36,6 +36,7 @@ config X86
 	select HAVE_KRETPROBES
 	select HAVE_OPTPROBES
 	select HAVE_FTRACE_MCOUNT_RECORD
+	select HAVE_FENTRY if X86_64
 	select HAVE_C_RECORDMCOUNT
 	select HAVE_DYNAMIC_FTRACE
 	select HAVE_FUNCTION_TRACER
diff --git a/arch/x86/include/asm/ftrace.h b/arch/x86/include/asm/ftrace.h
index a6cae0c..9a25b52 100644
--- a/arch/x86/include/asm/ftrace.h
+++ b/arch/x86/include/asm/ftrace.h
@@ -35,7 +35,11 @@
 #endif
 
 #ifdef CONFIG_FUNCTION_TRACER
-#define MCOUNT_ADDR		((long)(mcount))
+#ifdef CC_USING_FENTRY
+# define MCOUNT_ADDR		((long)(__fentry__))
+#else
+# define MCOUNT_ADDR		((long)(mcount))
+#endif
 #define MCOUNT_INSN_SIZE	5 /* sizeof mcount call */
 
 #ifdef CONFIG_DYNAMIC_FTRACE
@@ -46,6 +50,7 @@
 #ifndef __ASSEMBLY__
 extern void mcount(void);
 extern atomic_t modifying_ftrace_code;
+extern void __fentry__(void);
 
 static inline unsigned long ftrace_call_adjust(unsigned long addr)
 {
diff --git a/arch/x86/kernel/entry_64.S b/arch/x86/kernel/entry_64.S
index 38308fa..a698521 100644
--- a/arch/x86/kernel/entry_64.S
+++ b/arch/x86/kernel/entry_64.S
@@ -68,10 +68,18 @@
 	.section .entry.text, "ax"
 
 #ifdef CONFIG_FUNCTION_TRACER
+
+#ifdef CC_USING_FENTRY
+# define function_hook	__fentry__
+#else
+# define function_hook	mcount
+#endif
+
 #ifdef CONFIG_DYNAMIC_FTRACE
-ENTRY(mcount)
+
+ENTRY(function_hook)
 	retq
-END(mcount)
+END(function_hook)
 
 /* skip is set if stack has been adjusted */
 .macro ftrace_caller_setup skip=0
@@ -84,7 +92,11 @@ END(mcount)
 	movq RIP(%rsp), %rdi
 	subq $MCOUNT_INSN_SIZE, %rdi
 	/* Load the parent_ip into the second parameter */
+#ifdef CC_USING_FENTRY
+	movq SS+16(%rsp), %rsi
+#else
 	movq 8(%rbp), %rsi
+#endif
 .endm
 
 ENTRY(ftrace_caller)
@@ -177,7 +189,8 @@ END(ftrace_regs_caller)
 
 
 #else /* ! CONFIG_DYNAMIC_FTRACE */
-ENTRY(mcount)
+
+ENTRY(function_hook)
 	cmpl $0, function_trace_stop
 	jne  ftrace_stub
 
@@ -199,7 +212,11 @@ trace:
 	MCOUNT_SAVE_FRAME
 
 	movq RIP(%rsp), %rdi
+#ifdef CC_USING_FENTRY
+	movq SS+16(%rsp), %rsi
+#else
 	movq 8(%rbp), %rsi
+#endif
 	subq $MCOUNT_INSN_SIZE, %rdi
 
 	call   *ftrace_trace_function
@@ -207,7 +224,7 @@ trace:
 	MCOUNT_RESTORE_FRAME
 
 	jmp ftrace_stub
-END(mcount)
+END(function_hook)
 #endif /* CONFIG_DYNAMIC_FTRACE */
 #endif /* CONFIG_FUNCTION_TRACER */
 
@@ -215,9 +232,14 @@ END(mcount)
 ENTRY(ftrace_graph_caller)
 	MCOUNT_SAVE_FRAME
 
+#ifdef CC_USING_FENTRY
+	leaq SS+16(%rsp), %rdi
+	movq $0, %rdx	/* No framepointers needed */
+#else
 	leaq 8(%rbp), %rdi
-	movq RIP(%rsp), %rsi
 	movq (%rbp), %rdx
+#endif
+	movq RIP(%rsp), %rsi
 	subq $MCOUNT_INSN_SIZE, %rsi
 
 	call	prepare_ftrace_return
diff --git a/arch/x86/kernel/x8664_ksyms_64.c b/arch/x86/kernel/x8664_ksyms_64.c
index 9796c2f..643b236 100644
--- a/arch/x86/kernel/x8664_ksyms_64.c
+++ b/arch/x86/kernel/x8664_ksyms_64.c
@@ -13,9 +13,13 @@
 #include <asm/ftrace.h>
 
 #ifdef CONFIG_FUNCTION_TRACER
-/* mcount is defined in assembly */
+/* mcount and __fentry__ are defined in assembly */
+#ifdef CC_USING_FENTRY
+EXPORT_SYMBOL(__fentry__);
+#else
 EXPORT_SYMBOL(mcount);
 #endif
+#endif
 
 EXPORT_SYMBOL(__get_user_1);
 EXPORT_SYMBOL(__get_user_2);



^ permalink raw reply related	[flat|nested] 25+ messages in thread

* Re: [RFC PATCH 4/4] ftrace/x86: Add support for -mfentry to x86_64
  2012-08-09 13:46   ` Steven Rostedt
@ 2012-08-09 13:48     ` Steven Rostedt
  2012-08-10  7:45     ` Masami Hiramatsu
  1 sibling, 0 replies; 25+ messages in thread
From: Steven Rostedt @ 2012-08-09 13:48 UTC (permalink / raw)
  To: linux-kernel
  Cc: H. Peter Anvin, Ingo Molnar, Andrew Morton, Thomas Gleixner,
	Frederic Weisbecker, Masami Hiramatsu, Linus Torvalds,
	Andi Kleen

On Thu, 2012-08-09 at 09:46 -0400, Steven Rostedt wrote:
> Peter and Masami
> 
> During my final tests, I found that this change breaks the
> !DYNAMIC_FTRACE config. That is, when we don't do the run-time updates
> of mcount calls to nops, the compiler will use fentry but the code still
> uses mcount.
> 
> I fixed this in the patch below. But as you two have acked and reviewed
> it, I can't add your tags if I have changed the code. Can you ack/review
> it again.
> 
> Thanks!
> 

This is the changes that were made against the original patch:

diff --git a/arch/x86/kernel/entry_64.S b/arch/x86/kernel/entry_64.S
index 2add3bb..a698521 100644
--- a/arch/x86/kernel/entry_64.S
+++ b/arch/x86/kernel/entry_64.S
@@ -68,18 +68,19 @@
 	.section .entry.text, "ax"
 
 #ifdef CONFIG_FUNCTION_TRACER
-#ifdef CONFIG_DYNAMIC_FTRACE
 
 #ifdef CC_USING_FENTRY
-ENTRY(__fentry__)
-	retq
-END(__fentry__)
+# define function_hook	__fentry__
 #else
-ENTRY(mcount)
-	retq
-END(mcount)
+# define function_hook	mcount
 #endif
 
+#ifdef CONFIG_DYNAMIC_FTRACE
+
+ENTRY(function_hook)
+	retq
+END(function_hook)
+
 /* skip is set if stack has been adjusted */
 .macro ftrace_caller_setup skip=0
 	MCOUNT_SAVE_FRAME \skip
@@ -188,7 +189,8 @@ END(ftrace_regs_caller)
 
 
 #else /* ! CONFIG_DYNAMIC_FTRACE */
-ENTRY(mcount)
+
+ENTRY(function_hook)
 	cmpl $0, function_trace_stop
 	jne  ftrace_stub
 
@@ -210,7 +212,11 @@ trace:
 	MCOUNT_SAVE_FRAME
 
 	movq RIP(%rsp), %rdi
+#ifdef CC_USING_FENTRY
+	movq SS+16(%rsp), %rsi
+#else
 	movq 8(%rbp), %rsi
+#endif
 	subq $MCOUNT_INSN_SIZE, %rdi
 
 	call   *ftrace_trace_function
@@ -218,7 +224,7 @@ trace:
 	MCOUNT_RESTORE_FRAME
 
 	jmp ftrace_stub
-END(mcount)
+END(function_hook)
 #endif /* CONFIG_DYNAMIC_FTRACE */
 #endif /* CONFIG_FUNCTION_TRACER */
 



^ permalink raw reply related	[flat|nested] 25+ messages in thread

* Re: Re: [RFC PATCH 4/4] ftrace/x86: Add support for -mfentry to x86_64
  2012-08-09 13:46   ` Steven Rostedt
  2012-08-09 13:48     ` Steven Rostedt
@ 2012-08-10  7:45     ` Masami Hiramatsu
  1 sibling, 0 replies; 25+ messages in thread
From: Masami Hiramatsu @ 2012-08-10  7:45 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: linux-kernel, H. Peter Anvin, Ingo Molnar, Andrew Morton,
	Thomas Gleixner, Frederic Weisbecker, Linus Torvalds, Andi Kleen,
	yrl.pp-manager.tt

(2012/08/09 22:46), Steven Rostedt wrote:
> Peter and Masami
> 
> During my final tests, I found that this change breaks the
> !DYNAMIC_FTRACE config. That is, when we don't do the run-time updates
> of mcount calls to nops, the compiler will use fentry but the code still
> uses mcount.

Ah, right. we have to take care about it.

> 
> I fixed this in the patch below. But as you two have acked and reviewed
> it, I can't add your tags if I have changed the code. Can you ack/review
> it again.

This looks good for me.

Reviewed-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>

> 
> Thanks!
> 
> -- Steve
> 
> 
> On Tue, 2012-08-07 at 15:38 -0400, Steven Rostedt wrote:
>> From: Steven Rostedt <srostedt@redhat.com>
>>
>> If the kernel is compiled with gcc 4.6.0 which supports -mfentry,
>> then use that instead of mcount.
>>
>> With mcount, frame pointers are forced with the -pg option and we
>> get something like:
>>
>> <can_vma_merge_before>:
>>        55                      push   %rbp
>>        48 89 e5                mov    %rsp,%rbp
>>        53                      push   %rbx
>>        41 51                   push   %r9
>>        e8 fe 6a 39 00          callq  ffffffff81483d00 <mcount>
>>        31 c0                   xor    %eax,%eax
>>        48 89 fb                mov    %rdi,%rbx
>>        48 89 d7                mov    %rdx,%rdi
>>        48 33 73 30             xor    0x30(%rbx),%rsi
>>        48 f7 c6 ff ff ff f7    test   $0xfffffffff7ffffff,%rsi
>>
>> With -mfentry, frame pointers are no longer forced and the call looks
>> like this:
>>
>> <can_vma_merge_before>:
>>        e8 33 af 37 00          callq  ffffffff81461b40 <__fentry__>
>>        53                      push   %rbx
>>        48 89 fb                mov    %rdi,%rbx
>>        31 c0                   xor    %eax,%eax
>>        48 89 d7                mov    %rdx,%rdi
>>        41 51                   push   %r9
>>        48 33 73 30             xor    0x30(%rbx),%rsi
>>        48 f7 c6 ff ff ff f7    test   $0xfffffffff7ffffff,%rsi
>>
>> This adds the ftrace hook at the beginning of the function before a
>> frame is set up, and allows the function callbacks to be able to access
>> parameters. As kprobes now can use function tracing (at least on x86)
>> this speeds up the kprobe hooks that are at the beginning of the
>> function.
>>
>> Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
>> Cc: Andi Kleen <andi@firstfloor.org>
>> Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
> 
> (change log kept the same)
> 
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index c70684f..bbbf5d8 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -36,6 +36,7 @@ config X86
>  	select HAVE_KRETPROBES
>  	select HAVE_OPTPROBES
>  	select HAVE_FTRACE_MCOUNT_RECORD
> +	select HAVE_FENTRY if X86_64
>  	select HAVE_C_RECORDMCOUNT
>  	select HAVE_DYNAMIC_FTRACE
>  	select HAVE_FUNCTION_TRACER
> diff --git a/arch/x86/include/asm/ftrace.h b/arch/x86/include/asm/ftrace.h
> index a6cae0c..9a25b52 100644
> --- a/arch/x86/include/asm/ftrace.h
> +++ b/arch/x86/include/asm/ftrace.h
> @@ -35,7 +35,11 @@
>  #endif
>  
>  #ifdef CONFIG_FUNCTION_TRACER
> -#define MCOUNT_ADDR		((long)(mcount))
> +#ifdef CC_USING_FENTRY
> +# define MCOUNT_ADDR		((long)(__fentry__))
> +#else
> +# define MCOUNT_ADDR		((long)(mcount))
> +#endif
>  #define MCOUNT_INSN_SIZE	5 /* sizeof mcount call */
>  
>  #ifdef CONFIG_DYNAMIC_FTRACE
> @@ -46,6 +50,7 @@
>  #ifndef __ASSEMBLY__
>  extern void mcount(void);
>  extern atomic_t modifying_ftrace_code;
> +extern void __fentry__(void);
>  
>  static inline unsigned long ftrace_call_adjust(unsigned long addr)
>  {
> diff --git a/arch/x86/kernel/entry_64.S b/arch/x86/kernel/entry_64.S
> index 38308fa..a698521 100644
> --- a/arch/x86/kernel/entry_64.S
> +++ b/arch/x86/kernel/entry_64.S
> @@ -68,10 +68,18 @@
>  	.section .entry.text, "ax"
>  
>  #ifdef CONFIG_FUNCTION_TRACER
> +
> +#ifdef CC_USING_FENTRY
> +# define function_hook	__fentry__
> +#else
> +# define function_hook	mcount
> +#endif
> +
>  #ifdef CONFIG_DYNAMIC_FTRACE
> -ENTRY(mcount)
> +
> +ENTRY(function_hook)
>  	retq
> -END(mcount)
> +END(function_hook)
>  
>  /* skip is set if stack has been adjusted */
>  .macro ftrace_caller_setup skip=0
> @@ -84,7 +92,11 @@ END(mcount)
>  	movq RIP(%rsp), %rdi
>  	subq $MCOUNT_INSN_SIZE, %rdi
>  	/* Load the parent_ip into the second parameter */
> +#ifdef CC_USING_FENTRY
> +	movq SS+16(%rsp), %rsi
> +#else
>  	movq 8(%rbp), %rsi
> +#endif
>  .endm
>  
>  ENTRY(ftrace_caller)
> @@ -177,7 +189,8 @@ END(ftrace_regs_caller)
>  
>  
>  #else /* ! CONFIG_DYNAMIC_FTRACE */
> -ENTRY(mcount)
> +
> +ENTRY(function_hook)
>  	cmpl $0, function_trace_stop
>  	jne  ftrace_stub
>  
> @@ -199,7 +212,11 @@ trace:
>  	MCOUNT_SAVE_FRAME
>  
>  	movq RIP(%rsp), %rdi
> +#ifdef CC_USING_FENTRY
> +	movq SS+16(%rsp), %rsi
> +#else
>  	movq 8(%rbp), %rsi
> +#endif
>  	subq $MCOUNT_INSN_SIZE, %rdi
>  
>  	call   *ftrace_trace_function
> @@ -207,7 +224,7 @@ trace:
>  	MCOUNT_RESTORE_FRAME
>  
>  	jmp ftrace_stub
> -END(mcount)
> +END(function_hook)
>  #endif /* CONFIG_DYNAMIC_FTRACE */
>  #endif /* CONFIG_FUNCTION_TRACER */
>  
> @@ -215,9 +232,14 @@ END(mcount)
>  ENTRY(ftrace_graph_caller)
>  	MCOUNT_SAVE_FRAME
>  
> +#ifdef CC_USING_FENTRY
> +	leaq SS+16(%rsp), %rdi
> +	movq $0, %rdx	/* No framepointers needed */
> +#else
>  	leaq 8(%rbp), %rdi
> -	movq RIP(%rsp), %rsi
>  	movq (%rbp), %rdx
> +#endif
> +	movq RIP(%rsp), %rsi
>  	subq $MCOUNT_INSN_SIZE, %rsi
>  
>  	call	prepare_ftrace_return
> diff --git a/arch/x86/kernel/x8664_ksyms_64.c b/arch/x86/kernel/x8664_ksyms_64.c
> index 9796c2f..643b236 100644
> --- a/arch/x86/kernel/x8664_ksyms_64.c
> +++ b/arch/x86/kernel/x8664_ksyms_64.c
> @@ -13,9 +13,13 @@
>  #include <asm/ftrace.h>
>  
>  #ifdef CONFIG_FUNCTION_TRACER
> -/* mcount is defined in assembly */
> +/* mcount and __fentry__ are defined in assembly */
> +#ifdef CC_USING_FENTRY
> +EXPORT_SYMBOL(__fentry__);
> +#else
>  EXPORT_SYMBOL(mcount);
>  #endif
> +#endif
>  
>  EXPORT_SYMBOL(__get_user_1);
>  EXPORT_SYMBOL(__get_user_2);
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 


-- 
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [RFC PATCH 0/4] ftrace: Add use of -mfentry for x86_64
  2012-08-07 20:23 ` [RFC PATCH 0/4] ftrace: Add use of -mfentry for x86_64 H. Peter Anvin
@ 2012-08-13  8:42   ` Ingo Molnar
  0 siblings, 0 replies; 25+ messages in thread
From: Ingo Molnar @ 2012-08-13  8:42 UTC (permalink / raw)
  To: H. Peter Anvin
  Cc: Steven Rostedt, linux-kernel, Ingo Molnar, Andrew Morton,
	Thomas Gleixner, Frederic Weisbecker, Masami Hiramatsu,
	Linus Torvalds, Andi Kleen


* H. Peter Anvin <hpa@zytor.com> wrote:

> On 08/07/2012 12:38 PM, Steven Rostedt wrote:
> >
> >This is an RFC patch set that makes gcc use the -mfentry option with
> >-pg. This will set the ftrace 'hook' to the beginning of the function
> >and also remove the requirement that -pg enables frame pointers.
> >
> >This has a couple of benefits (and probably more).
> >
> >1) removal of the frame pointer requirement makes for smaller and faster code.
> >
> >2) Having the function trace hook at the beginning of the function instead
> >  of after the frame is set up, gives the function tracing callbacks access
> >  to the parameters. This means that kprobes can take advantage of this.
> >  When a kprobe is set on top of a ftrace hook (nop), it will automatically
> >  use the function tracing callback. This makes it into an 'optimized' probe
> >  as there's no need to hit a breakpoint and trigger the probe that way.
> >  The function tracing code can do the work for it. Note, optimized probes
> >  are only allowed with !PREEMPT, but a ftrace optimize probe is allowed
> >  in any context (another benefit).
> >
> >This only implements fentry for x86_64.
> >
> 
> Acked-by: H. Peter Anvin <hpa@linux.intel.com>

Acked-by: Ingo Molnar <mingo@kernel.org>

Nice feature!

Thanks,

	Ingo

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [tip:perf/core] ftrace: Make recordmcount.c handle __fentry__
  2012-08-07 19:38 ` [RFC PATCH 1/4] ftrace: Make recordmcount.c handle __fentry__ Steven Rostedt
  2012-08-07 23:57   ` John Reiser
@ 2012-08-27 17:03   ` tip-bot for Steven Rostedt
  1 sibling, 0 replies; 25+ messages in thread
From: tip-bot for Steven Rostedt @ 2012-08-27 17:03 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: linux-kernel, hpa, mingo, andi, rostedt, srostedt, jreiser, tglx, hpa

Commit-ID:  48bb5dc6cd9d30fe0d594947563da1f8bd9abada
Gitweb:     http://git.kernel.org/tip/48bb5dc6cd9d30fe0d594947563da1f8bd9abada
Author:     Steven Rostedt <srostedt@redhat.com>
AuthorDate: Wed, 9 Feb 2011 13:13:23 -0500
Committer:  Steven Rostedt <rostedt@goodmis.org>
CommitDate: Thu, 23 Aug 2012 11:24:43 -0400

ftrace: Make recordmcount.c handle __fentry__

With gcc 4.6.0 the -mfentry feature places the function profiling
call at the start of the function. When this is used, the call is
to __fentry__ and not mcount.

Change recordmcount.c to record both callers to __fentry__ and
mcount.

Link: http://lkml.kernel.org/r/20120807194058.990674363@goodmis.org

Acked-by: H. Peter Anvin <hpa@linux.intel.com>
Acked-by: John Reiser <jreiser@bitwagon.com>
Acked-by: Ingo Molnar <mingo@kernel.org>
Cc: Andi Kleen <andi@firstfloor.org>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 scripts/recordmcount.h |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/scripts/recordmcount.h b/scripts/recordmcount.h
index 54e35c1..9d1421e 100644
--- a/scripts/recordmcount.h
+++ b/scripts/recordmcount.h
@@ -261,11 +261,13 @@ static unsigned get_mcountsym(Elf_Sym const *const sym0,
 		&sym0[Elf_r_sym(relp)];
 	char const *symname = &str0[w(symp->st_name)];
 	char const *mcount = gpfx == '_' ? "_mcount" : "mcount";
+	char const *fentry = "__fentry__";
 
 	if (symname[0] == '.')
 		++symname;  /* ppc64 hack */
 	if (strcmp(mcount, symname) == 0 ||
-	    (altmcount && strcmp(altmcount, symname) == 0))
+	    (altmcount && strcmp(altmcount, symname) == 0) ||
+	    (strcmp(fentry, symname) == 0))
 		mcountsym = Elf_r_sym(relp);
 
 	return mcountsym;

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [tip:perf/core] ftrace: Add -mfentry to Makefile on function tracer
  2012-08-07 19:38 ` [RFC PATCH 2/4] ftrace: Add -mfentry to Makefile on function tracer Steven Rostedt
@ 2012-08-27 17:04   ` tip-bot for Steven Rostedt
  0 siblings, 0 replies; 25+ messages in thread
From: tip-bot for Steven Rostedt @ 2012-08-27 17:04 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: linux-kernel, hpa, mingo, andi, rostedt, srostedt, mmarek, tglx, hpa

Commit-ID:  a2546fae01124fb8063747439300fcf39bac033a
Gitweb:     http://git.kernel.org/tip/a2546fae01124fb8063747439300fcf39bac033a
Author:     Steven Rostedt <srostedt@redhat.com>
AuthorDate: Wed, 9 Feb 2011 13:15:59 -0500
Committer:  Steven Rostedt <rostedt@goodmis.org>
CommitDate: Thu, 23 Aug 2012 11:25:02 -0400

ftrace: Add -mfentry to Makefile on function tracer

Thanks to Andi Kleen, gcc 4.6.0 now supports -mfentry for x86
(and hopefully soon for other archs). What this does is to have
the function profiler start at the beginning of the function
instead of after the stack is set up. As plain -pg (mcount) is
called after the stack is set up, and in some cases can have issues
with the function graph tracer. It also requires frame pointers to
be enabled.

The -mfentry now calls __fentry__ at the beginning of the function.
This allows for compiling without frame pointers and even has the
ability to access parameters if needed.

If the architecture and the compiler both support -mfentry then
use that instead.

Link: http://lkml.kernel.org/r/20120807194059.392617243@goodmis.org

Acked-by: H. Peter Anvin <hpa@linux.intel.com>
Acked-by: Ingo Molnar <mingo@kernel.org>
Cc: Michal Marek <mmarek@suse.cz>
Cc: Andi Kleen <andi@firstfloor.org>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 Makefile             |    6 +++++-
 kernel/trace/Kconfig |    5 +++++
 2 files changed, 10 insertions(+), 1 deletions(-)

diff --git a/Makefile b/Makefile
index ddf5be9..e7ca93f 100644
--- a/Makefile
+++ b/Makefile
@@ -609,7 +609,11 @@ KBUILD_CFLAGS 	+= $(call cc-option, -femit-struct-debug-baseonly)
 endif
 
 ifdef CONFIG_FUNCTION_TRACER
-KBUILD_CFLAGS	+= -pg
+ifdef CONFIG_HAVE_FENTRY
+CC_USING_FENTRY	:= $(call cc-option, -mfentry -DCC_USING_FENTRY)
+endif
+KBUILD_CFLAGS	+= -pg $(CC_USING_FENTRY)
+KBUILD_AFLAGS	+= $(CC_USING_FENTRY)
 ifdef CONFIG_DYNAMIC_FTRACE
 	ifdef CONFIG_HAVE_C_RECORDMCOUNT
 		BUILD_C_RECORDMCOUNT := y
diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
index 8c4c070..9301a0e 100644
--- a/kernel/trace/Kconfig
+++ b/kernel/trace/Kconfig
@@ -49,6 +49,11 @@ config HAVE_SYSCALL_TRACEPOINTS
 	help
 	  See Documentation/trace/ftrace-design.txt
 
+config HAVE_FENTRY
+	bool
+	help
+	  Arch supports the gcc options -pg with -mfentry
+
 config HAVE_C_RECORDMCOUNT
 	bool
 	help

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [tip:perf/core] ftrace: Do not test frame pointers if -mfentry is used
  2012-08-07 19:38 ` [RFC PATCH 3/4] ftrace: Do not test frame pointers if -mfentry is used Steven Rostedt
  2012-08-08  4:34   ` Masami Hiramatsu
@ 2012-08-27 17:05   ` tip-bot for Steven Rostedt
  1 sibling, 0 replies; 25+ messages in thread
From: tip-bot for Steven Rostedt @ 2012-08-27 17:05 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: linux-kernel, hpa, mingo, andi, rostedt, srostedt, tglx, hpa

Commit-ID:  781d06248234e221edb560a18461d65808a8a942
Gitweb:     http://git.kernel.org/tip/781d06248234e221edb560a18461d65808a8a942
Author:     Steven Rostedt <srostedt@redhat.com>
AuthorDate: Wed, 9 Feb 2011 13:27:22 -0500
Committer:  Steven Rostedt <rostedt@goodmis.org>
CommitDate: Thu, 23 Aug 2012 11:25:29 -0400

ftrace: Do not test frame pointers if -mfentry is used

The function graph has a test to check if the frame pointer is
corrupted, which can happen with various options of gcc with mcount.
But this is not an issue with -mfentry as -mfentry does not need nor use
frame pointers for function graph tracing.

Link: http://lkml.kernel.org/r/20120807194059.773895870@goodmis.org

Acked-by: H. Peter Anvin <hpa@linux.intel.com>
Acked-by: Ingo Molnar <mingo@kernel.org>
Cc: Andi Kleen <andi@firstfloor.org>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 kernel/trace/trace_functions_graph.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/kernel/trace/trace_functions_graph.c b/kernel/trace/trace_functions_graph.c
index ce27c8b..99b4378 100644
--- a/kernel/trace/trace_functions_graph.c
+++ b/kernel/trace/trace_functions_graph.c
@@ -143,7 +143,7 @@ ftrace_pop_return_trace(struct ftrace_graph_ret *trace, unsigned long *ret,
 		return;
 	}
 
-#ifdef CONFIG_HAVE_FUNCTION_GRAPH_FP_TEST
+#if defined(CONFIG_HAVE_FUNCTION_GRAPH_FP_TEST) && !defined(CC_USING_FENTRY)
 	/*
 	 * The arch may choose to record the frame pointer used
 	 * and check it here to make sure that it is what we expect it
@@ -154,6 +154,9 @@ ftrace_pop_return_trace(struct ftrace_graph_ret *trace, unsigned long *ret,
 	 *
 	 * Currently, x86_32 with optimize for size (-Os) makes the latest
 	 * gcc do the above.
+	 *
+	 * Note, -mfentry does not use frame pointers, and this test
+	 *  is not needed if CC_USING_FENTRY is set.
 	 */
 	if (unlikely(current->ret_stack[index].fp != frame_pointer)) {
 		ftrace_graph_stop();

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [tip:perf/core] ftrace/x86: Add support for -mfentry to x86_64
  2012-08-07 19:38 ` [RFC PATCH 4/4] ftrace/x86: Add support for -mfentry to x86_64 Steven Rostedt
  2012-08-09  8:34   ` Masami Hiramatsu
  2012-08-09 13:46   ` Steven Rostedt
@ 2012-08-27 17:06   ` tip-bot for Steven Rostedt
  2 siblings, 0 replies; 25+ messages in thread
From: tip-bot for Steven Rostedt @ 2012-08-27 17:06 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: linux-kernel, hpa, mingo, andi, masami.hiramatsu.pt, rostedt,
	srostedt, tglx

Commit-ID:  d57c5d51a30152f3175d2344cb6395f08bf8ee0c
Gitweb:     http://git.kernel.org/tip/d57c5d51a30152f3175d2344cb6395f08bf8ee0c
Author:     Steven Rostedt <srostedt@redhat.com>
AuthorDate: Wed, 9 Feb 2011 13:32:18 -0500
Committer:  Steven Rostedt <rostedt@goodmis.org>
CommitDate: Thu, 23 Aug 2012 11:26:36 -0400

ftrace/x86: Add support for -mfentry to x86_64

If the kernel is compiled with gcc 4.6.0 which supports -mfentry,
then use that instead of mcount.

With mcount, frame pointers are forced with the -pg option and we
get something like:

<can_vma_merge_before>:
       55                      push   %rbp
       48 89 e5                mov    %rsp,%rbp
       53                      push   %rbx
       41 51                   push   %r9
       e8 fe 6a 39 00          callq  ffffffff81483d00 <mcount>
       31 c0                   xor    %eax,%eax
       48 89 fb                mov    %rdi,%rbx
       48 89 d7                mov    %rdx,%rdi
       48 33 73 30             xor    0x30(%rbx),%rsi
       48 f7 c6 ff ff ff f7    test   $0xfffffffff7ffffff,%rsi

With -mfentry, frame pointers are no longer forced and the call looks
like this:

<can_vma_merge_before>:
       e8 33 af 37 00          callq  ffffffff81461b40 <__fentry__>
       53                      push   %rbx
       48 89 fb                mov    %rdi,%rbx
       31 c0                   xor    %eax,%eax
       48 89 d7                mov    %rdx,%rdi
       41 51                   push   %r9
       48 33 73 30             xor    0x30(%rbx),%rsi
       48 f7 c6 ff ff ff f7    test   $0xfffffffff7ffffff,%rsi

This adds the ftrace hook at the beginning of the function before a
frame is set up, and allows the function callbacks to be able to access
parameters. As kprobes now can use function tracing (at least on x86)
this speeds up the kprobe hooks that are at the beginning of the
function.

Link: http://lkml.kernel.org/r/20120807194100.130477900@goodmis.org

Acked-by: Ingo Molnar <mingo@kernel.org>
Reviewed-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Andi Kleen <andi@firstfloor.org>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 arch/x86/Kconfig                 |    1 +
 arch/x86/include/asm/ftrace.h    |    7 ++++++-
 arch/x86/kernel/entry_64.S       |   32 +++++++++++++++++++++++++++-----
 arch/x86/kernel/x8664_ksyms_64.c |    6 +++++-
 4 files changed, 39 insertions(+), 7 deletions(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index a2d19ee..28dd891 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -36,6 +36,7 @@ config X86
 	select HAVE_KRETPROBES
 	select HAVE_OPTPROBES
 	select HAVE_FTRACE_MCOUNT_RECORD
+	select HAVE_FENTRY if X86_64
 	select HAVE_C_RECORDMCOUNT
 	select HAVE_DYNAMIC_FTRACE
 	select HAVE_FUNCTION_TRACER
diff --git a/arch/x86/include/asm/ftrace.h b/arch/x86/include/asm/ftrace.h
index a6cae0c..9a25b52 100644
--- a/arch/x86/include/asm/ftrace.h
+++ b/arch/x86/include/asm/ftrace.h
@@ -35,7 +35,11 @@
 #endif
 
 #ifdef CONFIG_FUNCTION_TRACER
-#define MCOUNT_ADDR		((long)(mcount))
+#ifdef CC_USING_FENTRY
+# define MCOUNT_ADDR		((long)(__fentry__))
+#else
+# define MCOUNT_ADDR		((long)(mcount))
+#endif
 #define MCOUNT_INSN_SIZE	5 /* sizeof mcount call */
 
 #ifdef CONFIG_DYNAMIC_FTRACE
@@ -46,6 +50,7 @@
 #ifndef __ASSEMBLY__
 extern void mcount(void);
 extern atomic_t modifying_ftrace_code;
+extern void __fentry__(void);
 
 static inline unsigned long ftrace_call_adjust(unsigned long addr)
 {
diff --git a/arch/x86/kernel/entry_64.S b/arch/x86/kernel/entry_64.S
index b7a81dc..ed767b7 100644
--- a/arch/x86/kernel/entry_64.S
+++ b/arch/x86/kernel/entry_64.S
@@ -68,10 +68,18 @@
 	.section .entry.text, "ax"
 
 #ifdef CONFIG_FUNCTION_TRACER
+
+#ifdef CC_USING_FENTRY
+# define function_hook	__fentry__
+#else
+# define function_hook	mcount
+#endif
+
 #ifdef CONFIG_DYNAMIC_FTRACE
-ENTRY(mcount)
+
+ENTRY(function_hook)
 	retq
-END(mcount)
+END(function_hook)
 
 /* skip is set if stack has been adjusted */
 .macro ftrace_caller_setup skip=0
@@ -84,7 +92,11 @@ END(mcount)
 	movq RIP(%rsp), %rdi
 	subq $MCOUNT_INSN_SIZE, %rdi
 	/* Load the parent_ip into the second parameter */
+#ifdef CC_USING_FENTRY
+	movq SS+16(%rsp), %rsi
+#else
 	movq 8(%rbp), %rsi
+#endif
 .endm
 
 ENTRY(ftrace_caller)
@@ -177,7 +189,8 @@ END(ftrace_regs_caller)
 
 
 #else /* ! CONFIG_DYNAMIC_FTRACE */
-ENTRY(mcount)
+
+ENTRY(function_hook)
 	cmpl $0, function_trace_stop
 	jne  ftrace_stub
 
@@ -199,7 +212,11 @@ trace:
 	MCOUNT_SAVE_FRAME
 
 	movq RIP(%rsp), %rdi
+#ifdef CC_USING_FENTRY
+	movq SS+16(%rsp), %rsi
+#else
 	movq 8(%rbp), %rsi
+#endif
 	subq $MCOUNT_INSN_SIZE, %rdi
 
 	call   *ftrace_trace_function
@@ -207,7 +224,7 @@ trace:
 	MCOUNT_RESTORE_FRAME
 
 	jmp ftrace_stub
-END(mcount)
+END(function_hook)
 #endif /* CONFIG_DYNAMIC_FTRACE */
 #endif /* CONFIG_FUNCTION_TRACER */
 
@@ -215,9 +232,14 @@ END(mcount)
 ENTRY(ftrace_graph_caller)
 	MCOUNT_SAVE_FRAME
 
+#ifdef CC_USING_FENTRY
+	leaq SS+16(%rsp), %rdi
+	movq $0, %rdx	/* No framepointers needed */
+#else
 	leaq 8(%rbp), %rdi
-	movq RIP(%rsp), %rsi
 	movq (%rbp), %rdx
+#endif
+	movq RIP(%rsp), %rsi
 	subq $MCOUNT_INSN_SIZE, %rsi
 
 	call	prepare_ftrace_return
diff --git a/arch/x86/kernel/x8664_ksyms_64.c b/arch/x86/kernel/x8664_ksyms_64.c
index 6020f6f..1330dd1 100644
--- a/arch/x86/kernel/x8664_ksyms_64.c
+++ b/arch/x86/kernel/x8664_ksyms_64.c
@@ -13,9 +13,13 @@
 #include <asm/ftrace.h>
 
 #ifdef CONFIG_FUNCTION_TRACER
-/* mcount is defined in assembly */
+/* mcount and __fentry__ are defined in assembly */
+#ifdef CC_USING_FENTRY
+EXPORT_SYMBOL(__fentry__);
+#else
 EXPORT_SYMBOL(mcount);
 #endif
+#endif
 
 EXPORT_SYMBOL(__get_user_1);
 EXPORT_SYMBOL(__get_user_2);

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [RFC][PATCH 4/4] ftrace/x86: Add support for -mfentry to x86_64
  2011-02-09 20:02 [RFC][PATCH 0/4] ftrace: Use -mfentry when supported (this is for x86_64 right now) Steven Rostedt
@ 2011-02-09 20:02 ` Steven Rostedt
  0 siblings, 0 replies; 25+ messages in thread
From: Steven Rostedt @ 2011-02-09 20:02 UTC (permalink / raw)
  To: linux-kernel
  Cc: Ingo Molnar, Andrew Morton, Thomas Gleixner, Frederic Weisbecker,
	H. Peter Anvin, Mathieu Desnoyers, Andi Kleen, Masami Hiramatsu

[-- Attachment #1: 0004-ftrace-x86-Add-support-for-mfentry-to-x86_64.patch --]
[-- Type: text/plain, Size: 2913 bytes --]

From: Steven Rostedt <srostedt@redhat.com>

If the kernel is compiled with gcc 4.6.0 which supports -mfentry,
then use that instead of mcount.

Cc: Andi Kleen <andi@firstfloor.org>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 arch/x86/Kconfig                 |    1 +
 arch/x86/include/asm/ftrace.h    |    7 ++++++-
 arch/x86/kernel/entry_64.S       |   17 ++++++++++++++++-
 arch/x86/kernel/x8664_ksyms_64.c |    6 +++++-
 4 files changed, 28 insertions(+), 3 deletions(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index d5ed94d..ac1a47e 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -32,6 +32,7 @@ config X86
 	select HAVE_KRETPROBES
 	select HAVE_OPTPROBES
 	select HAVE_FTRACE_MCOUNT_RECORD
+	select HAVE_FENTRY if X86_64
 	select HAVE_C_RECORDMCOUNT
 	select HAVE_DYNAMIC_FTRACE
 	select HAVE_FUNCTION_TRACER
diff --git a/arch/x86/include/asm/ftrace.h b/arch/x86/include/asm/ftrace.h
index db24c22..b1386d8 100644
--- a/arch/x86/include/asm/ftrace.h
+++ b/arch/x86/include/asm/ftrace.h
@@ -29,11 +29,16 @@
 #endif
 
 #ifdef CONFIG_FUNCTION_TRACER
-#define MCOUNT_ADDR		((long)(mcount))
+#ifdef CC_HAS_FENTRY
+# define MCOUNT_ADDR		((long)(__fentry__))
+#else
+# define MCOUNT_ADDR		((long)(mcount))
+#endif
 #define MCOUNT_INSN_SIZE	5 /* sizeof mcount call */
 
 #ifndef __ASSEMBLY__
 extern void mcount(void);
+extern void __fentry__(void);
 
 static inline unsigned long ftrace_call_adjust(unsigned long addr)
 {
diff --git a/arch/x86/kernel/entry_64.S b/arch/x86/kernel/entry_64.S
index aed1ffb..44031ee 100644
--- a/arch/x86/kernel/entry_64.S
+++ b/arch/x86/kernel/entry_64.S
@@ -63,9 +63,16 @@
 	.code64
 #ifdef CONFIG_FUNCTION_TRACER
 #ifdef CONFIG_DYNAMIC_FTRACE
+
+#ifdef CC_HAS_FENTRY
+ENTRY(__fentry__)
+	retq
+END(__fentry__)
+#else
 ENTRY(mcount)
 	retq
 END(mcount)
+#endif
 
 ENTRY(ftrace_caller)
 	cmpl $0, function_trace_stop
@@ -74,7 +81,11 @@ ENTRY(ftrace_caller)
 	MCOUNT_SAVE_FRAME
 
 	movq 0x38(%rsp), %rdi
+#ifdef CC_HAS_FENTRY
+	movq 0x40(%rsp), %rsi
+#else
 	movq 8(%rbp), %rsi
+#endif
 	subq $MCOUNT_INSN_SIZE, %rdi
 
 GLOBAL(ftrace_call)
@@ -133,9 +144,13 @@ ENTRY(ftrace_graph_caller)
 
 	MCOUNT_SAVE_FRAME
 
+#ifdef CC_HAS_FENTRY
+	leaq 0x40(%rsp), %rdi
+#else
 	leaq 8(%rbp), %rdi
-	movq 0x38(%rsp), %rsi
 	movq (%rbp), %rdx
+#endif
+	movq 0x38(%rsp), %rsi
 	subq $MCOUNT_INSN_SIZE, %rsi
 
 	call	prepare_ftrace_return
diff --git a/arch/x86/kernel/x8664_ksyms_64.c b/arch/x86/kernel/x8664_ksyms_64.c
index 1b950d1..7fa3a78 100644
--- a/arch/x86/kernel/x8664_ksyms_64.c
+++ b/arch/x86/kernel/x8664_ksyms_64.c
@@ -13,9 +13,13 @@
 #include <asm/ftrace.h>
 
 #ifdef CONFIG_FUNCTION_TRACER
-/* mcount is defined in assembly */
+/* mcount and __fentry__ are defined in assembly */
+#ifdef CC_HAS_FENTRY
+EXPORT_SYMBOL(__fentry__);
+#else
 EXPORT_SYMBOL(mcount);
 #endif
+#endif
 
 EXPORT_SYMBOL(__get_user_1);
 EXPORT_SYMBOL(__get_user_2);
-- 
1.7.2.3



^ permalink raw reply related	[flat|nested] 25+ messages in thread

end of thread, other threads:[~2012-08-27 17:06 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-08-07 19:38 [RFC PATCH 0/4] ftrace: Add use of -mfentry for x86_64 Steven Rostedt
2012-08-07 19:38 ` [RFC PATCH 1/4] ftrace: Make recordmcount.c handle __fentry__ Steven Rostedt
2012-08-07 23:57   ` John Reiser
2012-08-08  0:05     ` Steven Rostedt
2012-08-27 17:03   ` [tip:perf/core] " tip-bot for Steven Rostedt
2012-08-07 19:38 ` [RFC PATCH 2/4] ftrace: Add -mfentry to Makefile on function tracer Steven Rostedt
2012-08-27 17:04   ` [tip:perf/core] " tip-bot for Steven Rostedt
2012-08-07 19:38 ` [RFC PATCH 3/4] ftrace: Do not test frame pointers if -mfentry is used Steven Rostedt
2012-08-08  4:34   ` Masami Hiramatsu
2012-08-08 12:49     ` Steven Rostedt
2012-08-09  2:58       ` Masami Hiramatsu
2012-08-09  3:45       ` Linus Torvalds
2012-08-09  3:57         ` Steven Rostedt
2012-08-09  4:15         ` H. Peter Anvin
2012-08-09 12:37         ` Andi Kleen
2012-08-27 17:05   ` [tip:perf/core] " tip-bot for Steven Rostedt
2012-08-07 19:38 ` [RFC PATCH 4/4] ftrace/x86: Add support for -mfentry to x86_64 Steven Rostedt
2012-08-09  8:34   ` Masami Hiramatsu
2012-08-09 13:46   ` Steven Rostedt
2012-08-09 13:48     ` Steven Rostedt
2012-08-10  7:45     ` Masami Hiramatsu
2012-08-27 17:06   ` [tip:perf/core] " tip-bot for Steven Rostedt
2012-08-07 20:23 ` [RFC PATCH 0/4] ftrace: Add use of -mfentry for x86_64 H. Peter Anvin
2012-08-13  8:42   ` Ingo Molnar
  -- strict thread matches above, loose matches on Subject: below --
2011-02-09 20:02 [RFC][PATCH 0/4] ftrace: Use -mfentry when supported (this is for x86_64 right now) Steven Rostedt
2011-02-09 20:02 ` [RFC][PATCH 4/4] ftrace/x86: Add support for -mfentry to x86_64 Steven Rostedt

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.