tracing: MIPS: add misc fixups and cleanups

PATCH Applied Wu Zhangjin 3 participants 23 messages May 14, 2010 – May 27, 2010

Subsystems (2)

MIPS ftrace

lore

Share & tools

lore

Wu Zhangjin

author

2010-05-14 11:09 UTC #0 lore raw

v7 cover tracing: MIPS: add misc fixups and cleanups

From: Wu Zhangjin <[email protected]>

This patchset adds misc fixups and cleanups for Ftrace of MIPS:

+ Fix the support of 32bit support with -mmcount-ra-address of gcc 4.5
o tracing: MIPS: mcount.S: Fix the argument passing of the 32bit support with gcc 4.5, The argument is passed by $12 in 32bit, not t0.
o tracing: MIPS: Fixup of the 32bit support with -mmcount-ra-address For 32bit kernel, the offset of "b 1f" should be 5 instructions, not only 4.

+ Speedup the dynamic function tracer
o tracing: MIPS: Reduce the overhead of dynamic Function Tracer In the old implementation, we have encode the 'nop' instruction and the instruction of calling to mcount at run-time, which may add some overhead. We reduce this overhead via encoding them when initializing the dynamic function tracer.

+ Lots of cleanups
o The other patches.

----------------
v6 -> v7:

o Apply the feedback from David Daney:
define a macro MCOUNT_RA_ADDRESS_REG instead of the magic number $12 for the patch "tracing: MIPS: mcount.S: Fix the argument passing of the 32bit
support with gcc 4.5"

v5 -> v6:

o splits up the old v5 revision into several patches to make the maintainer happier to review it.

Regards,
Wu Zhangjin

Wu Zhangjin (9):
tracing: MIPS: mcount.S: merge the same continuous #ifdefs
tracing: MIPS: mcount.S: cleanup the arguments of
prepare_ftrace_return
tracing: MIPS: mcount.S: cleanup of the comments
tracing: MIPS: mcount.S: Fix the argument passing of the 32bit
support with gcc 4.5
tracing: MIPS: Fixup of the 32bit support with -mmcount-ra-address
tracing: MIPS: cleanup of the instructions
tracing: MIPS: Reduce the overhead of dynamic Function Tracer
tracing: MIPS: cleanup of function graph tracer
tracing: MIPS: cleanup of the address space checking

 arch/mips/kernel/ftrace.c |  184 +++++++++++++++++++++++++++------------------
 arch/mips/kernel/mcount.S |   55 ++++++++-----
 2 files changed, 146 insertions(+), 93 deletions(-)

Wu Zhangjin

author

2010-05-14 11:09 UTC #1 lore raw

PATCH v7 1/9 tracing: MIPS: mcount.S: merge the same continuous #ifdefs

From: Wu Zhangjin <[email protected]>

There are two same continuous #ifdefs, this patch merges them to reduce two lines.

Signed-off-by Wu Zhangjin <[email protected]>

---
 arch/mips/kernel/mcount.S |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

arch/mips/kernel/mcount.S +0 -2

index 6851fc9..e256bf9 100644
@@ -45,8 +45,6 @@
 	PTR_L	a5, PT_R9(sp)
 	PTR_L	a6, PT_R10(sp)
 	PTR_L	a7, PT_R11(sp)
-#endif
-#ifdef CONFIG_64BIT
 	PTR_ADDIU	sp, PT_SIZE
 #else
 	PTR_ADDIU	sp, (PT_SIZE + 8)

Signature · 1 line

1.7.0.4

Ralf Baechle

Applied

2010-05-27 11:27 UTC #2 lore raw

Re: [PATCH 1/9] tracing: MIPS: mcount.S: merge the same continuous #ifdefs

Applied, thanks!

Ralf

Wu Zhangjin

author

2010-05-14 11:10 UTC #3 lore raw

PATCH v7 2/9 tracing: MIPS: mcount.S: cleanup the arguments of prepare_ftrace_return

In reply to Wu Zhangjin

From: Wu Zhangjin <[email protected]>

The old arguments handling for prepare_ftrace_return is awlful, this patch cleans it up.

Signed-off-by Wu Zhangjin <[email protected]>

---
 arch/mips/kernel/mcount.S |   36 ++++++++++++++++++++++--------------
 1 files changed, 22 insertions(+), 14 deletions(-)

arch/mips/kernel/mcount.S +22 ~~-14~~

index e256bf9..d4a00d2 100644
@@ -132,28 +132,34 @@ ftrace_stub:
 #ifdef CONFIG_FUNCTION_GRAPH_TRACER
 
 NESTED(ftrace_graph_caller, PT_SIZE, ra)
-#ifdef CONFIG_DYNAMIC_FTRACE
-	PTR_L	a1, PT_R31(sp)	/* load the original ra from the stack */
-#ifdef KBUILD_MCOUNT_RA_ADDRESS
-	PTR_L	t0, PT_R12(sp)	/* load the original t0 from the stack */
-#endif
-#else
+#ifndef CONFIG_DYNAMIC_FTRACE
 	MCOUNT_SAVE_REGS
