LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH] riscv/ftrace: Fix the problem modules cannot find _mcount
@ 2018-05-08  3:21 Alan Kao
  2018-05-08 13:11 ` Steven Rostedt
  2018-06-04 20:30 ` Palmer Dabbelt
  0 siblings, 2 replies; 7+ messages in thread
From: Alan Kao @ 2018-05-08  3:21 UTC (permalink / raw)
  To: Palmer Dabbelt, Albert Ou, linux-riscv, linux-kernel,
	Steven Rostedt, Ingo Molnar
  Cc: Alan Kao, Greentime Hu, Zong Li

Enabling ftrace and module support at the same time fails the kernel
build process, because modules cannot find the _mcount symbol.  This
patch fixes this issue.

Signed-off-by: Alan Kao <alankao@andestech.com>
Cc: Greentime Hu <greentime@andestech.com>
Cc: Zong Li <zong@andestech.com>
---
 arch/riscv/kernel/mcount.S | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/riscv/kernel/mcount.S b/arch/riscv/kernel/mcount.S
index ce9bdc57a2a1..5721624886a1 100644
--- a/arch/riscv/kernel/mcount.S
+++ b/arch/riscv/kernel/mcount.S
@@ -126,5 +126,5 @@ do_trace:
 	RESTORE_ABI_STATE
 	ret
 ENDPROC(_mcount)
-EXPORT_SYMBOL(_mcount)
 #endif
+EXPORT_SYMBOL(_mcount)
-- 
2.17.0

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

* Re: [PATCH] riscv/ftrace: Fix the problem modules cannot find _mcount
  2018-05-08  3:21 [PATCH] riscv/ftrace: Fix the problem modules cannot find _mcount Alan Kao
@ 2018-05-08 13:11 ` Steven Rostedt
  2018-05-09  2:36   ` Alan Kao
  2018-06-04 20:30 ` Palmer Dabbelt
  1 sibling, 1 reply; 7+ messages in thread
From: Steven Rostedt @ 2018-05-08 13:11 UTC (permalink / raw)
  To: Alan Kao
  Cc: Palmer Dabbelt, Albert Ou, linux-riscv, linux-kernel,
	Ingo Molnar, Greentime Hu, Zong Li

On Tue, 8 May 2018 11:21:57 +0800
Alan Kao <alankao@andestech.com> wrote:

> Enabling ftrace and module support at the same time fails the kernel
> build process, because modules cannot find the _mcount symbol.  This
> patch fixes this issue.

I think you have a much bigger issue.

> 
> Signed-off-by: Alan Kao <alankao@andestech.com>
> Cc: Greentime Hu <greentime@andestech.com>
> Cc: Zong Li <zong@andestech.com>
> ---
>  arch/riscv/kernel/mcount.S | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/riscv/kernel/mcount.S b/arch/riscv/kernel/mcount.S
> index ce9bdc57a2a1..5721624886a1 100644
> --- a/arch/riscv/kernel/mcount.S
> +++ b/arch/riscv/kernel/mcount.S
> @@ -126,5 +126,5 @@ do_trace:
>  	RESTORE_ABI_STATE
>  	ret
>  ENDPROC(_mcount)
> -EXPORT_SYMBOL(_mcount)
>  #endif
> +EXPORT_SYMBOL(_mcount)

How does this work? How do you export _mcount if _mcount isn't even
defined?

-- Steve

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

* Re: [PATCH] riscv/ftrace: Fix the problem modules cannot find _mcount
  2018-05-08 13:11 ` Steven Rostedt
@ 2018-05-09  2:36   ` Alan Kao
  2018-05-09  3:02     ` Steven Rostedt
  0 siblings, 1 reply; 7+ messages in thread
From: Alan Kao @ 2018-05-09  2:36 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Palmer Dabbelt, Albert Ou, linux-riscv, linux-kernel,
	Ingo Molnar, Greentime Hu, Zong Li

On Tue, May 08, 2018 at 09:11:42AM -0400, Steven Rostedt wrote:
> On Tue, 8 May 2018 11:21:57 +0800
> Alan Kao <alankao@andestech.com> wrote:
> 
> > Enabling ftrace and module support at the same time fails the kernel
> > build process, because modules cannot find the _mcount symbol.  This
> > patch fixes this issue.
> 
> I think you have a much bigger issue.
>

Yes, we do, but not this one.  Let me state more detail later.
 
> > 
> > Signed-off-by: Alan Kao <alankao@andestech.com>
> > Cc: Greentime Hu <greentime@andestech.com>
> > Cc: Zong Li <zong@andestech.com>
> > ---
> >  arch/riscv/kernel/mcount.S | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/arch/riscv/kernel/mcount.S b/arch/riscv/kernel/mcount.S
> > index ce9bdc57a2a1..5721624886a1 100644
> > --- a/arch/riscv/kernel/mcount.S
> > +++ b/arch/riscv/kernel/mcount.S
> > @@ -126,5 +126,5 @@ do_trace:
> >  	RESTORE_ABI_STATE
> >  	ret
> >  ENDPROC(_mcount)
> > -EXPORT_SYMBOL(_mcount)
> >  #endif
> > +EXPORT_SYMBOL(_mcount)
> 
> How does this work? How do you export _mcount if _mcount isn't even
> defined?

