All of lore.kernel.org
 help / color / mirror / Atom feed
From: Steven Rostedt <rostedt@goodmis.org>
To: linux-kernel@vger.kernel.org, "H. Peter Anvin" <hpa@linux.intel.com>
Cc: Ingo Molnar <mingo@elte.hu>,
	Andrew Morton <akpm@linux-foundation.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Frederic Weisbecker <fweisbec@gmail.com>,
	Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Andi Kleen <andi@firstfloor.org>
Subject: Re: [RFC PATCH 4/4] ftrace/x86: Add support for -mfentry to x86_64
Date: Thu, 09 Aug 2012 09:46:10 -0400	[thread overview]
Message-ID: <1344519970.6935.34.camel@gandalf.stny.rr.com> (raw)
In-Reply-To: <20120807194100.130477900@goodmis.org>

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);



  parent reply	other threads:[~2012-08-09 13:46 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1344519970.6935.34.camel@gandalf.stny.rr.com \
    --to=rostedt@goodmis.org \
    --cc=akpm@linux-foundation.org \
    --cc=andi@firstfloor.org \
    --cc=fweisbec@gmail.com \
    --cc=hpa@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=masami.hiramatsu.pt@hitachi.com \
    --cc=mingo@elte.hu \
    --cc=tglx@linutronix.de \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.