[PATCH 0/6 v3] [GIT PULL] ftrace/x86: Ftrace cleanup and add support for -mfentry on x86_32

* [PATCH 0/6 v3] [GIT PULL] ftrace/x86: Ftrace cleanup and add support for -mfentry on x86_32
@ 2017-03-18 21:09 Steven Rostedt
  2017-03-18 21:09 ` [PATCH 1/6 v3] ftrace/x86_64: Rename mcount_64.S to ftrace_64.S Steven Rostedt
                   ` (5 more replies)
  0 siblings, 6 replies; 11+ messages in thread
From: Steven Rostedt @ 2017-03-18 21:09 UTC (permalink / raw)
  To: linux-kernel
  Cc: Ingo Molnar, Andrew Morton, Thomas Gleixner, Peter Zijlstra,
	Masami Hiramatsu, H. Peter Anvin, Andy Lutomirski,
	Josh Poimboeuf, Linus Torvalds


[
 Ingo, Thomas or Peter,

 I believe this is all set to go now. I updated those patches that Linus
 commented on and I don't believe there are any more issues. I ran this
 through several tests (although some of my tests are failing due to
 bugs introduced by others in 4.11-rc2). You can take this as a patch
 series, or you can pull from my tree defined below. It's based on 4.11-rc2
 as I noticed that tip/x86/core is rather outdated, and Linus is fine with
 basing off of his tagged releases.
]

With the issues of gcc screwing around with the mcount stack frame causing
function graph tracer to panic on x86_32, and with Linus saying that we
should start deprecating mcount (at least on x86), I figured that x86_32
needs to support fentry.

First, I renamed mcount_64.S to ftrace_64.S. As we want to get away from
mcount, having the ftrace code in a file called mcount seems rather backwards.

Next I moved the ftrace code out of entry_32.S. It's not in entry_64.S
and it does not belong in entry_32.S.

I noticed that the x86_32 code has the same issue as the x86_64 did
in the past with respect to a stack frame. I fixed that just for the main
ftrace_caller. The ftrace_regs_caller is rather special, and so is
function graph tracing.

I realized the ftrace_regs_caller code was complex due to me aggressively
saving flags, even though I could still do push, lea and mov without
changing them. That made the logic a little nicer.

Finally I added the fentry code.

I tested this with an older compiler (for mcount) with and without
FRAME_POINTER set. I also did it with a new compiler (with fentry), with and
without FRAME_POINTER. I tested function tracing, stack tracing, function_graph
tracing, and kprobes (as that uses the ftrace_regs_caller).

Please pull (or take the patch series) from:

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

Head SHA1: 7506fa66bcd267c587b43d39f9fc9acb48925b5a


Steven Rostedt (VMware) (6):
      ftrace/x86_64: Rename mcount_64.S to ftrace_64.S
      ftrace/x86_32: Move the ftrace specific code out of entry_32.S
      ftrace/x86_32: Add stack frame pointer to ftrace_caller
      ftrace/x86_32: Clean up ftrace_regs_caller
      ftrace/x86_32: Add -mfentry support to x86_32 with DYNAMIC_FTRACE set
      ftrace/x86: Use Makefile logic instead of #ifdef of compling ftrace_*.o

----
Changes from v2:

 * removed accidental committing of asm goto code from someone else

 * Better code for ftrace_regs_caller (Linus Torvalds)

 * Added patch to only compile ftrace_*.o if FUNCTION_TRACER is defined
    (Josh Poimboeuf)

 * Fixed comment whitespace issue (Peter Zijlstra)


 arch/x86/Kconfig                             |   2 +-
 arch/x86/entry/entry_32.S                    | 169 ------------------
 arch/x86/kernel/Makefile                     |   5 +-
 arch/x86/kernel/ftrace_32.S                  | 246 +++++++++++++++++++++++++++
 arch/x86/kernel/{mcount_64.S => ftrace_64.S} |   4 -
 5 files changed, 250 insertions(+), 176 deletions(-)
 create mode 100644 arch/x86/kernel/ftrace_32.S
 rename arch/x86/kernel/{mcount_64.S => ftrace_64.S} (99%)

Diff against v2:

