* [PATCH 1/6 v3] ftrace/x86_64: Rename mcount_64.S to ftrace_64.S
2017-03-18 21:09 [PATCH 0/6 v3] [GIT PULL] ftrace/x86: Ftrace cleanup and add support for -mfentry on x86_32 Steven Rostedt
@ 2017-03-18 21:09 ` Steven Rostedt
2017-03-18 21:09 ` [PATCH 2/6 v3] ftrace/x86_32: Move the ftrace specific code out of entry_32.S Steven Rostedt
` (4 subsequent siblings)
5 siblings, 0 replies; 11+ messages in thread
From: Steven Rostedt @ 2017-03-18 21:09 UTC (permalink / raw)
To: linux-kernel
Cc: Ingo Molnar, Andrew Morton, Thomas Gleixner, Peter Zijlstra,
Masami Hiramatsu, H. Peter Anvin, Andy Lutomirski,
Josh Poimboeuf, Linus Torvalds
[-- 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] 11+ messages in thread
* [PATCH 2/6 v3] ftrace/x86_32: Move the ftrace specific code out of entry_32.S
2017-03-18 21:09 [PATCH 0/6 v3] [GIT PULL] ftrace/x86: Ftrace cleanup and add support for -mfentry on x86_32 Steven Rostedt
2017-03-18 21:09 ` [PATCH 1/6 v3] ftrace/x86_64: Rename mcount_64.S to ftrace_64.S Steven Rostedt
@ 2017-03-18 21:09 ` Steven Rostedt
2017-03-18 21:09 ` [PATCH 3/6 v3] ftrace/x86_32: Add stack frame pointer to ftrace_caller Steven Rostedt
` (3 subsequent siblings)
5 siblings, 0 replies; 11+ messages in thread
From: Steven Rostedt @ 2017-03-18 21:09 UTC (permalink / raw)
To: linux-kernel
Cc: Ingo Molnar, Andrew Morton, Thomas Gleixner, Peter Zijlstra,
Masami Hiramatsu, H. Peter Anvin, Andy Lutomirski,
Josh Poimboeuf, Linus Torvalds
[-- Attachment #1: 0002-ftrace-x86_32-Move-the-ftrace-specific-code-out-of-e.patch --]
[-- Type: text/plain, Size: 9718 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>
---
arch/x86/entry/entry_32.S | 169 ------------------------------------------
arch/x86/kernel/Makefile | 1 +
arch/x86/kernel/ftrace_32.S | 177 ++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 178 insertions(+), 169 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..1889a74823ce
--- /dev/null
+++ b/arch/x86/kernel/ftrace_32.S
@@ -0,0 +1,177 @@
+/*
+ * linux/arch/x86_64/mcount_64.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
--
2.10.2
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 3/6 v3] ftrace/x86_32: Add stack frame pointer to ftrace_caller
2017-03-18 21:09 [PATCH 0/6 v3] [GIT PULL] ftrace/x86: Ftrace cleanup and add support for -mfentry on x86_32 Steven Rostedt
2017-03-18 21:09 ` [PATCH 1/6 v3] ftrace/x86_64: Rename mcount_64.S to ftrace_64.S Steven Rostedt
2017-03-18 21:09 ` [PATCH 2/6 v3] ftrace/x86_32: Move the ftrace specific code out of entry_32.S Steven Rostedt
@ 2017-03-18 21:09 ` Steven Rostedt
2017-03-18 21:09 ` [PATCH 4/6 v3] ftrace/x86_32: Clean up ftrace_regs_caller Steven Rostedt
` (2 subsequent siblings)
5 siblings, 0 replies; 11+ messages in thread
From: Steven Rostedt @ 2017-03-18 21:09 UTC (permalink / raw)
To: linux-kernel
Cc: Ingo Molnar, Andrew Morton, Thomas Gleixner, Peter Zijlstra,
Masami Hiramatsu, H. Peter Anvin, Andy Lutomirski,
Josh Poimboeuf, Linus Torvalds
[-- 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 1889a74823ce..f991e723c3e4 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] 11+ messages in thread
* [PATCH 4/6 v3] ftrace/x86_32: Clean up ftrace_regs_caller
2017-03-18 21:09 [PATCH 0/6 v3] [GIT PULL] ftrace/x86: Ftrace cleanup and add support for -mfentry on x86_32 Steven Rostedt
` (2 preceding siblings ...)
2017-03-18 21:09 ` [PATCH 3/6 v3] ftrace/x86_32: Add stack frame pointer to ftrace_caller Steven Rostedt
@ 2017-03-18 21:09 ` Steven Rostedt
2017-03-20 14:01 ` Josh Poimboeuf
2017-03-18 21:09 ` [PATCH 5/6 v3] ftrace/x86_32: Add -mfentry support to x86_32 with DYNAMIC_FTRACE set Steven Rostedt
2017-03-18 21:09 ` [PATCH 6/6 v3] ftrace/x86: Use Makefile logic instead of #ifdef of compling ftrace_*.o Steven Rostedt
5 siblings, 1 reply; 11+ messages in thread
From: Steven Rostedt @ 2017-03-18 21:09 UTC (permalink / raw)
To: linux-kernel
Cc: Ingo Molnar, Andrew Morton, Thomas Gleixner, Peter Zijlstra,
Masami Hiramatsu, H. Peter Anvin, Andy Lutomirski,
Josh Poimboeuf, Linus Torvalds
[-- Attachment #1: 0004-ftrace-x86_32-Clean-up-ftrace_regs_caller.patch --]
[-- Type: text/plain, Size: 3384 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>
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 f991e723c3e4..7cd27cf56578 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 flags */
+
+ jmp .Lftrace_ret
#else /* ! CONFIG_DYNAMIC_FTRACE */
ENTRY(mcount)
--
2.10.2
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 4/6 v3] ftrace/x86_32: Clean up ftrace_regs_caller
2017-03-18 21:09 ` [PATCH 4/6 v3] ftrace/x86_32: Clean up ftrace_regs_caller Steven Rostedt
@ 2017-03-20 14:01 ` Josh Poimboeuf
0 siblings, 0 replies; 11+ messages in thread
From: Josh Poimboeuf @ 2017-03-20 14:01 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 Sat, Mar 18, 2017 at 05:09:27PM -0400, Steven Rostedt wrote:
> @@ -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 flags */
> +
> + jmp .Lftrace_ret
> #else /* ! CONFIG_DYNAMIC_FTRACE */
>
> ENTRY(mcount)
That last comment should be
/* Skip orig_ax, ip and cs */
Otherwise:
Reviewed-by: Josh Poimboeuf <jpoimboe@redhat.com>
--
Josh
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 5/6 v3] ftrace/x86_32: Add -mfentry support to x86_32 with DYNAMIC_FTRACE set
2017-03-18 21:09 [PATCH 0/6 v3] [GIT PULL] ftrace/x86: Ftrace cleanup and add support for -mfentry on x86_32 Steven Rostedt
` (3 preceding siblings ...)
2017-03-18 21:09 ` [PATCH 4/6 v3] ftrace/x86_32: Clean up ftrace_regs_caller Steven Rostedt
@ 2017-03-18 21:09 ` Steven Rostedt
2017-03-20 14:02 ` Josh Poimboeuf
2017-03-18 21:09 ` [PATCH 6/6 v3] ftrace/x86: Use Makefile logic instead of #ifdef of compling ftrace_*.o Steven Rostedt
5 siblings, 1 reply; 11+ messages in thread
From: Steven Rostedt @ 2017-03-18 21:09 UTC (permalink / raw)
To: linux-kernel
Cc: Ingo Molnar, Andrew Morton, Thomas Gleixner, Peter Zijlstra,
Masami Hiramatsu, H. Peter Anvin, Andy Lutomirski,
Josh Poimboeuf, Linus Torvalds
[-- Attachment #1: 0005-ftrace-x86_32-Add-mfentry-support-to-x86_32-with-DYN.patch --]
[-- Type: text/plain, Size: 5127 bytes --]
From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
x86_64 has had fentry support for some time. I did not add support to x86_32
as I was unsure if it will be used much in the future. It is still very much
used, and there's issues with function graph tracing with gcc playing around
with the mcount frames, causing function graph to panic. The fentry code
does not have this issue, and is able to cope as there is no frame to mess
up.
Note, this only add support for fentry when DYNAMIC_FTRACE is set. There's
really no reason to not have that set, because the performance of the
machine drops significantly when it's not enabled. I only keep
!DYNAMIC_FTRACE around to test it off, as there's still some archs that have
FTRACE but not DYNAMIC_FTRACE.
Reviewed-by: Masami Hiramatsu <mhiramat@kernel.org>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
arch/x86/Kconfig | 2 +-
arch/x86/kernel/ftrace_32.S | 82 +++++++++++++++++++++++++++++++++++++++------
2 files changed, 73 insertions(+), 11 deletions(-)
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index cc98d5a294ee..8c17146427ca 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -127,7 +127,7 @@ config X86
select HAVE_EBPF_JIT if X86_64
select HAVE_EFFICIENT_UNALIGNED_ACCESS
select HAVE_EXIT_THREAD
- select HAVE_FENTRY if X86_64
+ select HAVE_FENTRY if X86_64 || DYNAMIC_FTRACE
select HAVE_FTRACE_MCOUNT_RECORD
select HAVE_FUNCTION_GRAPH_TRACER
select HAVE_FUNCTION_TRACER
diff --git a/arch/x86/kernel/ftrace_32.S b/arch/x86/kernel/ftrace_32.S
index 7cd27cf56578..fe64fa596266 100644
--- a/arch/x86/kernel/ftrace_32.S
+++ b/arch/x86/kernel/ftrace_32.S
@@ -11,26 +11,68 @@
#include <asm/ftrace.h>
#ifdef CONFIG_FUNCTION_TRACER
+
+#ifdef CC_USING_FENTRY
+# define function_hook __fentry__
+EXPORT_SYMBOL(__fentry__)
+#else
+# define function_hook mcount
+EXPORT_SYMBOL(mcount)
+#endif
+
#ifdef CONFIG_DYNAMIC_FTRACE
-ENTRY(mcount)
+/* mcount uses a frame pointer even if CONFIG_FRAME_POINTER is not set */
+#if !defined(CC_USING_FENTRY) || defined(CONFIG_FRAME_POINTER)
+# define USING_FRAME_POINTER
+#endif
+
+#ifdef USING_FRAME_POINTER
+# define MCOUNT_FRAME 1 /* using frame = true */
+#else
+# define MCOUNT_FRAME 0 /* using frame = false */
+#endif
+
+ENTRY(function_hook)
ret
-END(mcount)
+END(function_hook)
ENTRY(ftrace_caller)
+#ifdef USING_FRAME_POINTER
+# ifdef CC_USING_FENTRY
+ /*
+ * Frame pointers are of ip followed by bp.
+ * Since fentry is an immediate jump, we are left with
+ * parent-ip, function-ip. We need to add a frame with
+ * parent-ip followed by ebp.
+ */
+ pushl 4(%esp) /* parent ip */
pushl %ebp
movl %esp, %ebp
-
+ pushl 2*4(%esp) /* function ip */
+# endif
+ /* For mcount, the function ip is directly above */
+ pushl %ebp
+ movl %esp, %ebp
+#endif
pushl %eax
pushl %ecx
pushl %edx
pushl $0 /* Pass NULL as regs pointer */
- movl 5*4(%esp), %eax
- /* Copy original ebp into %edx */
+
+#ifdef USING_FRAME_POINTER
+ /* Load parent ebp into edx */
movl 4*4(%esp), %edx
+#else
+ /* There's no frame pointer, load the appropriate stack addr instead */
+ lea 4*4(%esp), %edx
+#endif
+
+ movl (MCOUNT_FRAME+4)*4(%esp), %eax /* load the rip */
/* Get the parent ip */
- movl 0x4(%edx), %edx
+ movl 4(%edx), %edx /* edx has ebp */
+
movl function_trace_op, %ecx
subl $MCOUNT_INSN_SIZE, %eax
@@ -42,7 +84,14 @@ ftrace_call:
popl %edx
popl %ecx
popl %eax
+#ifdef USING_FRAME_POINTER
popl %ebp
+# ifdef CC_USING_FENTRY
+ addl $4,%esp /* skip function ip */
+ popl %ebp /* this is the orig bp */
+ addl $4, %esp /* skip parent ip */
+# endif
+#endif
.Lftrace_ret:
#ifdef CONFIG_FUNCTION_GRAPH_TRACER
.globl ftrace_graph_call
@@ -83,6 +132,10 @@ ENTRY(ftrace_regs_caller)
pushl %edx
pushl %ecx
pushl %ebx
+#ifdef CC_USING_FENTRY
+ /* Load 4 off of the parent ip addr into ebp */
+ lea 14*4(%esp), %ebp
+#endif
movl 12*4(%esp), %eax /* Load ip (1st parameter) */
subl $MCOUNT_INSN_SIZE, %eax /* Adjust ip */
@@ -121,7 +174,7 @@ GLOBAL(ftrace_regs_call)
jmp .Lftrace_ret
#else /* ! CONFIG_DYNAMIC_FTRACE */
-ENTRY(mcount)
+ENTRY(function_hook)
cmpl $__PAGE_OFFSET, %esp
jb ftrace_stub /* Paging not enabled yet? */
@@ -153,9 +206,8 @@ ftrace_stub:
popl %ecx
popl %eax
jmp ftrace_stub
-END(mcount)
+END(function_hook)
#endif /* CONFIG_DYNAMIC_FTRACE */
-EXPORT_SYMBOL(mcount)
#endif /* CONFIG_FUNCTION_TRACER */
#ifdef CONFIG_FUNCTION_GRAPH_TRACER
@@ -163,9 +215,15 @@ ENTRY(ftrace_graph_caller)
pushl %eax
pushl %ecx
pushl %edx
- movl 0xc(%esp), %eax
+ movl 3*4(%esp), %eax
+ /* Even with frame pointers, fentry doesn't have one here */
+#ifdef CC_USING_FENTRY
+ lea 4*4(%esp), %edx
+ movl $0, %ecx
+#else
lea 0x4(%ebp), %edx
movl (%ebp), %ecx
+#endif
subl $MCOUNT_INSN_SIZE, %eax
call prepare_ftrace_return
popl %edx
@@ -178,7 +236,11 @@ END(ftrace_graph_caller)
return_to_handler:
pushl %eax
pushl %edx
+#ifdef CC_USING_FENTRY
+ movl $0, %eax
+#else
movl %ebp, %eax
+#endif
call ftrace_return_to_handler
movl %eax, %ecx
popl %edx
--
2.10.2
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 5/6 v3] ftrace/x86_32: Add -mfentry support to x86_32 with DYNAMIC_FTRACE set
2017-03-18 21:09 ` [PATCH 5/6 v3] ftrace/x86_32: Add -mfentry support to x86_32 with DYNAMIC_FTRACE set Steven Rostedt
@ 2017-03-20 14:02 ` Josh Poimboeuf
0 siblings, 0 replies; 11+ messages in thread
From: Josh Poimboeuf @ 2017-03-20 14:02 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 Sat, Mar 18, 2017 at 05:09:28PM -0400, Steven Rostedt wrote:
> From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
>
> x86_64 has had fentry support for some time. I did not add support to x86_32
> as I was unsure if it will be used much in the future. It is still very much
> used, and there's issues with function graph tracing with gcc playing around
> with the mcount frames, causing function graph to panic. The fentry code
> does not have this issue, and is able to cope as there is no frame to mess
> up.
>
> Note, this only add support for fentry when DYNAMIC_FTRACE is set. There's
> really no reason to not have that set, because the performance of the
> machine drops significantly when it's not enabled. I only keep
> !DYNAMIC_FTRACE around to test it off, as there's still some archs that have
> FTRACE but not DYNAMIC_FTRACE.
>
> Reviewed-by: Masami Hiramatsu <mhiramat@kernel.org>
> Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
Reviewed-by: Josh Poimboeuf <jpoimboe@redhat.com>
--
Josh
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 6/6 v3] ftrace/x86: Use Makefile logic instead of #ifdef of compling ftrace_*.o
2017-03-18 21:09 [PATCH 0/6 v3] [GIT PULL] ftrace/x86: Ftrace cleanup and add support for -mfentry on x86_32 Steven Rostedt
` (4 preceding siblings ...)
2017-03-18 21:09 ` [PATCH 5/6 v3] ftrace/x86_32: Add -mfentry support to x86_32 with DYNAMIC_FTRACE set Steven Rostedt
@ 2017-03-18 21:09 ` Steven Rostedt
2017-03-20 14:04 ` Josh Poimboeuf
5 siblings, 1 reply; 11+ messages in thread
From: Steven Rostedt @ 2017-03-18 21:09 UTC (permalink / raw)
To: linux-kernel
Cc: Ingo Molnar, Andrew Morton, Thomas Gleixner, Peter Zijlstra,
Masami Hiramatsu, H. Peter Anvin, Andy Lutomirski,
Josh Poimboeuf, Linus Torvalds
[-- Attachment #1: 0006-ftrace-x86-Use-Makefile-logic-instead-of-ifdef-of-co.patch --]
[-- Type: text/plain, Size: 2759 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>
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 fe64fa596266..4d52e0d49e26 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 7b0d3da52fb4..b9c46919d4fc 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] 11+ messages in thread
* Re: [PATCH 6/6 v3] ftrace/x86: Use Makefile logic instead of #ifdef of compling ftrace_*.o
2017-03-18 21:09 ` [PATCH 6/6 v3] ftrace/x86: Use Makefile logic instead of #ifdef of compling ftrace_*.o Steven Rostedt
@ 2017-03-20 14:04 ` Josh Poimboeuf
2017-03-22 0:56 ` Steven Rostedt
0 siblings, 1 reply; 11+ messages in thread
From: Josh Poimboeuf @ 2017-03-20 14:04 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 Sat, Mar 18, 2017 at 05:09:29PM -0400, Steven Rostedt wrote:
> 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>
> Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
The subject has a typo ("compling") and would parse better if the second
"of" where changed to a "for": "for compiling". Otherwise:
Reviewed-by: Josh Poimboeuf <jpoimboe@redhat.com>
--
Josh
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 6/6 v3] ftrace/x86: Use Makefile logic instead of #ifdef of compling ftrace_*.o
2017-03-20 14:04 ` Josh Poimboeuf
@ 2017-03-22 0:56 ` Steven Rostedt
0 siblings, 0 replies; 11+ messages in thread
From: Steven Rostedt @ 2017-03-22 0:56 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 Mon, 20 Mar 2017 09:04:25 -0500
Josh Poimboeuf <jpoimboe@redhat.com> wrote:
> On Sat, Mar 18, 2017 at 05:09:29PM -0400, Steven Rostedt wrote:
> > 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> Signed-off-by:
> > Steven Rostedt (VMware) <rostedt@goodmis.org>
>
> The subject has a typo ("compling") and would parse better if the
> second "of" where changed to a "for": "for compiling". Otherwise:
But this was to comply with #ifdef of ftrace_*.o!
;-)
>
> Reviewed-by: Josh Poimboeuf <jpoimboe@redhat.com>
>
Thanks!
-- Steve
^ permalink raw reply [flat|nested] 11+ messages in thread