Re: [PATCH] ftrace/x86: Fix function graph tracer reset path

On Mon, 16 May 2016 09:58:33 -0400 Steven Rostedt wrote: > > Nice work Masami! > > On Mon, 16 May 2016 21:32:50 +0900 > Namhyung Kim wrote: > > > > > -/* This is global to keep gas from relaxing the jumps */ > > >

Re: [PATCH] ftrace/x86: Fix function graph tracer reset path

On Mon, 16 May 2016 09:58:33 -0400 Steven Rostedt wrote: > > Nice work Masami! > > On Mon, 16 May 2016 21:32:50 +0900 > Namhyung Kim wrote: > > > > > -/* This is global to keep gas from relaxing the jumps */ > > > -ENTRY(early_idt_handler) > > > +/* This is weak to keep gas

Re: [PATCH] ftrace/x86: Fix function graph tracer reset path

On Mon, 16 May, at 09:58:33AM, Steven Rostedt wrote: > > Can we solve this by doing the same thing it did for the kernel? > > -- Steve > > diff --git a/arch/x86/kernel/mcount_64.S b/arch/x86/kernel/mcount_64.S > index ed48a9f465f8..e13a695c3084 100644 > --- a/arch/x86/kernel/mcount_64.S > +++

Re: [PATCH] ftrace/x86: Fix function graph tracer reset path

On Mon, 16 May, at 09:58:33AM, Steven Rostedt wrote: > > Can we solve this by doing the same thing it did for the kernel? > > -- Steve > > diff --git a/arch/x86/kernel/mcount_64.S b/arch/x86/kernel/mcount_64.S > index ed48a9f465f8..e13a695c3084 100644 > --- a/arch/x86/kernel/mcount_64.S > +++

Re: [PATCH] ftrace/x86: Fix function graph tracer reset path

On Mon, 16 May 2016 21:19:18 +0200 Borislav Petkov wrote: > Btw, arch_static_branch_jump() spells that 5-byte JMP too and not until > too long ago we had it in static_cpu_has()... Those are "special" too. If we can get the compiler to do the Right Thing (TM) then we should let

Re: [PATCH] ftrace/x86: Fix function graph tracer reset path

On Mon, 16 May 2016 21:19:18 +0200 Borislav Petkov wrote: > Btw, arch_static_branch_jump() spells that 5-byte JMP too and not until > too long ago we had it in static_cpu_has()... Those are "special" too. If we can get the compiler to do the Right Thing (TM) then we should let it. > > I

Re: [PATCH] ftrace/x86: Fix function graph tracer reset path

On Mon, May 16, 2016 at 03:13:57PM -0400, Steven Rostedt wrote: > I actually thought about this first, but I thought it rather a hack > (although one could argue all of function tracing is a hack ;-) ... I was about to say... > But as the "weak" call was used to fix one location, why not use >

Re: [PATCH] ftrace/x86: Fix function graph tracer reset path

On Mon, May 16, 2016 at 03:13:57PM -0400, Steven Rostedt wrote: > I actually thought about this first, but I thought it rather a hack > (although one could argue all of function tracing is a hack ;-) ... I was about to say... > But as the "weak" call was used to fix one location, why not use >

Re: [PATCH] ftrace/x86: Fix function graph tracer reset path

On Mon, 16 May 2016 21:03:59 +0200 Borislav Petkov wrote: > On Mon, May 16, 2016 at 11:24:53PM +0900, Namhyung Kim wrote: > > > -GLOBAL(ftrace_stub) > > > +/* This is weak to keep gas from relaxing the jumps */ > > > +WEAK(ftrace_stub) > > > retq > > > END(ftrace_caller) > >

Re: [PATCH] ftrace/x86: Fix function graph tracer reset path

On Mon, 16 May 2016 21:03:59 +0200 Borislav Petkov wrote: > On Mon, May 16, 2016 at 11:24:53PM +0900, Namhyung Kim wrote: > > > -GLOBAL(ftrace_stub) > > > +/* This is weak to keep gas from relaxing the jumps */ > > > +WEAK(ftrace_stub) > > > retq > > > END(ftrace_caller) > > You could also

Re: [PATCH] ftrace/x86: Fix function graph tracer reset path

On Mon, May 16, 2016 at 11:24:53PM +0900, Namhyung Kim wrote: > > -GLOBAL(ftrace_stub) > > +/* This is weak to keep gas from relaxing the jumps */ > > +WEAK(ftrace_stub) > > retq > > END(ftrace_caller) You could also force the 5-byte jump. I guess you could also write simply ".long 0" in

Re: [PATCH] ftrace/x86: Fix function graph tracer reset path

On Mon, May 16, 2016 at 11:24:53PM +0900, Namhyung Kim wrote: > > -GLOBAL(ftrace_stub) > > +/* This is weak to keep gas from relaxing the jumps */ > > +WEAK(ftrace_stub) > > retq > > END(ftrace_caller) You could also force the 5-byte jump. I guess you could also write simply ".long 0" in

Re: [PATCH] ftrace/x86: Fix function graph tracer reset path

Hi Steve, On Mon, May 16, 2016 at 09:58:33AM -0400, Steven Rostedt wrote: > > Nice work Masami! > > On Mon, 16 May 2016 21:32:50 +0900 > Namhyung Kim wrote: > > > > > -/* This is global to keep gas from relaxing the jumps */ > > > -ENTRY(early_idt_handler) >

Re: [PATCH] ftrace/x86: Fix function graph tracer reset path