-	move	a1, ra		/* arg2: next ip, selfaddr */
 #endif
 
+	/* arg1: Get the location of the parent's return address */
 #ifdef KBUILD_MCOUNT_RA_ADDRESS
-	bnez	t0, 1f		/* non-leaf func: t0 saved the location of the return address */
+#ifdef CONFIG_DYNAMIC_FTRACE
+	PTR_L	a0, PT_R12(sp)
+#else
+	move	a0, t0
+#endif
+	bnez	a0, 1f		/* non-leaf func: stored in t0 */
 	 nop
-	PTR_LA	t0, PT_R1(sp)	/* leaf func: get the location of at(old ra) from our own stack */
-1:	move	a0, t0		/* arg1: the location of the return address */
+#endif
+	PTR_LA	a0, PT_R1(sp)	/* leaf func: the location in current stack */
+1:
+
+	/* arg2: Get self return address */
+#ifdef CONFIG_DYNAMIC_FTRACE
+	PTR_L	a1, PT_R31(sp)
 #else
-	PTR_LA	a0, PT_R1(sp)	/* arg1: &AT -> a0 */
+	move	a1, ra
 #endif
-	jal	prepare_ftrace_return
+
+	/* arg3: Get frame pointer of current stack */
 #ifdef CONFIG_FRAME_POINTER
-	 move	a2, fp		/* arg3: frame pointer */
-#else
+	 move	a2, fp
+#else /* ! CONFIG_FRAME_POINTER */
 #ifdef CONFIG_64BIT
 	 PTR_LA	a2, PT_SIZE(sp)
 #else

@@ -161,6 +167,8 @@ NESTED(ftrace_graph_caller, PT_SIZE, ra)
 #endif
 #endif
 
+	jal	prepare_ftrace_return
+	 nop
 	MCOUNT_RESTORE_REGS
 	RETURN_BACK
 	END(ftrace_graph_caller)

Signature · 1 line

1.7.0.4

Ralf Baechle

Applied

2010-05-27 11:27 UTC #4 lore raw

Re: [PATCH 2/9] tracing: MIPS: mcount.S: cleanup the arguments of prepare_ftrace_return

Applied, thanks!

Ralf

Wu Zhangjin

author

2010-05-14 11:09 UTC #5 lore raw

PATCH v7 3/9 tracing: MIPS: mcount.S: cleanup of the comments

In reply to Wu Zhangjin

From: Wu Zhangjin <[email protected]>

This patch cleans up the comments.

Signed-off-by Wu Zhangjin <[email protected]>

---
 arch/mips/kernel/mcount.S |   11 ++++++-----
 1 files changed, 6 insertions(+), 5 deletions(-)

arch/mips/kernel/mcount.S +6 -5

index d4a00d2..9a029d4 100644
@@ -6,6 +6,7 @@
  * more details.
  *
  * Copyright (C) 2009 Lemote Inc. & DSLab, Lanzhou University, China
+ * Copyright (C) 2010 DSLab, Lanzhou University, China
  * Author: Wu Zhangjin <[email protected]>
  */

@@ -69,14 +70,14 @@ _mcount:
 
 	MCOUNT_SAVE_REGS
 #ifdef KBUILD_MCOUNT_RA_ADDRESS
-	PTR_S	t0, PT_R12(sp)	/* t0 saved the location of the return address(at) by -mmcount-ra-address */
+	PTR_S	t0, PT_R12(sp)	/* save location of parent's return address */
 #endif
 
-	move	a0, ra		/* arg1: next ip, selfaddr */
+	move	a0, ra		/* arg1: self return address */
 	.globl ftrace_call
 ftrace_call:
 	nop	/* a placeholder for the call to a real tracing function */
-	 move	a1, AT		/* arg2: the caller's next ip, parent */
+	 move	a1, AT		/* arg2: parent's return address */
 
 #ifdef CONFIG_FUNCTION_GRAPH_TRACER
 	.globl ftrace_graph_call

@@ -117,9 +118,9 @@ NESTED(_mcount, PT_SIZE, ra)
 static_trace:
 	MCOUNT_SAVE_REGS
 
-	move	a0, ra		/* arg1: next ip, selfaddr */
+	move	a0, ra		/* arg1: self return address */
 	jalr	t2		/* (1) call *ftrace_trace_function */
-	 move	a1, AT		/* arg2: the caller's next ip, parent */
+	 move	a1, AT		/* arg2: parent's return address */
 
 	MCOUNT_RESTORE_REGS
 	.globl ftrace_stub

Signature · 1 line

1.7.0.4

Ralf Baechle

Applied

2010-05-27 11:28 UTC #6 lore raw

Re: [PATCH 3/9] tracing: MIPS: mcount.S: cleanup of the comments

Applied, thanks!

Ralf

Wu Zhangjin

author

2010-05-14 11:10 UTC #7 lore raw

PATCH v7 4/9 tracing: MIPS: mcount.S: Fix the argument passing of the 32bit support with gcc 4.5

In reply to Wu Zhangjin

