linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* powerpc ftrace broken due to "manual merge of the ftrace tree with the arm64 tree"
@ 2019-11-15 20:28 Qian Cai
  2019-11-15 21:02 ` Steven Rostedt
  0 siblings, 1 reply; 7+ messages in thread
From: Qian Cai @ 2019-11-15 20:28 UTC (permalink / raw)
  To: Stephen Rothwell
  Cc: Steven Rostedt, Catalin Marinas, Will Deacon, Mark Rutland,
	Michael Ellerman, linux-kernel, linuxppc-dev

# echo function >/sys/kernel/debug/tracing/current_tracer

It hangs forever with today's linux-next on powerpc. Reverted the conflict fix
[1] as below fixes the issue.

[1] https://lore.kernel.org/linux-next/20191115135357.10386fac@canb.auug.org.au/

diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-
generic/vmlinux.lds.h
index 7d0d03a03d4d..a92222f79b53 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -136,29 +136,20 @@
 #endif
 
 #ifdef CONFIG_FTRACE_MCOUNT_RECORD
-/*
- * The ftrace call sites are logged to a section whose name depends on the
- * compiler option used. A given kernel image will only use one, AKA
- * FTRACE_CALLSITE_SECTION. We capture all of them here to avoid header
- * dependencies for FTRACE_CALLSITE_SECTION's definition.
- */
-/*
- * Need to also make ftrace_graph_stub point to ftrace_stub
- * so that the same stub location may have different protocols
- * and not mess up with C verifiers.
- */
-#define MCOUNT_REC()	. = ALIGN(8);				\
+#ifdef CC_USING_PATCHABLE_FUNCTION_ENTRY
+#define MCOUNT_REC()	. = ALIGN(8)				\
 			__start_mcount_loc = .;			\
-			KEEP(*(__mcount_loc))			\
 			KEEP(*(__patchable_function_entries))	\
 			__stop_mcount_loc = .;			\
 			ftrace_graph_stub = ftrace_stub;
 #else
-# ifdef CONFIG_FUNCTION_TRACER
-#  define MCOUNT_REC()	ftrace_graph_stub = ftrace_stub;
-# else
-#  define MCOUNT_REC()
-# endif
+#define MCOUNT_REC()	. = ALIGN(8);				\
+			__start_mcount_loc = .;			\
+			KEEP(*(__mcount_loc))			\
+			__stop_mcount_loc = .;
+#endif
+#else
+#define MCOUNT_REC()
 #endif
 
 #ifdef CONFIG_TRACE_BRANCH_PROFILING

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

* Re: powerpc ftrace broken due to "manual merge of the ftrace tree with the arm64 tree"
  2019-11-15 20:28 powerpc ftrace broken due to "manual merge of the ftrace tree with the arm64 tree" Qian Cai
@ 2019-11-15 21:02 ` Steven Rostedt
  2019-11-15 21:06   ` Qian Cai
  0 siblings, 1 reply; 7+ messages in thread
From: Steven Rostedt @ 2019-11-15 21:02 UTC (permalink / raw)
  To: Qian Cai
  Cc: Stephen Rothwell, Catalin Marinas, Will Deacon, Mark Rutland,
	Michael Ellerman, linux-kernel, linuxppc-dev

On Fri, 15 Nov 2019 15:28:52 -0500
Qian Cai <cai@lca.pw> wrote:

> # echo function >/sys/kernel/debug/tracing/current_tracer
> 
> It hangs forever with today's linux-next on powerpc. Reverted the conflict fix
> [1] as below fixes the issue.
> 
> [1] https://lore.kernel.org/linux-next/20191115135357.10386fac@canb.auug.org.au/

What's your config file.

And can you test the two conflicting commits to see which one caused
your error?

Test this commit please: b83b43ffc6e4b514ca034a0fbdee01322e2f7022

And see if the issue is with that one, and not with the one without it.

-- Steve


> 
> diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-
> generic/vmlinux.lds.h
> index 7d0d03a03d4d..a92222f79b53 100644
> --- a/include/asm-generic/vmlinux.lds.h
> +++ b/include/asm-generic/vmlinux.lds.h
> @@ -136,29 +136,20 @@
>  #endif
>  
>  #ifdef CONFIG_FTRACE_MCOUNT_RECORD
> -/*
> - * The ftrace call sites are logged to a section whose name depends on the
> - * compiler option used. A given kernel image will only use one, AKA
> - * FTRACE_CALLSITE_SECTION. We capture all of them here to avoid header
> - * dependencies for FTRACE_CALLSITE_SECTION's definition.
> - */
> -/*
> - * Need to also make ftrace_graph_stub point to ftrace_stub
> - * so that the same stub location may have different protocols
> - * and not mess up with C verifiers.
> - */
> -#define MCOUNT_REC()	. = ALIGN(8);				\
> +#ifdef CC_USING_PATCHABLE_FUNCTION_ENTRY
> +#define MCOUNT_REC()	. = ALIGN(8)				\
>  			__start_mcount_loc = .;			\
> -			KEEP(*(__mcount_loc))			\
>  			KEEP(*(__patchable_function_entries))	\
>  			__stop_mcount_loc = .;			\
>  			ftrace_graph_stub = ftrace_stub;
>  #else
> -# ifdef CONFIG_FUNCTION_TRACER
> -#  define MCOUNT_REC()	ftrace_graph_stub = ftrace_stub;
> -# else
> -#  define MCOUNT_REC()
> -# endif
> +#define MCOUNT_REC()	. = ALIGN(8);				\
> +			__start_mcount_loc = .;			\
> +			KEEP(*(__mcount_loc))			\
> +			__stop_mcount_loc = .;
> +#endif
> +#else
> +#define MCOUNT_REC()
>  #endif
>  
>  #ifdef CONFIG_TRACE_BRANCH_PROFILING


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

* Re: powerpc ftrace broken due to "manual merge of the ftrace tree with the arm64 tree"
  2019-11-15 21:02 ` Steven Rostedt