_mcount is defined.

This EXPORT_SYMBOL and the _mcount body was inside a
"#ifndef CONFIG_DYNAMIC_FTRACE" section, so if the config has dynamic ftrace
disabled, _mcount is visible.

For the dynamic ftrace case, there is a code snippet in this file:

> ENTRY(ftrace_stub)
> #ifdef CONFIG_DYNAMIC_FTRACE         
>       .global _mcount
>       .set    _mcount, ftrace_stub
> #endif
>        ret 
> ENDPROC(ftrace_stub)

so the _mcount symbol is visible in the kernel, but not for modules.
As a result, a module build always fails, because _mcount is neither exported,
nor aliased to a defined symbol.

That's the simple purpose of this patch:  we just want to eliminate the errors
during the kernel build process.

> 
> -- Steve

Talking about some bigger issues here, there will be twofold.

1. This patch lacks call-site collecting mechanism.

This feature requires a pattern check in recordmcount.pl.  With this,
section __mcount_loc is properly constructed, and the call-sites will be
registered during the loading stage.

However, the loading will then fail at the first try of make_nop, due to
the instruction check.  This is thus the second issue.


2. The instruction check for kernel texts is not applicable to module texts.

The check expects a call instruction to _mcount, but here it is a call to
the .plt section of the module.  After referencing the implementation of arm64,
I think it would need much more work to have make_nop's and make_call's behavior
right.


Quick summary: This patch ensures that a kernel build will not fail because of
the _mcount-missing problem.  But ftrace cannot trace loading modules for now
due to the stated reasons.

Thanks,
Alan Kao

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

* Re: [PATCH] riscv/ftrace: Fix the problem modules cannot find _mcount
  2018-05-09  2:36   ` Alan Kao
@ 2018-05-09  3:02     ` Steven Rostedt
  0 siblings, 0 replies; 7+ messages in thread
From: Steven Rostedt @ 2018-05-09  3:02 UTC (permalink / raw)
  To: Alan Kao
  Cc: Palmer Dabbelt, Albert Ou, linux-riscv, linux-kernel,
	Ingo Molnar, Greentime Hu, Zong Li

On Wed, 9 May 2018 10:36:05 +0800
Alan Kao <alankao@andestech.com> wrote:

> This EXPORT_SYMBOL and the _mcount body was inside a
> "#ifndef CONFIG_DYNAMIC_FTRACE" section, so if the config has dynamic ftrace
> disabled, _mcount is visible.
> 
> For the dynamic ftrace case, there is a code snippet in this file:
> 
> > ENTRY(ftrace_stub)
> > #ifdef CONFIG_DYNAMIC_FTRACE         
> >       .global _mcount
> >       .set    _mcount, ftrace_stub
> > #endif
> >        ret 
> > ENDPROC(ftrace_stub)  

I missed that, thanks.

> 
> Talking about some bigger issues here, there will be twofold.
> 
> 1. This patch lacks call-site collecting mechanism.
> 
> This feature requires a pattern check in recordmcount.pl.  With this,
> section __mcount_loc is properly constructed, and the call-sites will be
> registered during the loading stage.
> 
> However, the loading will then fail at the first try of make_nop, due to
> the instruction check.  This is thus the second issue.
> 
> 
> 2. The instruction check for kernel texts is not applicable to module texts.
> 
> The check expects a call instruction to _mcount, but here it is a call to
> the .plt section of the module.  After referencing the implementation of arm64,
> I think it would need much more work to have make_nop's and make_call's behavior
> right.

And powerpc64 I believe does something similar (which I think arm64
based it's work from).

> 
> 
> Quick summary: This patch ensures that a kernel build will not fail because of
> the _mcount-missing problem.  But ftrace cannot trace loading modules for now
> due to the stated reasons.

Probably why they had modules break to begin with ;-)

-- Steve

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

* Re: [PATCH] riscv/ftrace: Fix the problem modules cannot find _mcount
  2018-05-08  3:21 [PATCH] riscv/ftrace: Fix the problem modules cannot find _mcount Alan Kao
  2018-05-08 13:11 ` Steven Rostedt
@ 2018-06-04 20:30 ` Palmer Dabbelt
  2018-06-04 20:30   ` [PATCH] riscv/ftrace: Export _mcount when FUNCTION_GRAPH_TRACER isn't set Palmer Dabbelt
  1 sibling, 1 reply; 7+ messages in thread
From: Palmer Dabbelt @ 2018-06-04 20:30 UTC (permalink / raw)
  To: alankao, linux-riscv; +Cc: linux-kernel

I've rewritten the commit message to match what this patch actually
does.  Assuming it's OK, I'll include it as part of the commits for this
merge window.

Thanks!

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

* [PATCH] riscv/ftrace: Export _mcount when FUNCTION_GRAPH_TRACER isn't set
  2018-06-04 20:30 ` Palmer Dabbelt
