On Thu, 2012-07-19 at 08:52 -0400, Steven Rostedt wrote:
>
> GLOBAL(ftrace_regs_call)
> call ftrace_stub
> @@ -1195,8 +1194,8 @@ 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)
> */
> + lea 8(%esp), %esp /* Skip orig_ax and ip */
> + popf /* Pop flags at end */
> jmp ftrace_ret
>
> ftrace_restore_flags:
>
>
> Because we no longer have that 4 byte offset on the stack when we need
> to load the 4th parameter, we can just load the current stack pointer
> into the stack (pushl %esp), without the save to %ecx step.
>
> also, because lea is faster than add (and doesn't even modify flags), I
> changed the last part to use lea instead of addl.
Now I'm told that this is not always the case (at least not for Atom),
so I reverted this part and put back the addl. But can you still give
you reviewed by for the first part?
>
> Can you give your reviewed-by tag for this too? I'd like to push this
> out today so we can still make 3.6.
>
> Thanks!
>
> -- Steve
>
> here's the full patch:
>
> diff --git a/arch/x86/include/asm/ftrace.h b/arch/x86/include/asm/ftrace.h
> index a847501..a6cae0c 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 5da11d1..ca5a146 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,72 @@ ftrace_stub:
> ret
> END(ftrace_caller)
>
> +ENTRY(ftrace_regs_caller)
> + pushf /* push flags before compare (in cs location) */
> + cmpl $0, function_trace_stop
> + jne ftrace_restore_flags
> +
> + /*
> + * 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 */
> + subl $MCOUNT_INSN_SIZE, (%esp) /* Adjust ip */
> +
> + 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) */
> + movl 0x4(%ebp), %edx /* Load parent ip (2nd parameter) */
> + leal 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 */
> + addl $MCOUNT_INSN_SIZE, %eax
> + 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)
*/
The above has been changed to this again.
-- Steve
> + jmp ftrace_ret
> +
> +ftrace_restore_flags:
> + popf
> + jmp ftrace_stub
> #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);
>
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