diff --git a/Makefile b/Makefile
index 7df3247..b841fb3 100644
--- a/Makefile
+++ b/Makefile
@@ -653,12 +653,6 @@ KBUILD_CFLAGS += $(call cc-ifversion, -lt, 0409, \
 # Tell gcc to never replace conditional load with a non-conditional one
 KBUILD_CFLAGS	+= $(call cc-option,--param=allow-store-data-races=0)
 
-# check for 'asm goto'
-ifeq ($(shell $(CONFIG_SHELL) $(srctree)/scripts/gcc-goto.sh $(CC) $(KBUILD_CFLAGS)), y)
-	KBUILD_CFLAGS += -DCC_HAVE_ASM_GOTO
-	KBUILD_AFLAGS += -DCC_HAVE_ASM_GOTO
-endif
-
 include scripts/Makefile.gcc-plugins
 
 ifdef CONFIG_READABLE_ASM
@@ -804,6 +798,12 @@ KBUILD_CFLAGS   += $(call cc-option,-Werror=incompatible-pointer-types)
 # use the deterministic mode of AR if available
 KBUILD_ARFLAGS := $(call ar-option,D)
 
+# check for 'asm goto'
+ifeq ($(shell $(CONFIG_SHELL) $(srctree)/scripts/gcc-goto.sh $(CC) $(KBUILD_CFLAGS)), y)
+	KBUILD_CFLAGS += -DCC_HAVE_ASM_GOTO
+	KBUILD_AFLAGS += -DCC_HAVE_ASM_GOTO
+endif
+
 include scripts/Makefile.kasan
 include scripts/Makefile.extrawarn
 include scripts/Makefile.ubsan
diff --git a/arch/x86/entry/entry_32.S b/arch/x86/entry/entry_32.S
index c576352..169b3b0 100644
--- a/arch/x86/entry/entry_32.S
+++ b/arch/x86/entry/entry_32.S
@@ -35,7 +35,6 @@
 #include <asm/errno.h>
 #include <asm/segment.h>
 #include <asm/smp.h>
-#include <asm/page_types.h>
 #include <asm/percpu.h>
 #include <asm/processor-flags.h>
 #include <asm/irq_vectors.h>
diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
index 55e8902..4b99423 100644
--- a/arch/x86/kernel/Makefile
+++ b/arch/x86/kernel/Makefile
@@ -46,8 +46,7 @@ obj-$(CONFIG_MODIFY_LDT_SYSCALL)	+= ldt.o
 obj-y			+= setup.o x86_init.o i8259.o irqinit.o jump_label.o
 obj-$(CONFIG_IRQ_WORK)  += irq_work.o
 obj-y			+= probe_roms.o
-obj-$(CONFIG_X86_64)	+= sys_x86_64.o ftrace_64.o
-obj-$(CONFIG_X86_32)	+= ftrace_32.o
+obj-$(CONFIG_X86_64)	+= sys_x86_64.o
 obj-$(CONFIG_X86_ESPFIX64)	+= espfix_64.o
 obj-$(CONFIG_SYSFS)	+= ksysfs.o
 obj-y			+= bootflag.o e820.o
@@ -83,6 +82,7 @@ obj-y				+= apic/
 obj-$(CONFIG_X86_REBOOTFIXUPS)	+= reboot_fixups_32.o
 obj-$(CONFIG_DYNAMIC_FTRACE)	+= ftrace.o
 obj-$(CONFIG_LIVEPATCH)	+= livepatch.o
+obj-$(CONFIG_FUNCTION_TRACER)	+= ftrace_$(BITS).o
 obj-$(CONFIG_FUNCTION_GRAPH_TRACER) += ftrace.o
 obj-$(CONFIG_FTRACE_SYSCALLS)	+= ftrace.o
 obj-$(CONFIG_X86_TSC)		+= trace_clock.o
diff --git a/arch/x86/kernel/ftrace_32.S b/arch/x86/kernel/ftrace_32.S
index 7ed4137..4d52e0d 100644
--- a/arch/x86/kernel/ftrace_32.S
+++ b/arch/x86/kernel/ftrace_32.S
@@ -5,6 +5,7 @@
  */
 
 #include <linux/linkage.h>
+#include <asm/page_types.h>
 #include <asm/segment.h>
 #include <asm/export.h>
 #include <asm/ftrace.h>
@@ -17,6 +18,8 @@ EXPORT_SYMBOL(__fentry__)
 EXPORT_SYMBOL(mcount)
 #endif
 
+#ifdef CONFIG_DYNAMIC_FTRACE
+
 /* 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
@@ -28,9 +31,6 @@ EXPORT_SYMBOL(mcount)
 # define MCOUNT_FRAME			0	/* using frame = false */
 #endif
 
-#ifdef CONFIG_FUNCTION_TRACER
-#ifdef CONFIG_DYNAMIC_FTRACE
-
 ENTRY(function_hook)
 	ret
 END(function_hook)
@@ -109,23 +109,20 @@ ENTRY(ftrace_regs_caller)
 	 * Unfortunately, that means eflags must be at the same location
 	 * as the current return ip is. We move the return ip into the
 	 * regs->ip location, and move flags into the return ip location.
-         */
+	 */
 	pushl	$__KERNEL_CS
 	pushl	4(%esp)				/* Save the return ip */
-
-	/* temporarily save flags in the orig_ax location */
-	pushf
-
+	pushl	$0				/* Load 0 into orig_ax */
 	pushl	%gs
 	pushl	%fs
 	pushl	%es
 	pushl	%ds
 	pushl	%eax
 
-	/* move flags into the location of where the return ip was */
-	movl	5*4(%esp), %eax
-	movl	$0, 5*4(%esp)			/* Load 0 into orig_ax */
-	movl	%eax, 8*4(%esp)			/* Load flags in return ip */
+	/* Get flags and place them into the return ip slot */
+	pushf
+	popl	%eax
+	movl	%eax, 8*4(%esp)
 
 	pushl	%ebp
 	pushl	%edi
@@ -149,9 +146,9 @@ GLOBAL(ftrace_regs_call)
 
 	addl	$4, %esp			/* Skip pt_regs */
 
-	/* Since we don't care about cs, move flags there to simplify return */
-	movl	14*4(%esp), %eax
-	movl	%eax, 13*4(%esp)
+	/* restore flags */
+	push	14*4(%esp)
+	popf
 
 	/* Move return ip back to its original location */
 	movl	12*4(%esp), %eax
@@ -168,9 +165,9 @@ GLOBAL(ftrace_regs_call)
 	popl	%es
 	popl	%fs
 	popl	%gs
-	addl	$8, %esp			/* Skip orig_ax and old ip */
 
-	popf					/* flags is in the cs location */
+	/* use lea to not affect flags */
+	lea	3*4(%esp), %esp			/* Skip orig_ax, ip and flags */
 
 	jmp	.Lftrace_ret
 #else /* ! CONFIG_DYNAMIC_FTRACE */
@@ -209,7 +206,6 @@ ftrace_stub:
 	jmp	ftrace_stub
 END(function_hook)
 #endif /* CONFIG_DYNAMIC_FTRACE */
-#endif /* CONFIG_FUNCTION_TRACER */
 
 #ifdef CONFIG_FUNCTION_GRAPH_TRACER
 ENTRY(ftrace_graph_caller)
diff --git a/arch/x86/kernel/ftrace_64.S b/arch/x86/kernel/ftrace_64.S
index 7b0d3da..b9c4691 100644
--- a/arch/x86/kernel/ftrace_64.S
+++ b/arch/x86/kernel/ftrace_64.S
@@ -13,9 +13,6 @@
 	.code64
 	.section .entry.text, "ax"
 
-
-#ifdef CONFIG_FUNCTION_TRACER
-
 #ifdef CC_USING_FENTRY
 # define function_hook	__fentry__
 EXPORT_SYMBOL(__fentry__)
@@ -297,7 +294,6 @@ trace:
 	jmp fgraph_trace
 END(function_hook)
 #endif /* CONFIG_DYNAMIC_FTRACE */
-#endif /* CONFIG_FUNCTION_TRACER */
 
 #ifdef CONFIG_FUNCTION_GRAPH_TRACER
 ENTRY(ftrace_graph_caller)

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