On Thu, 16 Mar 2017 16:40:36 -0400 Steven Rostedt <[email protected]> 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)" <[email protected]> > > 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 <[email protected]> Thanks! > > Signed-off-by: Steven Rostedt (VMware) <[email protected]> > --- > 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 <[email protected]>