From: Wu Zhangjin <[email protected]>

As the doc[1] of gcc-4.5 shows, the -mmcount-ra-address uses register $12 to transfer the stack offset of the return address to the _mcount function. in 64bit kernel, $12 is t0, but in 32bit kernel, it is t4, so, we need to use $12 instead of t0 here to cover the 64bit and 32bit support.

[1] Gcc doc: MIPS Options
http://gcc.gnu.org/onlinedocs/gcc/MIPS-Options.html

Changes from old revision:
o Apply the feedback from David Daney:
define a macro MCOUNT_RA_ADDRESS_REG instead of the magic number $12

Signed-off-by Wu Zhangjin <[email protected]>

---
 arch/mips/kernel/mcount.S |   12 +++++++++---
 1 files changed, 9 insertions(+), 3 deletions(-)

arch/mips/kernel/mcount.S +9 -3

index 9a029d4..6bfcb7a 100644
@@ -57,6 +57,12 @@
 	 move ra, AT
 	.endm
 
+/*
+ * The -mmcount-ra-address option of gcc 4.5 uses register $12 to pass
+ * the location of the parent's return address.
+ */
+#define MCOUNT_RA_ADDRESS_REG	$12
+
 #ifdef CONFIG_DYNAMIC_FTRACE
 
 NESTED(ftrace_caller, PT_SIZE, ra)

@@ -70,7 +76,7 @@ _mcount:
 
 	MCOUNT_SAVE_REGS
 #ifdef KBUILD_MCOUNT_RA_ADDRESS
-	PTR_S	t0, PT_R12(sp)	/* save location of parent's return address */
+	PTR_S	MCOUNT_RA_ADDRESS_REG, PT_R12(sp)
 #endif
 
 	move	a0, ra		/* arg1: self return address */

@@ -142,9 +148,9 @@ NESTED(ftrace_graph_caller, PT_SIZE, ra)
 #ifdef CONFIG_DYNAMIC_FTRACE
 	PTR_L	a0, PT_R12(sp)
 #else
-	move	a0, t0
+	move	a0, MCOUNT_RA_ADDRESS_REG
 #endif
-	bnez	a0, 1f		/* non-leaf func: stored in t0 */
+	bnez	a0, 1f	/* non-leaf func: stored in MCOUNT_RA_ADDRESS_REG */
 	 nop
 #endif
 	PTR_LA	a0, PT_R1(sp)	/* leaf func: the location in current stack */

Signature · 1 line

1.7.0.4

Ralf Baechle

Applied

2010-05-27 11:28 UTC #8 lore raw

Re: [PATCH 4/9] tracing: MIPS: mcount.S: Fix the argument passing of the 32bit support with gcc 4.5

Applied, thanks!

Ralf

Wu Zhangjin

author

2010-05-14 11:11 UTC #9 lore raw

PATCH v7 5/9 tracing: MIPS: Fixup of the 32bit support with -mmcount-ra-address

In reply to Wu Zhangjin

From: Wu Zhangjin <[email protected]>

For 32bit kernel, the -mmcount-ra-address option of gcc 4.5 has introduced one more instruction before calling to _mcount, so we need to use a different "b 1f" for it.

Signed-off-by Wu Zhangjin <[email protected]>

---
 arch/mips/kernel/ftrace.c |   16 ++++++++++++++--
 1 files changed, 14 insertions(+), 2 deletions(-)

arch/mips/kernel/ftrace.c +14 -2

index e9e64e0..37aa767 100644
@@ -62,14 +62,26 @@ int ftrace_make_nop(struct module *mod,
 				return -EFAULT;
 		}
 
+#if defined(KBUILD_MCOUNT_RA_ADDRESS) && defined(CONFIG_32BIT)
+		/* lui v1, hi_16bit_of_mcount        --> b 1f (0x10000005)
+		 * addiu v1, v1, low_16bit_of_mcount
+		 * move at, ra
+		 * move $12, ra_address
+		 * jalr v1
+		 *  sub sp, sp, 8
+		 *                                  1: offset = 5 instructions
+		 */
+		new = 0x10000005;
+#else
 		/* lui v1, hi_16bit_of_mcount        --> b 1f (0x10000004)
 		 * addiu v1, v1, low_16bit_of_mcount
 		 * move at, ra
 		 * jalr v1
-		 * nop
-		 * 				     1f: (ip + 12)
+		 *  nop | move $12, ra_address | sub sp, sp, 8
+		 *                                  1: offset = 4 instructions
 		 */
 		new = 0x10000004;
