All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] ftrace/x86: Set ftrace_stub to weak to prevent gcc from using short jumps to it
@ 2016-05-17  3:00 Steven Rostedt
  2016-05-18 14:31 ` Steven Rostedt
  0 siblings, 1 reply; 3+ messages in thread
From: Steven Rostedt @ 2016-05-17  3:00 UTC (permalink / raw)
  To: LKML
  Cc: Ingo Molnar, H. Peter Anvin, Thomas Gleixner, Masami Hiramatsu,
	Namhyung Kim, Matt Fleming

Matt Fleming reported seeing crashes when enabling and disabling
function profiling which uses function graph tracer. Later Namhyung Kim
hit a similar issue and he found that the issue was due to the jmp to
ftrace_stub in ftrace_graph_call was only two bytes, and when it was
changed to jump to the tracing code, it overwrote the ftrace_stub that
was after it.

Masami Hiramatsu bisected this down to a binutils change:

8dcea93252a9ea7dff57e85220a719e2a5e8ab41 is the first bad commit
commit 8dcea93252a9ea7dff57e85220a719e2a5e8ab41
Author: H.J. Lu <hjl.tools@gmail.com>
Date:   Fri May 15 03:17:31 2015 -0700

    Add -mshared option to x86 ELF assembler
    
    This patch adds -mshared option to x86 ELF assembler.  By default,
    assembler will optimize out non-PLT relocations against defined non-weak
    global branch targets with default visibility.  The -mshared option tells
    the assembler to generate code which may go into a shared library
    where all non-weak global branch targets with default visibility can
    be preempted.  The resulting code is slightly bigger.  This option
    only affects the handling of branch instructions.

Declaring ftrace_stub as a weak call prevents gas from using two byte
jumps to it, which would be converted to a jump to the function graph
code.

Reported-by: Matt Fleming <matt@codeblueprint.co.uk>
Reported-by: Namhyung Kim <namhyung@kernel.org>
Tested-by: Matt Fleming <matt@codeblueprint.co.uk>
Reviewed-by: Masami Hiramatsu <mhiramat@kernel.org>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
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
+++ b/arch/x86/kernel/mcount_64.S
@@ -182,7 +182,8 @@ GLOBAL(ftrace_graph_call)
 	jmp ftrace_stub
 #endif
 
-GLOBAL(ftrace_stub)
+/* This is weak to keep gas from relaxing the jumps */
+WEAK(ftrace_stub)
 	retq
 END(ftrace_caller)
 

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] ftrace/x86: Set ftrace_stub to weak to prevent gcc from using short jumps to it
  2016-05-17  3:00 [PATCH] ftrace/x86: Set ftrace_stub to weak to prevent gcc from using short jumps to it Steven Rostedt
@ 2016-05-18 14:31 ` Steven Rostedt
  2016-05-20  7:08   ` Ingo Molnar
  0 siblings, 1 reply; 3+ messages in thread
From: Steven Rostedt @ 2016-05-18 14:31 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: LKML, H. Peter Anvin, Thomas Gleixner, Masami Hiramatsu,
	Namhyung Kim, Matt Fleming


Ingo,

Did you want to take this patch through the tip tree, or do you want me
to take it?

Thanks,

-- Steve



On Mon, 16 May 2016 23:00:35 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:

> Matt Fleming reported seeing crashes when enabling and disabling
> function profiling which uses function graph tracer. Later Namhyung Kim
> hit a similar issue and he found that the issue was due to the jmp to
> ftrace_stub in ftrace_graph_call was only two bytes, and when it was
> changed to jump to the tracing code, it overwrote the ftrace_stub that
> was after it.
> 
> Masami Hiramatsu bisected this down to a binutils change:
> 
> 8dcea93252a9ea7dff57e85220a719e2a5e8ab41 is the first bad commit
> commit 8dcea93252a9ea7dff57e85220a719e2a5e8ab41
> Author: H.J. Lu <hjl.tools@gmail.com>
> Date:   Fri May 15 03:17:31 2015 -0700
> 
>     Add -mshared option to x86 ELF assembler
>     
>     This patch adds -mshared option to x86 ELF assembler.  By default,
>     assembler will optimize out non-PLT relocations against defined non-weak
>     global branch targets with default visibility.  The -mshared option tells
>     the assembler to generate code which may go into a shared library
>     where all non-weak global branch targets with default visibility can
>     be preempted.  The resulting code is slightly bigger.  This option
>     only affects the handling of branch instructions.
> 
> Declaring ftrace_stub as a weak call prevents gas from using two byte
> jumps to it, which would be converted to a jump to the function graph
> code.
> 
> Reported-by: Matt Fleming <matt@codeblueprint.co.uk>
> Reported-by: Namhyung Kim <namhyung@kernel.org>
> Tested-by: Matt Fleming <matt@codeblueprint.co.uk>
> Reviewed-by: Masami Hiramatsu <mhiramat@kernel.org>
> Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
> ---
> 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
> +++ b/arch/x86/kernel/mcount_64.S
> @@ -182,7 +182,8 @@ GLOBAL(ftrace_graph_call)
>  	jmp ftrace_stub
>  #endif
>  
> -GLOBAL(ftrace_stub)
> +/* This is weak to keep gas from relaxing the jumps */
> +WEAK(ftrace_stub)
>  	retq
>  END(ftrace_caller)
>  
> 

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] ftrace/x86: Set ftrace_stub to weak to prevent gcc from using short jumps to it
  2016-05-18 14:31 ` Steven Rostedt
@ 2016-05-20  7:08   ` Ingo Molnar
  0 siblings, 0 replies; 3+ messages in thread
From: Ingo Molnar @ 2016-05-20  7:08 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: LKML, H. Peter Anvin, Thomas Gleixner, Masami Hiramatsu,
	Namhyung Kim, Matt Fleming


* Steven Rostedt <rostedt@goodmis.org> wrote:

> 
> Ingo,
> 
> Did you want to take this patch through the tip tree, or do you want me
> to take it?

Feel free to take it!

Thanks,

	Ingo

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2016-05-20  7:08 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-05-17  3:00 [PATCH] ftrace/x86: Set ftrace_stub to weak to prevent gcc from using short jumps to it Steven Rostedt
2016-05-18 14:31 ` Steven Rostedt
2016-05-20  7:08   ` Ingo Molnar

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.