All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jisheng Zhang <Jisheng.Zhang@synaptics.com>
To: Miroslav Benes <mbenes@suse.cz>,
	Steven Rostedt <rostedt@goodmis.org>,
	Mark Rutland <mark.rutland@arm.com>, Torsten Duwe <duwe@suse.de>
Cc: Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will@kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH 1/3] ftrace: introdue ftrace_call_init
Date: Mon, 26 Aug 2019 10:14:09 +0000	[thread overview]
Message-ID: <20190826180253.69f28971@xhacker.debian> (raw)
In-Reply-To: <alpine.LSU.2.21.1908201118190.9536@pobox.suse.cz>

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 <Jisheng.Zhang@synaptics.com>
> > ---
> >  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 <Jisheng.Zhang@synaptics.com>
---
 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, (unsigned long)__gnu_mcount_nc);
+}
+
 int __init ftrace_dyn_arch_init(void)
 {
 	return 0;
diff --git a/arch/arm64/include/asm/ftrace.h b/arch/arm64/include/asm/ftrace.h
index 5ab5200b2bdc..de6f35ef3825 100644
--- a/arch/arm64/include/asm/ftrace.h
+++ b/arch/arm64/include/asm/ftrace.h
@@ -11,7 +11,6 @@
 #include <asm/insn.h>
 
 #define HAVE_FUNCTION_GRAPH_FP_TEST
-#define MCOUNT_ADDR		((unsigned long)_mcount)
 #define MCOUNT_INSN_SIZE	AARCH64_INSN_SIZE
 
 #ifndef __ASSEMBLY__
diff --git a/arch/arm64/kernel/ftrace.c b/arch/arm64/kernel/ftrace.c
index 171773257974..8298b8a7d8f6 100644
--- a/arch/arm64/kernel/ftrace.c
+++ b/arch/arm64/kernel/ftrace.c
@@ -196,6 +196,11 @@ int ftrace_make_nop(struct module *mod, struct dyn_ftrace *rec,
 	return ftrace_modify_code(pc, old, new, validate);
 }
 
+int ftrace_call_init(struct module *mod, struct dyn_ftrace *rec)
+{
+	return ftrace_make_nop(mod, rec, (unsigned long)_mcount);
+}
+
 void arch_ftrace_update_code(int command)
 {
 	command |= FTRACE_MAY_SLEEP;
diff --git a/arch/csky/kernel/ftrace.c b/arch/csky/kernel/ftrace.c
index 44f4880179b7..c1fce15caaa4 100644
--- a/arch/csky/kernel/ftrace.c
+++ b/arch/csky/kernel/ftrace.c
@@ -122,6 +122,11 @@ int ftrace_make_nop(struct module *mod, struct dyn_ftrace *rec,
 	return ftrace_modify_code(rec->ip, addr, false, false);
 }
 
+int ftrace_call_init(struct module *mod, struct dyn_ftrace *rec)
+{
+	return ftrace_make_nop(mod, rec, (unsigned long)_mcount);
+}
+
 int ftrace_update_ftrace_func(ftrace_func_t func)
 {
 	int ret = ftrace_modify_code((unsigned long)&ftrace_call,
diff --git a/arch/ia64/include/asm/ftrace.h b/arch/ia64/include/asm/ftrace.h
index a07a8e575453..2aefd2098fc5 100644
--- a/arch/ia64/include/asm/ftrace.h
+++ b/arch/ia64/include/asm/ftrace.h
@@ -9,8 +9,6 @@
 extern void _mcount(unsigned long pfs, unsigned long r1, unsigned long b0, unsigned long r0);
 #define mcount _mcount
 
-/* In IA64, MCOUNT_ADDR is set in link time, so it's not a constant at compile time */
-#define MCOUNT_ADDR (((struct fnptr *)mcount)->ip)
 #define FTRACE_ADDR (((struct fnptr *)ftrace_caller)->ip)
 
 static inline unsigned long ftrace_call_adjust(unsigned long addr)
diff --git a/arch/ia64/kernel/ftrace.c b/arch/ia64/kernel/ftrace.c
index cee411e647ca..0cb2cbcea4fc 100644
--- a/arch/ia64/kernel/ftrace.c
+++ b/arch/ia64/kernel/ftrace.c
@@ -169,6 +169,11 @@ int ftrace_make_nop(struct module *mod,
 	return ftrace_modify_code(rec->ip, NULL, new, 0);
 }
 
+int ftrace_call_init(struct module *mod, struct dyn_ftrace *rec)
+{
+	return ftrace_make_nop(mod, rec, ((struct fnptr *)mcount)->ip);
+}
+
 int ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
 {
 	unsigned long ip = rec->ip;
diff --git a/arch/microblaze/include/asm/ftrace.h b/arch/microblaze/include/asm/ftrace.h
index 5db7f4489f05..02d185ef22fe 100644
--- a/arch/microblaze/include/asm/ftrace.h
+++ b/arch/microblaze/include/asm/ftrace.h
@@ -4,7 +4,6 @@
 
 #ifdef CONFIG_FUNCTION_TRACER
 
-#define MCOUNT_ADDR		((unsigned long)(_mcount))
 #define MCOUNT_INSN_SIZE	8 /* sizeof mcount call */
 
 #ifndef __ASSEMBLY__
diff --git a/arch/microblaze/kernel/ftrace.c b/arch/microblaze/kernel/ftrace.c
index 224eea40e1ee..f3606e3cba94 100644
--- a/arch/microblaze/kernel/ftrace.c
+++ b/arch/microblaze/kernel/ftrace.c
@@ -149,6 +149,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, (unsigned long)_mcount);
+}
+
 /* I believe that first is called ftrace_make_nop before this function */
 int ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
 {
diff --git a/arch/mips/include/asm/ftrace.h b/arch/mips/include/asm/ftrace.h
index b463f2aa5a61..45b3f7064fcd 100644
--- a/arch/mips/include/asm/ftrace.h
+++ b/arch/mips/include/asm/ftrace.h
@@ -12,7 +12,6 @@
 
 #ifdef CONFIG_FUNCTION_TRACER
 
-#define MCOUNT_ADDR ((unsigned long)(_mcount))
 #define MCOUNT_INSN_SIZE 4		/* sizeof mcount call */
 
 #ifndef __ASSEMBLY__
diff --git a/arch/mips/kernel/ftrace.c b/arch/mips/kernel/ftrace.c
index 2625232bfe52..85de81985e6f 100644
--- a/arch/mips/kernel/ftrace.c
+++ b/arch/mips/kernel/ftrace.c
@@ -61,7 +61,7 @@ static inline void ftrace_dyn_arch_init_insns(void)
 	/* la v1, _mcount */
 	v1 = 3;
 	buf = (u32 *)&insn_la_mcount[0];
-	UASM_i_LA(&buf, v1, MCOUNT_ADDR);
+	UASM_i_LA(&buf, v1, (unsigned long)_mcount);
 
 	/* jal (ftrace_caller + 8), jump over the first two instruction */
 	buf = (u32 *)&insn_jal_ftrace_caller;
@@ -200,6 +200,11 @@ int ftrace_make_nop(struct module *mod,
 #endif
 }
 
+int ftrace_call_init(struct module *mod, struct dyn_ftrace *rec)
+{
+	return ftrace_make_nop(mod, rec, (unsigned long)_mcount);
+}
+
 int ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
 {
 	unsigned int new;
@@ -232,7 +237,7 @@ int __init ftrace_dyn_arch_init(void)
 	ftrace_dyn_arch_init_insns();
 
 	/* Remove "b ftrace_stub" to ensure ftrace_caller() is executed */
-	ftrace_modify_code(MCOUNT_ADDR, INSN_NOP);
+	ftrace_modify_code((unsigned long)_mcount, INSN_NOP);
 
 	return 0;
 }
diff --git a/arch/nds32/include/asm/ftrace.h b/arch/nds32/include/asm/ftrace.h
index 2f96cc96aa35..97263241249d 100644
--- a/arch/nds32/include/asm/ftrace.h
+++ b/arch/nds32/include/asm/ftrace.h
@@ -7,7 +7,6 @@
 
 #define HAVE_FUNCTION_GRAPH_FP_TEST
 
-#define MCOUNT_ADDR ((unsigned long)(_mcount))
 /* mcount call is composed of three instructions:
  * sethi + ori + jral
  */
diff --git a/arch/nds32/kernel/ftrace.c b/arch/nds32/kernel/ftrace.c
index fd2a54b8cd57..8f0c34f7bf5b 100644
--- a/arch/nds32/kernel/ftrace.c
+++ b/arch/nds32/kernel/ftrace.c
@@ -203,6 +203,11 @@ int ftrace_make_nop(struct module *mod, struct dyn_ftrace *rec,
 
 	return ftrace_modify_code(pc, call_insn, nop_insn, true);
 }
+
+int ftrace_call_init(struct module *mod, struct dyn_ftrace *rec)
+{
+	return ftrace_make_nop(mod, rec, (unsigned long)_mcount);
+}
 #endif /* CONFIG_DYNAMIC_FTRACE */
 
 #ifdef CONFIG_FUNCTION_GRAPH_TRACER
diff --git a/arch/parisc/include/asm/ftrace.h b/arch/parisc/include/asm/ftrace.h
index 958c0aa5dbb2..e0c7cc7958b8 100644
--- a/arch/parisc/include/asm/ftrace.h
+++ b/arch/parisc/include/asm/ftrace.h
@@ -5,7 +5,6 @@
 #ifndef __ASSEMBLY__
 extern void mcount(void);
 
-#define MCOUNT_ADDR		((unsigned long)mcount)
 #define MCOUNT_INSN_SIZE	4
 #define CC_USING_NOP_MCOUNT
 extern unsigned long sys_call_table[];
diff --git a/arch/parisc/kernel/ftrace.c b/arch/parisc/kernel/ftrace.c
index b6fb30f2e4bf..4c9d8d1e5262 100644
--- a/arch/parisc/kernel/ftrace.c
+++ b/arch/parisc/kernel/ftrace.c
@@ -186,4 +186,9 @@ int ftrace_make_nop(struct module *mod, struct dyn_ftrace *rec,
 			      insn, sizeof(insn)-4);
 	return 0;
 }
+
+int ftrace_call_init(struct module *mod, struct dyn_ftrace *rec)
+{
+	return ftrace_make_nop(mod, rec, (unsigned long)mcount);
+}
 #endif
diff --git a/arch/powerpc/include/asm/ftrace.h b/arch/powerpc/include/asm/ftrace.h
index 3dfb80b86561..bac76b982d2c 100644
--- a/arch/powerpc/include/asm/ftrace.h
+++ b/arch/powerpc/include/asm/ftrace.h
@@ -5,7 +5,6 @@
 #include <asm/types.h>
 
 #ifdef CONFIG_FUNCTION_TRACER
-#define MCOUNT_ADDR		((unsigned long)(_mcount))
 #define MCOUNT_INSN_SIZE	4 /* sizeof mcount call */
 
 #ifdef __ASSEMBLY__
diff --git a/arch/powerpc/kernel/trace/ftrace.c b/arch/powerpc/kernel/trace/ftrace.c
index be1ca98fce5c..728a357c51f0 100644
--- a/arch/powerpc/kernel/trace/ftrace.c
+++ b/arch/powerpc/kernel/trace/ftrace.c
@@ -472,6 +472,11 @@ int ftrace_make_nop(struct module *mod,
 #endif /* CONFIG_MODULES */
 }
 
+int ftrace_call_init(struct module *mod, struct dyn_ftrace *rec)
+{
+	return ftrace_make_nop(mod, rec, (unsigned long)_mcount);
+}
+
 #ifdef CONFIG_MODULES
 #ifdef CONFIG_PPC64
 /*
diff --git a/arch/riscv/include/asm/ftrace.h b/arch/riscv/include/asm/ftrace.h
index c6dcc5291f97..5ca88b80de44 100644
--- a/arch/riscv/include/asm/ftrace.h
+++ b/arch/riscv/include/asm/ftrace.h
@@ -33,7 +33,6 @@ struct dyn_arch_ftrace {
  * both auipc and jalr at the same time.
  */
 
-#define MCOUNT_ADDR		((unsigned long)_mcount)
 #define JALR_SIGN_MASK		(0x00000800)
 #define JALR_OFFSET_MASK	(0x00000fff)
 #define AUIPC_OFFSET_MASK	(0xfffff000)
diff --git a/arch/riscv/kernel/ftrace.c b/arch/riscv/kernel/ftrace.c
index b94d8db5ddcc..88fcdbf84ad8 100644
--- a/arch/riscv/kernel/ftrace.c
+++ b/arch/riscv/kernel/ftrace.c
@@ -88,6 +88,11 @@ int ftrace_make_nop(struct module *mod, struct dyn_ftrace *rec,
 	return __ftrace_modify_call(rec->ip, addr, false);
 }
 
+int ftrace_call_init(struct module *mod, struct dyn_ftrace *rec)
+{
+	return ftrace_make_nop(mod, rec, (unsigned long)_mcount);
+}
+
 int ftrace_update_ftrace_func(ftrace_func_t func)
 {
 	int ret = __ftrace_modify_call((unsigned long)&ftrace_call,
diff --git a/arch/s390/include/asm/ftrace.h b/arch/s390/include/asm/ftrace.h
index 68d362f8d6c1..e70717c37f68 100644
--- a/arch/s390/include/asm/ftrace.h
+++ b/arch/s390/include/asm/ftrace.h
@@ -30,7 +30,6 @@ extern unsigned long ftrace_plt;
 
 struct dyn_arch_ftrace { };
 
-#define MCOUNT_ADDR ((unsigned long)_mcount)
 #define FTRACE_ADDR ((unsigned long)ftrace_caller)
 
 #define KPROBE_ON_FTRACE_NOP	0
diff --git a/arch/s390/kernel/ftrace.c b/arch/s390/kernel/ftrace.c
index 1bb85f60c0dd..b74e35cc2262 100644
--- a/arch/s390/kernel/ftrace.c
+++ b/arch/s390/kernel/ftrace.c
@@ -110,11 +110,7 @@ int ftrace_make_nop(struct module *mod, struct dyn_ftrace *rec,
 
 	if (probe_kernel_read(&old, (void *) rec->ip, sizeof(old)))
 		return -EFAULT;
-	if (addr == MCOUNT_ADDR) {
-		/* Initial code replacement */
-		ftrace_generate_orig_insn(&orig);
-		ftrace_generate_nop_insn(&new);
-	} else if (is_kprobe_on_ftrace(&old)) {
+	if (is_kprobe_on_ftrace(&old)) {
 		/*
 		 * If we find a breakpoint instruction, a kprobe has been
 		 * placed at the beginning of the function. We write the
@@ -136,6 +132,22 @@ int ftrace_make_nop(struct module *mod, struct dyn_ftrace *rec,
 	return 0;
 }
 
+int ftrace_call_init(struct module *mod, struct dyn_ftrace *rec)
+{
+	struct ftrace_insn orig, new, old;
+
+	if (probe_kernel_read(&old, (void *) rec->ip, sizeof(old)))
+		return -EFAULT;
+	ftrace_generate_orig_insn(&orig);
+	ftrace_generate_nop_insn(&new);
+	/* Verify that the to be replaced code matches what we expect. */
+	if (memcmp(&orig, &old, sizeof(old)))
+		return -EINVAL;
+	s390_kernel_write((void *) rec->ip, &new, sizeof(new));
+
+	return 0;
+}
+
 int ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
 {
 	struct ftrace_insn orig, new, old;
diff --git a/arch/sh/include/asm/ftrace.h b/arch/sh/include/asm/ftrace.h
index b1c1dc0cc261..5a7890b7a0d2 100644
--- a/arch/sh/include/asm/ftrace.h
+++ b/arch/sh/include/asm/ftrace.h
@@ -10,8 +10,6 @@
 #ifndef __ASSEMBLY__
 extern void mcount(void);
 
-#define MCOUNT_ADDR		((unsigned long)(mcount))
-
 #ifdef CONFIG_DYNAMIC_FTRACE
 #define CALL_ADDR		((long)(ftrace_call))
 #define STUB_ADDR		((long)(ftrace_stub))
diff --git a/arch/sh/kernel/ftrace.c b/arch/sh/kernel/ftrace.c
index 1b04270e5460..f7a12102a93d 100644
--- a/arch/sh/kernel/ftrace.c
+++ b/arch/sh/kernel/ftrace.c
@@ -242,6 +242,11 @@ int ftrace_make_nop(struct module *mod,
 	return ftrace_modify_code(rec->ip, old, new);
 }
 
+int ftrace_call_init(struct module *mod, struct dyn_ftrace *rec)
+{
+	return ftrace_make_nop(mod, rec, (unsigned long)mcount);
+}
+
 int ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
 {
 	unsigned char *new, *old;
diff --git a/arch/sparc/include/asm/ftrace.h b/arch/sparc/include/asm/ftrace.h
index d3aa1a524431..a1eecda8976b 100644
--- a/arch/sparc/include/asm/ftrace.h
+++ b/arch/sparc/include/asm/ftrace.h
@@ -3,7 +3,6 @@
 #define _ASM_SPARC64_FTRACE
 
 #ifdef CONFIG_MCOUNT
-#define MCOUNT_ADDR		((unsigned long)(_mcount))
 #define MCOUNT_INSN_SIZE	4 /* sizeof mcount call */
 
 #ifndef __ASSEMBLY__
diff --git a/arch/sparc/kernel/ftrace.c b/arch/sparc/kernel/ftrace.c
index 684b84ce397f..7a16482fdd29 100644
--- a/arch/sparc/kernel/ftrace.c
+++ b/arch/sparc/kernel/ftrace.c
@@ -63,6 +63,11 @@ int ftrace_make_nop(struct module *mod, struct dyn_ftrace *rec, unsigned long ad
 	return ftrace_modify_code(ip, old, new);
 }
 
+int ftrace_call_init(struct module *mod, struct dyn_ftrace *rec)
+{
+	return ftrace_make_nop(mod, rec, (unsigned long)_mcount);
+}
+
 int ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
 {
 	unsigned long ip = rec->ip;
diff --git a/arch/x86/include/asm/ftrace.h b/arch/x86/include/asm/ftrace.h
index 287f1f7b2e52..58fe65fc5a63 100644
--- a/arch/x86/include/asm/ftrace.h
+++ b/arch/x86/include/asm/ftrace.h
@@ -6,7 +6,6 @@
 #ifndef CC_USING_FENTRY
 # error Compiler does not support fentry?
 #endif
-# define MCOUNT_ADDR		((unsigned long)(__fentry__))
 #define MCOUNT_INSN_SIZE	5 /* sizeof mcount call */
 
 #ifdef CONFIG_DYNAMIC_FTRACE
diff --git a/arch/x86/kernel/ftrace.c b/arch/x86/kernel/ftrace.c
index 024c3053dbba..3cdba006f848 100644
--- a/arch/x86/kernel/ftrace.c
+++ b/arch/x86/kernel/ftrace.c
@@ -149,26 +149,28 @@ ftrace_modify_code_direct(unsigned long ip, unsigned const char *old_code,
 	return 0;
 }
 
-int ftrace_make_nop(struct module *mod,
-		    struct dyn_ftrace *rec, unsigned long addr)
+/*
+ * On boot up, and when modules are loaded, the __fentry__
+ * is converted to a nop, and will never become __fentry__
+ * again. This code is either running before SMP (on boot up)
+ * or before the code will ever be executed (module load).
+ * We do not want to use the breakpoint version in this case,
+ * just modify the code directly.
+ */
+int ftrace_call_init(struct module *mod, struct dyn_ftrace *rec)
 {
 	unsigned const char *new, *old;
 	unsigned long ip = rec->ip;
 
-	old = ftrace_call_replace(ip, addr);
+	old = ftrace_call_replace(ip, (unsigned long)__fentry__);
 	new = ftrace_nop_replace();
 
-	/*
-	 * On boot up, and when modules are loaded, the MCOUNT_ADDR
-	 * is converted to a nop, and will never become MCOUNT_ADDR
-	 * again. This code is either running before SMP (on boot up)
-	 * or before the code will ever be executed (module load).
-	 * We do not want to use the breakpoint version in this case,
-	 * just modify the code directly.
-	 */
-	if (addr == MCOUNT_ADDR)
-		return ftrace_modify_code_direct(rec->ip, old, new);
+	return ftrace_modify_code_direct(ip, old, new);
+}
 
+int ftrace_make_nop(struct module *mod,
+		    struct dyn_ftrace *rec, unsigned long addr)
+{
 	ftrace_expected = NULL;
 
 	/* Normal cases use add_brk_on_nop */
diff --git a/arch/xtensa/include/asm/ftrace.h b/arch/xtensa/include/asm/ftrace.h
index 6c6d9a9f185f..3ce2170291cd 100644
--- a/arch/xtensa/include/asm/ftrace.h
+++ b/arch/xtensa/include/asm/ftrace.h
@@ -28,7 +28,6 @@ extern unsigned long return_address(unsigned level);
 
 #ifdef CONFIG_FUNCTION_TRACER
 
-#define MCOUNT_ADDR ((unsigned long)(_mcount))
 #define MCOUNT_INSN_SIZE 3
 
 #ifndef __ASSEMBLY__
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..62ca6977e570 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -2500,7 +2500,7 @@ ftrace_code_disable(struct module *mod, struct dyn_ftrace *rec)
 	if (unlikely(ftrace_disabled))
 		return 0;
 
-	ret = ftrace_make_nop(mod, rec, MCOUNT_ADDR);
+	ret = ftrace_call_init(mod, rec);
 	if (ret) {
 		ftrace_bug_type = FTRACE_BUG_INIT;
 		ftrace_bug(ret, rec);
-- 
2.23.0.rc1




_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

WARNING: multiple messages have this Message-ID (diff)
From: Jisheng Zhang <Jisheng.Zhang@synaptics.com>
To: Miroslav Benes <mbenes@suse.cz>,
	Steven Rostedt <rostedt@goodmis.org>,
	Mark Rutland <mark.rutland@arm.com>, Torsten Duwe <duwe@suse.de>
Cc: Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will@kernel.org>,
	"linux-arm-kernel@lists.infradead.org" 
	<linux-arm-kernel@lists.infradead.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 1/3] ftrace: introdue ftrace_call_init
Date: Mon, 26 Aug 2019 10:14:09 +0000	[thread overview]
Message-ID: <20190826180253.69f28971@xhacker.debian> (raw)
In-Reply-To: <alpine.LSU.2.21.1908201118190.9536@pobox.suse.cz>

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 <Jisheng.Zhang@synaptics.com>
> > ---
> >  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 <Jisheng.Zhang@synaptics.com>
---
 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, (unsigned long)__gnu_mcount_nc);
+}
+
 int __init ftrace_dyn_arch_init(void)
 {
 	return 0;
diff --git a/arch/arm64/include/asm/ftrace.h b/arch/arm64/include/asm/ftrace.h
index 5ab5200b2bdc..de6f35ef3825 100644
--- a/arch/arm64/include/asm/ftrace.h
+++ b/arch/arm64/include/asm/ftrace.h
@@ -11,7 +11,6 @@
 #include <asm/insn.h>
 
 #define HAVE_FUNCTION_GRAPH_FP_TEST
-#define MCOUNT_ADDR		((unsigned long)_mcount)
 #define MCOUNT_INSN_SIZE	AARCH64_INSN_SIZE
 
 #ifndef __ASSEMBLY__
diff --git a/arch/arm64/kernel/ftrace.c b/arch/arm64/kernel/ftrace.c
index 171773257974..8298b8a7d8f6 100644
--- a/arch/arm64/kernel/ftrace.c
+++ b/arch/arm64/kernel/ftrace.c
@@ -196,6 +196,11 @@ int ftrace_make_nop(struct module *mod, struct dyn_ftrace *rec,
 	return ftrace_modify_code(pc, old, new, validate);
 }
 
+int ftrace_call_init(struct module *mod, struct dyn_ftrace *rec)
+{
+	return ftrace_make_nop(mod, rec, (unsigned long)_mcount);
+}
+
 void arch_ftrace_update_code(int command)
 {
 	command |= FTRACE_MAY_SLEEP;
diff --git a/arch/csky/kernel/ftrace.c b/arch/csky/kernel/ftrace.c
index 44f4880179b7..c1fce15caaa4 100644
--- a/arch/csky/kernel/ftrace.c
+++ b/arch/csky/kernel/ftrace.c
@@ -122,6 +122,11 @@ int ftrace_make_nop(struct module *mod, struct dyn_ftrace *rec,
 	return ftrace_modify_code(rec->ip, addr, false, false);
 }
 
+int ftrace_call_init(struct module *mod, struct dyn_ftrace *rec)
+{
+	return ftrace_make_nop(mod, rec, (unsigned long)_mcount);
+}
+
 int ftrace_update_ftrace_func(ftrace_func_t func)
 {
 	int ret = ftrace_modify_code((unsigned long)&ftrace_call,
diff --git a/arch/ia64/include/asm/ftrace.h b/arch/ia64/include/asm/ftrace.h
index a07a8e575453..2aefd2098fc5 100644
--- a/arch/ia64/include/asm/ftrace.h
+++ b/arch/ia64/include/asm/ftrace.h
@@ -9,8 +9,6 @@
 extern void _mcount(unsigned long pfs, unsigned long r1, unsigned long b0, unsigned long r0);
 #define mcount _mcount
 
-/* In IA64, MCOUNT_ADDR is set in link time, so it's not a constant at compile time */
-#define MCOUNT_ADDR (((struct fnptr *)mcount)->ip)
 #define FTRACE_ADDR (((struct fnptr *)ftrace_caller)->ip)
 
 static inline unsigned long ftrace_call_adjust(unsigned long addr)
diff --git a/arch/ia64/kernel/ftrace.c b/arch/ia64/kernel/ftrace.c
index cee411e647ca..0cb2cbcea4fc 100644
--- a/arch/ia64/kernel/ftrace.c
+++ b/arch/ia64/kernel/ftrace.c
@@ -169,6 +169,11 @@ int ftrace_make_nop(struct module *mod,
 	return ftrace_modify_code(rec->ip, NULL, new, 0);
 }
 
+int ftrace_call_init(struct module *mod, struct dyn_ftrace *rec)
+{
+	return ftrace_make_nop(mod, rec, ((struct fnptr *)mcount)->ip);
+}
+
 int ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
 {
 	unsigned long ip = rec->ip;
diff --git a/arch/microblaze/include/asm/ftrace.h b/arch/microblaze/include/asm/ftrace.h
index 5db7f4489f05..02d185ef22fe 100644
--- a/arch/microblaze/include/asm/ftrace.h
+++ b/arch/microblaze/include/asm/ftrace.h
@@ -4,7 +4,6 @@
 
 #ifdef CONFIG_FUNCTION_TRACER
 
-#define MCOUNT_ADDR		((unsigned long)(_mcount))
 #define MCOUNT_INSN_SIZE	8 /* sizeof mcount call */
 
 #ifndef __ASSEMBLY__
diff --git a/arch/microblaze/kernel/ftrace.c b/arch/microblaze/kernel/ftrace.c
index 224eea40e1ee..f3606e3cba94 100644
--- a/arch/microblaze/kernel/ftrace.c
+++ b/arch/microblaze/kernel/ftrace.c
@@ -149,6 +149,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, (unsigned long)_mcount);
+}
+
 /* I believe that first is called ftrace_make_nop before this function */
 int ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
 {
diff --git a/arch/mips/include/asm/ftrace.h b/arch/mips/include/asm/ftrace.h
index b463f2aa5a61..45b3f7064fcd 100644
--- a/arch/mips/include/asm/ftrace.h
+++ b/arch/mips/include/asm/ftrace.h
@@ -12,7 +12,6 @@
 
 #ifdef CONFIG_FUNCTION_TRACER
 
-#define MCOUNT_ADDR ((unsigned long)(_mcount))
 #define MCOUNT_INSN_SIZE 4		/* sizeof mcount call */
 
 #ifndef __ASSEMBLY__
diff --git a/arch/mips/kernel/ftrace.c b/arch/mips/kernel/ftrace.c
index 2625232bfe52..85de81985e6f 100644
--- a/arch/mips/kernel/ftrace.c
+++ b/arch/mips/kernel/ftrace.c
@@ -61,7 +61,7 @@ static inline void ftrace_dyn_arch_init_insns(void)
 	/* la v1, _mcount */
 	v1 = 3;
 	buf = (u32 *)&insn_la_mcount[0];
-	UASM_i_LA(&buf, v1, MCOUNT_ADDR);
+	UASM_i_LA(&buf, v1, (unsigned long)_mcount);
 
 	/* jal (ftrace_caller + 8), jump over the first two instruction */
 	buf = (u32 *)&insn_jal_ftrace_caller;
@@ -200,6 +200,11 @@ int ftrace_make_nop(struct module *mod,
 #endif
 }
 
+int ftrace_call_init(struct module *mod, struct dyn_ftrace *rec)
+{
+	return ftrace_make_nop(mod, rec, (unsigned long)_mcount);
+}
+
 int ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
 {
 	unsigned int new;
@@ -232,7 +237,7 @@ int __init ftrace_dyn_arch_init(void)
 	ftrace_dyn_arch_init_insns();
 
 	/* Remove "b ftrace_stub" to ensure ftrace_caller() is executed */
-	ftrace_modify_code(MCOUNT_ADDR, INSN_NOP);
+	ftrace_modify_code((unsigned long)_mcount, INSN_NOP);
 
 	return 0;
 }
diff --git a/arch/nds32/include/asm/ftrace.h b/arch/nds32/include/asm/ftrace.h
index 2f96cc96aa35..97263241249d 100644
--- a/arch/nds32/include/asm/ftrace.h
+++ b/arch/nds32/include/asm/ftrace.h
@@ -7,7 +7,6 @@
 
 #define HAVE_FUNCTION_GRAPH_FP_TEST
 
-#define MCOUNT_ADDR ((unsigned long)(_mcount))
 /* mcount call is composed of three instructions:
  * sethi + ori + jral
  */
diff --git a/arch/nds32/kernel/ftrace.c b/arch/nds32/kernel/ftrace.c
index fd2a54b8cd57..8f0c34f7bf5b 100644
--- a/arch/nds32/kernel/ftrace.c
+++ b/arch/nds32/kernel/ftrace.c
@@ -203,6 +203,11 @@ int ftrace_make_nop(struct module *mod, struct dyn_ftrace *rec,
 
 	return ftrace_modify_code(pc, call_insn, nop_insn, true);
 }
+
+int ftrace_call_init(struct module *mod, struct dyn_ftrace *rec)
+{
+	return ftrace_make_nop(mod, rec, (unsigned long)_mcount);
+}
 #endif /* CONFIG_DYNAMIC_FTRACE */
 
 #ifdef CONFIG_FUNCTION_GRAPH_TRACER
diff --git a/arch/parisc/include/asm/ftrace.h b/arch/parisc/include/asm/ftrace.h
index 958c0aa5dbb2..e0c7cc7958b8 100644
--- a/arch/parisc/include/asm/ftrace.h
+++ b/arch/parisc/include/asm/ftrace.h
@@ -5,7 +5,6 @@
 #ifndef __ASSEMBLY__
 extern void mcount(void);
 
-#define MCOUNT_ADDR		((unsigned long)mcount)
 #define MCOUNT_INSN_SIZE	4
 #define CC_USING_NOP_MCOUNT
 extern unsigned long sys_call_table[];
diff --git a/arch/parisc/kernel/ftrace.c b/arch/parisc/kernel/ftrace.c
index b6fb30f2e4bf..4c9d8d1e5262 100644
--- a/arch/parisc/kernel/ftrace.c
+++ b/arch/parisc/kernel/ftrace.c
@@ -186,4 +186,9 @@ int ftrace_make_nop(struct module *mod, struct dyn_ftrace *rec,
 			      insn, sizeof(insn)-4);
 	return 0;
 }
+
+int ftrace_call_init(struct module *mod, struct dyn_ftrace *rec)
+{
+	return ftrace_make_nop(mod, rec, (unsigned long)mcount);
+}
 #endif
diff --git a/arch/powerpc/include/asm/ftrace.h b/arch/powerpc/include/asm/ftrace.h
index 3dfb80b86561..bac76b982d2c 100644
--- a/arch/powerpc/include/asm/ftrace.h
+++ b/arch/powerpc/include/asm/ftrace.h
@@ -5,7 +5,6 @@
 #include <asm/types.h>
 
 #ifdef CONFIG_FUNCTION_TRACER
-#define MCOUNT_ADDR		((unsigned long)(_mcount))
 #define MCOUNT_INSN_SIZE	4 /* sizeof mcount call */
 
 #ifdef __ASSEMBLY__
diff --git a/arch/powerpc/kernel/trace/ftrace.c b/arch/powerpc/kernel/trace/ftrace.c
index be1ca98fce5c..728a357c51f0 100644
--- a/arch/powerpc/kernel/trace/ftrace.c
+++ b/arch/powerpc/kernel/trace/ftrace.c
@@ -472,6 +472,11 @@ int ftrace_make_nop(struct module *mod,
 #endif /* CONFIG_MODULES */
 }
 
+int ftrace_call_init(struct module *mod, struct dyn_ftrace *rec)
+{
+	return ftrace_make_nop(mod, rec, (unsigned long)_mcount);
+}
+
 #ifdef CONFIG_MODULES
 #ifdef CONFIG_PPC64
 /*
diff --git a/arch/riscv/include/asm/ftrace.h b/arch/riscv/include/asm/ftrace.h
index c6dcc5291f97..5ca88b80de44 100644
--- a/arch/riscv/include/asm/ftrace.h
+++ b/arch/riscv/include/asm/ftrace.h
@@ -33,7 +33,6 @@ struct dyn_arch_ftrace {
  * both auipc and jalr at the same time.
  */
 
-#define MCOUNT_ADDR		((unsigned long)_mcount)
 #define JALR_SIGN_MASK		(0x00000800)
 #define JALR_OFFSET_MASK	(0x00000fff)
 #define AUIPC_OFFSET_MASK	(0xfffff000)
diff --git a/arch/riscv/kernel/ftrace.c b/arch/riscv/kernel/ftrace.c
index b94d8db5ddcc..88fcdbf84ad8 100644
--- a/arch/riscv/kernel/ftrace.c
+++ b/arch/riscv/kernel/ftrace.c
@@ -88,6 +88,11 @@ int ftrace_make_nop(struct module *mod, struct dyn_ftrace *rec,
 	return __ftrace_modify_call(rec->ip, addr, false);
 }
 
+int ftrace_call_init(struct module *mod, struct dyn_ftrace *rec)
+{
+	return ftrace_make_nop(mod, rec, (unsigned long)_mcount);
+}
+
 int ftrace_update_ftrace_func(ftrace_func_t func)
 {
 	int ret = __ftrace_modify_call((unsigned long)&ftrace_call,
diff --git a/arch/s390/include/asm/ftrace.h b/arch/s390/include/asm/ftrace.h
index 68d362f8d6c1..e70717c37f68 100644
--- a/arch/s390/include/asm/ftrace.h
+++ b/arch/s390/include/asm/ftrace.h
@@ -30,7 +30,6 @@ extern unsigned long ftrace_plt;
 
 struct dyn_arch_ftrace { };
 
-#define MCOUNT_ADDR ((unsigned long)_mcount)
 #define FTRACE_ADDR ((unsigned long)ftrace_caller)
 
 #define KPROBE_ON_FTRACE_NOP	0
diff --git a/arch/s390/kernel/ftrace.c b/arch/s390/kernel/ftrace.c
index 1bb85f60c0dd..b74e35cc2262 100644
--- a/arch/s390/kernel/ftrace.c
+++ b/arch/s390/kernel/ftrace.c
@@ -110,11 +110,7 @@ int ftrace_make_nop(struct module *mod, struct dyn_ftrace *rec,
 
 	if (probe_kernel_read(&old, (void *) rec->ip, sizeof(old)))
 		return -EFAULT;
-	if (addr == MCOUNT_ADDR) {
-		/* Initial code replacement */
-		ftrace_generate_orig_insn(&orig);
-		ftrace_generate_nop_insn(&new);
-	} else if (is_kprobe_on_ftrace(&old)) {
+	if (is_kprobe_on_ftrace(&old)) {
 		/*
 		 * If we find a breakpoint instruction, a kprobe has been
 		 * placed at the beginning of the function. We write the
@@ -136,6 +132,22 @@ int ftrace_make_nop(struct module *mod, struct dyn_ftrace *rec,
 	return 0;
 }
 
+int ftrace_call_init(struct module *mod, struct dyn_ftrace *rec)
+{
+	struct ftrace_insn orig, new, old;
+
+	if (probe_kernel_read(&old, (void *) rec->ip, sizeof(old)))
+		return -EFAULT;
+	ftrace_generate_orig_insn(&orig);
+	ftrace_generate_nop_insn(&new);
+	/* Verify that the to be replaced code matches what we expect. */
+	if (memcmp(&orig, &old, sizeof(old)))
+		return -EINVAL;
+	s390_kernel_write((void *) rec->ip, &new, sizeof(new));
+
+	return 0;
+}
+
 int ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
 {
 	struct ftrace_insn orig, new, old;
diff --git a/arch/sh/include/asm/ftrace.h b/arch/sh/include/asm/ftrace.h
index b1c1dc0cc261..5a7890b7a0d2 100644
--- a/arch/sh/include/asm/ftrace.h
+++ b/arch/sh/include/asm/ftrace.h
@@ -10,8 +10,6 @@
 #ifndef __ASSEMBLY__
 extern void mcount(void);
 
-#define MCOUNT_ADDR		((unsigned long)(mcount))
-
 #ifdef CONFIG_DYNAMIC_FTRACE
 #define CALL_ADDR		((long)(ftrace_call))
 #define STUB_ADDR		((long)(ftrace_stub))
diff --git a/arch/sh/kernel/ftrace.c b/arch/sh/kernel/ftrace.c
index 1b04270e5460..f7a12102a93d 100644
--- a/arch/sh/kernel/ftrace.c
+++ b/arch/sh/kernel/ftrace.c
@@ -242,6 +242,11 @@ int ftrace_make_nop(struct module *mod,
 	return ftrace_modify_code(rec->ip, old, new);
 }
 
+int ftrace_call_init(struct module *mod, struct dyn_ftrace *rec)
+{
+	return ftrace_make_nop(mod, rec, (unsigned long)mcount);
+}
+
 int ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
 {
 	unsigned char *new, *old;
diff --git a/arch/sparc/include/asm/ftrace.h b/arch/sparc/include/asm/ftrace.h
index d3aa1a524431..a1eecda8976b 100644
--- a/arch/sparc/include/asm/ftrace.h
+++ b/arch/sparc/include/asm/ftrace.h
@@ -3,7 +3,6 @@
 #define _ASM_SPARC64_FTRACE
 
 #ifdef CONFIG_MCOUNT
-#define MCOUNT_ADDR		((unsigned long)(_mcount))
 #define MCOUNT_INSN_SIZE	4 /* sizeof mcount call */
 
 #ifndef __ASSEMBLY__
diff --git a/arch/sparc/kernel/ftrace.c b/arch/sparc/kernel/ftrace.c
index 684b84ce397f..7a16482fdd29 100644
--- a/arch/sparc/kernel/ftrace.c
+++ b/arch/sparc/kernel/ftrace.c
@@ -63,6 +63,11 @@ int ftrace_make_nop(struct module *mod, struct dyn_ftrace *rec, unsigned long ad
 	return ftrace_modify_code(ip, old, new);
 }
 
+int ftrace_call_init(struct module *mod, struct dyn_ftrace *rec)
+{
+	return ftrace_make_nop(mod, rec, (unsigned long)_mcount);
+}
+
 int ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
 {
 	unsigned long ip = rec->ip;
diff --git a/arch/x86/include/asm/ftrace.h b/arch/x86/include/asm/ftrace.h
index 287f1f7b2e52..58fe65fc5a63 100644
--- a/arch/x86/include/asm/ftrace.h
+++ b/arch/x86/include/asm/ftrace.h
@@ -6,7 +6,6 @@
 #ifndef CC_USING_FENTRY
 # error Compiler does not support fentry?
 #endif
-# define MCOUNT_ADDR		((unsigned long)(__fentry__))
 #define MCOUNT_INSN_SIZE	5 /* sizeof mcount call */
 
 #ifdef CONFIG_DYNAMIC_FTRACE
diff --git a/arch/x86/kernel/ftrace.c b/arch/x86/kernel/ftrace.c
index 024c3053dbba..3cdba006f848 100644
--- a/arch/x86/kernel/ftrace.c
+++ b/arch/x86/kernel/ftrace.c
@@ -149,26 +149,28 @@ ftrace_modify_code_direct(unsigned long ip, unsigned const char *old_code,
 	return 0;
 }
 
-int ftrace_make_nop(struct module *mod,
-		    struct dyn_ftrace *rec, unsigned long addr)
+/*
+ * On boot up, and when modules are loaded, the __fentry__
+ * is converted to a nop, and will never become __fentry__
+ * again. This code is either running before SMP (on boot up)
+ * or before the code will ever be executed (module load).
+ * We do not want to use the breakpoint version in this case,
+ * just modify the code directly.
+ */
+int ftrace_call_init(struct module *mod, struct dyn_ftrace *rec)
 {
 	unsigned const char *new, *old;
 	unsigned long ip = rec->ip;
 
-	old = ftrace_call_replace(ip, addr);
+	old = ftrace_call_replace(ip, (unsigned long)__fentry__);
 	new = ftrace_nop_replace();
 
-	/*
-	 * On boot up, and when modules are loaded, the MCOUNT_ADDR
-	 * is converted to a nop, and will never become MCOUNT_ADDR
-	 * again. This code is either running before SMP (on boot up)
-	 * or before the code will ever be executed (module load).
-	 * We do not want to use the breakpoint version in this case,
-	 * just modify the code directly.
-	 */
-	if (addr == MCOUNT_ADDR)
-		return ftrace_modify_code_direct(rec->ip, old, new);
+	return ftrace_modify_code_direct(ip, old, new);
+}
 
+int ftrace_make_nop(struct module *mod,
+		    struct dyn_ftrace *rec, unsigned long addr)
+{
 	ftrace_expected = NULL;
 
 	/* Normal cases use add_brk_on_nop */
diff --git a/arch/xtensa/include/asm/ftrace.h b/arch/xtensa/include/asm/ftrace.h
index 6c6d9a9f185f..3ce2170291cd 100644
--- a/arch/xtensa/include/asm/ftrace.h
+++ b/arch/xtensa/include/asm/ftrace.h
@@ -28,7 +28,6 @@ extern unsigned long return_address(unsigned level);
 
 #ifdef CONFIG_FUNCTION_TRACER
 
-#define MCOUNT_ADDR ((unsigned long)(_mcount))
 #define MCOUNT_INSN_SIZE 3
 
 #ifndef __ASSEMBLY__
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..62ca6977e570 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -2500,7 +2500,7 @@ ftrace_code_disable(struct module *mod, struct dyn_ftrace *rec)
 	if (unlikely(ftrace_disabled))
 		return 0;
 
-	ret = ftrace_make_nop(mod, rec, MCOUNT_ADDR);
+	ret = ftrace_call_init(mod, rec);
 	if (ret) {
 		ftrace_bug_type = FTRACE_BUG_INIT;
 		ftrace_bug(ret, rec);
-- 
2.23.0.rc1




  parent reply	other threads:[~2019-08-26 10:14 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-19 11:26 [PATCH 0/3] arm64: ftrace with regs Jisheng Zhang
2019-08-19 11:26 ` Jisheng Zhang
2019-08-19 11:27 ` [PATCH 1/3] ftrace: introdue ftrace_call_init Jisheng Zhang
2019-08-19 11:27   ` Jisheng Zhang
2019-08-19 11:30   ` Jisheng Zhang
2019-08-19 11:30     ` Jisheng Zhang
2019-08-20  9:27   ` Miroslav Benes
2019-08-20  9:27     ` Miroslav Benes
2019-08-21  2:12     ` Jisheng Zhang
2019-08-21  2:12       ` Jisheng Zhang
2019-08-26 10:14     ` Jisheng Zhang [this message]
2019-08-26 10:14       ` Jisheng Zhang
2019-08-19 11:28 ` [PATCH 2/3] arm64: implement ftrace with regs Jisheng Zhang
2019-08-19 11:28   ` Jisheng Zhang
2019-08-19 11:29 ` [PATCH 3/3] arm64: use -fpatchable-function-entry if available Jisheng Zhang
2019-08-19 11:29   ` Jisheng Zhang

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=20190826180253.69f28971@xhacker.debian \
    --to=jisheng.zhang@synaptics.com \
    --cc=catalin.marinas@arm.com \
    --cc=duwe@suse.de \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=mbenes@suse.cz \
    --cc=rostedt@goodmis.org \
    --cc=will@kernel.org \
    /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: link
Be 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.