+#endif
 	} else {
 		/* record/calculate it for ftrace_make_call */
 		if (jal_mcount == 0) {

Signature · 1 line

1.7.0.4

Ralf Baechle

Applied

2010-05-27 11:29 UTC #10 lore raw

Re: [PATCH 5/9] tracing: MIPS: Fixup of the 32bit support with -mmcount-ra-address

Applied, thanks!

Ralf

Wu Zhangjin

author

2010-05-14 11:11 UTC #11 lore raw

PATCH v7 6/9 tracing: MIPS: cleanup of the instructions

In reply to Wu Zhangjin

From: Wu Zhangjin <[email protected]>

This patch adds some cleanups of the instructions:
o use macro instead of magic numbers
o use macro instead of variables to reduce some overhead
o add a new macro for the jal instruction

Signed-off-by Wu Zhangjin <[email protected]>

---
 arch/mips/kernel/ftrace.c |   19 +++++++++++--------
 1 files changed, 11 insertions(+), 8 deletions(-)

arch/mips/kernel/ftrace.c +11 -8

index 37aa767..b1b8fec 100644
@@ -23,7 +23,10 @@
 #define jump_insn_encode(op_code, addr) \
 	((unsigned int)((op_code) | (((addr) >> 2) & ADDR_MASK)))
 
-static unsigned int ftrace_nop = 0x00000000;
+#define INSN_B_1F_4 0x10000004	/* b 1f; offset = 4 */
+#define INSN_B_1F_5 0x10000005	/* b 1f; offset = 5 */
+#define INSN_NOP 0x00000000	/* nop */
+#define INSN_JAL(addr)	jump_insn_encode(JAL, addr)
 
 static int ftrace_modify_code(unsigned long ip, unsigned int new_code)
 {

@@ -71,7 +74,7 @@ int ftrace_make_nop(struct module *mod,
 		 *  sub sp, sp, 8
 		 *                                  1: offset = 5 instructions
 		 */
-		new = 0x10000005;
+		new = INSN_B_1F_5;
 #else
 		/* lui v1, hi_16bit_of_mcount        --> b 1f (0x10000004)
 		 * addiu v1, v1, low_16bit_of_mcount

@@ -80,7 +83,7 @@ int ftrace_make_nop(struct module *mod,
 		 *  nop | move $12, ra_address | sub sp, sp, 8
 		 *                                  1: offset = 4 instructions
 		 */
-		new = 0x10000004;
+		new = INSN_B_1F_4;
 #endif
 	} else {
 		/* record/calculate it for ftrace_make_call */

@@ -88,13 +91,13 @@ int ftrace_make_nop(struct module *mod,
 			/* We can record it directly like this:
 			 *     jal_mcount = *(unsigned int *)ip;
 			 * Herein, jump over the first two nop instructions */
-			jal_mcount = jump_insn_encode(JAL, (MCOUNT_ADDR + 8));
+			jal_mcount = INSN_JAL(MCOUNT_ADDR + 8);
 		}
 
 		/* move at, ra
 		 * jalr v1		--> nop
 		 */
-		new = ftrace_nop;
+		new = INSN_NOP;
 	}
 	return ftrace_modify_code(ip, new);
 }

@@ -109,7 +112,7 @@ int ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
 	/* We just need to remove the "b ftrace_stub" at the fist time! */
 	if (modified == 0) {
 		modified = 1;
-		ftrace_modify_code(addr, ftrace_nop);
+		ftrace_modify_code(addr, INSN_NOP);
 	}
 	/* ip, module: 0xc0000000, kernel: 0x80000000 */
 	new = (ip & 0x40000000) ? lui_v1 : jal_mcount;

@@ -123,7 +126,7 @@ int ftrace_update_ftrace_func(ftrace_func_t func)
 {
 	unsigned int new;
 
-	new = jump_insn_encode(JAL, (unsigned long)func);
+	new = INSN_JAL((unsigned long)func);
 
 	return ftrace_modify_code(FTRACE_CALL_IP, new);
 }

@@ -155,7 +158,7 @@ int ftrace_enable_ftrace_graph_caller(void)
 
 int ftrace_disable_ftrace_graph_caller(void)
 {
-	return ftrace_modify_code(FTRACE_GRAPH_CALL_IP, ftrace_nop);
+	return ftrace_modify_code(FTRACE_GRAPH_CALL_IP, INSN_NOP);
 }
 
 #endif				/* !CONFIG_DYNAMIC_FTRACE */

Signature · 1 line

1.7.0.4

Ralf Baechle

Applied

2010-05-27 11:29 UTC #12 lore raw

Re: [PATCH 6/9] tracing: MIPS: cleanup of the instructions

Applied, thanks!

Ralf

Wu Zhangjin

author

2010-05-14 11:11 UTC #13 lore raw

PATCH v7 7/9 tracing: MIPS: Reduce the overhead of dynamic Function Tracer

In reply to Wu Zhangjin

From: Wu Zhangjin <[email protected]>

With the help of uasm, this patch encodes the instructions of dynamic Function Tracer in ftrace_dyn_arch_init() when initializing it.

As a result, we can remove the dynamic encoding of instructions in ftrace_make_nop()/call(), ftrace_enable_ftrace_graph_caller() and remove the macro jump_insn_encode() and at last, reduces the overhead of dynamic Function Tracer and also make the source code looks better.

Signed-off-by Wu Zhangjin <[email protected]>

---
 arch/mips/kernel/ftrace.c |   93 +++++++++++++++++++++++---------------------
 1 files changed, 49 insertions(+), 44 deletions(-)

arch/mips/kernel/ftrace.c +49 ~~-44~~

index b1b8fec..c4042ca 100644
@@ -2,7 +2,7 @@
  * Code for replacing ftrace calls with jumps.
  *
  * Copyright (C) 2007-2008 Steven Rostedt <[email protected]>
- * Copyright (C) 2009 DSLab, Lanzhou University, China
+ * Copyright (C) 2009, 2010 DSLab, Lanzhou University, China
  * Author: Wu Zhangjin <[email protected]>
  *
  * Thanks goes to Steven Rostedt for writing the original x86 version.

@@ -12,21 +12,46 @@
 #include <linux/init.h>
 #include <linux/ftrace.h>
 
-#include <asm/cacheflush.h>
 #include <asm/asm.h>
 #include <asm/asm-offsets.h>
+#include <asm/cacheflush.h>
+#include <asm/uasm.h>
 
 #ifdef CONFIG_DYNAMIC_FTRACE
 
 #define JAL 0x0c000000		/* jump & link: ip --> ra, jump to target */
 #define ADDR_MASK 0x03ffffff	/*  op_code|addr : 31...26|25 ....0 */
-#define jump_insn_encode(op_code, addr) \
-	((unsigned int)((op_code) | (((addr) >> 2) & ADDR_MASK)))
 
 #define INSN_B_1F_4 0x10000004	/* b 1f; offset = 4 */
 #define INSN_B_1F_5 0x10000005	/* b 1f; offset = 5 */
 #define INSN_NOP 0x00000000	/* nop */
-#define INSN_JAL(addr)	jump_insn_encode(JAL, addr)
+#define INSN_JAL(addr)	\
+	((unsigned int)(JAL | (((addr) >> 2) & ADDR_MASK)))
+
+static unsigned int insn_jal_ftrace_caller __read_mostly;
+static unsigned int insn_lui_v1_hi16_mcount __read_mostly;
+static unsigned int insn_j_ftrace_graph_caller __maybe_unused __read_mostly;
+
+static inline void ftrace_dyn_arch_init_insns(void)
+{
+	u32 *buf;
+	unsigned int v1;
+
+	/* lui v1, hi16_mcount */
+	v1 = 3;
+	buf = (u32 *)&insn_lui_v1_hi16_mcount;
+	UASM_i_LA_mostly(&buf, v1, MCOUNT_ADDR);
+
+	/* jal (ftrace_caller + 8), jump over the first two instruction */
+	buf = (u32 *)&insn_jal_ftrace_caller;
+	uasm_i_jal(&buf, (FTRACE_ADDR + 8));
+
+#ifdef CONFIG_FUNCTION_GRAPH_TRACER
+	/* j ftrace_graph_caller */
+	buf = (u32 *)&insn_j_ftrace_graph_caller;
+	uasm_i_j(&buf, (unsigned long)ftrace_graph_caller);
+#endif
+}
 
 static int ftrace_modify_code(unsigned long ip, unsigned int new_code)
 {

@@ -43,30 +68,20 @@ static int ftrace_modify_code(unsigned long ip, unsigned int new_code)
 	return 0;
 }
 
-static int lui_v1;
-static int jal_mcount;
-
 int ftrace_make_nop(struct module *mod,
 		    struct dyn_ftrace *rec, unsigned long addr)
 {
 	unsigned int new;
-	int faulted;
 	unsigned long ip = rec->ip;
 
-	/* We have compiled module with -mlong-calls, but compiled the kernel
-	 * without it, we need to cope with them respectively. */
+	/*
+	 * We have compiled module with -mlong-calls, but compiled the kernel
+	 * without it, we need to cope with them respectively.
+	 */
 	if (ip & 0x40000000) {
-		/* record it for ftrace_make_call */
-		if (lui_v1 == 0) {
-			/* lui_v1 = *(unsigned int *)ip; */
-			safe_load_code(lui_v1, ip, faulted);
-
-			if (unlikely(faulted))
-				return -EFAULT;
-		}
-
 #if defined(KBUILD_MCOUNT_RA_ADDRESS) && defined(CONFIG_32BIT)
-		/* lui v1, hi_16bit_of_mcount        --> b 1f (0x10000005)
+		/*
+		 * lui v1, hi_16bit_of_mcount        --> b 1f (0x10000005)
 		 * addiu v1, v1, low_16bit_of_mcount
 		 * move at, ra
 		 * move $12, ra_address

@@ -76,7 +91,8 @@ int ftrace_make_nop(struct module *mod,
 		 */
 		new = INSN_B_1F_5;
 #else
-		/* lui v1, hi_16bit_of_mcount        --> b 1f (0x10000004)
+		/*
+		 * lui v1, hi_16bit_of_mcount        --> b 1f (0x10000004)
 		 * addiu v1, v1, low_16bit_of_mcount
 		 * move at, ra
 		 * jalr v1

@@ -86,36 +102,22 @@ int ftrace_make_nop(struct module *mod,
 		new = INSN_B_1F_4;
 #endif
 	} else {
-		/* record/calculate it for ftrace_make_call */
-		if (jal_mcount == 0) {
-			/* We can record it directly like this:
-			 *     jal_mcount = *(unsigned int *)ip;
-			 * Herein, jump over the first two nop instructions */
-			jal_mcount = INSN_JAL(MCOUNT_ADDR + 8);
-		}
-
-		/* move at, ra
-		 * jalr v1		--> nop
+		/*
+		 * move at, ra
+		 * jal _mcount		--> nop
 		 */
 		new = INSN_NOP;
 	}
 	return ftrace_modify_code(ip, new);
 }
 
-static int modified;	/* initialized as 0 by default */
-
 int ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
 {
 	unsigned int new;
 	unsigned long ip = rec->ip;
 
-	/* We just need to remove the "b ftrace_stub" at the fist time! */
-	if (modified == 0) {
-		modified = 1;
-		ftrace_modify_code(addr, INSN_NOP);
-	}
 	/* ip, module: 0xc0000000, kernel: 0x80000000 */
-	new = (ip & 0x40000000) ? lui_v1 : jal_mcount;
+	new = (ip & 0x40000000) ? insn_lui_v1_hi16_mcount : insn_jal_ftrace_caller;
 
 	return ftrace_modify_code(ip, new);
 }

@@ -133,6 +135,12 @@ int ftrace_update_ftrace_func(ftrace_func_t func)
 
 int __init ftrace_dyn_arch_init(void *data)
 {
+	/* Encode the instructions when booting */
+	ftrace_dyn_arch_init_insns();
+
+	/* Remove "b ftrace_stub" to ensure ftrace_caller() is executed */
+	ftrace_modify_code(MCOUNT_ADDR, INSN_NOP);
+
 	/* The return code is retured via data */
 	*(unsigned long *)data = 0;

@@ -145,15 +153,12 @@ int __init ftrace_dyn_arch_init(void *data)
 #ifdef CONFIG_DYNAMIC_FTRACE
 
 extern void ftrace_graph_call(void);
-#define JMP	0x08000000	/* jump to target directly */
-#define CALL_FTRACE_GRAPH_CALLER \
-	jump_insn_encode(JMP, (unsigned long)(&ftrace_graph_caller))
 #define FTRACE_GRAPH_CALL_IP	((unsigned long)(&ftrace_graph_call))
 
 int ftrace_enable_ftrace_graph_caller(void)
 {
 	return ftrace_modify_code(FTRACE_GRAPH_CALL_IP,
-				  CALL_FTRACE_GRAPH_CALLER);
+			insn_j_ftrace_graph_caller);
 }
 
 int ftrace_disable_ftrace_graph_caller(void)

Signature · 1 line

1.7.0.4

David Daney

2010-05-14 17:15 UTC #14 lore raw

Re: [PATCH 7/9] tracing: MIPS: Reduce the overhead of dynamic Function Tracer

Quoted 5 lines · Wu Zhangjin

From: Wu Zhangjin<[email protected]>
With the help of uasm, this patch encodes the instructions of dynamic Function Tracer in ftrace_dyn_arch_init() when initializing it.

[...]

Quoted 2 lines

+#include<asm/uasm.h>

All of uasm is _cpuinit, I haven't checked everything, but are you sure you aren't calling if from non-_cpuinit code?

David Daney

Wu Zhangjin

author

2010-05-15 02:44 UTC #15 lore raw

Re: [PATCH 7/9] tracing: MIPS: Reduce the overhead of dynamic Function Tracer

Quoted 6 lines · David Daney

Quoted 5 lines · Wu Zhangjin
From: Wu Zhangjin<[email protected]>
With the help of uasm, this patch encodes the instructions of dynamic Function Tracer in ftrace_dyn_arch_init() when initializing it.

Quoted 6 lines

[...]
Quoted 2 lines
+#include<asm/uasm.h>
All of uasm is _cpuinit, I haven't checked everything, but are you sure you aren't calling if from non-_cpuinit code?

The calling tree looks like this:

start_kernel() // __init
--> ftrace_init() // __init

        --> ftrace_dyn_arch_init() // __init
              --> ftrace_dyn_arch_init_insns() // inline
	            --> uasm_*/UASM_*

Nobody else will call uasm_*/UASM_* in this patch, I have checked the uasm usage in arch/mips/kernel/traps.c. Seems the usam functions are also called in the __init *set_except_vector(). so, it will also be safe in this patch, is it?

Thanks & Regards,
Wu Zhangjin

David Daney

2010-05-16 17:04 UTC #16 lore raw

Re: [PATCH 7/9] tracing: MIPS: Reduce the overhead of dynamic Function Tracer

Quoted 22 lines · Wu Zhangjin

Quoted 8 lines · David Daney
Quoted 6 lines · Wu Zhangjin
From: Wu Zhangjin<[email protected]>
With the help of uasm, this patch encodes the instructions of dynamic Function Tracer in ftrace_dyn_arch_init() when initializing it.
Quoted 8 lines
[...]
Quoted 3 lines
+#include<asm/uasm.h>
All of uasm is _cpuinit, I haven't checked everything, but are you sure you aren't calling if from non-_cpuinit code?
The calling tree looks like this:
start_kernel() // __init
--> ftrace_init() // __init

Quoted 10 lines

         -->  ftrace_dyn_arch_init() // __init
               -->  ftrace_dyn_arch_init_insns() // inline
	            -->  uasm_*/UASM_*
Nobody else will call uasm_*/UASM_* in this patch, I have checked the uasm usage in arch/mips/kernel/traps.c. Seems the usam functions are also called in the __init *set_except_vector(). so, it will also be safe in this patch, is it?

Yes, that seems good to me. I just wanted to make sure that this wasn't being called from non-init code.

David Daney

Ralf Baechle

2010-05-27 11:22 UTC #17 lore raw

Re: [PATCH 7/9] tracing: MIPS: Reduce the overhead of dynamic Function Tracer

Quoted 2 lines · David Daney

Yes, that seems good to me. I just wanted to make sure that this wasn't being called from non-init code.

Worst case there's always CONFIG_DEBUG_SECTION_MISMATCH.

Ralf

Ralf Baechle

Applied

2010-05-27 11:29 UTC #18 lore raw

Re: [PATCH 7/9] tracing: MIPS: Reduce the overhead of dynamic Function Tracer

Applied, thanks!

Ralf

Wu Zhangjin

author

2010-05-14 11:12 UTC #19 lore raw

PATCH v7 8/9 tracing: MIPS: cleanup of function graph tracer

In reply to Wu Zhangjin

From: Wu Zhangjin <[email protected]>

This patch cleans up the comments and ftrace_get_parent_addr() of function graph tracer.

Signed-off-by Wu Zhangjin <[email protected]>

---
 arch/mips/kernel/ftrace.c |   48 ++++++++++++++++++++++++--------------------
 1 files changed, 26 insertions(+), 22 deletions(-)

arch/mips/kernel/ftrace.c +26 ~~-22~~

index c4042ca..628e90b 100644
@@ -146,7 +146,7 @@ int __init ftrace_dyn_arch_init(void *data)
 
 	return 0;
 }
-#endif				/* CONFIG_DYNAMIC_FTRACE */
+#endif	/* CONFIG_DYNAMIC_FTRACE */
 
 #ifdef CONFIG_FUNCTION_GRAPH_TRACER

@@ -166,9 +166,10 @@ int ftrace_disable_ftrace_graph_caller(void)
 	return ftrace_modify_code(FTRACE_GRAPH_CALL_IP, INSN_NOP);
 }
 