@ 2019-11-15 21:06   ` Qian Cai
  2019-11-18 14:51     ` Steven Rostedt
  0 siblings, 1 reply; 7+ messages in thread
From: Qian Cai @ 2019-11-15 21:06 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Stephen Rothwell, Catalin Marinas, Will Deacon, Mark Rutland,
	Michael Ellerman, linux-kernel, linuxppc-dev

On Fri, 2019-11-15 at 16:02 -0500, Steven Rostedt wrote:
> On Fri, 15 Nov 2019 15:28:52 -0500
> Qian Cai <cai@lca.pw> wrote:
> 
> > # echo function >/sys/kernel/debug/tracing/current_tracer
> > 
> > It hangs forever with today's linux-next on powerpc. Reverted the conflict fix
> > [1] as below fixes the issue.
> > 
> > [1] https://lore.kernel.org/linux-next/20191115135357.10386fac@canb.auug.org.au/
> 
> What's your config file.

https://raw.githubusercontent.com/cailca/linux-mm/master/powerpc.config

> 
> And can you test the two conflicting commits to see which one caused
> your error?
> 
> Test this commit please: b83b43ffc6e4b514ca034a0fbdee01322e2f7022

# git reset --hard b83b43ffc6e4b514ca034a0fbdee01322e2f7022

Yes, that one is bad.

> 
> And see if the issue is with that one, and not with the one without it.
> 
> -- Steve
> 
> 
> > 
> > diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-
> > generic/vmlinux.lds.h
> > index 7d0d03a03d4d..a92222f79b53 100644
> > --- a/include/asm-generic/vmlinux.lds.h
> > +++ b/include/asm-generic/vmlinux.lds.h
> > @@ -136,29 +136,20 @@
> >  #endif
> >  
> >  #ifdef CONFIG_FTRACE_MCOUNT_RECORD
> > -/*
> > - * The ftrace call sites are logged to a section whose name depends on the
> > - * compiler option used. A given kernel image will only use one, AKA
> > - * FTRACE_CALLSITE_SECTION. We capture all of them here to avoid header
> > - * dependencies for FTRACE_CALLSITE_SECTION's definition.
> > - */
> > -/*
> > - * Need to also make ftrace_graph_stub point to ftrace_stub
> > - * so that the same stub location may have different protocols
> > - * and not mess up with C verifiers.
> > - */
> > -#define MCOUNT_REC()	. = ALIGN(8);				\
> > +#ifdef CC_USING_PATCHABLE_FUNCTION_ENTRY
> > +#define MCOUNT_REC()	. = ALIGN(8)				\
> >  			__start_mcount_loc = .;			\
> > -			KEEP(*(__mcount_loc))			\
> >  			KEEP(*(__patchable_function_entries))	\
> >  			__stop_mcount_loc = .;			\
> >  			ftrace_graph_stub = ftrace_stub;
> >  #else
> > -# ifdef CONFIG_FUNCTION_TRACER
> > -#  define MCOUNT_REC()	ftrace_graph_stub = ftrace_stub;
> > -# else
> > -#  define MCOUNT_REC()
> > -# endif
> > +#define MCOUNT_REC()	. = ALIGN(8);				\
> > +			__start_mcount_loc = .;			\
> > +			KEEP(*(__mcount_loc))			\
> > +			__stop_mcount_loc = .;
> > +#endif
> > +#else
> > +#define MCOUNT_REC()
> >  #endif
> >  
> >  #ifdef CONFIG_TRACE_BRANCH_PROFILING
> 
> 

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

