From: Alan Kao <alankao@andestech.com> To: Steven Rostedt <rostedt@goodmis.org> Cc: Palmer Dabbelt <palmer@sifive.com>, Albert Ou <albert@sifive.com>, <linux-riscv@lists.infradead.org>, <linux-kernel@vger.kernel.org>, "Ingo Molnar" <mingo@redhat.com>, Greentime Hu <greentime@andestech.com>, Zong Li <zong@andestech.com> Subject: Re: [PATCH] riscv/ftrace: Fix the problem modules cannot find _mcount Date: Wed, 9 May 2018 10:36:05 +0800 [thread overview] Message-ID: <20180509023605.GB7303@andestech.com> (raw) In-Reply-To: <20180508091142.12b5231a@gandalf.local.home> 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
WARNING: multiple messages have this Message-ID (diff)
From: alankao@andestech.com (Alan Kao) To: linux-riscv@lists.infradead.org Subject: [PATCH] riscv/ftrace: Fix the problem modules cannot find _mcount Date: Wed, 9 May 2018 10:36:05 +0800 [thread overview] Message-ID: <20180509023605.GB7303@andestech.com> (raw) In-Reply-To: <20180508091142.12b5231a@gandalf.local.home> 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
next prev parent reply other threads:[~2018-05-09 2:36 UTC|newest] Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-05-08 3:21 [PATCH] riscv/ftrace: Fix the problem modules cannot find _mcount Alan Kao 2018-05-08 3:21 ` Alan Kao 2018-05-08 13:11 ` Steven Rostedt 2018-05-08 13:11 ` Steven Rostedt 2018-05-09 2:36 ` Alan Kao [this message] 2018-05-09 2:36 ` Alan Kao 2018-05-09 3:02 ` Steven Rostedt 2018-05-09 3:02 ` Steven Rostedt 2018-06-04 20:30 ` Palmer Dabbelt 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-04 20:30 ` Palmer Dabbelt 2018-06-05 1:12 ` Alan Kao 2018-06-05 1:12 ` Alan Kao
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20180509023605.GB7303@andestech.com \ --to=alankao@andestech.com \ --cc=albert@sifive.com \ --cc=greentime@andestech.com \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-riscv@lists.infradead.org \ --cc=mingo@redhat.com \ --cc=palmer@sifive.com \ --cc=rostedt@goodmis.org \ --cc=zong@andestech.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
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.