* [PATCH 0/5 v2] ftrace/x86_32: Ftrace cleanup and add support for -mfentry
@ 2017-03-16 17:20 Steven Rostedt
2017-03-16 17:20 ` [PATCH 1/5 v2] x86/ftrace: Rename mcount_64.S to ftrace_64.S Steven Rostedt
` (4 more replies)
0 siblings, 5 replies; 37+ messages in thread
From: Steven Rostedt @ 2017-03-16 17:20 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
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).
Steven Rostedt (VMware) (5):
x86/ftrace: 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
----
Makefile | 12 +-
arch/x86/Kconfig | 2 +-
arch/x86/entry/entry_32.S | 168 ------------------
arch/x86/kernel/Makefile | 5 +-
arch/x86/kernel/ftrace_32.S | 250 +++++++++++++++++++++++++++
arch/x86/kernel/{mcount_64.S => ftrace_64.S} | 0
6 files changed, 260 insertions(+), 177 deletions(-)
create mode 100644 arch/x86/kernel/ftrace_32.S
rename arch/x86/kernel/{mcount_64.S => ftrace_64.S} (100%)
Changes from v1:
Removed MCOUNT_FRAME_SIZE macro, as it is not used.
(Masami Hiramatsu)
Fixed vertical alignment of args (Josh Poimboeuf)
s/ndef USING_FRAME_POINTER/def CC_USING_FENTRY/ (Josh Poimboeuf)
Fixed adding $0 into orig_ax and not ax. (Josh Poimboeuf)
Updated some comments.
Here's the diff of v1 to v2:
diff --git a/arch/x86/kernel/ftrace_32.S b/arch/x86/kernel/ftrace_32.S
index 4bf8223..7ed4137 100644
--- a/arch/x86/kernel/ftrace_32.S
+++ b/arch/x86/kernel/ftrace_32.S
@@ -9,7 +9,6 @@
#include <asm/export.h>
#include <asm/ftrace.h>
-
#ifdef CC_USING_FENTRY
# define function_hook __fentry__
EXPORT_SYMBOL(__fentry__)
@@ -24,14 +23,8 @@ EXPORT_SYMBOL(mcount)
#endif
#ifdef USING_FRAME_POINTER
-# ifdef CC_USING_FENTRY
-# define MCOUNT_FRAME_SIZE (4*4) /* bp,ip and parent's */
-# else
-# define MCOUNT_FRAME_SIZE 4 /* just the bp */
-# endif
# define MCOUNT_FRAME 1 /* using frame = true */
#else
-# define MCOUNT_FRAME_SIZE 0 /* no stack frame */
# define MCOUNT_FRAME 0 /* using frame = false */
#endif
@@ -52,14 +45,14 @@ ENTRY(ftrace_caller)
* 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 */
+ 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
+ pushl %ebp
+ movl %esp, %ebp
#endif
pushl %eax
pushl %ecx
@@ -131,8 +124,8 @@ ENTRY(ftrace_regs_caller)
/* move flags into the location of where the return ip was */
movl 5*4(%esp), %eax
- movl $0, (%esp) /* Load 0 into orig_ax */
- movl %eax, 8*4(%esp)
+ movl $0, 5*4(%esp) /* Load 0 into orig_ax */
+ movl %eax, 8*4(%esp) /* Load flags in return ip */
pushl %ebp
pushl %edi
@@ -140,7 +133,7 @@ ENTRY(ftrace_regs_caller)
pushl %edx
pushl %ecx
pushl %ebx
-#ifndef USING_FRAME_POINTER
+#ifdef CC_USING_FENTRY
/* Load 4 off of the parent ip addr into ebp */
lea 14*4(%esp), %ebp
#endif
@@ -160,7 +153,7 @@ GLOBAL(ftrace_regs_call)
movl 14*4(%esp), %eax
movl %eax, 13*4(%esp)
- /* Move ip back to its original location */
+ /* Move return ip back to its original location */
movl 12*4(%esp), %eax
movl %eax, 14*4(%esp)
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [PATCH 1/5 v2] x86/ftrace: Rename mcount_64.S to ftrace_64.S
2017-03-16 17:20 [PATCH 0/5 v2] ftrace/x86_32: Ftrace cleanup and add support for -mfentry Steven Rostedt
@ 2017-03-16 17:20 ` Steven Rostedt
2017-03-16 17:20 ` [PATCH 2/5 v2] ftrace/x86-32: Move the ftrace specific code out of entry_32.S Steven Rostedt
` (3 subsequent siblings)
4 siblings, 0 replies; 37+ messages in thread
From: Steven Rostedt @ 2017-03-16 17:20 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
[-- Attachment #1: 0001-x86-ftrace-Rename-mcount_64.S-to-ftrace_64.S.patch --]
[-- Type: text/plain, Size: 1673 bytes --]
From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
With the advent of -mfentry that uses the new "fentry" hook over mcount, the
mcount name is obsolete. Having the code file that ftrace hooks into called
"mcount*.S" is rather misleading. Rename it to ftrace_64.S
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
arch/x86/kernel/Makefile | 4 ++--
arch/x86/kernel/{mcount_64.S => ftrace_64.S} | 0
2 files changed, 2 insertions(+), 2 deletions(-)
rename arch/x86/kernel/{mcount_64.S => ftrace_64.S} (100%)
diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
index 84c00592d359..d3743a37e990 100644
--- a/arch/x86/kernel/Makefile
+++ b/arch/x86/kernel/Makefile
@@ -27,7 +27,7 @@ KASAN_SANITIZE_stacktrace.o := n
OBJECT_FILES_NON_STANDARD_head_$(BITS).o := y
OBJECT_FILES_NON_STANDARD_relocate_kernel_$(BITS).o := y
-OBJECT_FILES_NON_STANDARD_mcount_$(BITS).o := y
+OBJECT_FILES_NON_STANDARD_ftrace_$(BITS).o := y
OBJECT_FILES_NON_STANDARD_test_nx.o := y
# If instrumentation of this dir is enabled, boot hangs during first second.
@@ -46,7 +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 mcount_64.o
+obj-$(CONFIG_X86_64) += sys_x86_64.o ftrace_64.o
obj-$(CONFIG_X86_ESPFIX64) += espfix_64.o
obj-$(CONFIG_SYSFS) += ksysfs.o
obj-y += bootflag.o e820.o
diff --git a/arch/x86/kernel/mcount_64.S b/arch/x86/kernel/ftrace_64.S
similarity index 100%
rename from arch/x86/kernel/mcount_64.S
rename to arch/x86/kernel/ftrace_64.S
--
2.10.2
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [PATCH 2/5 v2] ftrace/x86-32: Move the ftrace specific code out of entry_32.S
2017-03-16 17:20 [PATCH 0/5 v2] ftrace/x86_32: Ftrace cleanup and add support for -mfentry Steven Rostedt
2017-03-16 17:20 ` [PATCH 1/5 v2] x86/ftrace: Rename mcount_64.S to ftrace_64.S Steven Rostedt
@ 2017-03-16 17:20 ` Steven Rostedt
2017-03-17 13:21 ` Steven Rostedt
2017-03-16 17:20 ` [PATCH 3/5 v2] ftrace/x86_32: Add stack frame pointer to ftrace_caller Steven Rostedt
` (2 subsequent siblings)
4 siblings, 1 reply; 37+ messages in thread
From: Steven Rostedt @ 2017-03-16 17:20 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
[-- Attachment #1: 0002-ftrace-x86-32-Move-the-ftrace-specific-code-out-of-e.patch --]
[-- Type: text/plain, Size: 10723 bytes --]
From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
The function tracing hook code for ftrace is not an entry point from
userspace and does not belong in the entry_*.S files. It has already been
moved out of entry_64.S. This moves it out of entry_32.S into its own
ftrace_32.S file.
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
Makefile | 12 +--
arch/x86/entry/entry_32.S | 168 ------------------------------------------
arch/x86/kernel/Makefile | 1 +
arch/x86/kernel/ftrace_32.S | 176 ++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 183 insertions(+), 174 deletions(-)
create mode 100644 arch/x86/kernel/ftrace_32.S
diff --git a/Makefile b/Makefile
index b841fb36beb2..7df32471c206 100644
--- a/Makefile
+++ b/Makefile
@@ -653,6 +653,12 @@ 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
@@ -798,12 +804,6 @@ 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 57f7ec35216e..c576352361a7 100644
--- a/arch/x86/entry/entry_32.S
+++ b/arch/x86/entry/entry_32.S
@@ -38,13 +38,11 @@
#include <asm/page_types.h>
#include <asm/percpu.h>
#include <asm/processor-flags.h>
-#include <asm/ftrace.h>
#include <asm/irq_vectors.h>
#include <asm/cpufeatures.h>
#include <asm/alternative-asm.h>
#include <asm/asm.h>
#include <asm/smap.h>
-#include <asm/export.h>
#include <asm/frame.h>
.section .entry.text, "ax"
@@ -886,172 +884,6 @@ BUILD_INTERRUPT3(hyperv_callback_vector, HYPERVISOR_CALLBACK_VECTOR,
#endif /* CONFIG_HYPERV */
-#ifdef CONFIG_FUNCTION_TRACER
-#ifdef CONFIG_DYNAMIC_FTRACE
-
-ENTRY(mcount)
- ret
-END(mcount)
-
-ENTRY(ftrace_caller)
- pushl %eax
- pushl %ecx
- pushl %edx
- pushl $0 /* Pass NULL as regs pointer */
- movl 4*4(%esp), %eax
- movl 0x4(%ebp), %edx
- movl function_trace_op, %ecx
- subl $MCOUNT_INSN_SIZE, %eax
-
-.globl ftrace_call
-ftrace_call:
- call ftrace_stub
-
- addl $4, %esp /* skip NULL pointer */
- popl %edx
- popl %ecx
- popl %eax
-.Lftrace_ret:
-#ifdef CONFIG_FUNCTION_GRAPH_TRACER
-.globl ftrace_graph_call
-ftrace_graph_call:
- jmp ftrace_stub
-#endif
-
-/* This is weak to keep gas from relaxing the jumps */
-WEAK(ftrace_stub)
- ret
-END(ftrace_caller)
-
-ENTRY(ftrace_regs_caller)
- pushf /* push flags before compare (in cs location) */
-
- /*
- * i386 does not save SS and ESP when coming from kernel.
- * Instead, to get sp, ®s->sp is used (see ptrace.h).
- * Unfortunately, that means eflags must be at the same location
- * as the current return ip is. We move the return ip into the
- * ip location, and move flags into the return ip location.
- */
- pushl 4(%esp) /* save return ip into ip slot */
-
- pushl $0 /* Load 0 into orig_ax */
- pushl %gs
- pushl %fs
- pushl %es
- pushl %ds
- pushl %eax
- pushl %ebp
- pushl %edi
- pushl %esi
- pushl %edx
- pushl %ecx
- pushl %ebx
-
- movl 13*4(%esp), %eax /* Get the saved flags */
- movl %eax, 14*4(%esp) /* Move saved flags into regs->flags location */
- /* clobbering return ip */
- movl $__KERNEL_CS, 13*4(%esp)
-
- movl 12*4(%esp), %eax /* Load ip (1st parameter) */
- subl $MCOUNT_INSN_SIZE, %eax /* Adjust ip */
- movl 0x4(%ebp), %edx /* Load parent ip (2nd parameter) */
- movl function_trace_op, %ecx /* Save ftrace_pos in 3rd parameter */
- pushl %esp /* Save pt_regs as 4th parameter */
-
-GLOBAL(ftrace_regs_call)
- call ftrace_stub
-
- addl $4, %esp /* Skip pt_regs */
- movl 14*4(%esp), %eax /* Move flags back into cs */
- movl %eax, 13*4(%esp) /* Needed to keep addl from modifying flags */
- movl 12*4(%esp), %eax /* Get return ip from regs->ip */
- movl %eax, 14*4(%esp) /* Put return ip back for ret */
-
- popl %ebx
- popl %ecx
- popl %edx
- popl %esi
- popl %edi
- popl %ebp
- popl %eax
- popl %ds
- popl %es
- popl %fs
- popl %gs
- addl $8, %esp /* Skip orig_ax and ip */
- popf /* Pop flags at end (no addl to corrupt flags) */
- jmp .Lftrace_ret
-
- popf
- jmp ftrace_stub
-#else /* ! CONFIG_DYNAMIC_FTRACE */
-
-ENTRY(mcount)
- cmpl $__PAGE_OFFSET, %esp
- jb ftrace_stub /* Paging not enabled yet? */
-
- cmpl $ftrace_stub, ftrace_trace_function
- jnz .Ltrace
-#ifdef CONFIG_FUNCTION_GRAPH_TRACER
- cmpl $ftrace_stub, ftrace_graph_return
- jnz ftrace_graph_caller
-
- cmpl $ftrace_graph_entry_stub, ftrace_graph_entry
- jnz ftrace_graph_caller
-#endif
-.globl ftrace_stub
-ftrace_stub:
- ret
-
- /* taken from glibc */
-.Ltrace:
- pushl %eax
- pushl %ecx
- pushl %edx
- movl 0xc(%esp), %eax
- movl 0x4(%ebp), %edx
- subl $MCOUNT_INSN_SIZE, %eax
-
- call *ftrace_trace_function
-
- popl %edx
- popl %ecx
- popl %eax
- jmp ftrace_stub
-END(mcount)
-#endif /* CONFIG_DYNAMIC_FTRACE */
-EXPORT_SYMBOL(mcount)
-#endif /* CONFIG_FUNCTION_TRACER */
-
-#ifdef CONFIG_FUNCTION_GRAPH_TRACER
-ENTRY(ftrace_graph_caller)
- pushl %eax
- pushl %ecx
- pushl %edx
- movl 0xc(%esp), %eax
- lea 0x4(%ebp), %edx
- movl (%ebp), %ecx
- subl $MCOUNT_INSN_SIZE, %eax
- call prepare_ftrace_return
- popl %edx
- popl %ecx
- popl %eax
- ret
-END(ftrace_graph_caller)
-
-.globl return_to_handler
-return_to_handler:
- pushl %eax
- pushl %edx
- movl %ebp, %eax
- call ftrace_return_to_handler
- movl %eax, %ecx
- popl %edx
- popl %eax
- jmp *%ecx
-#endif
-
#ifdef CONFIG_TRACING
ENTRY(trace_page_fault)
ASM_CLAC
diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
index d3743a37e990..55e8902c461f 100644
--- a/arch/x86/kernel/Makefile
+++ b/arch/x86/kernel/Makefile
@@ -47,6 +47,7 @@ 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_ESPFIX64) += espfix_64.o
obj-$(CONFIG_SYSFS) += ksysfs.o
obj-y += bootflag.o e820.o
diff --git a/arch/x86/kernel/ftrace_32.S b/arch/x86/kernel/ftrace_32.S
new file mode 100644
index 000000000000..4ff30f31f0a5
--- /dev/null
+++ b/arch/x86/kernel/ftrace_32.S
@@ -0,0 +1,176 @@
+/*
+ * linux/arch/x86_64/mcount_64.S
+ *
+ * Copyright (C) 2017 Steven Rostedt, VMware Inc.
+ */
+
+#include <linux/linkage.h>
+#include <asm/segment.h>
+#include <asm/export.h>
+#include <asm/ftrace.h>
+
+#ifdef CONFIG_FUNCTION_TRACER
+#ifdef CONFIG_DYNAMIC_FTRACE
+
+ENTRY(mcount)
+ ret
+END(mcount)
+
+ENTRY(ftrace_caller)
+ pushl %eax
+ pushl %ecx
+ pushl %edx
+ pushl $0 /* Pass NULL as regs pointer */
+ movl 4*4(%esp), %eax
+ movl 0x4(%ebp), %edx
+ movl function_trace_op, %ecx
+ subl $MCOUNT_INSN_SIZE, %eax
+
+.globl ftrace_call
+ftrace_call:
+ call ftrace_stub
+
+ addl $4, %esp /* skip NULL pointer */
+ popl %edx
+ popl %ecx
+ popl %eax
+.Lftrace_ret:
+#ifdef CONFIG_FUNCTION_GRAPH_TRACER
+.globl ftrace_graph_call
+ftrace_graph_call:
+ jmp ftrace_stub
+#endif
+
+/* This is weak to keep gas from relaxing the jumps */
+WEAK(ftrace_stub)
+ ret
+END(ftrace_caller)
+
+ENTRY(ftrace_regs_caller)
+ pushf /* push flags before compare (in cs location) */
+
+ /*
+ * i386 does not save SS and ESP when coming from kernel.
+ * Instead, to get sp, ®s->sp is used (see ptrace.h).
+ * Unfortunately, that means eflags must be at the same location
+ * as the current return ip is. We move the return ip into the
+ * ip location, and move flags into the return ip location.
+ */
+ pushl 4(%esp) /* save return ip into ip slot */
+
+ pushl $0 /* Load 0 into orig_ax */
+ pushl %gs
+ pushl %fs
+ pushl %es
+ pushl %ds
+ pushl %eax
+ pushl %ebp
+ pushl %edi
+ pushl %esi
+ pushl %edx
+ pushl %ecx
+ pushl %ebx
+
+ movl 13*4(%esp), %eax /* Get the saved flags */
+ movl %eax, 14*4(%esp) /* Move saved flags into regs->flags location */
+ /* clobbering return ip */
+ movl $__KERNEL_CS, 13*4(%esp)
+
+ movl 12*4(%esp), %eax /* Load ip (1st parameter) */
+ subl $MCOUNT_INSN_SIZE, %eax /* Adjust ip */
+ movl 0x4(%ebp), %edx /* Load parent ip (2nd parameter) */
+ movl function_trace_op, %ecx /* Save ftrace_pos in 3rd parameter */
+ pushl %esp /* Save pt_regs as 4th parameter */
+
+GLOBAL(ftrace_regs_call)
+ call ftrace_stub
+
+ addl $4, %esp /* Skip pt_regs */
+ movl 14*4(%esp), %eax /* Move flags back into cs */
+ movl %eax, 13*4(%esp) /* Needed to keep addl from modifying flags */
+ movl 12*4(%esp), %eax /* Get return ip from regs->ip */
+ movl %eax, 14*4(%esp) /* Put return ip back for ret */
+
+ popl %ebx
+ popl %ecx
+ popl %edx
+ popl %esi
+ popl %edi
+ popl %ebp
+ popl %eax
+ popl %ds
+ popl %es
+ popl %fs
+ popl %gs
+ addl $8, %esp /* Skip orig_ax and ip */
+ popf /* Pop flags at end (no addl to corrupt flags) */
+ jmp .Lftrace_ret
+
+ popf
+ jmp ftrace_stub
+#else /* ! CONFIG_DYNAMIC_FTRACE */
+
+ENTRY(mcount)
+ cmpl $__PAGE_OFFSET, %esp
+ jb ftrace_stub /* Paging not enabled yet? */
+
+ cmpl $ftrace_stub, ftrace_trace_function
+ jnz .Ltrace
+#ifdef CONFIG_FUNCTION_GRAPH_TRACER
+ cmpl $ftrace_stub, ftrace_graph_return
+ jnz ftrace_graph_caller
+
+ cmpl $ftrace_graph_entry_stub, ftrace_graph_entry
+ jnz ftrace_graph_caller
+#endif
+.globl ftrace_stub
+ftrace_stub:
+ ret
+
+ /* taken from glibc */
+.Ltrace:
+ pushl %eax
+ pushl %ecx
+ pushl %edx
+ movl 0xc(%esp), %eax
+ movl 0x4(%ebp), %edx
+ subl $MCOUNT_INSN_SIZE, %eax
+
+ call *ftrace_trace_function
+
+ popl %edx
+ popl %ecx
+ popl %eax
+ jmp ftrace_stub
+END(mcount)
+#endif /* CONFIG_DYNAMIC_FTRACE */
+EXPORT_SYMBOL(mcount)
+#endif /* CONFIG_FUNCTION_TRACER */
+
+#ifdef CONFIG_FUNCTION_GRAPH_TRACER
+ENTRY(ftrace_graph_caller)
+ pushl %eax
+ pushl %ecx
+ pushl %edx
+ movl 0xc(%esp), %eax
+ lea 0x4(%ebp), %edx
+ movl (%ebp), %ecx
+ subl $MCOUNT_INSN_SIZE, %eax
+ call prepare_ftrace_return
+ popl %edx
+ popl %ecx
+ popl %eax
+ ret
+END(ftrace_graph_caller)
+
+.globl return_to_handler
+return_to_handler:
+ pushl %eax
+ pushl %edx
+ movl %ebp, %eax
+ call ftrace_return_to_handler
+ movl %eax, %ecx
+ popl %edx
+ popl %eax
+ jmp *%ecx
+#endif
--
2.10.2
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [PATCH 3/5 v2] ftrace/x86_32: Add stack frame pointer to ftrace_caller
2017-03-16 17:20 [PATCH 0/5 v2] ftrace/x86_32: Ftrace cleanup and add support for -mfentry Steven Rostedt
2017-03-16 17:20 ` [PATCH 1/5 v2] x86/ftrace: Rename mcount_64.S to ftrace_64.S Steven Rostedt
2017-03-16 17:20 ` [PATCH 2/5 v2] ftrace/x86-32: Move the ftrace specific code out of entry_32.S Steven Rostedt
@ 2017-03-16 17:20 ` Steven Rostedt
2017-03-16 17:20 ` [PATCH 4/5 v2] ftrace/x86_32: Clean up ftrace_regs_caller Steven Rostedt
2017-03-16 17:20 ` [PATCH 5/5 v2] ftrace/x86-32: Add -mfentry support to x86_32 with DYNAMIC_FTRACE set Steven Rostedt
4 siblings, 0 replies; 37+ messages in thread
From: Steven Rostedt @ 2017-03-16 17:20 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
[-- Attachment #1: 0003-ftrace-x86_32-Add-stack-frame-pointer-to-ftrace_call.patch --]
[-- Type: text/plain, Size: 2128 bytes --]
From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
The function hook ftrace_caller does not create its own stack frame, and
this causes the ftrace stack trace to miss the first function when doing
stack traces.
# echo schedule:stacktrace > /sys/kernel/tracing/set_ftrace_filter
Before:
<idle>-0 [002] .N.. 29.865807: <stack trace>
=> cpu_startup_entry
=> start_secondary
=> startup_32_smp
<...>-7 [001] .... 29.866509: <stack trace>
=> kthread
=> ret_from_fork
<...>-1 [000] .... 29.865377: <stack trace>
=> poll_schedule_timeout
=> do_select
=> core_sys_select
=> SyS_select
=> do_fast_syscall_32
=> entry_SYSENTER_32
After:
<idle>-0 [002] .N.. 31.234853: <stack trace>
=> do_idle
=> cpu_startup_entry
=> start_secondary
=> startup_32_smp
<...>-7 [003] .... 31.235140: <stack trace>
=> rcu_gp_kthread
=> kthread
=> ret_from_fork
<...>-1819 [000] .... 31.264172: <stack trace>
=> schedule_hrtimeout_range
=> poll_schedule_timeout
=> do_sys_poll
=> SyS_ppoll
=> do_fast_syscall_32
=> entry_SYSENTER_32
Reviewed-by: Josh Poimboeuf <jpoimboe@redhat.com>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
arch/x86/kernel/ftrace_32.S | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/arch/x86/kernel/ftrace_32.S b/arch/x86/kernel/ftrace_32.S
index 4ff30f31f0a5..6f855dfb786d 100644
--- a/arch/x86/kernel/ftrace_32.S
+++ b/arch/x86/kernel/ftrace_32.S
@@ -17,12 +17,19 @@ ENTRY(mcount)
END(mcount)
ENTRY(ftrace_caller)
+
+ pushl %ebp
+ movl %esp, %ebp
+
pushl %eax
pushl %ecx
pushl %edx
pushl $0 /* Pass NULL as regs pointer */
- movl 4*4(%esp), %eax
- movl 0x4(%ebp), %edx
+ movl 5*4(%esp), %eax
+ /* Copy original ebp into %edx */
+ movl 4*4(%esp), %edx
+ /* Get the parent ip */
+ movl 0x4(%edx), %edx
movl function_trace_op, %ecx
subl $MCOUNT_INSN_SIZE, %eax
@@ -34,6 +41,7 @@ ftrace_call:
popl %edx
popl %ecx
popl %eax
+ popl %ebp
.Lftrace_ret:
#ifdef CONFIG_FUNCTION_GRAPH_TRACER
.globl ftrace_graph_call
--
2.10.2
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [PATCH 4/5 v2] ftrace/x86_32: Clean up ftrace_regs_caller
2017-03-16 17:20 [PATCH 0/5 v2] ftrace/x86_32: Ftrace cleanup and add support for -mfentry Steven Rostedt
` (2 preceding siblings ...)
2017-03-16 17:20 ` [PATCH 3/5 v2] ftrace/x86_32: Add stack frame pointer to ftrace_caller Steven Rostedt
@ 2017-03-16 17:20 ` Steven Rostedt
2017-03-16 17:40 ` Linus Torvalds
2017-03-16 17:20 ` [PATCH 5/5 v2] ftrace/x86-32: Add -mfentry support to x86_32 with DYNAMIC_FTRACE set Steven Rostedt
4 siblings, 1 reply; 37+ messages in thread
From: Steven Rostedt @ 2017-03-16 17:20 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
[-- Attachment #1: 0004-ftrace-x86_32-Clean-up-ftrace_regs_caller.patch --]
[-- Type: text/plain, Size: 3466 bytes --]
From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
When ftrace_regs_caller was created, it was designed to preserve flags as
much as possible as it needed to act just like a breakpoint triggered on the
same location. But the design is over complicated as it treated all
operations as modifying flags. But push, mov and lea do not modify flags.
This means the code can become more simplified by allowing flags to be
stored further down.
Making ftrace_regs_caller simpler will also be useful in implementing fentry
logic.
Reviewed-by: Masami Hiramatsu <mhiramat@kernel.org>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
arch/x86/kernel/ftrace_32.S | 45 +++++++++++++++++++++++++--------------------
1 file changed, 25 insertions(+), 20 deletions(-)
diff --git a/arch/x86/kernel/ftrace_32.S b/arch/x86/kernel/ftrace_32.S
index 6f855dfb786d..2002aadd25f8 100644
--- a/arch/x86/kernel/ftrace_32.S
+++ b/arch/x86/kernel/ftrace_32.S
@@ -55,23 +55,30 @@ WEAK(ftrace_stub)
END(ftrace_caller)
ENTRY(ftrace_regs_caller)
- pushf /* push flags before compare (in cs location) */
-
/*
* i386 does not save SS and ESP when coming from kernel.
* Instead, to get sp, ®s->sp is used (see ptrace.h).
* Unfortunately, that means eflags must be at the same location
* as the current return ip is. We move the return ip into the
- * ip location, and move flags into the return ip location.
- */
- pushl 4(%esp) /* save return ip into ip slot */
+ * 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 */
+
pushl %ebp
pushl %edi
pushl %esi
@@ -79,11 +86,6 @@ ENTRY(ftrace_regs_caller)
pushl %ecx
pushl %ebx
- movl 13*4(%esp), %eax /* Get the saved flags */
- movl %eax, 14*4(%esp) /* Move saved flags into regs->flags location */
- /* clobbering return ip */
- movl $__KERNEL_CS, 13*4(%esp)
-
movl 12*4(%esp), %eax /* Load ip (1st parameter) */
subl $MCOUNT_INSN_SIZE, %eax /* Adjust ip */
movl 0x4(%ebp), %edx /* Load parent ip (2nd parameter) */
@@ -94,10 +96,14 @@ GLOBAL(ftrace_regs_call)
call ftrace_stub
addl $4, %esp /* Skip pt_regs */
- movl 14*4(%esp), %eax /* Move flags back into cs */
- movl %eax, 13*4(%esp) /* Needed to keep addl from modifying flags */
- movl 12*4(%esp), %eax /* Get return ip from regs->ip */
- movl %eax, 14*4(%esp) /* Put return ip back for ret */
+
+ /* Since we don't care about cs, move flags there to simplify return */
+ movl 14*4(%esp), %eax
+ movl %eax, 13*4(%esp)
+
+ /* Move return ip back to its original location */
+ movl 12*4(%esp), %eax
+ movl %eax, 14*4(%esp)
popl %ebx
popl %ecx
@@ -110,12 +116,11 @@ GLOBAL(ftrace_regs_call)
popl %es
popl %fs
popl %gs
- addl $8, %esp /* Skip orig_ax and ip */
- popf /* Pop flags at end (no addl to corrupt flags) */
- jmp .Lftrace_ret
+ addl $8, %esp /* Skip orig_ax and old ip */
- popf
- jmp ftrace_stub
+ popf /* flags is in the cs location */
+
+ jmp .Lftrace_ret
#else /* ! CONFIG_DYNAMIC_FTRACE */
ENTRY(mcount)
--
2.10.2
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [PATCH 5/5 v2] ftrace/x86-32: Add -mfentry support to x86_32 with DYNAMIC_FTRACE set
2017-03-16 17:20 [PATCH 0/5 v2] ftrace/x86_32: Ftrace cleanup and add support for -mfentry Steven Rostedt
` (3 preceding siblings ...)
2017-03-16 17:20 ` [PATCH 4/5 v2] ftrace/x86_32: Clean up ftrace_regs_caller Steven Rostedt
@ 2017-03-16 17:20 ` Steven Rostedt
2017-03-16 20:40 ` [PATCH 5/5 v3] " Steven Rostedt
4 siblings, 1 reply; 37+ messages in thread
From: Steven Rostedt @ 2017-03-16 17:20 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
[-- Attachment #1: 0005-ftrace-x86-32-Add-mfentry-support-to-x86_32-with-DYN.patch --]
[-- Type: text/plain, Size: 5096 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.
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
arch/x86/Kconfig | 2 +-
arch/x86/kernel/ftrace_32.S | 81 +++++++++++++++++++++++++++++++++++++++------
2 files changed, 72 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 2002aadd25f8..7ed4137aa81a 100644
--- a/arch/x86/kernel/ftrace_32.S
+++ b/arch/x86/kernel/ftrace_32.S
@@ -9,27 +9,68 @@
#include <asm/export.h>
#include <asm/ftrace.h>
+#ifdef CC_USING_FENTRY
+# define function_hook __fentry__
+EXPORT_SYMBOL(__fentry__)
+#else
+# define function_hook mcount
+EXPORT_SYMBOL(mcount)
+#endif
+
+/* 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
+
#ifdef CONFIG_FUNCTION_TRACER
#ifdef CONFIG_DYNAMIC_FTRACE
-ENTRY(mcount)
+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
@@ -41,7 +82,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
@@ -85,6 +133,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 */
@@ -123,7 +175,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? */
@@ -155,9 +207,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
@@ -165,9 +216,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
@@ -180,7 +237,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
^ permalink raw reply related [flat|nested] 37+ messages in thread
* Re: [PATCH 4/5 v2] ftrace/x86_32: Clean up ftrace_regs_caller
2017-03-16 17:20 ` [PATCH 4/5 v2] ftrace/x86_32: Clean up ftrace_regs_caller Steven Rostedt
@ 2017-03-16 17:40 ` Linus Torvalds
2017-03-16 17:55 ` Steven Rostedt
2017-03-16 18:09 ` [PATCH 4/5 v3] " Steven Rostedt
0 siblings, 2 replies; 37+ messages in thread
From: Linus Torvalds @ 2017-03-16 17:40 UTC (permalink / raw)
To: Steven Rostedt
Cc: Linux Kernel Mailing List, Ingo Molnar, Andrew Morton,
Thomas Gleixner, Peter Zijlstra, Masami Hiramatsu,
H. Peter Anvin, Andy Lutomirski, Josh Poimboeuf
On Thu, Mar 16, 2017 at 10:20 AM, Steven Rostedt <rostedt@goodmis.org> wrote:
>
> When ftrace_regs_caller was created, it was designed to preserve flags as
> much as possible as it needed to act just like a breakpoint triggered on the
> same location. But the design is over complicated as it treated all
> operations as modifying flags. But push, mov and lea do not modify flags.
> This means the code can become more simplified by allowing flags to be
> stored further down.
It still looks overly complicated to me.
The snippet below is the patch without the "-" lines, so it's the end result:
> ENTRY(ftrace_regs_caller)
> /*
> * i386 does not save SS and ESP when coming from kernel.
> * Instead, to get sp, ®s->sp is used (see ptrace.h).
> * 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 %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 */
Why do you do that silly "temporarily save flags" thing?
Why not just push $0 there?
Afaik, the sequence could/should be:
pushl $__KERNEL_CS
pushl 4(%esp) /* Save the return ip */
pushl $0
pushl %gs
pushl %fs
pushl %es
pushl %ds
pushl %eax
/* Fix up eflags now that we have a scratch register */
pushfl
popl %eax
movl %eax,8(%rsp)
Or something. There's no point in the unnecessary "shuffle values back
and forth with odd stack offsets".
Linus
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 4/5 v2] ftrace/x86_32: Clean up ftrace_regs_caller
2017-03-16 17:40 ` Linus Torvalds
@ 2017-03-16 17:55 ` Steven Rostedt
2017-03-16 18:09 ` [PATCH 4/5 v3] " Steven Rostedt
1 sibling, 0 replies; 37+ messages in thread
From: Steven Rostedt @ 2017-03-16 17:55 UTC (permalink / raw)
To: Linus Torvalds
Cc: Linux Kernel Mailing List, Ingo Molnar, Andrew Morton,
Thomas Gleixner, Peter Zijlstra, Masami Hiramatsu,
H. Peter Anvin, Andy Lutomirski, Josh Poimboeuf
[ Resending again with a "reply-all" instead of just "reply" ]
On Thu, 16 Mar 2017 10:40:24 -0700
Linus Torvalds <torvalds@linux-foundation.org> wrote:
> On Thu, Mar 16, 2017 at 10:20 AM, Steven Rostedt <rostedt@goodmis.org> wrote:
> >
> > When ftrace_regs_caller was created, it was designed to preserve flags as
> > much as possible as it needed to act just like a breakpoint triggered on the
> > same location. But the design is over complicated as it treated all
> > operations as modifying flags. But push, mov and lea do not modify flags.
> > This means the code can become more simplified by allowing flags to be
> > stored further down.
>
> It still looks overly complicated to me.
>
> The snippet below is the patch without the "-" lines, so it's the end result:
>
> > ENTRY(ftrace_regs_caller)
> > /*
> > * i386 does not save SS and ESP when coming from kernel.
> > * Instead, to get sp, ®s->sp is used (see ptrace.h).
> > * 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 %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 */
>
> Why do you do that silly "temporarily save flags" thing?
>
> Why not just push $0 there?
>
> Afaik, the sequence could/should be:
>
> pushl $__KERNEL_CS
> pushl 4(%esp) /* Save the return ip */
> pushl $0
> pushl %gs
> pushl %fs
> pushl %es
> pushl %ds
> pushl %eax
>
> /* Fix up eflags now that we have a scratch register */
> pushfl
> popl %eax
> movl %eax,8(%rsp)
>
> Or something. There's no point in the unnecessary "shuffle values back
> and forth with odd stack offsets".
Sure, I can do this. This is the issue of trying to do too much at
first and then eliminating what you don't need. :-p
I think previous versions (where I was trying to horribly add a stack
frame here) had some more logic.
-- Steve
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH 4/5 v3] ftrace/x86_32: Clean up ftrace_regs_caller
2017-03-16 17:40 ` Linus Torvalds
2017-03-16 17:55 ` Steven Rostedt
@ 2017-03-16 18:09 ` Steven Rostedt
2017-03-16 18:19 ` Linus Torvalds
1 sibling, 1 reply; 37+ messages in thread
From: Steven Rostedt @ 2017-03-16 18:09 UTC (permalink / raw)
To: Linus Torvalds
Cc: Linux Kernel Mailing List, Ingo Molnar, Andrew Morton,
Thomas Gleixner, Peter Zijlstra, Masami Hiramatsu,
H. Peter Anvin, Andy Lutomirski, Josh Poimboeuf
[ I'll wait to post the full v3 series in case there is more comments ]
From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
When ftrace_regs_caller was created, it was designed to preserve flags as
much as possible as it needed to act just like a breakpoint triggered on the
same location. But the design is over complicated as it treated all
operations as modifying flags. But push, mov and lea do not modify flags.
This means the code can become more simplified by allowing flags to be
stored further down.
Making ftrace_regs_caller simpler will also be useful in implementing fentry
logic.
Link: http://lkml.kernel.org/r/20170316135328.36123c3e@gandalf.local.home
[ simpler logic to save flags ]
Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
Reviewed-by: Masami Hiramatsu <mhiramat@kernel.org>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
arch/x86/kernel/ftrace_32.S | 42 ++++++++++++++++++++++--------------------
1 file changed, 22 insertions(+), 20 deletions(-)
diff --git a/arch/x86/kernel/ftrace_32.S b/arch/x86/kernel/ftrace_32.S
index 6f855df..691897e 100644
--- a/arch/x86/kernel/ftrace_32.S
+++ b/arch/x86/kernel/ftrace_32.S
@@ -55,23 +55,27 @@ WEAK(ftrace_stub)
END(ftrace_caller)
ENTRY(ftrace_regs_caller)
- pushf /* push flags before compare (in cs location) */
-
/*
* i386 does not save SS and ESP when coming from kernel.
* Instead, to get sp, ®s->sp is used (see ptrace.h).
* Unfortunately, that means eflags must be at the same location
* as the current return ip is. We move the return ip into the
- * ip location, and move flags into the return ip location.
- */
- pushl 4(%esp) /* save return ip into ip slot */
-
+ * regs->ip location, and move flags into the return ip location.
+ */
+ pushl $__KERNEL_CS
+ pushl 4(%esp) /* Save the return ip */
pushl $0 /* Load 0 into orig_ax */
pushl %gs
pushl %fs
pushl %es
pushl %ds
pushl %eax
+
+ /* Get flags and place them into the return ip slot */
+ pushf
+ popl %eax
+ movl %eax, 8*4(%esp)
+
pushl %ebp
pushl %edi
pushl %esi
@@ -79,11 +83,6 @@ ENTRY(ftrace_regs_caller)
pushl %ecx
pushl %ebx
- movl 13*4(%esp), %eax /* Get the saved flags */
- movl %eax, 14*4(%esp) /* Move saved flags into regs->flags location */
- /* clobbering return ip */
- movl $__KERNEL_CS, 13*4(%esp)
-
movl 12*4(%esp), %eax /* Load ip (1st parameter) */
subl $MCOUNT_INSN_SIZE, %eax /* Adjust ip */
movl 0x4(%ebp), %edx /* Load parent ip (2nd parameter) */
@@ -94,10 +93,14 @@ GLOBAL(ftrace_regs_call)
call ftrace_stub
addl $4, %esp /* Skip pt_regs */
- movl 14*4(%esp), %eax /* Move flags back into cs */
- movl %eax, 13*4(%esp) /* Needed to keep addl from modifying flags */
- movl 12*4(%esp), %eax /* Get return ip from regs->ip */
- movl %eax, 14*4(%esp) /* Put return ip back for ret */
+
+ /* Since we don't care about cs, move flags there to simplify return */
+ movl 14*4(%esp), %eax
+ movl %eax, 13*4(%esp)
+
+ /* Move return ip back to its original location */
+ movl 12*4(%esp), %eax
+ movl %eax, 14*4(%esp)
popl %ebx
popl %ecx
@@ -110,12 +113,11 @@ GLOBAL(ftrace_regs_call)
popl %es
popl %fs
popl %gs
- addl $8, %esp /* Skip orig_ax and ip */
- popf /* Pop flags at end (no addl to corrupt flags) */
- jmp .Lftrace_ret
+ addl $8, %esp /* Skip orig_ax and old ip */
- popf
- jmp ftrace_stub
+ popf /* flags is in the cs location */
+
+ jmp .Lftrace_ret
#else /* ! CONFIG_DYNAMIC_FTRACE */
ENTRY(mcount)
--
2.9.3
^ permalink raw reply related [flat|nested] 37+ messages in thread
* Re: [PATCH 4/5 v3] ftrace/x86_32: Clean up ftrace_regs_caller
2017-03-16 18:09 ` [PATCH 4/5 v3] " Steven Rostedt
@ 2017-03-16 18:19 ` Linus Torvalds
2017-03-16 19:19 ` Steven Rostedt
2017-03-16 20:14 ` [PATCH 4/5 v3.1] " Steven Rostedt
0 siblings, 2 replies; 37+ messages in thread
From: Linus Torvalds @ 2017-03-16 18:19 UTC (permalink / raw)
To: Steven Rostedt
Cc: Linux Kernel Mailing List, Ingo Molnar, Andrew Morton,
Thomas Gleixner, Peter Zijlstra, Masami Hiramatsu,
H. Peter Anvin, Andy Lutomirski, Josh Poimboeuf
On Thu, Mar 16, 2017 at 11:09 AM, Steven Rostedt <rostedt@goodmis.org> wrote:
> +
> + /* Since we don't care about cs, move flags there to simplify return */
> + movl 14*4(%esp), %eax
> + movl %eax, 13*4(%esp)
> +
> + /* Move return ip back to its original location */
> + movl 12*4(%esp), %eax
> + movl %eax, 14*4(%esp)
Could this perhaps be removed entirely?
The return code could instead do:
... restore all the normal registers ..
# Now restore flags that is under the return address and our
fake __KERNEL_CS
pushl 8(%esp)
popfl
# and then return, skipping __KERNEL_CS and %flasg
ret $8
which is smaller and simpler than (again) playing games with stack entries.
Linus
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 4/5 v3] ftrace/x86_32: Clean up ftrace_regs_caller
2017-03-16 18:19 ` Linus Torvalds
@ 2017-03-16 19:19 ` Steven Rostedt
2017-03-16 19:24 ` Steven Rostedt
2017-03-16 19:28 ` Linus Torvalds
2017-03-16 20:14 ` [PATCH 4/5 v3.1] " Steven Rostedt
1 sibling, 2 replies; 37+ messages in thread
From: Steven Rostedt @ 2017-03-16 19:19 UTC (permalink / raw)
To: Linus Torvalds
Cc: Linux Kernel Mailing List, Ingo Molnar, Andrew Morton,
Thomas Gleixner, Peter Zijlstra, Masami Hiramatsu,
H. Peter Anvin, Andy Lutomirski, Josh Poimboeuf
On Thu, 16 Mar 2017 11:19:44 -0700
Linus Torvalds <torvalds@linux-foundation.org> wrote:
> On Thu, Mar 16, 2017 at 11:09 AM, Steven Rostedt <rostedt@goodmis.org> wrote:
> > +
> > + /* Since we don't care about cs, move flags there to simplify return */
> > + movl 14*4(%esp), %eax
> > + movl %eax, 13*4(%esp)
> > +
> > + /* Move return ip back to its original location */
> > + movl 12*4(%esp), %eax
> > + movl %eax, 14*4(%esp)
>
> Could this perhaps be removed entirely?
>
> The return code could instead do:
>
> ... restore all the normal registers ..
>
> # Now restore flags that is under the return address and our
> fake __KERNEL_CS
> pushl 8(%esp)
> popfl
>
> # and then return, skipping __KERNEL_CS and %flasg
> ret $8
>
> which is smaller and simpler than (again) playing games with stack entries.
>
> Linus
The thing is we don't return, we jump to the location that may be
modified to run the function graph tracer.
.Lftrace_ret:
#ifdef CONFIG_FUNCTION_GRAPH_TRACER
.globl ftrace_graph_call
ftrace_graph_call:
jmp ftrace_stub <- this can turn to a jump to function graph tracing
#endif
WEAK(ftrace_stub)
ret
END(ftrace_caller)
[...]
popf /* flags is in the cs location */
jmp .Lftrace_ret
-- Steve
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 4/5 v3] ftrace/x86_32: Clean up ftrace_regs_caller
2017-03-16 19:19 ` Steven Rostedt
@ 2017-03-16 19:24 ` Steven Rostedt
2017-03-16 19:30 ` Linus Torvalds
2017-03-16 19:28 ` Linus Torvalds
1 sibling, 1 reply; 37+ messages in thread
From: Steven Rostedt @ 2017-03-16 19:24 UTC (permalink / raw)
To: Linus Torvalds
Cc: Linux Kernel Mailing List, Ingo Molnar, Andrew Morton,
Thomas Gleixner, Peter Zijlstra, Masami Hiramatsu,
H. Peter Anvin, Andy Lutomirski, Josh Poimboeuf
On Thu, 16 Mar 2017 15:19:32 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:
>
>
> The thing is we don't return, we jump to the location that may be
> modified to run the function graph tracer.
That said, maybe the below is better?
/* restore flags */
pushl 14*4(%esp)
popf
/* Move return ip back to its original location */
movl 12*4(%esp), %eax
movl %eax, 14*4(%esp)
popl %ebx
popl %ecx
popl %edx
popl %esi
popl %edi
popl %ebp
popl %eax
popl %ds
popl %es
popl %fs
popl %gs
/* use lea to not affect flags */
lea (3*4)%esp, %esp /* Skip orig_ax, ip and flags */
jmp .Lftrace_ret
-- Steve
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 4/5 v3] ftrace/x86_32: Clean up ftrace_regs_caller
2017-03-16 19:19 ` Steven Rostedt
2017-03-16 19:24 ` Steven Rostedt
@ 2017-03-16 19:28 ` Linus Torvalds
2017-03-16 19:49 ` Steven Rostedt
1 sibling, 1 reply; 37+ messages in thread
From: Linus Torvalds @ 2017-03-16 19:28 UTC (permalink / raw)
To: Steven Rostedt
Cc: Linux Kernel Mailing List, Ingo Molnar, Andrew Morton,
Thomas Gleixner, Peter Zijlstra, Masami Hiramatsu,
H. Peter Anvin, Andy Lutomirski, Josh Poimboeuf
On Thu, Mar 16, 2017 at 12:19 PM, Steven Rostedt <rostedt@goodmis.org> wrote:
>
> The thing is we don't return, we jump to the location that may be
> modified to run the function graph tracer.
Hmm.
How about just making the stack frame a tiny bit bigger, and getting
rid of *all* the games.
IOW, just duplicate the return address, and make the entry code do
pushfl
pushl $__KERNEL_CS
pushl 8(%esp) /* Save the return ip *again* */
pushl $0
pushl %gs
pushl %fs
pushl %es
pushl %ds
pushl %eax
....
and not have any silly code to modify the old stack frame at all. Just
skip the values (all the segments, ORIG_EAX, duplicated return
address, __KERNEL_CS), and you can finish off with a "popf", and all
you have left i the original return ip that you didn't touch.
Hmm?
Linus
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 4/5 v3] ftrace/x86_32: Clean up ftrace_regs_caller
2017-03-16 19:24 ` Steven Rostedt
@ 2017-03-16 19:30 ` Linus Torvalds
2017-03-16 19:50 ` Steven Rostedt
0 siblings, 1 reply; 37+ messages in thread
From: Linus Torvalds @ 2017-03-16 19:30 UTC (permalink / raw)
To: Steven Rostedt
Cc: Linux Kernel Mailing List, Ingo Molnar, Andrew Morton,
Thomas Gleixner, Peter Zijlstra, Masami Hiramatsu,
H. Peter Anvin, Andy Lutomirski, Josh Poimboeuf
On Thu, Mar 16, 2017 at 12:24 PM, Steven Rostedt <rostedt@goodmis.org> wrote:
> popl %ds
> popl %es
> popl %fs
> popl %gs
Do you even need these? Can they be changed by ftrace?
And see the previous email about not touching the original location at
all, adn just duplicating that entry, avoiding all the need for
shuffling. I *really* hope that ftrace doesn't depend on changing the
return %eip on the stack. That would be truly nasty.
Linus
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 4/5 v3] ftrace/x86_32: Clean up ftrace_regs_caller
2017-03-16 19:28 ` Linus Torvalds
@ 2017-03-16 19:49 ` Steven Rostedt
0 siblings, 0 replies; 37+ messages in thread
From: Steven Rostedt @ 2017-03-16 19:49 UTC (permalink / raw)
To: Linus Torvalds
Cc: Linux Kernel Mailing List, Ingo Molnar, Andrew Morton,
Thomas Gleixner, Peter Zijlstra, Masami Hiramatsu,
H. Peter Anvin, Andy Lutomirski, Josh Poimboeuf
On Thu, 16 Mar 2017 12:28:13 -0700
Linus Torvalds <torvalds@linux-foundation.org> wrote:
> On Thu, Mar 16, 2017 at 12:19 PM, Steven Rostedt <rostedt@goodmis.org> wrote:
> >
> > The thing is we don't return, we jump to the location that may be
> > modified to run the function graph tracer.
>
> Hmm.
>
> How about just making the stack frame a tiny bit bigger, and getting
> rid of *all* the games.
Unfortunately, x86_32 causes us to play these games. The
ftrace_regs_caller is used by kprobes to simulate an int3 breakpoint.
That means I need to have it behave exactly the same as int3. On
x86_32, the int3 does not save ESP or SS for that matter, but instead
kprobes, et al. uses the address of the regs->sp parameter to get the
stack location when the interrupt triggered (coming from kernel).
That means, I can't duplicate pt_regs any place else. The pt_regs passed
into the caller (kprobes) has to have ®s->esp be equal to the stack
address when the int3 was hit. Or in this case, when fentry was called.
>
> IOW, just duplicate the return address, and make the entry code do
>
> pushfl
> pushl $__KERNEL_CS
> pushl 8(%esp) /* Save the return ip *again* */
> pushl $0
> pushl %gs
> pushl %fs
> pushl %es
> pushl %ds
> pushl %eax
> ....
>
> and not have any silly code to modify the old stack frame at all. Just
> skip the values (all the segments, ORIG_EAX, duplicated return
> address, __KERNEL_CS), and you can finish off with a "popf", and all
> you have left i the original return ip that you didn't touch.
Most likely those will not be touched, but as ftrace_regs_caller (which
is much heavier weight than ftrace_caller) must act the same as an
int3. If a kprobes callback modifies any of those, the
ftrace_regs_caller must act the same as if it was done by int3.
Now, most likely a kprobe's callback will never touch those. But we
never know.
-- Steve
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 4/5 v3] ftrace/x86_32: Clean up ftrace_regs_caller
2017-03-16 19:30 ` Linus Torvalds
@ 2017-03-16 19:50 ` Steven Rostedt
0 siblings, 0 replies; 37+ messages in thread
From: Steven Rostedt @ 2017-03-16 19:50 UTC (permalink / raw)
To: Linus Torvalds
Cc: Linux Kernel Mailing List, Ingo Molnar, Andrew Morton,
Thomas Gleixner, Peter Zijlstra, Masami Hiramatsu,
H. Peter Anvin, Andy Lutomirski, Josh Poimboeuf
On Thu, 16 Mar 2017 12:30:25 -0700
Linus Torvalds <torvalds@linux-foundation.org> wrote:
> On Thu, Mar 16, 2017 at 12:24 PM, Steven Rostedt <rostedt@goodmis.org> wrote:
> > popl %ds
> > popl %es
> > popl %fs
> > popl %gs
>
> Do you even need these? Can they be changed by ftrace?
ftrace no, kprobes, maybe.
>
> And see the previous email about not touching the original location at
> all, adn just duplicating that entry, avoiding all the need for
> shuffling. I *really* hope that ftrace doesn't depend on changing the
> return %eip on the stack. That would be truly nasty.
How do you think the live patching works?
-- Steve
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH 4/5 v3.1] ftrace/x86_32: Clean up ftrace_regs_caller
2017-03-16 18:19 ` Linus Torvalds
2017-03-16 19:19 ` Steven Rostedt
@ 2017-03-16 20:14 ` Steven Rostedt
1 sibling, 0 replies; 37+ messages in thread
From: Steven Rostedt @ 2017-03-16 20:14 UTC (permalink / raw)
To: Linus Torvalds
Cc: Linux Kernel Mailing List, Ingo Molnar, Andrew Morton,
Thomas Gleixner, Peter Zijlstra, Masami Hiramatsu,
H. Peter Anvin, Andy Lutomirski, Josh Poimboeuf
[ Unless we come up with something else, here's the latest ]
From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
When ftrace_regs_caller was created, it was designed to preserve flags as
much as possible as it needed to act just like a breakpoint triggered on the
same location. But the design is over complicated as it treated all
operations as modifying flags. But push, mov and lea do not modify flags.
This means the code can become more simplified by allowing flags to be
stored further down.
Making ftrace_regs_caller simpler will also be useful in implementing fentry
logic.
Link: http://lkml.kernel.org/r/20170316135328.36123c3e@gandalf.local.home
[ simpler logic to save flags ]
Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
Reviewed-by: Masami Hiramatsu <mhiramat@kernel.org>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
arch/x86/kernel/ftrace_32.S | 40 +++++++++++++++++++++-------------------
1 file changed, 21 insertions(+), 19 deletions(-)
diff --git a/arch/x86/kernel/ftrace_32.S b/arch/x86/kernel/ftrace_32.S
index 6f855df..068a582 100644
--- a/arch/x86/kernel/ftrace_32.S
+++ b/arch/x86/kernel/ftrace_32.S
@@ -55,23 +55,27 @@ WEAK(ftrace_stub)
END(ftrace_caller)
ENTRY(ftrace_regs_caller)
- pushf /* push flags before compare (in cs location) */
-
/*
* i386 does not save SS and ESP when coming from kernel.
* Instead, to get sp, ®s->sp is used (see ptrace.h).
* Unfortunately, that means eflags must be at the same location
* as the current return ip is. We move the return ip into the
- * ip location, and move flags into the return ip location.
+ * regs->ip location, and move flags into the return ip location.
*/
- pushl 4(%esp) /* save return ip into ip slot */
-
+ pushl $__KERNEL_CS
+ pushl 4(%esp) /* Save the return ip */
pushl $0 /* Load 0 into orig_ax */
pushl %gs
pushl %fs
pushl %es
pushl %ds
pushl %eax
+
+ /* Get flags and place them into the return ip slot */
+ pushf
+ popl %eax
+ movl %eax, 8*4(%esp)
+
pushl %ebp
pushl %edi
pushl %esi
@@ -79,11 +83,6 @@ ENTRY(ftrace_regs_caller)
pushl %ecx
pushl %ebx
- movl 13*4(%esp), %eax /* Get the saved flags */
- movl %eax, 14*4(%esp) /* Move saved flags into regs->flags location */
- /* clobbering return ip */
- movl $__KERNEL_CS, 13*4(%esp)
-
movl 12*4(%esp), %eax /* Load ip (1st parameter) */
subl $MCOUNT_INSN_SIZE, %eax /* Adjust ip */
movl 0x4(%ebp), %edx /* Load parent ip (2nd parameter) */
@@ -94,10 +93,14 @@ GLOBAL(ftrace_regs_call)
call ftrace_stub
addl $4, %esp /* Skip pt_regs */
- movl 14*4(%esp), %eax /* Move flags back into cs */
- movl %eax, 13*4(%esp) /* Needed to keep addl from modifying flags */
- movl 12*4(%esp), %eax /* Get return ip from regs->ip */
- movl %eax, 14*4(%esp) /* Put return ip back for ret */
+
+ /* restore flags */
+ push 14*4(%esp)
+ popf
+
+ /* Move return ip back to its original location */
+ movl 12*4(%esp), %eax
+ movl %eax, 14*4(%esp)
popl %ebx
popl %ecx
@@ -110,12 +113,11 @@ GLOBAL(ftrace_regs_call)
popl %es
popl %fs
popl %gs
- addl $8, %esp /* Skip orig_ax and ip */
- popf /* Pop flags at end (no addl to corrupt flags) */
- jmp .Lftrace_ret
- popf
- jmp ftrace_stub
+ /* use lea to not affect flags */
+ lea 3*4(%esp), %esp /* Skip orig_ax, ip and flags */
+
+ jmp .Lftrace_ret
#else /* ! CONFIG_DYNAMIC_FTRACE */
ENTRY(mcount)
--
2.9.3
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [PATCH 5/5 v3] ftrace/x86-32: Add -mfentry support to x86_32 with DYNAMIC_FTRACE set
2017-03-16 17:20 ` [PATCH 5/5 v2] ftrace/x86-32: Add -mfentry support to x86_32 with DYNAMIC_FTRACE set Steven Rostedt
@ 2017-03-16 20:40 ` Steven Rostedt
2017-03-16 21:00 ` Josh Poimboeuf
2017-03-17 10:38 ` Masami Hiramatsu
0 siblings, 2 replies; 37+ messages in thread
From: Steven Rostedt @ 2017-03-16 20:40 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
[
Fengguang Wu's bot told me that it failed to compile, as ftrace_32.S
always gets compiled (as does ftrace_64.S). And the content needs to
be protected within the #ifdef.
]
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.
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
arch/x86/Kconfig | 2 +-
arch/x86/kernel/ftrace_32.S | 81 +++++++++++++++++++++++++++++++++++++++------
2 files changed, 72 insertions(+), 11 deletions(-)
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index cc98d5a..8c17146 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 068a582..8530567 100644
--- a/arch/x86/kernel/ftrace_32.S
+++ b/arch/x86/kernel/ftrace_32.S
@@ -12,24 +12,65 @@
#ifdef CONFIG_FUNCTION_TRACER
#ifdef CONFIG_DYNAMIC_FTRACE
-ENTRY(mcount)
+#ifdef CC_USING_FENTRY
+# define function_hook __fentry__
+EXPORT_SYMBOL(__fentry__)
+#else
+# define function_hook mcount
+EXPORT_SYMBOL(mcount)
+#endif
+
+/* 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
@@ -41,7 +82,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
@@ -82,6 +130,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 */
@@ -120,7 +172,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? */
@@ -152,9 +204,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
@@ -162,9 +213,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
@@ -177,7 +234,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.9.3
^ permalink raw reply related [flat|nested] 37+ messages in thread
* Re: [PATCH 5/5 v3] ftrace/x86-32: Add -mfentry support to x86_32 with DYNAMIC_FTRACE set
2017-03-16 20:40 ` [PATCH 5/5 v3] " Steven Rostedt
@ 2017-03-16 21:00 ` Josh Poimboeuf
2017-03-16 21:25 ` Steven Rostedt
2017-03-17 10:38 ` Masami Hiramatsu
1 sibling, 1 reply; 37+ messages in thread
From: Josh Poimboeuf @ 2017-03-16 21:00 UTC (permalink / raw)
To: Steven Rostedt
Cc: linux-kernel, Ingo Molnar, Andrew Morton, Thomas Gleixner,
Peter Zijlstra, Masami Hiramatsu, H. Peter Anvin,
Andy Lutomirski, Linus Torvalds
On Thu, Mar 16, 2017 at 04:40:36PM -0400, Steven Rostedt wrote:
> [
> Fengguang Wu's bot told me that it failed to compile, as ftrace_32.S
> always gets compiled (as does ftrace_64.S). And the content needs to
> be protected within the #ifdef.
> ]
Why not instead make their compilation conditional in the Makefile?
Like:
obj-$(CONFIG_FUNCTION_TRACER) += ftrace_$(bits).o
--
Josh
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 5/5 v3] ftrace/x86-32: Add -mfentry support to x86_32 with DYNAMIC_FTRACE set
2017-03-16 21:00 ` Josh Poimboeuf
@ 2017-03-16 21:25 ` Steven Rostedt
0 siblings, 0 replies; 37+ messages in thread
From: Steven Rostedt @ 2017-03-16 21:25 UTC (permalink / raw)
To: Josh Poimboeuf
Cc: linux-kernel, Ingo Molnar, Andrew Morton, Thomas Gleixner,
Peter Zijlstra, Masami Hiramatsu, H. Peter Anvin,
Andy Lutomirski, Linus Torvalds
On Thu, 16 Mar 2017 16:00:43 -0500
Josh Poimboeuf <jpoimboe@redhat.com> wrote:
> On Thu, Mar 16, 2017 at 04:40:36PM -0400, Steven Rostedt wrote:
> > [
> > Fengguang Wu's bot told me that it failed to compile, as ftrace_32.S
> > always gets compiled (as does ftrace_64.S). And the content needs to
> > be protected within the #ifdef.
> > ]
>
> Why not instead make their compilation conditional in the Makefile?
>
> Like:
>
> obj-$(CONFIG_FUNCTION_TRACER) += ftrace_$(bits).o
>
That's another patch ;-)
I just followed suit with what was already there.
I'll add a 6th patch to clean that up.
-- Steve
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 5/5 v3] ftrace/x86-32: Add -mfentry support to x86_32 with DYNAMIC_FTRACE set
2017-03-16 20:40 ` [PATCH 5/5 v3] " Steven Rostedt
2017-03-16 21:00 ` Josh Poimboeuf
@ 2017-03-17 10:38 ` Masami Hiramatsu
1 sibling, 0 replies; 37+ messages in thread
From: Masami Hiramatsu @ 2017-03-17 10:38 UTC (permalink / raw)
To: Steven Rostedt
Cc: linux-kernel, Ingo Molnar, Andrew Morton, Thomas Gleixner,
Peter Zijlstra, Masami Hiramatsu, H. Peter Anvin,
Andy Lutomirski, Josh Poimboeuf, Linus Torvalds
On Thu, 16 Mar 2017 16:40:36 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:
> [
> Fengguang Wu's bot told me that it failed to compile, as ftrace_32.S
> always gets compiled (as does ftrace_64.S). And the content needs to
> be protected within the #ifdef.
> ]
>
> 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.
Looks good to me.
Reviewed-by: Masami Hiramatsu <mhiramat@kernel.org>
Thanks!
>
> Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
> ---
> arch/x86/Kconfig | 2 +-
> arch/x86/kernel/ftrace_32.S | 81 +++++++++++++++++++++++++++++++++++++++------
> 2 files changed, 72 insertions(+), 11 deletions(-)
>
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index cc98d5a..8c17146 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 068a582..8530567 100644
> --- a/arch/x86/kernel/ftrace_32.S
> +++ b/arch/x86/kernel/ftrace_32.S
> @@ -12,24 +12,65 @@
> #ifdef CONFIG_FUNCTION_TRACER
> #ifdef CONFIG_DYNAMIC_FTRACE
>
> -ENTRY(mcount)
> +#ifdef CC_USING_FENTRY
> +# define function_hook __fentry__
> +EXPORT_SYMBOL(__fentry__)
> +#else
> +# define function_hook mcount
> +EXPORT_SYMBOL(mcount)
> +#endif
> +
> +/* 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
>
> @@ -41,7 +82,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
> @@ -82,6 +130,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 */
> @@ -120,7 +172,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? */
>
> @@ -152,9 +204,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
> @@ -162,9 +213,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
> @@ -177,7 +234,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.9.3
>
--
Masami Hiramatsu <mhiramat@kernel.org>
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 2/5 v2] ftrace/x86-32: Move the ftrace specific code out of entry_32.S
2017-03-16 17:20 ` [PATCH 2/5 v2] ftrace/x86-32: Move the ftrace specific code out of entry_32.S Steven Rostedt
@ 2017-03-17 13:21 ` Steven Rostedt
0 siblings, 0 replies; 37+ messages in thread
From: Steven Rostedt @ 2017-03-17 13:21 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
On Thu, 16 Mar 2017 13:20:10 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:
> From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
>
> The function tracing hook code for ftrace is not an entry point from
> userspace and does not belong in the entry_*.S files. It has already been
> moved out of entry_64.S. This moves it out of entry_32.S into its own
> ftrace_32.S file.
>
> Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
> ---
> Makefile | 12 +--
> arch/x86/entry/entry_32.S | 168 ------------------------------------------
> arch/x86/kernel/Makefile | 1 +
> arch/x86/kernel/ftrace_32.S | 176 ++++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 183 insertions(+), 174 deletions(-)
> create mode 100644 arch/x86/kernel/ftrace_32.S
>
> diff --git a/Makefile b/Makefile
> index b841fb36beb2..7df32471c206 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -653,6 +653,12 @@ 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
> @@ -798,12 +804,6 @@ 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
Nobody noticed that I accidentally committed someone else's change. :-p
See http://lkml.kernel.org/r/20170310162411.GA18175@glebfm.cloud.tilaa.com
I was testing his patch during development, and forgot to revert it.
/me rebases.
-- Steve
> -
> 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
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH 0/6 v5] [GIT PULL] ftrace/x86: Ftrace cleanup and add support for -mfentry on x86_32
@ 2017-03-23 14:33 Steven Rostedt
2017-03-23 14:33 ` [PATCH 1/6 v5] ftrace/x86_64: Rename mcount_64.S to ftrace_64.S Steven Rostedt
` (5 more replies)
0 siblings, 6 replies; 37+ messages in thread
From: Steven Rostedt @ 2017-03-23 14:33 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 H.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: fadff4e942659f37a2374438dbb12388a9518107
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 for compiling ftrace_*.o
----
Changes since v4:
* s/mcount_/ftrace_/ in comment header that states the name.
(Namhyung Kim)
* Added Masami's Reviewed-by tag to patch 3
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} | 6 +-
5 files changed, 251 insertions(+), 177 deletions(-)
create mode 100644 arch/x86/kernel/ftrace_32.S
rename arch/x86/kernel/{mcount_64.S => ftrace_64.S} (98%)
Diff against v4:
diff --git a/arch/x86/kernel/ftrace_32.S b/arch/x86/kernel/ftrace_32.S
index de50c9084d16..97ede82aeb8c 100644
--- a/arch/x86/kernel/ftrace_32.S
+++ b/arch/x86/kernel/ftrace_32.S
@@ -1,5 +1,5 @@
/*
- * linux/arch/x86_64/mcount_64.S
+ * linux/arch/x86_64/ftrace_32.S
*
* Copyright (C) 2017 Steven Rostedt, VMware Inc.
*/
diff --git a/arch/x86/kernel/ftrace_64.S b/arch/x86/kernel/ftrace_64.S
index b9c46919d4fc..aef2361e7f83 100644
--- a/arch/x86/kernel/ftrace_64.S
+++ b/arch/x86/kernel/ftrace_64.S
@@ -1,5 +1,5 @@
/*
- * linux/arch/x86_64/mcount_64.S
+ * linux/arch/x86_64/ftrace_64.S
*
* Copyright (C) 2014 Steven Rostedt, Red Hat Inc
*/
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [PATCH 1/6 v5] ftrace/x86_64: Rename mcount_64.S to ftrace_64.S
2017-03-23 14:33 [PATCH 0/6 v5] [GIT PULL] ftrace/x86: Ftrace cleanup and add support for -mfentry on x86_32 Steven Rostedt
@ 2017-03-23 14:33 ` Steven Rostedt
2017-03-24 9:19 ` [tip:x86/asm] x86/ftrace: " tip-bot for Steven Rostedt (VMware)
2017-03-23 14:33 ` [PATCH 2/6 v5] ftrace/x86_32: Move the ftrace specific code out of entry_32.S Steven Rostedt
` (4 subsequent siblings)
5 siblings, 1 reply; 37+ messages in thread
From: Steven Rostedt @ 2017-03-23 14:33 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
[-- Attachment #1: 0001-ftrace-x86_64-Rename-mcount_64.S-to-ftrace_64.S.patch --]
[-- Type: text/plain, Size: 1673 bytes --]
From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
With the advent of -mfentry that uses the new "fentry" hook over mcount, the
mcount name is obsolete. Having the code file that ftrace hooks into called
"mcount*.S" is rather misleading. Rename it to ftrace_64.S
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
arch/x86/kernel/Makefile | 4 ++--
arch/x86/kernel/{mcount_64.S => ftrace_64.S} | 0
2 files changed, 2 insertions(+), 2 deletions(-)
rename arch/x86/kernel/{mcount_64.S => ftrace_64.S} (100%)
diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
index 84c00592d359..d3743a37e990 100644
--- a/arch/x86/kernel/Makefile
+++ b/arch/x86/kernel/Makefile
@@ -27,7 +27,7 @@ KASAN_SANITIZE_stacktrace.o := n
OBJECT_FILES_NON_STANDARD_head_$(BITS).o := y
OBJECT_FILES_NON_STANDARD_relocate_kernel_$(BITS).o := y
-OBJECT_FILES_NON_STANDARD_mcount_$(BITS).o := y
+OBJECT_FILES_NON_STANDARD_ftrace_$(BITS).o := y
OBJECT_FILES_NON_STANDARD_test_nx.o := y
# If instrumentation of this dir is enabled, boot hangs during first second.
@@ -46,7 +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 mcount_64.o
+obj-$(CONFIG_X86_64) += sys_x86_64.o ftrace_64.o
obj-$(CONFIG_X86_ESPFIX64) += espfix_64.o
obj-$(CONFIG_SYSFS) += ksysfs.o
obj-y += bootflag.o e820.o
diff --git a/arch/x86/kernel/mcount_64.S b/arch/x86/kernel/ftrace_64.S
similarity index 100%
rename from arch/x86/kernel/mcount_64.S
rename to arch/x86/kernel/ftrace_64.S
--
2.10.2
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [PATCH 2/6 v5] ftrace/x86_32: Move the ftrace specific code out of entry_32.S
2017-03-23 14:33 [PATCH 0/6 v5] [GIT PULL] ftrace/x86: Ftrace cleanup and add support for -mfentry on x86_32 Steven Rostedt
2017-03-23 14:33 ` [PATCH 1/6 v5] ftrace/x86_64: Rename mcount_64.S to ftrace_64.S Steven Rostedt
@ 2017-03-23 14:33 ` Steven Rostedt
2017-03-23 18:19 ` Thomas Gleixner
2017-03-24 9:20 ` [tip:x86/asm] x86/ftrace: " tip-bot for Steven Rostedt (VMware)
2017-03-23 14:33 ` [PATCH 3/6 v5] ftrace/x86_32: Add stack frame pointer to ftrace_caller Steven Rostedt
` (3 subsequent siblings)
5 siblings, 2 replies; 37+ messages in thread
From: Steven Rostedt @ 2017-03-23 14:33 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
[-- Attachment #1: 0002-ftrace-x86_32-Move-the-ftrace-specific-code-out-of-e.patch --]
[-- Type: text/plain, Size: 10160 bytes --]
From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
The function tracing hook code for ftrace is not an entry point from
userspace and does not belong in the entry_*.S files. It has already been
moved out of entry_64.S. This moves it out of entry_32.S into its own
ftrace_32.S file.
Also updated the comment in ftrace_64.S to state the correct file name.
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
arch/x86/entry/entry_32.S | 169 ------------------------------------------
arch/x86/kernel/Makefile | 1 +
arch/x86/kernel/ftrace_32.S | 177 ++++++++++++++++++++++++++++++++++++++++++++
arch/x86/kernel/ftrace_64.S | 2 +-
4 files changed, 179 insertions(+), 170 deletions(-)
create mode 100644 arch/x86/kernel/ftrace_32.S
diff --git a/arch/x86/entry/entry_32.S b/arch/x86/entry/entry_32.S
index 57f7ec35216e..169b3b0c5ec6 100644
--- a/arch/x86/entry/entry_32.S
+++ b/arch/x86/entry/entry_32.S
@@ -35,16 +35,13 @@
#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/ftrace.h>
#include <asm/irq_vectors.h>
#include <asm/cpufeatures.h>
#include <asm/alternative-asm.h>
#include <asm/asm.h>
#include <asm/smap.h>
-#include <asm/export.h>
#include <asm/frame.h>
.section .entry.text, "ax"
@@ -886,172 +883,6 @@ BUILD_INTERRUPT3(hyperv_callback_vector, HYPERVISOR_CALLBACK_VECTOR,
#endif /* CONFIG_HYPERV */
-#ifdef CONFIG_FUNCTION_TRACER
-#ifdef CONFIG_DYNAMIC_FTRACE
-
-ENTRY(mcount)
- ret
-END(mcount)
-
-ENTRY(ftrace_caller)
- pushl %eax
- pushl %ecx
- pushl %edx
- pushl $0 /* Pass NULL as regs pointer */
- movl 4*4(%esp), %eax
- movl 0x4(%ebp), %edx
- movl function_trace_op, %ecx
- subl $MCOUNT_INSN_SIZE, %eax
-
-.globl ftrace_call
-ftrace_call:
- call ftrace_stub
-
- addl $4, %esp /* skip NULL pointer */
- popl %edx
- popl %ecx
- popl %eax
-.Lftrace_ret:
-#ifdef CONFIG_FUNCTION_GRAPH_TRACER
-.globl ftrace_graph_call
-ftrace_graph_call:
- jmp ftrace_stub
-#endif
-
-/* This is weak to keep gas from relaxing the jumps */
-WEAK(ftrace_stub)
- ret
-END(ftrace_caller)
-
-ENTRY(ftrace_regs_caller)
- pushf /* push flags before compare (in cs location) */
-
- /*
- * i386 does not save SS and ESP when coming from kernel.
- * Instead, to get sp, ®s->sp is used (see ptrace.h).
- * Unfortunately, that means eflags must be at the same location
- * as the current return ip is. We move the return ip into the
- * ip location, and move flags into the return ip location.
- */
- pushl 4(%esp) /* save return ip into ip slot */
-
- pushl $0 /* Load 0 into orig_ax */
- pushl %gs
- pushl %fs
- pushl %es
- pushl %ds
- pushl %eax
- pushl %ebp
- pushl %edi
- pushl %esi
- pushl %edx
- pushl %ecx
- pushl %ebx
-
- movl 13*4(%esp), %eax /* Get the saved flags */
- movl %eax, 14*4(%esp) /* Move saved flags into regs->flags location */
- /* clobbering return ip */
- movl $__KERNEL_CS, 13*4(%esp)
-
- movl 12*4(%esp), %eax /* Load ip (1st parameter) */
- subl $MCOUNT_INSN_SIZE, %eax /* Adjust ip */
- movl 0x4(%ebp), %edx /* Load parent ip (2nd parameter) */
- movl function_trace_op, %ecx /* Save ftrace_pos in 3rd parameter */
- pushl %esp /* Save pt_regs as 4th parameter */
-
-GLOBAL(ftrace_regs_call)
- call ftrace_stub
-
- addl $4, %esp /* Skip pt_regs */
- movl 14*4(%esp), %eax /* Move flags back into cs */
- movl %eax, 13*4(%esp) /* Needed to keep addl from modifying flags */
- movl 12*4(%esp), %eax /* Get return ip from regs->ip */
- movl %eax, 14*4(%esp) /* Put return ip back for ret */
-
- popl %ebx
- popl %ecx
- popl %edx
- popl %esi
- popl %edi
- popl %ebp
- popl %eax
- popl %ds
- popl %es
- popl %fs
- popl %gs
- addl $8, %esp /* Skip orig_ax and ip */
- popf /* Pop flags at end (no addl to corrupt flags) */
- jmp .Lftrace_ret
-
- popf
- jmp ftrace_stub
-#else /* ! CONFIG_DYNAMIC_FTRACE */
-
-ENTRY(mcount)
- cmpl $__PAGE_OFFSET, %esp
- jb ftrace_stub /* Paging not enabled yet? */
-
- cmpl $ftrace_stub, ftrace_trace_function
- jnz .Ltrace
-#ifdef CONFIG_FUNCTION_GRAPH_TRACER
- cmpl $ftrace_stub, ftrace_graph_return
- jnz ftrace_graph_caller
-
- cmpl $ftrace_graph_entry_stub, ftrace_graph_entry
- jnz ftrace_graph_caller
-#endif
-.globl ftrace_stub
-ftrace_stub:
- ret
-
- /* taken from glibc */
-.Ltrace:
- pushl %eax
- pushl %ecx
- pushl %edx
- movl 0xc(%esp), %eax
- movl 0x4(%ebp), %edx
- subl $MCOUNT_INSN_SIZE, %eax
-
- call *ftrace_trace_function
-
- popl %edx
- popl %ecx
- popl %eax
- jmp ftrace_stub
-END(mcount)
-#endif /* CONFIG_DYNAMIC_FTRACE */
-EXPORT_SYMBOL(mcount)
-#endif /* CONFIG_FUNCTION_TRACER */
-
-#ifdef CONFIG_FUNCTION_GRAPH_TRACER
-ENTRY(ftrace_graph_caller)
- pushl %eax
- pushl %ecx
- pushl %edx
- movl 0xc(%esp), %eax
- lea 0x4(%ebp), %edx
- movl (%ebp), %ecx
- subl $MCOUNT_INSN_SIZE, %eax
- call prepare_ftrace_return
- popl %edx
- popl %ecx
- popl %eax
- ret
-END(ftrace_graph_caller)
-
-.globl return_to_handler
-return_to_handler:
- pushl %eax
- pushl %edx
- movl %ebp, %eax
- call ftrace_return_to_handler
- movl %eax, %ecx
- popl %edx
- popl %eax
- jmp *%ecx
-#endif
-
#ifdef CONFIG_TRACING
ENTRY(trace_page_fault)
ASM_CLAC
diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
index d3743a37e990..55e8902c461f 100644
--- a/arch/x86/kernel/Makefile
+++ b/arch/x86/kernel/Makefile
@@ -47,6 +47,7 @@ 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_ESPFIX64) += espfix_64.o
obj-$(CONFIG_SYSFS) += ksysfs.o
obj-y += bootflag.o e820.o
diff --git a/arch/x86/kernel/ftrace_32.S b/arch/x86/kernel/ftrace_32.S
new file mode 100644
index 000000000000..f380604514e4
--- /dev/null
+++ b/arch/x86/kernel/ftrace_32.S
@@ -0,0 +1,177 @@
+/*
+ * linux/arch/x86_64/ftrace_32.S
+ *
+ * Copyright (C) 2017 Steven Rostedt, VMware Inc.
+ */
+
+#include <linux/linkage.h>
+#include <asm/page_types.h>
+#include <asm/segment.h>
+#include <asm/export.h>
+#include <asm/ftrace.h>
+
+#ifdef CONFIG_FUNCTION_TRACER
+#ifdef CONFIG_DYNAMIC_FTRACE
+
+ENTRY(mcount)
+ ret
+END(mcount)
+
+ENTRY(ftrace_caller)
+ pushl %eax
+ pushl %ecx
+ pushl %edx
+ pushl $0 /* Pass NULL as regs pointer */
+ movl 4*4(%esp), %eax
+ movl 0x4(%ebp), %edx
+ movl function_trace_op, %ecx
+ subl $MCOUNT_INSN_SIZE, %eax
+
+.globl ftrace_call
+ftrace_call:
+ call ftrace_stub
+
+ addl $4, %esp /* skip NULL pointer */
+ popl %edx
+ popl %ecx
+ popl %eax
+.Lftrace_ret:
+#ifdef CONFIG_FUNCTION_GRAPH_TRACER
+.globl ftrace_graph_call
+ftrace_graph_call:
+ jmp ftrace_stub
+#endif
+
+/* This is weak to keep gas from relaxing the jumps */
+WEAK(ftrace_stub)
+ ret
+END(ftrace_caller)
+
+ENTRY(ftrace_regs_caller)
+ pushf /* push flags before compare (in cs location) */
+
+ /*
+ * i386 does not save SS and ESP when coming from kernel.
+ * Instead, to get sp, ®s->sp is used (see ptrace.h).
+ * Unfortunately, that means eflags must be at the same location
+ * as the current return ip is. We move the return ip into the
+ * ip location, and move flags into the return ip location.
+ */
+ pushl 4(%esp) /* save return ip into ip slot */
+
+ pushl $0 /* Load 0 into orig_ax */
+ pushl %gs
+ pushl %fs
+ pushl %es
+ pushl %ds
+ pushl %eax
+ pushl %ebp
+ pushl %edi
+ pushl %esi
+ pushl %edx
+ pushl %ecx
+ pushl %ebx
+
+ movl 13*4(%esp), %eax /* Get the saved flags */
+ movl %eax, 14*4(%esp) /* Move saved flags into regs->flags location */
+ /* clobbering return ip */
+ movl $__KERNEL_CS, 13*4(%esp)
+
+ movl 12*4(%esp), %eax /* Load ip (1st parameter) */
+ subl $MCOUNT_INSN_SIZE, %eax /* Adjust ip */
+ movl 0x4(%ebp), %edx /* Load parent ip (2nd parameter) */
+ movl function_trace_op, %ecx /* Save ftrace_pos in 3rd parameter */
+ pushl %esp /* Save pt_regs as 4th parameter */
+
+GLOBAL(ftrace_regs_call)
+ call ftrace_stub
+
+ addl $4, %esp /* Skip pt_regs */
+ movl 14*4(%esp), %eax /* Move flags back into cs */
+ movl %eax, 13*4(%esp) /* Needed to keep addl from modifying flags */
+ movl 12*4(%esp), %eax /* Get return ip from regs->ip */
+ movl %eax, 14*4(%esp) /* Put return ip back for ret */
+
+ popl %ebx
+ popl %ecx
+ popl %edx
+ popl %esi
+ popl %edi
+ popl %ebp
+ popl %eax
+ popl %ds
+ popl %es
+ popl %fs
+ popl %gs
+ addl $8, %esp /* Skip orig_ax and ip */
+ popf /* Pop flags at end (no addl to corrupt flags) */
+ jmp .Lftrace_ret
+
+ popf
+ jmp ftrace_stub
+#else /* ! CONFIG_DYNAMIC_FTRACE */
+
+ENTRY(mcount)
+ cmpl $__PAGE_OFFSET, %esp
+ jb ftrace_stub /* Paging not enabled yet? */
+
+ cmpl $ftrace_stub, ftrace_trace_function
+ jnz .Ltrace
+#ifdef CONFIG_FUNCTION_GRAPH_TRACER
+ cmpl $ftrace_stub, ftrace_graph_return
+ jnz ftrace_graph_caller
+
+ cmpl $ftrace_graph_entry_stub, ftrace_graph_entry
+ jnz ftrace_graph_caller
+#endif
+.globl ftrace_stub
+ftrace_stub:
+ ret
+
+ /* taken from glibc */
+.Ltrace:
+ pushl %eax
+ pushl %ecx
+ pushl %edx
+ movl 0xc(%esp), %eax
+ movl 0x4(%ebp), %edx
+ subl $MCOUNT_INSN_SIZE, %eax
+
+ call *ftrace_trace_function
+
+ popl %edx
+ popl %ecx
+ popl %eax
+ jmp ftrace_stub
+END(mcount)
+#endif /* CONFIG_DYNAMIC_FTRACE */
+EXPORT_SYMBOL(mcount)
+#endif /* CONFIG_FUNCTION_TRACER */
+
+#ifdef CONFIG_FUNCTION_GRAPH_TRACER
+ENTRY(ftrace_graph_caller)
+ pushl %eax
+ pushl %ecx
+ pushl %edx
+ movl 0xc(%esp), %eax
+ lea 0x4(%ebp), %edx
+ movl (%ebp), %ecx
+ subl $MCOUNT_INSN_SIZE, %eax
+ call prepare_ftrace_return
+ popl %edx
+ popl %ecx
+ popl %eax
+ ret
+END(ftrace_graph_caller)
+
+.globl return_to_handler
+return_to_handler:
+ pushl %eax
+ pushl %edx
+ movl %ebp, %eax
+ call ftrace_return_to_handler
+ movl %eax, %ecx
+ popl %edx
+ popl %eax
+ jmp *%ecx
+#endif
diff --git a/arch/x86/kernel/ftrace_64.S b/arch/x86/kernel/ftrace_64.S
index 7b0d3da52fb4..fc0fe0e6c2de 100644
--- a/arch/x86/kernel/ftrace_64.S
+++ b/arch/x86/kernel/ftrace_64.S
@@ -1,5 +1,5 @@
/*
- * linux/arch/x86_64/mcount_64.S
+ * linux/arch/x86_64/ftrace_64.S
*
* Copyright (C) 2014 Steven Rostedt, Red Hat Inc
*/
--
2.10.2
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [PATCH 3/6 v5] ftrace/x86_32: Add stack frame pointer to ftrace_caller
2017-03-23 14:33 [PATCH 0/6 v5] [GIT PULL] ftrace/x86: Ftrace cleanup and add support for -mfentry on x86_32 Steven Rostedt
2017-03-23 14:33 ` [PATCH 1/6 v5] ftrace/x86_64: Rename mcount_64.S to ftrace_64.S Steven Rostedt
2017-03-23 14:33 ` [PATCH 2/6 v5] ftrace/x86_32: Move the ftrace specific code out of entry_32.S Steven Rostedt
@ 2017-03-23 14:33 ` Steven Rostedt
2017-03-24 9:20 ` [tip:x86/asm] x86/ftrace: " tip-bot for Steven Rostedt (VMware)
2017-03-23 14:33 ` [PATCH 4/6 v5] ftrace/x86_32: Clean up ftrace_regs_caller Steven Rostedt
` (2 subsequent siblings)
5 siblings, 1 reply; 37+ messages in thread
From: Steven Rostedt @ 2017-03-23 14:33 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
[-- Attachment #1: 0003-ftrace-x86_32-Add-stack-frame-pointer-to-ftrace_call.patch --]
[-- Type: text/plain, Size: 2180 bytes --]
From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
The function hook ftrace_caller does not create its own stack frame, and
this causes the ftrace stack trace to miss the first function when doing
stack traces.
# echo schedule:stacktrace > /sys/kernel/tracing/set_ftrace_filter
Before:
<idle>-0 [002] .N.. 29.865807: <stack trace>
=> cpu_startup_entry
=> start_secondary
=> startup_32_smp
<...>-7 [001] .... 29.866509: <stack trace>
=> kthread
=> ret_from_fork
<...>-1 [000] .... 29.865377: <stack trace>
=> poll_schedule_timeout
=> do_select
=> core_sys_select
=> SyS_select
=> do_fast_syscall_32
=> entry_SYSENTER_32
After:
<idle>-0 [002] .N.. 31.234853: <stack trace>
=> do_idle
=> cpu_startup_entry
=> start_secondary
=> startup_32_smp
<...>-7 [003] .... 31.235140: <stack trace>
=> rcu_gp_kthread
=> kthread
=> ret_from_fork
<...>-1819 [000] .... 31.264172: <stack trace>
=> schedule_hrtimeout_range
=> poll_schedule_timeout
=> do_sys_poll
=> SyS_ppoll
=> do_fast_syscall_32
=> entry_SYSENTER_32
Reviewed-by: Josh Poimboeuf <jpoimboe@redhat.com>
Reviewed-by: Masami Hiramatsu <mhiramat@kernel.org>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
arch/x86/kernel/ftrace_32.S | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/arch/x86/kernel/ftrace_32.S b/arch/x86/kernel/ftrace_32.S
index f380604514e4..a28f1907e1e1 100644
--- a/arch/x86/kernel/ftrace_32.S
+++ b/arch/x86/kernel/ftrace_32.S
@@ -18,12 +18,19 @@ ENTRY(mcount)
END(mcount)
ENTRY(ftrace_caller)
+
+ pushl %ebp
+ movl %esp, %ebp
+
pushl %eax
pushl %ecx
pushl %edx
pushl $0 /* Pass NULL as regs pointer */
- movl 4*4(%esp), %eax
- movl 0x4(%ebp), %edx
+ movl 5*4(%esp), %eax
+ /* Copy original ebp into %edx */
+ movl 4*4(%esp), %edx
+ /* Get the parent ip */
+ movl 0x4(%edx), %edx
movl function_trace_op, %ecx
subl $MCOUNT_INSN_SIZE, %eax
@@ -35,6 +42,7 @@ ftrace_call:
popl %edx
popl %ecx
popl %eax
+ popl %ebp
.Lftrace_ret:
#ifdef CONFIG_FUNCTION_GRAPH_TRACER
.globl ftrace_graph_call
--
2.10.2
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [PATCH 4/6 v5] ftrace/x86_32: Clean up ftrace_regs_caller
2017-03-23 14:33 [PATCH 0/6 v5] [GIT PULL] ftrace/x86: Ftrace cleanup and add support for -mfentry on x86_32 Steven Rostedt
` (2 preceding siblings ...)
2017-03-23 14:33 ` [PATCH 3/6 v5] ftrace/x86_32: Add stack frame pointer to ftrace_caller Steven Rostedt
@ 2017-03-23 14:33 ` Steven Rostedt
2017-03-24 9:21 ` [tip:x86/asm] x86/ftrace: " tip-bot for Steven Rostedt (VMware)
2017-03-23 14:33 ` [PATCH 5/6 v5] ftrace/x86_32: Add -mfentry support to x86_32 with DYNAMIC_FTRACE set Steven Rostedt
2017-03-23 14:33 ` [PATCH 6/6 v5] ftrace/x86: Use Makefile logic instead of #ifdef for compiling ftrace_*.o Steven Rostedt
5 siblings, 1 reply; 37+ messages in thread
From: Steven Rostedt @ 2017-03-23 14:33 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
[-- Attachment #1: 0004-ftrace-x86_32-Clean-up-ftrace_regs_caller.patch --]
[-- Type: text/plain, Size: 3431 bytes --]
From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
When ftrace_regs_caller was created, it was designed to preserve flags as
much as possible as it needed to act just like a breakpoint triggered on the
same location. But the design is over complicated as it treated all
operations as modifying flags. But push, mov and lea do not modify flags.
This means the code can become more simplified by allowing flags to be
stored further down.
Making ftrace_regs_caller simpler will also be useful in implementing fentry
logic.
Link: http://lkml.kernel.org/r/20170316135328.36123c3e@gandalf.local.home
[ simpler logic to save flags ]
Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
Reviewed-by: Masami Hiramatsu <mhiramat@kernel.org>
Reviewed-by: Josh Poimboeuf <jpoimboe@redhat.com>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
arch/x86/kernel/ftrace_32.S | 40 +++++++++++++++++++++-------------------
1 file changed, 21 insertions(+), 19 deletions(-)
diff --git a/arch/x86/kernel/ftrace_32.S b/arch/x86/kernel/ftrace_32.S
index a28f1907e1e1..63e28504debf 100644
--- a/arch/x86/kernel/ftrace_32.S
+++ b/arch/x86/kernel/ftrace_32.S
@@ -56,23 +56,27 @@ WEAK(ftrace_stub)
END(ftrace_caller)
ENTRY(ftrace_regs_caller)
- pushf /* push flags before compare (in cs location) */
-
/*
* i386 does not save SS and ESP when coming from kernel.
* Instead, to get sp, ®s->sp is used (see ptrace.h).
* Unfortunately, that means eflags must be at the same location
* as the current return ip is. We move the return ip into the
- * ip location, and move flags into the return ip location.
+ * regs->ip location, and move flags into the return ip location.
*/
- pushl 4(%esp) /* save return ip into ip slot */
-
+ pushl $__KERNEL_CS
+ pushl 4(%esp) /* Save the return ip */
pushl $0 /* Load 0 into orig_ax */
pushl %gs
pushl %fs
pushl %es
pushl %ds
pushl %eax
+
+ /* Get flags and place them into the return ip slot */
+ pushf
+ popl %eax
+ movl %eax, 8*4(%esp)
+
pushl %ebp
pushl %edi
pushl %esi
@@ -80,11 +84,6 @@ ENTRY(ftrace_regs_caller)
pushl %ecx
pushl %ebx
- movl 13*4(%esp), %eax /* Get the saved flags */
- movl %eax, 14*4(%esp) /* Move saved flags into regs->flags location */
- /* clobbering return ip */
- movl $__KERNEL_CS, 13*4(%esp)
-
movl 12*4(%esp), %eax /* Load ip (1st parameter) */
subl $MCOUNT_INSN_SIZE, %eax /* Adjust ip */
movl 0x4(%ebp), %edx /* Load parent ip (2nd parameter) */
@@ -95,10 +94,14 @@ GLOBAL(ftrace_regs_call)
call ftrace_stub
addl $4, %esp /* Skip pt_regs */
- movl 14*4(%esp), %eax /* Move flags back into cs */
- movl %eax, 13*4(%esp) /* Needed to keep addl from modifying flags */
- movl 12*4(%esp), %eax /* Get return ip from regs->ip */
- movl %eax, 14*4(%esp) /* Put return ip back for ret */
+
+ /* restore flags */
+ push 14*4(%esp)
+ popf
+
+ /* Move return ip back to its original location */
+ movl 12*4(%esp), %eax
+ movl %eax, 14*4(%esp)
popl %ebx
popl %ecx
@@ -111,12 +114,11 @@ GLOBAL(ftrace_regs_call)
popl %es
popl %fs
popl %gs
- addl $8, %esp /* Skip orig_ax and ip */
- popf /* Pop flags at end (no addl to corrupt flags) */
- jmp .Lftrace_ret
- popf
- jmp ftrace_stub
+ /* use lea to not affect flags */
+ lea 3*4(%esp), %esp /* Skip orig_ax, ip and cs */
+
+ jmp .Lftrace_ret
#else /* ! CONFIG_DYNAMIC_FTRACE */
ENTRY(mcount)
--
2.10.2
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [PATCH 5/6 v5] ftrace/x86_32: Add -mfentry support to x86_32 with DYNAMIC_FTRACE set
2017-03-23 14:33 [PATCH 0/6 v5] [GIT PULL] ftrace/x86: Ftrace cleanup and add support for -mfentry on x86_32 Steven Rostedt
` (3 preceding siblings ...)
2017-03-23 14:33 ` [PATCH 4/6 v5] ftrace/x86_32: Clean up ftrace_regs_caller Steven Rostedt
@ 2017-03-23 14:33 ` Steven Rostedt
2017-03-24 9:21 ` [tip:x86/asm] x86/ftrace: " tip-bot for Steven Rostedt (VMware)
2017-03-23 14:33 ` [PATCH 6/6 v5] ftrace/x86: Use Makefile logic instead of #ifdef for compiling ftrace_*.o Steven Rostedt
5 siblings, 1 reply; 37+ messages in thread
From: Steven Rostedt @ 2017-03-23 14:33 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
[-- Attachment #1: 0005-ftrace-x86_32-Add-mfentry-support-to-x86_32-with-DYN.patch --]
[-- Type: text/plain, Size: 5177 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>
Reviewed-by: Josh Poimboeuf <jpoimboe@redhat.com>
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 63e28504debf..15acf54083c1 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
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [PATCH 6/6 v5] ftrace/x86: Use Makefile logic instead of #ifdef for compiling ftrace_*.o
2017-03-23 14:33 [PATCH 0/6 v5] [GIT PULL] ftrace/x86: Ftrace cleanup and add support for -mfentry on x86_32 Steven Rostedt
` (4 preceding siblings ...)
2017-03-23 14:33 ` [PATCH 5/6 v5] ftrace/x86_32: Add -mfentry support to x86_32 with DYNAMIC_FTRACE set Steven Rostedt
@ 2017-03-23 14:33 ` Steven Rostedt
2017-03-24 9:22 ` [tip:x86/asm] x86/ftrace: " tip-bot for Steven Rostedt (VMware)
5 siblings, 1 reply; 37+ messages in thread
From: Steven Rostedt @ 2017-03-23 14:33 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
[-- Attachment #1: 0006-ftrace-x86-Use-Makefile-logic-instead-of-ifdef-for-c.patch --]
[-- Type: text/plain, Size: 2809 bytes --]
From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
Currently ftrace_32.S and ftrace_64.S are compiled even when
CONFIG_FUNCTION_TRACER is not set. This means there's an unnecessary #ifdef
to protect the code. Instead of using preprocessor directives, only compile
those files when FUNCTION_TRACER is defined.
Link: http://lkml.kernel.org/r/20170316210043.peycxdxktwwn6cid@treble
Suggested-by: Josh Poimboeuf <jpoimboe@redhat.com>
Reviewed-by: Josh Poimboeuf <jpoimboe@redhat.com>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
arch/x86/kernel/Makefile | 4 ++--
arch/x86/kernel/ftrace_32.S | 3 ---
arch/x86/kernel/ftrace_64.S | 4 ----
3 files changed, 2 insertions(+), 9 deletions(-)
diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
index 55e8902c461f..4b994232cb57 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 15acf54083c1..97ede82aeb8c 100644
--- a/arch/x86/kernel/ftrace_32.S
+++ b/arch/x86/kernel/ftrace_32.S
@@ -10,8 +10,6 @@
#include <asm/export.h>
#include <asm/ftrace.h>
-#ifdef CONFIG_FUNCTION_TRACER
-
#ifdef CC_USING_FENTRY
# define function_hook __fentry__
EXPORT_SYMBOL(__fentry__)
@@ -208,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 fc0fe0e6c2de..aef2361e7f83 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)
--
2.10.2
^ permalink raw reply related [flat|nested] 37+ messages in thread
* Re: [PATCH 2/6 v5] ftrace/x86_32: Move the ftrace specific code out of entry_32.S
2017-03-23 14:33 ` [PATCH 2/6 v5] ftrace/x86_32: Move the ftrace specific code out of entry_32.S Steven Rostedt
@ 2017-03-23 18:19 ` Thomas Gleixner
2017-03-23 19:03 ` Steven Rostedt
2017-03-24 9:20 ` [tip:x86/asm] x86/ftrace: " tip-bot for Steven Rostedt (VMware)
1 sibling, 1 reply; 37+ messages in thread
From: Thomas Gleixner @ 2017-03-23 18:19 UTC (permalink / raw)
To: Steven Rostedt
Cc: linux-kernel, Ingo Molnar, Andrew Morton, Peter Zijlstra,
Masami Hiramatsu, H. Peter Anvin, Andy Lutomirski,
Josh Poimboeuf, Linus Torvalds
On Thu, 23 Mar 2017, Steven Rostedt wrote:
> --- a/arch/x86/kernel/ftrace_64.S
> +++ b/arch/x86/kernel/ftrace_64.S
> @@ -1,5 +1,5 @@
> /*
> - * linux/arch/x86_64/mcount_64.S
> + * linux/arch/x86_64/ftrace_64.S
Can we please get rid of that nonsense completely?
> *
> * Copyright (C) 2014 Steven Rostedt, Red Hat Inc
> */
> --
> 2.10.2
>
>
>
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 2/6 v5] ftrace/x86_32: Move the ftrace specific code out of entry_32.S
2017-03-23 18:19 ` Thomas Gleixner
@ 2017-03-23 19:03 ` Steven Rostedt
0 siblings, 0 replies; 37+ messages in thread
From: Steven Rostedt @ 2017-03-23 19:03 UTC (permalink / raw)
To: Thomas Gleixner
Cc: linux-kernel, Ingo Molnar, Andrew Morton, Peter Zijlstra,
Masami Hiramatsu, H. Peter Anvin, Andy Lutomirski,
Josh Poimboeuf, Linus Torvalds
On Thu, 23 Mar 2017 19:19:36 +0100 (CET)
Thomas Gleixner <tglx@linutronix.de> wrote:
> On Thu, 23 Mar 2017, Steven Rostedt wrote:
> > --- a/arch/x86/kernel/ftrace_64.S
> > +++ b/arch/x86/kernel/ftrace_64.S
> > @@ -1,5 +1,5 @@
> > /*
> > - * linux/arch/x86_64/mcount_64.S
> > + * linux/arch/x86_64/ftrace_64.S
>
> Can we please get rid of that nonsense completely?
Sure.
I'll have a v6 ready in a bit.
-- Steve
>
> > *
> > * Copyright (C) 2014 Steven Rostedt, Red Hat Inc
> > */
> > --
> > 2.10.2
> >
> >
> >
^ permalink raw reply [flat|nested] 37+ messages in thread
* [tip:x86/asm] x86/ftrace: Rename mcount_64.S to ftrace_64.S
2017-03-23 14:33 ` [PATCH 1/6 v5] ftrace/x86_64: Rename mcount_64.S to ftrace_64.S Steven Rostedt
@ 2017-03-24 9:19 ` tip-bot for Steven Rostedt (VMware)
0 siblings, 0 replies; 37+ messages in thread
From: tip-bot for Steven Rostedt (VMware) @ 2017-03-24 9:19 UTC (permalink / raw)
To: linux-tip-commits
Cc: luto, mhiramat, torvalds, linux-kernel, jpoimboe, akpm, peterz,
hpa, tglx, mingo, rostedt
Commit-ID: db65d7b6dcd313ad54c3589813f70b805dbb48ec
Gitweb: http://git.kernel.org/tip/db65d7b6dcd313ad54c3589813f70b805dbb48ec
Author: Steven Rostedt (VMware) <rostedt@goodmis.org>
AuthorDate: Thu, 23 Mar 2017 10:33:48 -0400
Committer: Thomas Gleixner <tglx@linutronix.de>
CommitDate: Fri, 24 Mar 2017 10:14:06 +0100
x86/ftrace: Rename mcount_64.S to ftrace_64.S
With the advent of -mfentry that uses the new "fentry" hook over mcount,
the mcount name is obsolete. Having the code file that ftrace hooks into
called "mcount*.S" is rather misleading. Rename it to ftrace_64.S and
remove the file name reference.
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
Reviewed-by: Ingo Molnar <mingo@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Link: http://lkml.kernel.org/r/20170323143445.490601451@goodmis.org
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
arch/x86/kernel/Makefile | 4 ++--
arch/x86/kernel/{mcount_64.S => ftrace_64.S} | 2 --
2 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
index 84c0059..d3743a37 100644
--- a/arch/x86/kernel/Makefile
+++ b/arch/x86/kernel/Makefile
@@ -27,7 +27,7 @@ KASAN_SANITIZE_stacktrace.o := n
OBJECT_FILES_NON_STANDARD_head_$(BITS).o := y
OBJECT_FILES_NON_STANDARD_relocate_kernel_$(BITS).o := y
-OBJECT_FILES_NON_STANDARD_mcount_$(BITS).o := y
+OBJECT_FILES_NON_STANDARD_ftrace_$(BITS).o := y
OBJECT_FILES_NON_STANDARD_test_nx.o := y
# If instrumentation of this dir is enabled, boot hangs during first second.
@@ -46,7 +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 mcount_64.o
+obj-$(CONFIG_X86_64) += sys_x86_64.o ftrace_64.o
obj-$(CONFIG_X86_ESPFIX64) += espfix_64.o
obj-$(CONFIG_SYSFS) += ksysfs.o
obj-y += bootflag.o e820.o
diff --git a/arch/x86/kernel/mcount_64.S b/arch/x86/kernel/ftrace_64.S
similarity index 99%
rename from arch/x86/kernel/mcount_64.S
rename to arch/x86/kernel/ftrace_64.S
index 7b0d3da..d1be3cb 100644
--- a/arch/x86/kernel/mcount_64.S
+++ b/arch/x86/kernel/ftrace_64.S
@@ -1,6 +1,4 @@
/*
- * linux/arch/x86_64/mcount_64.S
- *
* Copyright (C) 2014 Steven Rostedt, Red Hat Inc
*/
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [tip:x86/asm] x86/ftrace: Move the ftrace specific code out of entry_32.S
2017-03-23 14:33 ` [PATCH 2/6 v5] ftrace/x86_32: Move the ftrace specific code out of entry_32.S Steven Rostedt
2017-03-23 18:19 ` Thomas Gleixner
@ 2017-03-24 9:20 ` tip-bot for Steven Rostedt (VMware)
1 sibling, 0 replies; 37+ messages in thread
From: tip-bot for Steven Rostedt (VMware) @ 2017-03-24 9:20 UTC (permalink / raw)
To: linux-tip-commits
Cc: linux-kernel, mingo, peterz, akpm, luto, hpa, mhiramat, torvalds,
jpoimboe, rostedt, tglx
Commit-ID: 3d82c59c6e3cb168284d9b0a1143415d9c98ae40
Gitweb: http://git.kernel.org/tip/3d82c59c6e3cb168284d9b0a1143415d9c98ae40
Author: Steven Rostedt (VMware) <rostedt@goodmis.org>
AuthorDate: Thu, 23 Mar 2017 10:33:49 -0400
Committer: Thomas Gleixner <tglx@linutronix.de>
CommitDate: Fri, 24 Mar 2017 10:14:07 +0100
x86/ftrace: Move the ftrace specific code out of entry_32.S
The function tracing hook code for ftrace is not an entry point from
userspace and does not belong in the entry_*.S files. It has already been
moved out of entry_64.S.
Move it out of entry_32.S into its own ftrace_32.S file.
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
Reviewed-by: Ingo Molnar <mingo@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Link: http://lkml.kernel.org/r/20170323143445.645218946@goodmis.org
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
arch/x86/entry/entry_32.S | 169 ------------------------------------------
arch/x86/kernel/Makefile | 1 +
arch/x86/kernel/ftrace_32.S | 175 ++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 176 insertions(+), 169 deletions(-)
diff --git a/arch/x86/entry/entry_32.S b/arch/x86/entry/entry_32.S
index 5553475..50bc269 100644
--- a/arch/x86/entry/entry_32.S
+++ b/arch/x86/entry/entry_32.S
@@ -35,16 +35,13 @@
#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/ftrace.h>
#include <asm/irq_vectors.h>
#include <asm/cpufeatures.h>
#include <asm/alternative-asm.h>
#include <asm/asm.h>
#include <asm/smap.h>
-#include <asm/export.h>
#include <asm/frame.h>
.section .entry.text, "ax"
@@ -886,172 +883,6 @@ BUILD_INTERRUPT3(hyperv_callback_vector, HYPERVISOR_CALLBACK_VECTOR,
#endif /* CONFIG_HYPERV */
-#ifdef CONFIG_FUNCTION_TRACER
-#ifdef CONFIG_DYNAMIC_FTRACE
-
-ENTRY(mcount)
- ret
-END(mcount)
-
-ENTRY(ftrace_caller)
- pushl %eax
- pushl %ecx
- pushl %edx
- pushl $0 /* Pass NULL as regs pointer */
- movl 4*4(%esp), %eax
- movl 0x4(%ebp), %edx
- movl function_trace_op, %ecx
- subl $MCOUNT_INSN_SIZE, %eax
-
-.globl ftrace_call
-ftrace_call:
- call ftrace_stub
-
- addl $4, %esp /* skip NULL pointer */
- popl %edx
- popl %ecx
- popl %eax
-.Lftrace_ret:
-#ifdef CONFIG_FUNCTION_GRAPH_TRACER
-.globl ftrace_graph_call
-ftrace_graph_call:
- jmp ftrace_stub
-#endif
-
-/* This is weak to keep gas from relaxing the jumps */
-WEAK(ftrace_stub)
- ret
-END(ftrace_caller)
-
-ENTRY(ftrace_regs_caller)
- pushf /* push flags before compare (in cs location) */
-
- /*
- * i386 does not save SS and ESP when coming from kernel.
- * Instead, to get sp, ®s->sp is used (see ptrace.h).
- * Unfortunately, that means eflags must be at the same location
- * as the current return ip is. We move the return ip into the
- * ip location, and move flags into the return ip location.
- */
- pushl 4(%esp) /* save return ip into ip slot */
-
- pushl $0 /* Load 0 into orig_ax */
- pushl %gs
- pushl %fs
- pushl %es
- pushl %ds
- pushl %eax
- pushl %ebp
- pushl %edi
- pushl %esi
- pushl %edx
- pushl %ecx
- pushl %ebx
-
- movl 13*4(%esp), %eax /* Get the saved flags */
- movl %eax, 14*4(%esp) /* Move saved flags into regs->flags location */
- /* clobbering return ip */
- movl $__KERNEL_CS, 13*4(%esp)
-
- movl 12*4(%esp), %eax /* Load ip (1st parameter) */
- subl $MCOUNT_INSN_SIZE, %eax /* Adjust ip */
- movl 0x4(%ebp), %edx /* Load parent ip (2nd parameter) */
- movl function_trace_op, %ecx /* Save ftrace_pos in 3rd parameter */
- pushl %esp /* Save pt_regs as 4th parameter */
-
-GLOBAL(ftrace_regs_call)
- call ftrace_stub
-
- addl $4, %esp /* Skip pt_regs */
- movl 14*4(%esp), %eax /* Move flags back into cs */
- movl %eax, 13*4(%esp) /* Needed to keep addl from modifying flags */
- movl 12*4(%esp), %eax /* Get return ip from regs->ip */
- movl %eax, 14*4(%esp) /* Put return ip back for ret */
-
- popl %ebx
- popl %ecx
- popl %edx
- popl %esi
- popl %edi
- popl %ebp
- popl %eax
- popl %ds
- popl %es
- popl %fs
- popl %gs
- addl $8, %esp /* Skip orig_ax and ip */
- popf /* Pop flags at end (no addl to corrupt flags) */
- jmp .Lftrace_ret
-
- popf
- jmp ftrace_stub
-#else /* ! CONFIG_DYNAMIC_FTRACE */
-
-ENTRY(mcount)
- cmpl $__PAGE_OFFSET, %esp
- jb ftrace_stub /* Paging not enabled yet? */
-
- cmpl $ftrace_stub, ftrace_trace_function
- jnz .Ltrace
-#ifdef CONFIG_FUNCTION_GRAPH_TRACER
- cmpl $ftrace_stub, ftrace_graph_return
- jnz ftrace_graph_caller
-
- cmpl $ftrace_graph_entry_stub, ftrace_graph_entry
- jnz ftrace_graph_caller
-#endif
-.globl ftrace_stub
-ftrace_stub:
- ret
-
- /* taken from glibc */
-.Ltrace:
- pushl %eax
- pushl %ecx
- pushl %edx
- movl 0xc(%esp), %eax
- movl 0x4(%ebp), %edx
- subl $MCOUNT_INSN_SIZE, %eax
-
- call *ftrace_trace_function
-
- popl %edx
- popl %ecx
- popl %eax
- jmp ftrace_stub
-END(mcount)
-#endif /* CONFIG_DYNAMIC_FTRACE */
-EXPORT_SYMBOL(mcount)
-#endif /* CONFIG_FUNCTION_TRACER */
-
-#ifdef CONFIG_FUNCTION_GRAPH_TRACER
-ENTRY(ftrace_graph_caller)
- pushl %eax
- pushl %ecx
- pushl %edx
- movl 0xc(%esp), %eax
- lea 0x4(%ebp), %edx
- movl (%ebp), %ecx
- subl $MCOUNT_INSN_SIZE, %eax
- call prepare_ftrace_return
- popl %edx
- popl %ecx
- popl %eax
- ret
-END(ftrace_graph_caller)
-
-.globl return_to_handler
-return_to_handler:
- pushl %eax
- pushl %edx
- movl %ebp, %eax
- call ftrace_return_to_handler
- movl %eax, %ecx
- popl %edx
- popl %eax
- jmp *%ecx
-#endif
-
#ifdef CONFIG_TRACING
ENTRY(trace_page_fault)
ASM_CLAC
diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
index d3743a37..55e8902 100644
--- a/arch/x86/kernel/Makefile
+++ b/arch/x86/kernel/Makefile
@@ -47,6 +47,7 @@ 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_ESPFIX64) += espfix_64.o
obj-$(CONFIG_SYSFS) += ksysfs.o
obj-y += bootflag.o e820.o
diff --git a/arch/x86/kernel/ftrace_32.S b/arch/x86/kernel/ftrace_32.S
new file mode 100644
index 0000000..2b160f2
--- /dev/null
+++ b/arch/x86/kernel/ftrace_32.S
@@ -0,0 +1,175 @@
+/*
+ * Copyright (C) 2017 Steven Rostedt, VMware Inc.
+ */
+
+#include <linux/linkage.h>
+#include <asm/page_types.h>
+#include <asm/segment.h>
+#include <asm/export.h>
+#include <asm/ftrace.h>
+
+#ifdef CONFIG_FUNCTION_TRACER
+#ifdef CONFIG_DYNAMIC_FTRACE
+
+ENTRY(mcount)
+ ret
+END(mcount)
+
+ENTRY(ftrace_caller)
+ pushl %eax
+ pushl %ecx
+ pushl %edx
+ pushl $0 /* Pass NULL as regs pointer */
+ movl 4*4(%esp), %eax
+ movl 0x4(%ebp), %edx
+ movl function_trace_op, %ecx
+ subl $MCOUNT_INSN_SIZE, %eax
+
+.globl ftrace_call
+ftrace_call:
+ call ftrace_stub
+
+ addl $4, %esp /* skip NULL pointer */
+ popl %edx
+ popl %ecx
+ popl %eax
+.Lftrace_ret:
+#ifdef CONFIG_FUNCTION_GRAPH_TRACER
+.globl ftrace_graph_call
+ftrace_graph_call:
+ jmp ftrace_stub
+#endif
+
+/* This is weak to keep gas from relaxing the jumps */
+WEAK(ftrace_stub)
+ ret
+END(ftrace_caller)
+
+ENTRY(ftrace_regs_caller)
+ pushf /* push flags before compare (in cs location) */
+
+ /*
+ * i386 does not save SS and ESP when coming from kernel.
+ * Instead, to get sp, ®s->sp is used (see ptrace.h).
+ * Unfortunately, that means eflags must be at the same location
+ * as the current return ip is. We move the return ip into the
+ * ip location, and move flags into the return ip location.
+ */
+ pushl 4(%esp) /* save return ip into ip slot */
+
+ pushl $0 /* Load 0 into orig_ax */
+ pushl %gs
+ pushl %fs
+ pushl %es
+ pushl %ds
+ pushl %eax
+ pushl %ebp
+ pushl %edi
+ pushl %esi
+ pushl %edx
+ pushl %ecx
+ pushl %ebx
+
+ movl 13*4(%esp), %eax /* Get the saved flags */
+ movl %eax, 14*4(%esp) /* Move saved flags into regs->flags location */
+ /* clobbering return ip */
+ movl $__KERNEL_CS, 13*4(%esp)
+
+ movl 12*4(%esp), %eax /* Load ip (1st parameter) */
+ subl $MCOUNT_INSN_SIZE, %eax /* Adjust ip */
+ movl 0x4(%ebp), %edx /* Load parent ip (2nd parameter) */
+ movl function_trace_op, %ecx /* Save ftrace_pos in 3rd parameter */
+ pushl %esp /* Save pt_regs as 4th parameter */
+
+GLOBAL(ftrace_regs_call)
+ call ftrace_stub
+
+ addl $4, %esp /* Skip pt_regs */
+ movl 14*4(%esp), %eax /* Move flags back into cs */
+ movl %eax, 13*4(%esp) /* Needed to keep addl from modifying flags */
+ movl 12*4(%esp), %eax /* Get return ip from regs->ip */
+ movl %eax, 14*4(%esp) /* Put return ip back for ret */
+
+ popl %ebx
+ popl %ecx
+ popl %edx
+ popl %esi
+ popl %edi
+ popl %ebp
+ popl %eax
+ popl %ds
+ popl %es
+ popl %fs
+ popl %gs
+ addl $8, %esp /* Skip orig_ax and ip */
+ popf /* Pop flags at end (no addl to corrupt flags) */
+ jmp .Lftrace_ret
+
+ popf
+ jmp ftrace_stub
+#else /* ! CONFIG_DYNAMIC_FTRACE */
+
+ENTRY(mcount)
+ cmpl $__PAGE_OFFSET, %esp
+ jb ftrace_stub /* Paging not enabled yet? */
+
+ cmpl $ftrace_stub, ftrace_trace_function
+ jnz .Ltrace
+#ifdef CONFIG_FUNCTION_GRAPH_TRACER
+ cmpl $ftrace_stub, ftrace_graph_return
+ jnz ftrace_graph_caller
+
+ cmpl $ftrace_graph_entry_stub, ftrace_graph_entry
+ jnz ftrace_graph_caller
+#endif
+.globl ftrace_stub
+ftrace_stub:
+ ret
+
+ /* taken from glibc */
+.Ltrace:
+ pushl %eax
+ pushl %ecx
+ pushl %edx
+ movl 0xc(%esp), %eax
+ movl 0x4(%ebp), %edx
+ subl $MCOUNT_INSN_SIZE, %eax
+
+ call *ftrace_trace_function
+
+ popl %edx
+ popl %ecx
+ popl %eax
+ jmp ftrace_stub
+END(mcount)
+#endif /* CONFIG_DYNAMIC_FTRACE */
+EXPORT_SYMBOL(mcount)
+#endif /* CONFIG_FUNCTION_TRACER */
+
+#ifdef CONFIG_FUNCTION_GRAPH_TRACER
+ENTRY(ftrace_graph_caller)
+ pushl %eax
+ pushl %ecx
+ pushl %edx
+ movl 0xc(%esp), %eax
+ lea 0x4(%ebp), %edx
+ movl (%ebp), %ecx
+ subl $MCOUNT_INSN_SIZE, %eax
+ call prepare_ftrace_return
+ popl %edx
+ popl %ecx
+ popl %eax
+ ret
+END(ftrace_graph_caller)
+
+.globl return_to_handler
+return_to_handler:
+ pushl %eax
+ pushl %edx
+ movl %ebp, %eax
+ call ftrace_return_to_handler
+ movl %eax, %ecx
+ popl %edx
+ popl %eax
+ jmp *%ecx
+#endif
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [tip:x86/asm] x86/ftrace: Add stack frame pointer to ftrace_caller
2017-03-23 14:33 ` [PATCH 3/6 v5] ftrace/x86_32: Add stack frame pointer to ftrace_caller Steven Rostedt
@ 2017-03-24 9:20 ` tip-bot for Steven Rostedt (VMware)
0 siblings, 0 replies; 37+ messages in thread
From: tip-bot for Steven Rostedt (VMware) @ 2017-03-24 9:20 UTC (permalink / raw)
To: linux-tip-commits
Cc: jpoimboe, peterz, linux-kernel, hpa, mhiramat, akpm, tglx,
rostedt, mingo, luto, torvalds
Commit-ID: e6928e58d4d4a02f88838945f792c107623314ac
Gitweb: http://git.kernel.org/tip/e6928e58d4d4a02f88838945f792c107623314ac
Author: Steven Rostedt (VMware) <rostedt@goodmis.org>
AuthorDate: Thu, 23 Mar 2017 10:33:50 -0400
Committer: Thomas Gleixner <tglx@linutronix.de>
CommitDate: Fri, 24 Mar 2017 10:14:07 +0100
x86/ftrace: Add stack frame pointer to ftrace_caller
The function hook ftrace_caller does not create its own stack frame, and
this causes the ftrace stack trace to miss the first function when doing
stack traces.
# echo schedule:stacktrace > /sys/kernel/tracing/set_ftrace_filter
Before:
<idle>-0 [002] .N.. 29.865807: <stack trace>
=> cpu_startup_entry
=> start_secondary
=> startup_32_smp
<...>-7 [001] .... 29.866509: <stack trace>
=> kthread
=> ret_from_fork
<...>-1 [000] .... 29.865377: <stack trace>
=> poll_schedule_timeout
=> do_select
=> core_sys_select
=> SyS_select
=> do_fast_syscall_32
=> entry_SYSENTER_32
After:
<idle>-0 [002] .N.. 31.234853: <stack trace>
=> do_idle
=> cpu_startup_entry
=> start_secondary
=> startup_32_smp
<...>-7 [003] .... 31.235140: <stack trace>
=> rcu_gp_kthread
=> kthread
=> ret_from_fork
<...>-1819 [000] .... 31.264172: <stack trace>
=> schedule_hrtimeout_range
=> poll_schedule_timeout
=> do_sys_poll
=> SyS_ppoll
=> do_fast_syscall_32
=> entry_SYSENTER_32
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
Reviewed-by: Josh Poimboeuf <jpoimboe@redhat.com>
Reviewed-by: Masami Hiramatsu <mhiramat@kernel.org>
Reviewed-by: Ingo Molnar <mingo@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Link: http://lkml.kernel.org/r/20170323143445.771707773@goodmis.org
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
arch/x86/kernel/ftrace_32.S | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/arch/x86/kernel/ftrace_32.S b/arch/x86/kernel/ftrace_32.S
index 2b160f2..a4e2872 100644
--- a/arch/x86/kernel/ftrace_32.S
+++ b/arch/x86/kernel/ftrace_32.S
@@ -16,12 +16,19 @@ ENTRY(mcount)
END(mcount)
ENTRY(ftrace_caller)
+
+ pushl %ebp
+ movl %esp, %ebp
+
pushl %eax
pushl %ecx
pushl %edx
pushl $0 /* Pass NULL as regs pointer */
- movl 4*4(%esp), %eax
- movl 0x4(%ebp), %edx
+ movl 5*4(%esp), %eax
+ /* Copy original ebp into %edx */
+ movl 4*4(%esp), %edx
+ /* Get the parent ip */
+ movl 0x4(%edx), %edx
movl function_trace_op, %ecx
subl $MCOUNT_INSN_SIZE, %eax
@@ -33,6 +40,7 @@ ftrace_call:
popl %edx
popl %ecx
popl %eax
+ popl %ebp
.Lftrace_ret:
#ifdef CONFIG_FUNCTION_GRAPH_TRACER
.globl ftrace_graph_call
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [tip:x86/asm] x86/ftrace: Clean up ftrace_regs_caller
2017-03-23 14:33 ` [PATCH 4/6 v5] ftrace/x86_32: Clean up ftrace_regs_caller Steven Rostedt
@ 2017-03-24 9:21 ` tip-bot for Steven Rostedt (VMware)
0 siblings, 0 replies; 37+ messages in thread
From: tip-bot for Steven Rostedt (VMware) @ 2017-03-24 9:21 UTC (permalink / raw)
To: linux-tip-commits
Cc: mhiramat, torvalds, linux-kernel, mingo, akpm, tglx, luto, hpa,
peterz, jpoimboe, rostedt
Commit-ID: ff04b440d2d645fd8a5b3385b1b2e4d19d3fe746
Gitweb: http://git.kernel.org/tip/ff04b440d2d645fd8a5b3385b1b2e4d19d3fe746
Author: Steven Rostedt (VMware) <rostedt@goodmis.org>
AuthorDate: Thu, 23 Mar 2017 10:33:51 -0400
Committer: Thomas Gleixner <tglx@linutronix.de>
CommitDate: Fri, 24 Mar 2017 10:14:07 +0100
x86/ftrace: Clean up ftrace_regs_caller
When ftrace_regs_caller was created, it was designed to preserve flags as
much as possible as it needed to act just like a breakpoint triggered on the
same location. But the design is over complicated as it treated all
operations as modifying flags. But push, mov and lea do not modify flags.
This means the code can become more simplified by allowing flags to be
stored further down.
Making ftrace_regs_caller simpler will also be useful in implementing fentry
logic.
Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
Reviewed-by: Masami Hiramatsu <mhiramat@kernel.org>
Reviewed-by: Josh Poimboeuf <jpoimboe@redhat.com>
Reviewed-by: Ingo Molnar <mingo@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Andrew Morton <akpm@linux-foundation.org>
Link: http://lkml.kernel.org/r/20170316135328.36123c3e@gandalf.local.home
Link: http://lkml.kernel.org/r/20170323143445.917292592@goodmis.org
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
arch/x86/kernel/ftrace_32.S | 40 +++++++++++++++++++++-------------------
1 file changed, 21 insertions(+), 19 deletions(-)
diff --git a/arch/x86/kernel/ftrace_32.S b/arch/x86/kernel/ftrace_32.S
index a4e2872..93e2664 100644
--- a/arch/x86/kernel/ftrace_32.S
+++ b/arch/x86/kernel/ftrace_32.S
@@ -54,23 +54,27 @@ WEAK(ftrace_stub)
END(ftrace_caller)
ENTRY(ftrace_regs_caller)
- pushf /* push flags before compare (in cs location) */
-
/*
* i386 does not save SS and ESP when coming from kernel.
* Instead, to get sp, ®s->sp is used (see ptrace.h).
* Unfortunately, that means eflags must be at the same location
* as the current return ip is. We move the return ip into the
- * ip location, and move flags into the return ip location.
+ * regs->ip location, and move flags into the return ip location.
*/
- pushl 4(%esp) /* save return ip into ip slot */
-
+ pushl $__KERNEL_CS
+ pushl 4(%esp) /* Save the return ip */
pushl $0 /* Load 0 into orig_ax */
pushl %gs
pushl %fs
pushl %es
pushl %ds
pushl %eax
+
+ /* Get flags and place them into the return ip slot */
+ pushf
+ popl %eax
+ movl %eax, 8*4(%esp)
+
pushl %ebp
pushl %edi
pushl %esi
@@ -78,11 +82,6 @@ ENTRY(ftrace_regs_caller)
pushl %ecx
pushl %ebx
- movl 13*4(%esp), %eax /* Get the saved flags */
- movl %eax, 14*4(%esp) /* Move saved flags into regs->flags location */
- /* clobbering return ip */
- movl $__KERNEL_CS, 13*4(%esp)
-
movl 12*4(%esp), %eax /* Load ip (1st parameter) */
subl $MCOUNT_INSN_SIZE, %eax /* Adjust ip */
movl 0x4(%ebp), %edx /* Load parent ip (2nd parameter) */
@@ -93,10 +92,14 @@ GLOBAL(ftrace_regs_call)
call ftrace_stub
addl $4, %esp /* Skip pt_regs */
- movl 14*4(%esp), %eax /* Move flags back into cs */
- movl %eax, 13*4(%esp) /* Needed to keep addl from modifying flags */
- movl 12*4(%esp), %eax /* Get return ip from regs->ip */
- movl %eax, 14*4(%esp) /* Put return ip back for ret */
+
+ /* restore flags */
+ push 14*4(%esp)
+ popf
+
+ /* Move return ip back to its original location */
+ movl 12*4(%esp), %eax
+ movl %eax, 14*4(%esp)
popl %ebx
popl %ecx
@@ -109,12 +112,11 @@ GLOBAL(ftrace_regs_call)
popl %es
popl %fs
popl %gs
- addl $8, %esp /* Skip orig_ax and ip */
- popf /* Pop flags at end (no addl to corrupt flags) */
- jmp .Lftrace_ret
- popf
- jmp ftrace_stub
+ /* use lea to not affect flags */
+ lea 3*4(%esp), %esp /* Skip orig_ax, ip and cs */
+
+ jmp .Lftrace_ret
#else /* ! CONFIG_DYNAMIC_FTRACE */
ENTRY(mcount)
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [tip:x86/asm] x86/ftrace: Add -mfentry support to x86_32 with DYNAMIC_FTRACE set
2017-03-23 14:33 ` [PATCH 5/6 v5] ftrace/x86_32: Add -mfentry support to x86_32 with DYNAMIC_FTRACE set Steven Rostedt
@ 2017-03-24 9:21 ` tip-bot for Steven Rostedt (VMware)
0 siblings, 0 replies; 37+ messages in thread
From: tip-bot for Steven Rostedt (VMware) @ 2017-03-24 9:21 UTC (permalink / raw)
To: linux-tip-commits
Cc: mingo, mhiramat, jpoimboe, tglx, akpm, linux-kernel, luto,
torvalds, peterz, rostedt, hpa
Commit-ID: 644e0e8dc76b919976c44d3929164d42cbe656bc
Gitweb: http://git.kernel.org/tip/644e0e8dc76b919976c44d3929164d42cbe656bc
Author: Steven Rostedt (VMware) <rostedt@goodmis.org>
AuthorDate: Thu, 23 Mar 2017 10:33:52 -0400
Committer: Thomas Gleixner <tglx@linutronix.de>
CommitDate: Fri, 24 Mar 2017 10:14:07 +0100
x86/ftrace: Add -mfentry support to x86_32 with DYNAMIC_FTRACE set
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 adds 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.
Keep !DYNAMIC_FTRACE around to test it off, as there's still some archs
that have FTRACE but not DYNAMIC_FTRACE.
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
Reviewed-by: Masami Hiramatsu <mhiramat@kernel.org>
Reviewed-by: Josh Poimboeuf <jpoimboe@redhat.com>
Reviewed-by: Ingo Molnar <mingo@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Link: http://lkml.kernel.org/r/20170323143446.052202377@goodmis.org
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
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 cc98d5a..8c17146 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 93e2664..80518ec 100644
--- a/arch/x86/kernel/ftrace_32.S
+++ b/arch/x86/kernel/ftrace_32.S
@@ -9,26 +9,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
@@ -40,7 +82,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
@@ -81,6 +130,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 */
@@ -119,7 +172,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? */
@@ -151,9 +204,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
@@ -161,9 +213,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
@@ -176,7 +234,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
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [tip:x86/asm] x86/ftrace: Use Makefile logic instead of #ifdef for compiling ftrace_*.o
2017-03-23 14:33 ` [PATCH 6/6 v5] ftrace/x86: Use Makefile logic instead of #ifdef for compiling ftrace_*.o Steven Rostedt
@ 2017-03-24 9:22 ` tip-bot for Steven Rostedt (VMware)
0 siblings, 0 replies; 37+ messages in thread
From: tip-bot for Steven Rostedt (VMware) @ 2017-03-24 9:22 UTC (permalink / raw)
To: linux-tip-commits
Cc: mingo, linux-kernel, akpm, hpa, mhiramat, jpoimboe, torvalds,
tglx, rostedt, luto, peterz
Commit-ID: 1fa9d67a2f07893fc7e4a880867a6cbb81dda547
Gitweb: http://git.kernel.org/tip/1fa9d67a2f07893fc7e4a880867a6cbb81dda547
Author: Steven Rostedt (VMware) <rostedt@goodmis.org>
AuthorDate: Thu, 23 Mar 2017 10:33:53 -0400
Committer: Thomas Gleixner <tglx@linutronix.de>
CommitDate: Fri, 24 Mar 2017 10:14:08 +0100
x86/ftrace: Use Makefile logic instead of #ifdef for compiling ftrace_*.o
Currently ftrace_32.S and ftrace_64.S are compiled even when
CONFIG_FUNCTION_TRACER is not set. This means there's an unnecessary #ifdef
to protect the code. Instead of using preprocessor directives, only compile
those files when FUNCTION_TRACER is defined.
Suggested-by: Josh Poimboeuf <jpoimboe@redhat.com>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
Reviewed-by: Josh Poimboeuf <jpoimboe@redhat.com>
Reviewed-by: Ingo Molnar <mingo@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Link: http://lkml.kernel.org/r/20170316210043.peycxdxktwwn6cid@treble
Link: http://lkml.kernel.org/r/20170323143446.217684991@goodmis.org
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
arch/x86/kernel/Makefile | 4 ++--
arch/x86/kernel/ftrace_32.S | 3 ---
arch/x86/kernel/ftrace_64.S | 4 ----
3 files changed, 2 insertions(+), 9 deletions(-)
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 80518ec..07f4035 100644
--- a/arch/x86/kernel/ftrace_32.S
+++ b/arch/x86/kernel/ftrace_32.S
@@ -8,8 +8,6 @@
#include <asm/export.h>
#include <asm/ftrace.h>
-#ifdef CONFIG_FUNCTION_TRACER
-
#ifdef CC_USING_FENTRY
# define function_hook __fentry__
EXPORT_SYMBOL(__fentry__)
@@ -206,7 +204,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 d1be3cb..1dfac63 100644
--- a/arch/x86/kernel/ftrace_64.S
+++ b/arch/x86/kernel/ftrace_64.S
@@ -11,9 +11,6 @@
.code64
.section .entry.text, "ax"
-
-#ifdef CONFIG_FUNCTION_TRACER
-
#ifdef CC_USING_FENTRY
# define function_hook __fentry__
EXPORT_SYMBOL(__fentry__)
@@ -295,7 +292,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] 37+ messages in thread
end of thread, other threads:[~2017-03-24 9:27 UTC | newest]
Thread overview: 37+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-23 14:33 [PATCH 0/6 v5] [GIT PULL] ftrace/x86: Ftrace cleanup and add support for -mfentry on x86_32 Steven Rostedt
2017-03-23 14:33 ` [PATCH 1/6 v5] ftrace/x86_64: Rename mcount_64.S to ftrace_64.S Steven Rostedt
2017-03-24 9:19 ` [tip:x86/asm] x86/ftrace: " tip-bot for Steven Rostedt (VMware)
2017-03-23 14:33 ` [PATCH 2/6 v5] ftrace/x86_32: Move the ftrace specific code out of entry_32.S Steven Rostedt
2017-03-23 18:19 ` Thomas Gleixner
2017-03-23 19:03 ` Steven Rostedt
2017-03-24 9:20 ` [tip:x86/asm] x86/ftrace: " tip-bot for Steven Rostedt (VMware)
2017-03-23 14:33 ` [PATCH 3/6 v5] ftrace/x86_32: Add stack frame pointer to ftrace_caller Steven Rostedt
2017-03-24 9:20 ` [tip:x86/asm] x86/ftrace: " tip-bot for Steven Rostedt (VMware)
2017-03-23 14:33 ` [PATCH 4/6 v5] ftrace/x86_32: Clean up ftrace_regs_caller Steven Rostedt
2017-03-24 9:21 ` [tip:x86/asm] x86/ftrace: " tip-bot for Steven Rostedt (VMware)
2017-03-23 14:33 ` [PATCH 5/6 v5] ftrace/x86_32: Add -mfentry support to x86_32 with DYNAMIC_FTRACE set Steven Rostedt
2017-03-24 9:21 ` [tip:x86/asm] x86/ftrace: " tip-bot for Steven Rostedt (VMware)
2017-03-23 14:33 ` [PATCH 6/6 v5] ftrace/x86: Use Makefile logic instead of #ifdef for compiling ftrace_*.o Steven Rostedt
2017-03-24 9:22 ` [tip:x86/asm] x86/ftrace: " tip-bot for Steven Rostedt (VMware)
-- strict thread matches above, loose matches on Subject: below --
2017-03-16 17:20 [PATCH 0/5 v2] ftrace/x86_32: Ftrace cleanup and add support for -mfentry Steven Rostedt
2017-03-16 17:20 ` [PATCH 1/5 v2] x86/ftrace: Rename mcount_64.S to ftrace_64.S Steven Rostedt
2017-03-16 17:20 ` [PATCH 2/5 v2] ftrace/x86-32: Move the ftrace specific code out of entry_32.S Steven Rostedt
2017-03-17 13:21 ` Steven Rostedt
2017-03-16 17:20 ` [PATCH 3/5 v2] ftrace/x86_32: Add stack frame pointer to ftrace_caller Steven Rostedt
2017-03-16 17:20 ` [PATCH 4/5 v2] ftrace/x86_32: Clean up ftrace_regs_caller Steven Rostedt
2017-03-16 17:40 ` Linus Torvalds
2017-03-16 17:55 ` Steven Rostedt
2017-03-16 18:09 ` [PATCH 4/5 v3] " Steven Rostedt
2017-03-16 18:19 ` Linus Torvalds
2017-03-16 19:19 ` Steven Rostedt
2017-03-16 19:24 ` Steven Rostedt
2017-03-16 19:30 ` Linus Torvalds
2017-03-16 19:50 ` Steven Rostedt
2017-03-16 19:28 ` Linus Torvalds
2017-03-16 19:49 ` Steven Rostedt
2017-03-16 20:14 ` [PATCH 4/5 v3.1] " Steven Rostedt
2017-03-16 17:20 ` [PATCH 5/5 v2] ftrace/x86-32: Add -mfentry support to x86_32 with DYNAMIC_FTRACE set Steven Rostedt
2017-03-16 20:40 ` [PATCH 5/5 v3] " Steven Rostedt
2017-03-16 21:00 ` Josh Poimboeuf
2017-03-16 21:25 ` Steven Rostedt
2017-03-17 10:38 ` Masami Hiramatsu
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.