* Re: powerpc ftrace broken due to "manual merge of the ftrace tree with the arm64 tree"
  2019-11-15 21:06   ` Qian Cai
@ 2019-11-18 14:51     ` Steven Rostedt
  2019-11-18 14:58       ` Steven Rostedt
  0 siblings, 1 reply; 7+ messages in thread
From: Steven Rostedt @ 2019-11-18 14:51 UTC (permalink / raw)
  To: Qian Cai
  Cc: Stephen Rothwell, Catalin Marinas, Will Deacon, Mark Rutland,
	Michael Ellerman, linux-kernel, linuxppc-dev

On Fri, 15 Nov 2019 16:06:34 -0500
Qian Cai <cai@lca.pw> wrote:

> > Test this commit please: b83b43ffc6e4b514ca034a0fbdee01322e2f7022  
> 
> # git reset --hard b83b43ffc6e4b514ca034a0fbdee01322e2f7022
> 
> Yes, that one is bad.

Can you see if this patch fixes the issue for you?

Thanks!

-- Steve

diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index 0f358be551cd..fd8f4dc661dc 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -109,6 +109,13 @@
 #define MEM_DISCARD(sec) *(.mem##sec)
 #endif
 
+/* PowerPC defines ftrace_graph_stub in the code */
+#ifndef CONFIG_PPC
+# define DEFINE_FTRACE_GRAPH_STUB	ftrace_graph_stub = ftrace_stub;
+#else
+# define DEFINE_FTRACE_GRAPH_STUB
+#endif
+
 #ifdef CONFIG_FTRACE_MCOUNT_RECORD
 #ifdef CC_USING_PATCHABLE_FUNCTION_ENTRY
 /*
@@ -120,17 +127,17 @@
 			__start_mcount_loc = .;			\
 			KEEP(*(__patchable_function_entries))	\
 			__stop_mcount_loc = .;			\
-			ftrace_graph_stub = ftrace_stub;
+			DEFINE_FTRACE_GRAPH_STUB
 #else
 #define MCOUNT_REC()	. = ALIGN(8);				\
 			__start_mcount_loc = .;			\
 			KEEP(*(__mcount_loc))			\
 			__stop_mcount_loc = .;			\
-			ftrace_graph_stub = ftrace_stub;
+			DEFINE_FTRACE_GRAPH_STUB
 #endif
 #else
 # ifdef CONFIG_FUNCTION_TRACER
-#  define MCOUNT_REC()	ftrace_graph_stub = ftrace_stub;
+#  define MCOUNT_REC()	DEFINE_FTRACE_GRAPH_STUB
 # else
 #  define MCOUNT_REC()
 # endif

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

* Re: powerpc ftrace broken due to "manual merge of the ftrace tree with the arm64 tree"
  2019-11-18 14:51     ` Steven Rostedt
@ 2019-11-18 14:58       ` Steven Rostedt
  2019-11-18 15:16         ` Steven Rostedt
  0 siblings, 1 reply; 7+ messages in thread
From: Steven Rostedt @ 2019-11-18 14:58 UTC (permalink / raw)
  To: Qian Cai
  Cc: Stephen Rothwell, Catalin Marinas, Will Deacon, Mark Rutland,
	Michael Ellerman, linux-kernel, linuxppc-dev

On Mon, 18 Nov 2019 09:51:04 -0500
Steven Rostedt <rostedt@goodmis.org> wrote:

> > > Test this commit please: b83b43ffc6e4b514ca034a0fbdee01322e2f7022    
> > 
> > # git reset --hard b83b43ffc6e4b514ca034a0fbdee01322e2f7022
> > 
> > Yes, that one is bad.  
> 
> Can you see if this patch fixes the issue for you?

Don't bother. This isn't the right fix, I know see the real issue.

New fix coming shortly.

-- Steve

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

