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

From: Steven Rostedt
Date: Sat Mar 18 2017 - 17:13:27 EST



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