All of lore.kernel.org
 help / color / mirror / Atom feed
From: Steven Rostedt <rostedt@goodmis.org>
To: linux-kernel@vger.kernel.org
Cc: Ingo Molnar <mingo@kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Peter Zijlstra <peterz@infradead.org>,
	Masami Hiramatsu <mhiramat@kernel.org>,
	"H. Peter Anvin" <hpa@zytor.com>,
	Andy Lutomirski <luto@amacapital.net>,
	Josh Poimboeuf <jpoimboe@redhat.com>,
	Linus Torvalds <torvalds@linux-foundation.org>
Subject: [PATCH 5/6 v3] ftrace/x86_32: Add -mfentry support to x86_32 with DYNAMIC_FTRACE set
Date: Sat, 18 Mar 2017 17:09:28 -0400	[thread overview]
Message-ID: <20170318211149.565075180@goodmis.org> (raw)
In-Reply-To: 20170318210923.814509382@goodmis.org

[-- Attachment #1: 0005-ftrace-x86_32-Add-mfentry-support-to-x86_32-with-DYN.patch --]
[-- Type: text/plain, Size: 5127 bytes --]

From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>

x86_64 has had fentry support for some time. I did not add support to x86_32
as I was unsure if it will be used much in the future. It is still very much
used, and there's issues with function graph tracing with gcc playing around
with the mcount frames, causing function graph to panic. The fentry code
does not have this issue, and is able to cope as there is no frame to mess
up.

Note, this only add support for fentry when DYNAMIC_FTRACE is set. There's
really no reason to not have that set, because the performance of the
machine drops significantly when it's not enabled. I only keep
!DYNAMIC_FTRACE around to test it off, as there's still some archs that have
FTRACE but not DYNAMIC_FTRACE.

Reviewed-by: Masami Hiramatsu <mhiramat@kernel.org>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 arch/x86/Kconfig            |  2 +-
 arch/x86/kernel/ftrace_32.S | 82 +++++++++++++++++++++++++++++++++++++++------
 2 files changed, 73 insertions(+), 11 deletions(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index cc98d5a294ee..8c17146427ca 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -127,7 +127,7 @@ config X86
 	select HAVE_EBPF_JIT			if X86_64
 	select HAVE_EFFICIENT_UNALIGNED_ACCESS
 	select HAVE_EXIT_THREAD
-	select HAVE_FENTRY			if X86_64
+	select HAVE_FENTRY			if X86_64 || DYNAMIC_FTRACE
 	select HAVE_FTRACE_MCOUNT_RECORD
 	select HAVE_FUNCTION_GRAPH_TRACER
 	select HAVE_FUNCTION_TRACER
diff --git a/arch/x86/kernel/ftrace_32.S b/arch/x86/kernel/ftrace_32.S
index 7cd27cf56578..fe64fa596266 100644
--- a/arch/x86/kernel/ftrace_32.S
+++ b/arch/x86/kernel/ftrace_32.S
@@ -11,26 +11,68 @@
 #include <asm/ftrace.h>
 
 #ifdef CONFIG_FUNCTION_TRACER
+
+#ifdef CC_USING_FENTRY
+# define function_hook	__fentry__
+EXPORT_SYMBOL(__fentry__)
+#else
+# define function_hook	mcount
+EXPORT_SYMBOL(mcount)
+#endif
+
 #ifdef CONFIG_DYNAMIC_FTRACE
 
-ENTRY(mcount)
+/* mcount uses a frame pointer even if CONFIG_FRAME_POINTER is not set */
+#if !defined(CC_USING_FENTRY) || defined(CONFIG_FRAME_POINTER)
+# define USING_FRAME_POINTER
+#endif
+
+#ifdef USING_FRAME_POINTER
+# define MCOUNT_FRAME			1	/* using frame = true  */
+#else
+# define MCOUNT_FRAME			0	/* using frame = false */
+#endif
+
+ENTRY(function_hook)
 	ret
-END(mcount)
+END(function_hook)
 
 ENTRY(ftrace_caller)
 
+#ifdef USING_FRAME_POINTER
+# ifdef CC_USING_FENTRY
+	/*
+	 * Frame pointers are of ip followed by bp.
+	 * Since fentry is an immediate jump, we are left with
+	 * parent-ip, function-ip. We need to add a frame with
+	 * parent-ip followed by ebp.
+	 */
+	pushl	4(%esp)				/* parent ip */
 	pushl	%ebp
 	movl	%esp, %ebp
-
+	pushl	2*4(%esp)			/* function ip */
+# endif
+	/* For mcount, the function ip is directly above */
+	pushl	%ebp
+	movl	%esp, %ebp
+#endif
 	pushl	%eax
 	pushl	%ecx
 	pushl	%edx
 	pushl	$0				/* Pass NULL as regs pointer */
-	movl	5*4(%esp), %eax
-	/* Copy original ebp into %edx */
+
+#ifdef USING_FRAME_POINTER
+	/* Load parent ebp into edx */
 	movl	4*4(%esp), %edx
+#else
+	/* There's no frame pointer, load the appropriate stack addr instead */
+	lea	4*4(%esp), %edx
+#endif
+
+	movl	(MCOUNT_FRAME+4)*4(%esp), %eax	/* load the rip */
 	/* Get the parent ip */
-	movl	0x4(%edx), %edx
+	movl	4(%edx), %edx			/* edx has ebp */
+
 	movl	function_trace_op, %ecx
 	subl	$MCOUNT_INSN_SIZE, %eax
 
@@ -42,7 +84,14 @@ ftrace_call:
 	popl	%edx
 	popl	%ecx
 	popl	%eax
+#ifdef USING_FRAME_POINTER
 	popl	%ebp
+# ifdef CC_USING_FENTRY
+	addl	$4,%esp				/* skip function ip */
+	popl	%ebp				/* this is the orig bp */
+	addl	$4, %esp			/* skip parent ip */
+# endif
+#endif
 .Lftrace_ret:
 #ifdef CONFIG_FUNCTION_GRAPH_TRACER
 .globl ftrace_graph_call
@@ -83,6 +132,10 @@ ENTRY(ftrace_regs_caller)
 	pushl	%edx
 	pushl	%ecx
 	pushl	%ebx
+#ifdef CC_USING_FENTRY
+	/* Load 4 off of the parent ip addr into ebp */
+	lea	14*4(%esp), %ebp
+#endif
 
 	movl	12*4(%esp), %eax		/* Load ip (1st parameter) */
 	subl	$MCOUNT_INSN_SIZE, %eax		/* Adjust ip */
@@ -121,7 +174,7 @@ GLOBAL(ftrace_regs_call)
 	jmp	.Lftrace_ret
 #else /* ! CONFIG_DYNAMIC_FTRACE */
 
-ENTRY(mcount)
+ENTRY(function_hook)
 	cmpl	$__PAGE_OFFSET, %esp
 	jb	ftrace_stub			/* Paging not enabled yet? */
 
@@ -153,9 +206,8 @@ ftrace_stub:
 	popl	%ecx
 	popl	%eax
 	jmp	ftrace_stub
-END(mcount)
+END(function_hook)
 #endif /* CONFIG_DYNAMIC_FTRACE */
-EXPORT_SYMBOL(mcount)
 #endif /* CONFIG_FUNCTION_TRACER */
 
 #ifdef CONFIG_FUNCTION_GRAPH_TRACER
@@ -163,9 +215,15 @@ ENTRY(ftrace_graph_caller)
 	pushl	%eax
 	pushl	%ecx
 	pushl	%edx
-	movl	0xc(%esp), %eax
+	movl	3*4(%esp), %eax
+	/* Even with frame pointers, fentry doesn't have one here */
+#ifdef CC_USING_FENTRY
+	lea	4*4(%esp), %edx
+	movl	$0, %ecx
+#else
 	lea	0x4(%ebp), %edx
 	movl	(%ebp), %ecx
+#endif
 	subl	$MCOUNT_INSN_SIZE, %eax
 	call	prepare_ftrace_return
 	popl	%edx
@@ -178,7 +236,11 @@ END(ftrace_graph_caller)
 return_to_handler:
 	pushl	%eax
 	pushl	%edx
+#ifdef CC_USING_FENTRY
+	movl	$0, %eax
+#else
 	movl	%ebp, %eax
+#endif
 	call	ftrace_return_to_handler
 	movl	%eax, %ecx
 	popl	%edx
-- 
2.10.2

  parent reply	other threads:[~2017-03-18 21:13 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-03-18 21:09 [PATCH 0/6 v3] [GIT PULL] ftrace/x86: Ftrace cleanup and add support for -mfentry on x86_32 Steven Rostedt
2017-03-18 21:09 ` [PATCH 1/6 v3] ftrace/x86_64: Rename mcount_64.S to ftrace_64.S Steven Rostedt
2017-03-18 21:09 ` [PATCH 2/6 v3] ftrace/x86_32: Move the ftrace specific code out of entry_32.S Steven Rostedt
2017-03-18 21:09 ` [PATCH 3/6 v3] ftrace/x86_32: Add stack frame pointer to ftrace_caller Steven Rostedt
2017-03-18 21:09 ` [PATCH 4/6 v3] ftrace/x86_32: Clean up ftrace_regs_caller Steven Rostedt
2017-03-20 14:01   ` Josh Poimboeuf
2017-03-18 21:09 ` Steven Rostedt [this message]
2017-03-20 14:02   ` [PATCH 5/6 v3] ftrace/x86_32: Add -mfentry support to x86_32 with DYNAMIC_FTRACE set Josh Poimboeuf
2017-03-18 21:09 ` [PATCH 6/6 v3] ftrace/x86: Use Makefile logic instead of #ifdef of compling ftrace_*.o Steven Rostedt
2017-03-20 14:04   ` Josh Poimboeuf
2017-03-22  0:56     ` 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=20170318211149.565075180@goodmis.org \
    --to=rostedt@goodmis.org \
    --cc=akpm@linux-foundation.org \
    --cc=hpa@zytor.com \
    --cc=jpoimboe@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luto@amacapital.net \
    --cc=mhiramat@kernel.org \
    --cc=mingo@kernel.org \
    --cc=peterz@infradead.org \
    --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.