* Re: powerpc ftrace broken due to "manual merge of the ftrace tree with the arm64 tree"
  2019-11-18 14:58       ` Steven Rostedt
@ 2019-11-18 15:16         ` Steven Rostedt
  2019-11-18 17:11           ` Qian Cai
  0 siblings, 1 reply; 7+ messages in thread
From: Steven Rostedt @ 2019-11-18 15:16 UTC (permalink / raw)
  To: Qian Cai
  Cc: Stephen Rothwell, Catalin Marinas, Will Deacon, Mark Rutland,
	Michael Ellerman, linux-kernel, linuxppc-dev

On Mon, 18 Nov 2019 09:58:42 -0500
Steven Rostedt <rostedt@goodmis.org> wrote:

> On Mon, 18 Nov 2019 09:51:04 -0500
> Steven Rostedt <rostedt@goodmis.org> wrote:
> 
> > > > Test this commit please: b83b43ffc6e4b514ca034a0fbdee01322e2f7022      
> > > 
> > > # git reset --hard b83b43ffc6e4b514ca034a0fbdee01322e2f7022
> > > 
> > > Yes, that one is bad.    
> > 
> > Can you see if this patch fixes the issue for you?  
> 
> Don't bother. This isn't the right fix, I know see the real issue.
> 
> New fix coming shortly.
> 

Can you try this?

It appears that I picked a name "ftrace_graph_stub", that was already in
use by powerpc. This just renames the function stub I used.

-- Steve

diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index 0f358be551cd..996db32c491b 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -112,7 +112,7 @@
 #ifdef CONFIG_FTRACE_MCOUNT_RECORD
 #ifdef CC_USING_PATCHABLE_FUNCTION_ENTRY
 /*
- * Need to also make ftrace_graph_stub point to ftrace_stub
+ * Need to also make ftrace_stub_graph point to ftrace_stub
  * so that the same stub location may have different protocols
  * and not mess up with C verifiers.
  */
@@ -120,17 +120,17 @@
 			__start_mcount_loc = .;			\
 			KEEP(*(__patchable_function_entries))	\
 			__stop_mcount_loc = .;			\
-			ftrace_graph_stub = ftrace_stub;
+			ftrace_stub_graph = ftrace_stub;
 #else
 #define MCOUNT_REC()	. = ALIGN(8);				\
 			__start_mcount_loc = .;			\
 			KEEP(*(__mcount_loc))			\
 			__stop_mcount_loc = .;			\
-			ftrace_graph_stub = ftrace_stub;
+			ftrace_stub_graph = ftrace_stub;
 #endif
 #else
 # ifdef CONFIG_FUNCTION_TRACER
-#  define MCOUNT_REC()	ftrace_graph_stub = ftrace_stub;
+#  define MCOUNT_REC()	ftrace_stub_graph = ftrace_stub;
 # else
 #  define MCOUNT_REC()
 # endif
diff --git a/kernel/trace/fgraph.c b/kernel/trace/fgraph.c
index fa3ce10d0405..67e0c462b059 100644
--- a/kernel/trace/fgraph.c
+++ b/kernel/trace/fgraph.c
@@ -336,10 +336,10 @@ int ftrace_graph_entry_stub(struct ftrace_graph_ent *trace)
  * Simply points to ftrace_stub, but with the proper protocol.
  * Defined by the linker script in linux/vmlinux.lds.h
  */
-extern void ftrace_graph_stub(struct ftrace_graph_ret *);
+extern void ftrace_stub_graph(struct ftrace_graph_ret *);
 
 /* The callbacks that hook a function */
-trace_func_graph_ret_t ftrace_graph_return = ftrace_graph_stub;
+trace_func_graph_ret_t ftrace_graph_return = ftrace_stub_graph;
 trace_func_graph_ent_t ftrace_graph_entry = ftrace_graph_entry_stub;
 static trace_func_graph_ent_t __ftrace_graph_entry = ftrace_graph_entry_stub;
 
@@ -619,7 +619,7 @@ void unregister_ftrace_graph(struct fgraph_ops *gops)
 		goto out;
 
 	ftrace_graph_active--;
-	ftrace_graph_return = ftrace_graph_stub;
+	ftrace_graph_return = ftrace_stub_graph;
 	ftrace_graph_entry = ftrace_graph_entry_stub;
 	__ftrace_graph_entry = ftrace_graph_entry_stub;
 	ftrace_shutdown(&graph_ops, FTRACE_STOP_FUNC_RET);

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

* Re: powerpc ftrace broken due to "manual merge of the ftrace tree with the arm64 tree"
  2019-11-18 15:16         ` Steven Rostedt