-#endif				/* !CONFIG_DYNAMIC_FTRACE */
+#endif	/* CONFIG_DYNAMIC_FTRACE */
 
 #ifndef KBUILD_MCOUNT_RA_ADDRESS
+
 #define S_RA_SP	(0xafbf << 16)	/* s{d,w} ra, offset(sp) */
 #define S_R_SP	(0xafb0 << 16)  /* s{d,w} R, offset(sp) */
 #define OFFSET_MASK	0xffff	/* stack offset range: 0 ~ PT_SIZE */

@@ -182,17 +183,17 @@ unsigned long ftrace_get_parent_addr(unsigned long self_addr,
 	unsigned int code;
 	int faulted;
 
-	/* in module or kernel? */
-	if (self_addr & 0x40000000) {
-		/* module: move to the instruction "lui v1, HI_16BIT_OF_MCOUNT" */
-		ip = self_addr - 20;
-	} else {
-		/* kernel: move to the instruction "move ra, at" */
-		ip = self_addr - 12;
-	}
+	/*
+	 * For module, move the ip from calling site of mcount to the
+	 * instruction "lui v1, hi_16bit_of_mcount"(offset is 20), but for
+	 * kernel, move to the instruction "move ra, at"(offset is 12)
+	 */
+	ip = self_addr - ((self_addr & 0x40000000) ? 20 : 12);
 
-	/* search the text until finding the non-store instruction or "s{d,w}
-	 * ra, offset(sp)" instruction */
+	/*
+	 * search the text until finding the non-store instruction or "s{d,w}
+	 * ra, offset(sp)" instruction
+	 */
 	do {
 		ip -= 4;

@@ -201,10 +202,11 @@ unsigned long ftrace_get_parent_addr(unsigned long self_addr,
 
 		if (unlikely(faulted))
 			return 0;
-
-		/* If we hit the non-store instruction before finding where the
+		/*
+		 * If we hit the non-store instruction before finding where the
 		 * ra is stored, then this is a leaf function and it does not
-		 * store the ra on the stack. */
+		 * store the ra on the stack
+		 */
 		if ((code & S_R_SP) != S_R_SP)
 			return parent_addr;

@@ -222,7 +224,7 @@ unsigned long ftrace_get_parent_addr(unsigned long self_addr,
 	return 0;
 }
 
-#endif
+#endif	/* !KBUILD_MCOUNT_RA_ADDRESS */
 
 /*
  * Hook the return address and push it in the stack of return addrs

@@ -240,7 +242,8 @@ void prepare_ftrace_return(unsigned long *parent, unsigned long self_addr,
 	if (unlikely(atomic_read(&current->tracing_graph_pause)))
 		return;
 
-	/* "parent" is the stack address saved the return address of the caller
+	/*
+	 * "parent" is the stack address saved the return address of the caller
 	 * of _mcount.
 	 *
 	 * if the gcc < 4.5, a leaf function does not save the return address

@@ -262,10 +265,11 @@ void prepare_ftrace_return(unsigned long *parent, unsigned long self_addr,
 		goto out;
 #ifndef KBUILD_MCOUNT_RA_ADDRESS
 	parent = (unsigned long *)ftrace_get_parent_addr(self_addr, old,
-							 (unsigned long)parent,
-							 fp);
-	/* If fails when getting the stack address of the non-leaf function's
-	 * ra, stop function graph tracer and return */
+			(unsigned long)parent, fp);
+	/*
+	 * If fails when getting the stack address of the non-leaf function's
+	 * ra, stop function graph tracer and return
+	 */
 	if (parent == 0)
 		goto out;
 #endif

@@ -292,4 +296,4 @@ out:
 	ftrace_graph_stop();
 	WARN_ON(1);
 }
-#endif				/* CONFIG_FUNCTION_GRAPH_TRACER */
+#endif	/* CONFIG_FUNCTION_GRAPH_TRACER */

Signature · 1 line

1.7.0.4

Ralf Baechle

Applied

2010-05-27 11:30 UTC #20 lore raw

Re: [PATCH 8/9] tracing: MIPS: cleanup of function graph tracer

Applied, thanks!

Ralf

Wu Zhangjin

author

2010-05-14 11:12 UTC #21 lore raw

PATCH v7 9/9 tracing: MIPS: cleanup of the address space checking

In reply to Wu Zhangjin

From: Wu Zhangjin <[email protected]>

This patch adds an inline function in_module() to check which space the instruction pointer in, kernel space or module space.

Note: This will not work when the kernel space and module space are the same. If they are the same, we need to modify scripts/recordmcount.pl, ftrace_make_nop/call() and the other related parts to ensure the enabling/disabling of the calling site to _mcount is right for both kernel and module.

Signed-off-by Wu Zhangjin <[email protected]>

---
 arch/mips/kernel/ftrace.c |   22 +++++++++++++++++++---
 1 files changed, 19 insertions(+), 3 deletions(-)

arch/mips/kernel/ftrace.c +19 -3

index 628e90b..5a84a1f 100644
@@ -17,6 +17,22 @@
 #include <asm/cacheflush.h>
 #include <asm/uasm.h>
 
+/*
+ * If the Instruction Pointer is in module space (0xc0000000), return true;
+ * otherwise, it is in kernel space (0x80000000), return false.
+ *
+ * FIXME: This will not work when the kernel space and module space are the
+ * same. If they are the same, we need to modify scripts/recordmcount.pl,
+ * ftrace_make_nop/call() and the other related parts to ensure the
+ * enabling/disabling of the calling site to _mcount is right for both kernel
+ * and module.
+ */
+
+static inline int in_module(unsigned long ip)
+{
+	return ip & 0x40000000;
+}
+
 #ifdef CONFIG_DYNAMIC_FTRACE
 
 #define JAL 0x0c000000		/* jump & link: ip --> ra, jump to target */

@@ -78,7 +94,7 @@ int ftrace_make_nop(struct module *mod,
 	 * We have compiled module with -mlong-calls, but compiled the kernel
 	 * without it, we need to cope with them respectively.
 	 */
-	if (ip & 0x40000000) {
+	if (in_module(ip)) {
 #if defined(KBUILD_MCOUNT_RA_ADDRESS) && defined(CONFIG_32BIT)
 		/*
 		 * lui v1, hi_16bit_of_mcount        --> b 1f (0x10000005)

@@ -117,7 +133,7 @@ int ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
 	unsigned long ip = rec->ip;
 
 	/* ip, module: 0xc0000000, kernel: 0x80000000 */
-	new = (ip & 0x40000000) ? insn_lui_v1_hi16_mcount : insn_jal_ftrace_caller;
+	new = in_module(ip) ? insn_lui_v1_hi16_mcount : insn_jal_ftrace_caller;
 
 	return ftrace_modify_code(ip, new);
 }

@@ -188,7 +204,7 @@ unsigned long ftrace_get_parent_addr(unsigned long self_addr,
 	 * instruction "lui v1, hi_16bit_of_mcount"(offset is 20), but for
 	 * kernel, move to the instruction "move ra, at"(offset is 12)
 	 */
-	ip = self_addr - ((self_addr & 0x40000000) ? 20 : 12);
+	ip = self_addr - (in_module(self_addr) ? 20 : 12);
 
 	/*
 	 * search the text until finding the non-store instruction or "s{d,w}

Signature · 1 line

1.7.0.4

Ralf Baechle

Applied

2010-05-27 11:30 UTC #22 lore raw

Re: [PATCH 9/9] tracing: MIPS: cleanup of the address space checking

Applied, thanks!

Ralf