From: Steven Rostedt <[email protected]>
Add saving full regs for function tracing on i386.
The saving of regs was influenced by patches sent out by
Masami Hiramatsu.
Cc: Masami Hiramatsu <[email protected]>
Signed-off-by: Steven Rostedt <[email protected]>
---
arch/x86/include/asm/ftrace.h | 2 --
arch/x86/kernel/entry_32.S | 53 +++++++++++++++++++++++++++++++++++++++++
arch/x86/kernel/ftrace.c | 4 ----
3 files changed, 53 insertions(+), 6 deletions(-)
diff --git a/arch/x86/include/asm/ftrace.h b/arch/x86/include/asm/ftrace.h
index fd10faf..2b396cf 100644
--- a/arch/x86/include/asm/ftrace.h
+++ b/arch/x86/include/asm/ftrace.h
@@ -40,10 +40,8 @@
#ifdef CONFIG_DYNAMIC_FTRACE
#define ARCH_SUPPORTS_FTRACE_OPS 1
-#ifdef CONFIG_X86_64
#define ARCH_SUPPORTS_FTRACE_SAVE_REGS
#endif
-#endif
#ifndef __ASSEMBLY__
extern void mcount(void);
diff --git a/arch/x86/kernel/entry_32.S b/arch/x86/kernel/entry_32.S
index acd4963..f22802c 100644
--- a/arch/x86/kernel/entry_32.S
+++ b/arch/x86/kernel/entry_32.S
@@ -1123,6 +1123,7 @@ ftrace_call:
popl %edx
popl %ecx
popl %eax
+ftrace_ret:
#ifdef CONFIG_FUNCTION_GRAPH_TRACER
.globl ftrace_graph_call
ftrace_graph_call:
@@ -1134,6 +1135,58 @@ ftrace_stub:
ret
END(ftrace_caller)
+ENTRY(ftrace_regs_caller)
+ pushf /* push flags before compare */
+ cmpl $0, function_trace_stop
+ jne ftrace_exit
+
+ subl $8, %esp /* skip ip and 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 14*4(%esp), %eax /* Load return address */
+ pushl %eax /* Save return address (+4) */
+ subl $MCOUNT_INSN_SIZE, %eax
+ movl %eax, 12*4+4(%esp) /* Store IP */
+ movl 13*4+4(%esp), %edx /* Load flags */
+ movl %edx, 14*4+4(%esp) /* Store flags */
+ movl $__KERNEL_CS, %edx
+ movl %edx, 13*4+4(%esp) /* Store CS */
+
+ movl 0x4(%ebp), %edx
+ lea 4(%esp), %ecx
+ pushl %ecx /* Save pt_regs as 4th parameter */
+ leal function_trace_op, %ecx
+
+GLOBAL(ftrace_regs_call)
+ call ftrace_stub
+
+ addl $4,%esp /* Skip pt_regs */
+ popl %eax
+ movl %eax, 14*4(%esp) /* Restore return address */
+ 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
+ftrace_exit:
+ addl $4, %esp /* Skip eflags */
+ jmp ftrace_ret
#else /* ! CONFIG_DYNAMIC_FTRACE */
ENTRY(mcount)
diff --git a/arch/x86/kernel/ftrace.c b/arch/x86/kernel/ftrace.c
index b90eb1a..1d41402 100644
--- a/arch/x86/kernel/ftrace.c
+++ b/arch/x86/kernel/ftrace.c
@@ -206,7 +206,6 @@ static int
ftrace_modify_code(unsigned long ip, unsigned const char *old_code,
unsigned const char *new_code);
-#ifdef ARCH_SUPPORTS_FTRACE_SAVE_REGS
/*
* Should never be called:
* As it is only called by __ftrace_replace_code() which is called by
@@ -221,7 +220,6 @@ int ftrace_modify_call(struct dyn_ftrace *rec, unsigned long old_addr,
WARN_ON(1);
return -EINVAL;
}
-#endif
int ftrace_update_ftrace_func(ftrace_func_t func)
{
@@ -237,7 +235,6 @@ int ftrace_update_ftrace_func(ftrace_func_t func)
ret = ftrace_modify_code(ip, old, new);
-#ifdef ARCH_SUPPORTS_FTRACE_SAVE_REGS
/* Also update the regs callback function */
if (!ret) {
ip = (unsigned long)(&ftrace_regs_call);
@@ -245,7 +242,6 @@ int ftrace_update_ftrace_func(ftrace_func_t func)
new = ftrace_call_replace(ip, (unsigned long)func);
ret = ftrace_modify_code(ip, old, new);
}
-#endif
atomic_dec(&modifying_ftrace_code);
--
1.7.10
(2012/06/13 7:43), Steven Rostedt wrote:
> From: Steven Rostedt <[email protected]>
>
> Add saving full regs for function tracing on i386.
> The saving of regs was influenced by patches sent out by
> Masami Hiramatsu.
>
> Cc: Masami Hiramatsu <[email protected]>
> Signed-off-by: Steven Rostedt <[email protected]>
> ---
> arch/x86/include/asm/ftrace.h | 2 --
> arch/x86/kernel/entry_32.S | 53 +++++++++++++++++++++++++++++++++++++++++
> arch/x86/kernel/ftrace.c | 4 ----
> 3 files changed, 53 insertions(+), 6 deletions(-)
>
> diff --git a/arch/x86/include/asm/ftrace.h b/arch/x86/include/asm/ftrace.h
> index fd10faf..2b396cf 100644
> --- a/arch/x86/include/asm/ftrace.h
> +++ b/arch/x86/include/asm/ftrace.h
> @@ -40,10 +40,8 @@
>
> #ifdef CONFIG_DYNAMIC_FTRACE
> #define ARCH_SUPPORTS_FTRACE_OPS 1
> -#ifdef CONFIG_X86_64
> #define ARCH_SUPPORTS_FTRACE_SAVE_REGS
> #endif
> -#endif
>
> #ifndef __ASSEMBLY__
> extern void mcount(void);
> diff --git a/arch/x86/kernel/entry_32.S b/arch/x86/kernel/entry_32.S
> index acd4963..f22802c 100644
> --- a/arch/x86/kernel/entry_32.S
> +++ b/arch/x86/kernel/entry_32.S
> @@ -1123,6 +1123,7 @@ ftrace_call:
> popl %edx
> popl %ecx
> popl %eax
> +ftrace_ret:
> #ifdef CONFIG_FUNCTION_GRAPH_TRACER
> .globl ftrace_graph_call
> ftrace_graph_call:
> @@ -1134,6 +1135,58 @@ ftrace_stub:
> ret
> END(ftrace_caller)
>
> +ENTRY(ftrace_regs_caller)
> + pushf /* push flags before compare */
> + cmpl $0, function_trace_stop
> + jne ftrace_exit
> +
> + subl $8, %esp /* skip ip and 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 14*4(%esp), %eax /* Load return address */
> + pushl %eax /* Save return address (+4) */
> + subl $MCOUNT_INSN_SIZE, %eax
> + movl %eax, 12*4+4(%esp) /* Store IP */
> + movl 13*4+4(%esp), %edx /* Load flags */
> + movl %edx, 14*4+4(%esp) /* Store flags */
> + movl $__KERNEL_CS, %edx
> + movl %edx, 13*4+4(%esp) /* Store CS */
> +
> + movl 0x4(%ebp), %edx
> + lea 4(%esp), %ecx
> + pushl %ecx /* Save pt_regs as 4th parameter */
> + leal function_trace_op, %ecx
> +
> +GLOBAL(ftrace_regs_call)
> + call ftrace_stub
> +
> + addl $4,%esp /* Skip pt_regs */
> + popl %eax
> + movl %eax, 14*4(%esp) /* Restore return address */
> + 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
> +ftrace_exit:
> + addl $4, %esp /* Skip eflags */
Hmm, why don't you recover eflags, as x86-64 has done?
IMHO, it should be recovered if ftrace-based kprobe is
a transparent acceleration.
Thank you,
--
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: [email protected]
On Fri, 2012-06-15 at 15:03 +0900, Masami Hiramatsu wrote:
> (2012/06/13 7:43), Steven Rostedt wrote:
> > +GLOBAL(ftrace_regs_call)
> > + call ftrace_stub
> > +
> > + addl $4,%esp /* Skip pt_regs */
> > + popl %eax
> > + movl %eax, 14*4(%esp) /* Restore return address */
> > + 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
> > +ftrace_exit:
> > + addl $4, %esp /* Skip eflags */
>
> Hmm, why don't you recover eflags, as x86-64 has done?
> IMHO, it should be recovered if ftrace-based kprobe is
> a transparent acceleration.
As I copied this mostly from your patch I probably was thinking that it
saved it already ;-)
That is, we originally debated the usefulness of restoring eflags at the
start of the function call, as function calls give no guarantees to
them. I modified this patch to not restore flags. But later, I agreed
that I would keep kprobes the way it was, even though flags are pretty
meaningless here (unless you are going to have the kprobe modify the
enabling of interrupts), and decided to add back the flags. I simply
forgot to restore back to your original patch.
Will fix,
Thanks,
-- Steve