Re: [PATCH 1/3] ftrace: introdue ftrace_call_init
Hi all,
On Tue, 20 Aug 2019 11:27:38 +0200 (CEST) Miroslav Benes wrote:
>
> Hi,
>
> On Mon, 19 Aug 2019, Jisheng Zhang wrote:
>
> > On some arch, the FTRACE_WITH_REGS is implemented with gcc's
> > -fpatchable-function-entry (=2), gcc adds 2 NOPs at the beginning
> > of each function, so this makes the MCOUNT_ADDR useless. In ftrace
> > common framework, MCOUNT_ADDR is mostly used to "init" the nop, so
> > let's introcude ftrace_call_init().
> >
> > Signed-off-by: Jisheng Zhang
> > ---
> > include/linux/ftrace.h | 1 +
> > kernel/trace/ftrace.c | 4
> > 2 files changed, 5 insertions(+)
> >
> > diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
> > index 8a8cb3c401b2..8175ffb671f0 100644
> > --- a/include/linux/ftrace.h
> > +++ b/include/linux/ftrace.h
> > @@ -458,6 +458,7 @@ extern void ftrace_regs_caller(void);
> > extern void ftrace_call(void);
> > extern void ftrace_regs_call(void);
> > extern void mcount_call(void);
> > +extern int ftrace_call_init(struct module *mod, struct dyn_ftrace *rec);
> >
> > void ftrace_modify_all_code(int command);
> >
> > diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> > index eca34503f178..9df5a66a6811 100644
> > --- a/kernel/trace/ftrace.c
> > +++ b/kernel/trace/ftrace.c
> > @@ -2500,7 +2500,11 @@ ftrace_code_disable(struct module *mod, struct
> > dyn_ftrace *rec)
> > if (unlikely(ftrace_disabled))
> > return 0;
> >
> > +#ifdef MCOUNT_ADDR
> > ret = ftrace_make_nop(mod, rec, MCOUNT_ADDR);
> > +#else
> > + ret = ftrace_call_init(mod, rec);
> > +#endif
> > if (ret) {
> > ftrace_bug_type = FTRACE_BUG_INIT;
> > ftrace_bug(ret, rec);
>
> I may be missing something, but the patch does not seem to be complete.
> There is no ftrace_call_init() implemented. MCOUNT_ADDR is still defined,
> so it does not change much. I don't think it is what Mark had in his mind.
>
Here is patch to remove MCOUNT_ADDR from all arch, but I think it isn't
as good as current MCOUNT_ADDR. So how to remove the MCOUNT_ADDR depedency?
Could we
patch ftrace_make_nop to accept one more parameter? I.E
int ftrace_make_nop(struct module *mod, struct dyn_ftrace *rec,
unsigned long addr, bool init)
or
make "0" a special meaning to tell ftrace_make_nop to "init"?
Thanks
<---8
ftrace: introdue ftrace_call_init
On some arch, the FTRACE_WITH_REGS is implemented with gcc's
-fpatchable-function-entry (=2), gcc adds 2 NOPs at the beginning
of each function, so this makes the MCOUNT_ADDR useless. In ftrace
common framework, MCOUNT_ADDR is mostly used to "init" the nop, so
let's introcude ftrace_call_init() and remove MCOUNT_ADDR.
Signed-off-by: Jisheng Zhang
---
arch/arm/include/asm/ftrace.h| 1 -
arch/arm/kernel/ftrace.c | 5 +
arch/arm64/include/asm/ftrace.h | 1 -
arch/arm64/kernel/ftrace.c | 5 +
arch/csky/kernel/ftrace.c| 5 +
arch/ia64/include/asm/ftrace.h | 2 --
arch/ia64/kernel/ftrace.c| 5 +
arch/microblaze/include/asm/ftrace.h | 1 -
arch/microblaze/kernel/ftrace.c | 5 +
arch/mips/include/asm/ftrace.h | 1 -
arch/mips/kernel/ftrace.c| 9 +++--
arch/nds32/include/asm/ftrace.h | 1 -
arch/nds32/kernel/ftrace.c | 5 +
arch/parisc/include/asm/ftrace.h | 1 -
arch/parisc/kernel/ftrace.c | 5 +
arch/powerpc/include/asm/ftrace.h| 1 -
arch/powerpc/kernel/trace/ftrace.c | 5 +
arch/riscv/include/asm/ftrace.h | 1 -
arch/riscv/kernel/ftrace.c | 5 +
arch/s390/include/asm/ftrace.h | 1 -
arch/s390/kernel/ftrace.c| 22 +-
arch/sh/include/asm/ftrace.h | 2 --
arch/sh/kernel/ftrace.c | 5 +
arch/sparc/include/asm/ftrace.h | 1 -
arch/sparc/kernel/ftrace.c | 5 +
arch/x86/include/asm/ftrace.h| 1 -
arch/x86/kernel/ftrace.c | 28 +++-
arch/xtensa/include/asm/ftrace.h | 1 -
include/linux/ftrace.h | 1 +
kernel/trace/ftrace.c| 2 +-
30 files changed, 96 insertions(+), 37 deletions(-)
diff --git a/arch/arm/include/asm/ftrace.h b/arch/arm/include/asm/ftrace.h
index 18b0197f2384..afb01ab7f956 100644
--- a/arch/arm/include/asm/ftrace.h
+++ b/arch/arm/include/asm/ftrace.h
@@ -7,7 +7,6 @@
#endif
#ifdef CONFIG_FUNCTION_TRACER
-#define MCOUNT_ADDR((unsigned long)(__gnu_mcount_nc))
#define MCOUNT_INSN_SIZE 4 /* sizeof mcount call */
#ifndef __ASSEMBLY__
diff --git a/arch/arm/kernel/ftrace.c b/arch/arm/kernel/ftrace.c
index bda949fd84e8..fd3547bb4bfa 100644
--- a/arch/arm/kernel/ftrace.c
+++ b/arch/arm/kernel/ftrace.c
@@ -172,6 +172,11 @@ int ftrace_make_nop(struct module *mod,
return ret;
}
+int ftrace_call_init(struct module *mod, struct dyn_ftrace *rec)
+{
+ return ftrace_make_nop(mod, rec,
Re: [PATCH 1/3] ftrace: introdue ftrace_call_init
Hi,
On Tue, 20 Aug 2019 11:27:38 +0200 (CEST) Miroslav Benes wrote:
>
>
> Hi,
>
> On Mon, 19 Aug 2019, Jisheng Zhang wrote:
>
> > On some arch, the FTRACE_WITH_REGS is implemented with gcc's
> > -fpatchable-function-entry (=2), gcc adds 2 NOPs at the beginning
> > of each function, so this makes the MCOUNT_ADDR useless. In ftrace
> > common framework, MCOUNT_ADDR is mostly used to "init" the nop, so
> > let's introcude ftrace_call_init().
> >
> > Signed-off-by: Jisheng Zhang
> > ---
> > include/linux/ftrace.h | 1 +
> > kernel/trace/ftrace.c | 4
> > 2 files changed, 5 insertions(+)
> >
> > diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
> > index 8a8cb3c401b2..8175ffb671f0 100644
> > --- a/include/linux/ftrace.h
> > +++ b/include/linux/ftrace.h
> > @@ -458,6 +458,7 @@ extern void ftrace_regs_caller(void);
> > extern void ftrace_call(void);
> > extern void ftrace_regs_call(void);
> > extern void mcount_call(void);
> > +extern int ftrace_call_init(struct module *mod, struct dyn_ftrace *rec);
> >
> > void ftrace_modify_all_code(int command);
> >
> > diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> > index eca34503f178..9df5a66a6811 100644
> > --- a/kernel/trace/ftrace.c
> > +++ b/kernel/trace/ftrace.c
> > @@ -2500,7 +2500,11 @@ ftrace_code_disable(struct module *mod, struct
> > dyn_ftrace *rec)
> > if (unlikely(ftrace_disabled))
> > return 0;
> >
> > +#ifdef MCOUNT_ADDR
> > ret = ftrace_make_nop(mod, rec, MCOUNT_ADDR);
> > +#else
> > + ret = ftrace_call_init(mod, rec);
> > +#endif
> > if (ret) {
> > ftrace_bug_type = FTRACE_BUG_INIT;
> > ftrace_bug(ret, rec);
>
> I may be missing something, but the patch does not seem to be complete.
> There is no ftrace_call_init() implemented. MCOUNT_ADDR is still defined,
> so it does not change much. I don't think it is what Mark had in his mind.
>
Yes, except arm64, MCOUNT_ADDR is still defined in other arch. If we want
to remove MCOUNT_ADDR from all arch, I will cook patches for this purpose.
Thanks
Re: [PATCH 1/3] ftrace: introdue ftrace_call_init
Hi,
On Mon, 19 Aug 2019, Jisheng Zhang wrote:
> On some arch, the FTRACE_WITH_REGS is implemented with gcc's
> -fpatchable-function-entry (=2), gcc adds 2 NOPs at the beginning
> of each function, so this makes the MCOUNT_ADDR useless. In ftrace
> common framework, MCOUNT_ADDR is mostly used to "init" the nop, so
> let's introcude ftrace_call_init().
>
> Signed-off-by: Jisheng Zhang
> ---
> include/linux/ftrace.h | 1 +
> kernel/trace/ftrace.c | 4
> 2 files changed, 5 insertions(+)
>
> diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
> index 8a8cb3c401b2..8175ffb671f0 100644
> --- a/include/linux/ftrace.h
> +++ b/include/linux/ftrace.h
> @@ -458,6 +458,7 @@ extern void ftrace_regs_caller(void);
> extern void ftrace_call(void);
> extern void ftrace_regs_call(void);
> extern void mcount_call(void);
> +extern int ftrace_call_init(struct module *mod, struct dyn_ftrace *rec);
>
> void ftrace_modify_all_code(int command);
>
> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> index eca34503f178..9df5a66a6811 100644
> --- a/kernel/trace/ftrace.c
> +++ b/kernel/trace/ftrace.c
> @@ -2500,7 +2500,11 @@ ftrace_code_disable(struct module *mod, struct
> dyn_ftrace *rec)
> if (unlikely(ftrace_disabled))
> return 0;
>
> +#ifdef MCOUNT_ADDR
> ret = ftrace_make_nop(mod, rec, MCOUNT_ADDR);
> +#else
> + ret = ftrace_call_init(mod, rec);
> +#endif
> if (ret) {
> ftrace_bug_type = FTRACE_BUG_INIT;
> ftrace_bug(ret, rec);
I may be missing something, but the patch does not seem to be complete.
There is no ftrace_call_init() implemented. MCOUNT_ADDR is still defined,
so it does not change much. I don't think it is what Mark had in his mind.
Miroslav
Re: [PATCH 1/3] ftrace: introdue ftrace_call_init
On Mon, 19 Aug 2019 19:16:22 +0800 Jisheng Zhang wrote: > On some arch, the FTRACE_WITH_REGS is implemented with gcc's > -fpatchable-function-entry (=2), gcc adds 2 NOPs at the beginning > of each function, so this makes the MCOUNT_ADDR useless. In ftrace > common framework, MCOUNT_ADDR is mostly used to "init" the nop, so > let's introcude ftrace_call_init(). > > Signed-off-by: Jisheng Zhang > --- > include/linux/ftrace.h | 1 + > kernel/trace/ftrace.c | 4 > 2 files changed, 5 insertions(+) > > diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h > index 8a8cb3c401b2..8175ffb671f0 100644 > --- a/include/linux/ftrace.h > +++ b/include/linux/ftrace.h > @@ -458,6 +458,7 @@ extern void ftrace_regs_caller(void); > extern void ftrace_call(void); > extern void ftrace_regs_call(void); > extern void mcount_call(void); > +extern int ftrace_call_init(struct module *mod, struct dyn_ftrace *rec); > > void ftrace_modify_all_code(int command); > > diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c > index eca34503f178..9df5a66a6811 100644 > --- a/kernel/trace/ftrace.c > +++ b/kernel/trace/ftrace.c > @@ -2500,7 +2500,11 @@ ftrace_code_disable(struct module *mod, struct > dyn_ftrace *rec) > if (unlikely(ftrace_disabled)) > return 0; > > +#ifdef MCOUNT_ADDR > ret = ftrace_make_nop(mod, rec, MCOUNT_ADDR); > +#else > + ret = ftrace_call_init(mod, rec); > +#endif If we want to remove MCOUNT_ADDR from all arch, I could cook patches for this.
[PATCH 1/3] ftrace: introdue ftrace_call_init
On some arch, the FTRACE_WITH_REGS is implemented with gcc's
-fpatchable-function-entry (=2), gcc adds 2 NOPs at the beginning
of each function, so this makes the MCOUNT_ADDR useless. In ftrace
common framework, MCOUNT_ADDR is mostly used to "init" the nop, so
let's introcude ftrace_call_init().
Signed-off-by: Jisheng Zhang
---
include/linux/ftrace.h | 1 +
kernel/trace/ftrace.c | 4
2 files changed, 5 insertions(+)
diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
index 8a8cb3c401b2..8175ffb671f0 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -458,6 +458,7 @@ extern void ftrace_regs_caller(void);
extern void ftrace_call(void);
extern void ftrace_regs_call(void);
extern void mcount_call(void);
+extern int ftrace_call_init(struct module *mod, struct dyn_ftrace *rec);
void ftrace_modify_all_code(int command);
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index eca34503f178..9df5a66a6811 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -2500,7 +2500,11 @@ ftrace_code_disable(struct module *mod, struct
dyn_ftrace *rec)
if (unlikely(ftrace_disabled))
return 0;
+#ifdef MCOUNT_ADDR
ret = ftrace_make_nop(mod, rec, MCOUNT_ADDR);
+#else
+ ret = ftrace_call_init(mod, rec);
+#endif
if (ret) {
ftrace_bug_type = FTRACE_BUG_INIT;
ftrace_bug(ret, rec);
--
2.23.0.rc1