@ 2019-11-18 17:11           ` Qian Cai
  0 siblings, 0 replies; 7+ messages in thread
From: Qian Cai @ 2019-11-18 17:11 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Stephen Rothwell, Catalin Marinas, Will Deacon, Mark Rutland,
	Michael Ellerman, linux-kernel, linuxppc-dev

On Mon, 2019-11-18 at 10:16 -0500, Steven Rostedt wrote:
> On Mon, 18 Nov 2019 09:58:42 -0500
> Steven Rostedt <rostedt@goodmis.org> wrote:
> 
> > On Mon, 18 Nov 2019 09:51:04 -0500
> > Steven Rostedt <rostedt@goodmis.org> wrote:
> > 
> > > > > Test this commit please: b83b43ffc6e4b514ca034a0fbdee01322e2f7022      
> > > > 
> > > > # git reset --hard b83b43ffc6e4b514ca034a0fbdee01322e2f7022
> > > > 
> > > > Yes, that one is bad.    
> > > 
> > > Can you see if this patch fixes the issue for you?  
> > 
> > Don't bother. This isn't the right fix, I know see the real issue.
> > 
> > New fix coming shortly.
> > 
> 
> Can you try this?

Yes, it works fine.

> 
> It appears that I picked a name "ftrace_graph_stub", that was already in
> use by powerpc. This just renames the function stub I used.
> 
> -- Steve
> 
> diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
> index 0f358be551cd..996db32c491b 100644
> --- a/include/asm-generic/vmlinux.lds.h
> +++ b/include/asm-generic/vmlinux.lds.h
> @@ -112,7 +112,7 @@
>  #ifdef CONFIG_FTRACE_MCOUNT_RECORD
>  #ifdef CC_USING_PATCHABLE_FUNCTION_ENTRY
>  /*
> - * Need to also make ftrace_graph_stub point to ftrace_stub
> + * Need to also make ftrace_stub_graph point to ftrace_stub
>   * so that the same stub location may have different protocols
>   * and not mess up with C verifiers.
>   */
> @@ -120,17 +120,17 @@
>  			__start_mcount_loc = .;			\
>  			KEEP(*(__patchable_function_entries))	\
>  			__stop_mcount_loc = .;			\
> -			ftrace_graph_stub = ftrace_stub;
> +			ftrace_stub_graph = ftrace_stub;
>  #else
>  #define MCOUNT_REC()	. = ALIGN(8);				\
>  			__start_mcount_loc = .;			\
>  			KEEP(*(__mcount_loc))			\
>  			__stop_mcount_loc = .;			\
> -			ftrace_graph_stub = ftrace_stub;
> +			ftrace_stub_graph = ftrace_stub;
>  #endif
>  #else
>  # ifdef CONFIG_FUNCTION_TRACER
> -#  define MCOUNT_REC()	ftrace_graph_stub = ftrace_stub;
> +#  define MCOUNT_REC()	ftrace_stub_graph = ftrace_stub;
>  # else
>  #  define MCOUNT_REC()
>  # endif
> diff --git a/kernel/trace/fgraph.c b/kernel/trace/fgraph.c
> index fa3ce10d0405..67e0c462b059 100644
> --- a/kernel/trace/fgraph.c
> +++ b/kernel/trace/fgraph.c
> @@ -336,10 +336,10 @@ int ftrace_graph_entry_stub(struct ftrace_graph_ent *trace)
>   * Simply points to ftrace_stub, but with the proper protocol.
>   * Defined by the linker script in linux/vmlinux.lds.h
>   */
> -extern void ftrace_graph_stub(struct ftrace_graph_ret *);
> +extern void ftrace_stub_graph(struct ftrace_graph_ret *);
>  
>  /* The callbacks that hook a function */
> -trace_func_graph_ret_t ftrace_graph_return = ftrace_graph_stub;
> +trace_func_graph_ret_t ftrace_graph_return = ftrace_stub_graph;
>  trace_func_graph_ent_t ftrace_graph_entry = ftrace_graph_entry_stub;
>  static trace_func_graph_ent_t __ftrace_graph_entry = ftrace_graph_entry_stub;
>  
> @@ -619,7 +619,7 @@ void unregister_ftrace_graph(struct fgraph_ops *gops)
>  		goto out;
>  
>  	ftrace_graph_active--;
> -	ftrace_graph_return = ftrace_graph_stub;
> +	ftrace_graph_return = ftrace_stub_graph;
>  	ftrace_graph_entry = ftrace_graph_entry_stub;
>  	__ftrace_graph_entry = ftrace_graph_entry_stub;
>  	ftrace_shutdown(&graph_ops, FTRACE_STOP_FUNC_RET);

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

end of thread, other threads:[~2019-11-18 17:11 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-15 20:28 powerpc ftrace broken due to "manual merge of the ftrace tree with the arm64 tree" Qian Cai
2019-11-15 21:02 ` Steven Rostedt
2019-11-15 21:06   ` Qian Cai
2019-11-18 14:51     ` Steven Rostedt
2019-11-18 14:58       ` Steven Rostedt
2019-11-18 15:16         ` Steven Rostedt
2019-11-18 17:11           ` Qian Cai

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).