@ 2018-06-04 20:30   ` Palmer Dabbelt
  2018-06-05  1:12     ` Alan Kao
  0 siblings, 1 reply; 7+ messages in thread
From: Palmer Dabbelt @ 2018-06-04 20:30 UTC (permalink / raw)
  To: alankao, linux-riscv; +Cc: linux-kernel, Greentime Hu, Zong Li, Palmer Dabbelt

From: Alan Kao <alankao@andestech.com>

The EXPORT_SYMBOL(_mcount) for RISC-V ended up inside a
CONFIG_FUNCTION_GRAPH_TRACER ifdef.  If you enable modules without
enabling CONFIG_FUNCTION_GRAPH_TRACER then you'll get a build error
without this patch because the modules won't be able to find _mcount.

The new behavior is to export _mcount whenever CONFIG_FUNCTION_TRACER is
defined.  This matches what every other architecture is doing.

Signed-off-by: Alan Kao <alankao@andestech.com>
Cc: Greentime Hu <greentime@andestech.com>
Cc: Zong Li <zong@andestech.com>
Signed-off-by: Palmer Dabbelt <palmer@sifive.com>
---
 arch/riscv/kernel/mcount.S | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/riscv/kernel/mcount.S b/arch/riscv/kernel/mcount.S
index ce9bdc57a2a1..5721624886a1 100644
--- a/arch/riscv/kernel/mcount.S
+++ b/arch/riscv/kernel/mcount.S
@@ -126,5 +126,5 @@ do_trace:
 	RESTORE_ABI_STATE
 	ret
 ENDPROC(_mcount)
-EXPORT_SYMBOL(_mcount)
 #endif
+EXPORT_SYMBOL(_mcount)
-- 
2.16.4

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

* Re: [PATCH] riscv/ftrace: Export _mcount when FUNCTION_GRAPH_TRACER isn't set
  2018-06-04 20:30   ` [PATCH] riscv/ftrace: Export _mcount when FUNCTION_GRAPH_TRACER isn't set Palmer Dabbelt
@ 2018-06-05  1:12     ` Alan Kao
  0 siblings, 0 replies; 7+ messages in thread
From: Alan Kao @ 2018-06-05  1:12 UTC (permalink / raw)
  To: Palmer Dabbelt; +Cc: linux-riscv, linux-kernel, Greentime Hu, Zong Li

Hi Palmer,

Thanks for the refining work! But a small correction here:

On Mon, Jun 04, 2018 at 01:30:28PM -0700, Palmer Dabbelt wrote:
> From: Alan Kao <alankao@andestech.com>
> 
> The EXPORT_SYMBOL(_mcount) for RISC-V ended up inside a
> CONFIG_FUNCTION_GRAPH_TRACER ifdef.  If you enable modules without
> enabling CONFIG_FUNCTION_GRAPH_TRACER then you'll get a build error
> without this patch because the modules won't be able to find _mcount.

the export was inside a CONFIG_DYNAMIC_FTRACE section instead of
CONFIG_FUNCTION_GRAPH_TRACER.

> The new behavior is to export _mcount whenever CONFIG_FUNCTION_TRACER is
> defined.  This matches what every other architecture is doing.

Also, this patch is just a workaround and there are still issues in bringing
up ftrace for modules.  For details, please check previous emails between
Steve and me.

Thanks again for this better commit message.
Alan Kao

> 
> Signed-off-by: Alan Kao <alankao@andestech.com>
> Cc: Greentime Hu <greentime@andestech.com>
> Cc: Zong Li <zong@andestech.com>
> Signed-off-by: Palmer Dabbelt <palmer@sifive.com>
> ---
>  arch/riscv/kernel/mcount.S | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/riscv/kernel/mcount.S b/arch/riscv/kernel/mcount.S
> index ce9bdc57a2a1..5721624886a1 100644
> --- a/arch/riscv/kernel/mcount.S
> +++ b/arch/riscv/kernel/mcount.S
> @@ -126,5 +126,5 @@ do_trace:
>  	RESTORE_ABI_STATE
>  	ret
>  ENDPROC(_mcount)
> -EXPORT_SYMBOL(_mcount)
>  #endif
> +EXPORT_SYMBOL(_mcount)
> -- 
> 2.16.4
> 

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

end of thread, other threads:[~2018-06-05  1:13 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-08  3:21 [PATCH] riscv/ftrace: Fix the problem modules cannot find _mcount Alan Kao
2018-05-08 13:11 ` Steven Rostedt
2018-05-09  2:36   ` Alan Kao
2018-05-09  3:02     ` Steven Rostedt
2018-06-04 20:30 ` Palmer Dabbelt
2018-06-04 20:30   ` [PATCH] riscv/ftrace: Export _mcount when FUNCTION_GRAPH_TRACER isn't set Palmer Dabbelt
2018-06-05  1:12     ` Alan Kao

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).