Hi Steve, On Mon, May 16, 2016 at 09:58:33AM -0400, Steven Rostedt wrote: > > Nice work Masami! > > On Mon, 16 May 2016 21:32:50 +0900 > Namhyung Kim wrote: > > > > > -/* This is global to keep gas from relaxing the jumps */ > > > -ENTRY(early_idt_handler) > > > +/* This is

Re: [PATCH] ftrace/x86: Fix function graph tracer reset path

Nice work Masami! On Mon, 16 May 2016 21:32:50 +0900 Namhyung Kim wrote: > > -/* This is global to keep gas from relaxing the jumps */ > > -ENTRY(early_idt_handler) > > +/* This is weak to keep gas from relaxing the jumps */ > >

Re: [PATCH] ftrace/x86: Fix function graph tracer reset path

Nice work Masami! On Mon, 16 May 2016 21:32:50 +0900 Namhyung Kim wrote: > > -/* This is global to keep gas from relaxing the jumps */ > > -ENTRY(early_idt_handler) > > +/* This is weak to keep gas from relaxing the jumps */ > > +WEAK(early_idt_handler) > > cld

Re: [PATCH] ftrace/x86: Fix function graph tracer reset path

On Mon, May 16, 2016 at 05:56:14PM +0900, Masami Hiramatsu wrote: > On Sun, 15 May 2016 22:06:52 +0900 > Namhyung Kim wrote: > > > On Sun, May 15, 2016 at 09:41:41AM +0900, Masami Hiramatsu wrote: > > > > Hi Masami, > > > > > Hi Namhyung, > > > > > > I'm interested in

Re: [PATCH] ftrace/x86: Fix function graph tracer reset path

On Mon, May 16, 2016 at 05:56:14PM +0900, Masami Hiramatsu wrote: > On Sun, 15 May 2016 22:06:52 +0900 > Namhyung Kim wrote: > > > On Sun, May 15, 2016 at 09:41:41AM +0900, Masami Hiramatsu wrote: > > > > Hi Masami, > > > > > Hi Namhyung, > > > > > > I'm interested in this problem, and it

Re: [PATCH] ftrace/x86: Fix function graph tracer reset path

On Fri, 13 May, at 10:06:10AM, Steven Rostedt wrote: > Matt, > > This bug looks very similar to what you were hitting with the function > profiler. Can you apply this patch and see if it fixes the issue for > you. Yep, this patch fixes it for me. For the record, this is what objdump tells me

Re: [PATCH] ftrace/x86: Fix function graph tracer reset path

On Fri, 13 May, at 10:06:10AM, Steven Rostedt wrote: > Matt, > > This bug looks very similar to what you were hitting with the function > profiler. Can you apply this patch and see if it fixes the issue for > you. Yep, this patch fixes it for me. For the record, this is what objdump tells me

Re: [PATCH] ftrace/x86: Fix function graph tracer reset path

On Sun, May 15, 2016 at 09:41:41AM +0900, Masami Hiramatsu wrote: Hi Masami, > Hi Namhyung, > > I'm interested in this problem, and it seems compiling environment > related or kconfig related problem. > If you can reproduce this kernel, would you share what the "AS" > commandline shows? That

Re: [PATCH] ftrace/x86: Fix function graph tracer reset path

On Sun, May 15, 2016 at 09:41:41AM +0900, Masami Hiramatsu wrote: Hi Masami, > Hi Namhyung, > > I'm interested in this problem, and it seems compiling environment > related or kconfig related problem. > If you can reproduce this kernel, would you share what the "AS" > commandline shows? That

Re: [PATCH] ftrace/x86: Fix function graph tracer reset path

Hi Namhyung, I'm interested in this problem, and it seems compiling environment related or kconfig related problem. If you can reproduce this kernel, would you share what the "AS" commandline shows? That can be done as below; $ make V=1 arch/x86/kernel/mcount_64.o | grep mcount_64 Thank you,

Re: [PATCH] ftrace/x86: Fix function graph tracer reset path

Hi Namhyung, I'm interested in this problem, and it seems compiling environment related or kconfig related problem. If you can reproduce this kernel, would you share what the "AS" commandline shows? That can be done as below; $ make V=1 arch/x86/kernel/mcount_64.o | grep mcount_64 Thank you,

Re: [PATCH] ftrace/x86: Fix function graph tracer reset path

Matt, This bug looks very similar to what you were hitting with the function profiler. Can you apply this patch and see if it fixes the issue for you. Thanks! -- Steve On Fri, 13 May 2016 22:53:43 +0900 Namhyung Kim wrote: > On my system, simply enabling and disabling

Re: [PATCH] ftrace/x86: Fix function graph tracer reset path

Matt, This bug looks very similar to what you were hitting with the function profiler. Can you apply this patch and see if it fixes the issue for you. Thanks! -- Steve On Fri, 13 May 2016 22:53:43 +0900 Namhyung Kim wrote: > On my system, simply enabling and disabling function graph tracer

[PATCH] ftrace/x86: Fix function graph tracer reset path

On my system, simply enabling and disabling function graph tracer can crash the kernel. I don't know how it worked until now. The ftrace_disable_ftrace_graph_caller() modifies jmp instruction at ftrace_graph_call assuming it's a 5 bytes near jmp (e9 ). However it's a short jmp consisting of 2

[PATCH] ftrace/x86: Fix function graph tracer reset path

On my system, simply enabling and disabling function graph tracer can crash the kernel. I don't know how it worked until now. The ftrace_disable_ftrace_graph_caller() modifies jmp instruction at ftrace_graph_call assuming it's a 5 bytes near jmp (e9 ). However it's a short jmp consisting of 2