lkml.org 
[lkml]   [2012]   [Jul]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patches in this message
    /
    SubjectRe: Re: [RFC][PATCH 2/4 v4] ftrace/x86: Add save_regs for i386 function calls
    From
    Date
    On Thu, 2012-07-19 at 11:20 +0900, Masami Hiramatsu wrote:
    > (2012/07/19 0:59), Steven Rostedt wrote:
    > > On Fri, 2012-07-13 at 14:47 -0400, Steven Rostedt wrote:
    > >
    > > Masami, can you give your Reviewed-by tag for this version? Or is there
    > > something else needing to be fixed?
    >
    > No, that is OK for me. I've just missed that...
    >
    > Reviewed-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
    >
    > Thank you!
    >

    Thanks! Except I have one more version. Someone offlist gave me some
    ideas.

    Here's the diff from the previous patch.

    diff --git a/arch/x86/kernel/entry_32.S b/arch/x86/kernel/entry_32.S
    index 46caa56..ca5a146 100644
    --- a/arch/x86/kernel/entry_32.S
    +++ b/arch/x86/kernel/entry_32.S
    @@ -1169,10 +1169,9 @@ ENTRY(ftrace_regs_caller)
    movl $__KERNEL_CS,13*4(%esp)

    movl 12*4(%esp), %eax /* Load ip (1st parameter) */
    - movl 0x4(%ebp), %edx /* Load parent ip (2cd parameter) */
    - lea (%esp), %ecx
    - pushl %ecx /* Save pt_regs as 4th 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
    @@ -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.

    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, &regs->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
    + lea 8(%esp), %esp /* Skip orig_ax and ip */
    + popf /* Pop flags at end */
    + 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);





    \
     
     \ /
      Last update: 2012-07-19 15:21    [W:2.627 / U:0.304 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site