[PATCH v6 0/9] tracing: MIPS: add misc fixups and cleanups

[PATCH v6 0/9] tracing: MIPS: add misc fixups and cleanups

[Wu Zhangjin]

From: Wu Zhangjin <wuzhangjin@gmail.com>

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: Fixup 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.

----------------

Changes from v5:

  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: Fixup of the 32bit support with gcc 4.5
  tracing: MIPS: mcount.S: cleanup the arguments of
    prepare_ftrace_return
  tracing: MIPS: mcount.S: cleanup of the comments
  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 |  179 +++++++++++++++++++++++++++------------------
 arch/mips/kernel/mcount.S |   49 +++++++-----
 2 files changed, 135 insertions(+), 93 deletions(-)

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

[Wu Zhangjin]

From: Wu Zhangjin <wuzhangjin@gmail.com>

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

Signed-off-by: Wu Zhangjin <wuzhangjin@gmail.com>
---
 arch/mips/kernel/mcount.S |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/arch/mips/kernel/mcount.S b/arch/mips/kernel/mcount.S
index 6851fc9..e256bf9 100644
--- a/arch/mips/kernel/mcount.S
+++ b/arch/mips/kernel/mcount.S
@@ -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)
-- 
1.7.0.4

[PATCH 2/9] tracing: MIPS: mcount.S: Fixup of the 32bit support with gcc 4.5

[Wu Zhangjin]

From: Wu Zhangjin <wuzhangjin@gmail.com>

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

Signed-off-by: Wu Zhangjin <wuzhangjin@gmail.com>
---
 arch/mips/kernel/mcount.S |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/arch/mips/kernel/mcount.S b/arch/mips/kernel/mcount.S
index e256bf9..92d1540 100644
--- a/arch/mips/kernel/mcount.S
+++ b/arch/mips/kernel/mcount.S
@@ -69,7 +69,7 @@ _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	$12, PT_R12(sp)	/* $12 saved the location of the return address(at) by -mmcount-ra-address */
 #endif
 
 	move	a0, ra		/* arg1: next ip, selfaddr */
@@ -135,7 +135,7 @@ 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 */
+	PTR_L	$12, PT_R12(sp)	/* load the original $12 from the stack */
 #endif
 #else
 	MCOUNT_SAVE_REGS
@@ -143,10 +143,10 @@ NESTED(ftrace_graph_caller, PT_SIZE, ra)
 #endif
 
 #ifdef KBUILD_MCOUNT_RA_ADDRESS
-	bnez	t0, 1f		/* non-leaf func: t0 saved the location of the return address */
+	bnez	$12, 1f		/* non-leaf func: $12 saved the location of the return address */
 	 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 */
+	PTR_LA	$12, PT_R1(sp)	/* leaf func: get the location of at(old ra) from our own stack */
+1:	move	a0, $12		/* arg1: the location of the return address */
 #else
 	PTR_LA	a0, PT_R1(sp)	/* arg1: &AT -> a0 */
 #endif
-- 
1.7.0.4

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

[Wu Zhangjin]

From: Wu Zhangjin <wuzhangjin@gmail.com>

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

Signed-off-by: Wu Zhangjin <wuzhangjin@gmail.com>
---
 arch/mips/kernel/mcount.S |   36 ++++++++++++++++++++++--------------
 1 files changed, 22 insertions(+), 14 deletions(-)

diff --git a/arch/mips/kernel/mcount.S b/arch/mips/kernel/mcount.S
index 92d1540..991cbdf 100644
--- a/arch/mips/kernel/mcount.S
+++ b/arch/mips/kernel/mcount.S
@@ -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	$12, PT_R12(sp)	/* load the original $12 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	$12, 1f		/* non-leaf func: $12 saved the location of the return address */
+#ifdef CONFIG_DYNAMIC_FTRACE
+	PTR_L	a0, PT_R12(sp)
+#else
+	move	a0, $12
+#endif
+	bnez	a0, 1f		/* non-leaf func: stored in $12 */
 	 nop
-	PTR_LA	$12, PT_R1(sp)	/* leaf func: get the location of at(old ra) from our own stack */
-1:	move	a0, $12		/* 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)
-- 
1.7.0.4

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

[Wu Zhangjin]

From: Wu Zhangjin <wuzhangjin@gmail.com>

This patch cleans the comments up.

Signed-off-by: Wu Zhangjin <wuzhangjin@gmail.com>
---
 arch/mips/kernel/mcount.S |   11 ++++++-----
 1 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/arch/mips/kernel/mcount.S b/arch/mips/kernel/mcount.S
index 991cbdf..84303bf 100644
--- a/arch/mips/kernel/mcount.S
+++ b/arch/mips/kernel/mcount.S
@@ -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 <wuzhangjin@gmail.com>
  */
 
@@ -69,14 +70,14 @@ _mcount:
 
 	MCOUNT_SAVE_REGS
 #ifdef KBUILD_MCOUNT_RA_ADDRESS
-	PTR_S	$12, PT_R12(sp)	/* $12 saved the location of the return address(at) by -mmcount-ra-address */
+	PTR_S	$12, 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
-- 
1.7.0.4

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

[Wu Zhangjin]

From: Wu Zhangjin <wuzhangjin@gmail.com>

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 <wuzhangjin@gmail.com>
---
 arch/mips/kernel/ftrace.c |   16 ++++++++++++++--
 1 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/arch/mips/kernel/ftrace.c b/arch/mips/kernel/ftrace.c
index e9e64e0..37aa767 100644
--- a/arch/mips/kernel/ftrace.c
+++ b/arch/mips/kernel/ftrace.c
@@ -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) {
-- 
1.7.0.4

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

[Wu Zhangjin]

From: Wu Zhangjin <wuzhangjin@gmail.com>

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 <wuzhangjin@gmail.com>
---
 arch/mips/kernel/ftrace.c |   19 +++++++++++--------
 1 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/arch/mips/kernel/ftrace.c b/arch/mips/kernel/ftrace.c
index 37aa767..b1b8fec 100644
--- a/arch/mips/kernel/ftrace.c
+++ b/arch/mips/kernel/ftrace.c
@@ -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 */
-- 
1.7.0.4

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

[Wu Zhangjin]

From: Wu Zhangjin <wuzhangjin@gmail.com>

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 <wuzhangjin@gmail.com>
---
 arch/mips/kernel/ftrace.c |   93 +++++++++++++++++++++++---------------------
 1 files changed, 49 insertions(+), 44 deletions(-)

diff --git a/arch/mips/kernel/ftrace.c b/arch/mips/kernel/ftrace.c
index b1b8fec..c4042ca 100644
--- a/arch/mips/kernel/ftrace.c
+++ b/arch/mips/kernel/ftrace.c
@@ -2,7 +2,7 @@
  * Code for replacing ftrace calls with jumps.
  *
  * Copyright (C) 2007-2008 Steven Rostedt <srostedt@redhat.com>
- * Copyright (C) 2009 DSLab, Lanzhou University, China
+ * Copyright (C) 2009, 2010 DSLab, Lanzhou University, China
  * Author: Wu Zhangjin <wuzhangjin@gmail.com>
  *
  * 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)
-- 
1.7.0.4

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

[Wu Zhangjin]

From: Wu Zhangjin <wuzhangjin@gmail.com>

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

Signed-off-by: Wu Zhangjin <wuzhangjin@gmail.com>
---
 arch/mips/kernel/ftrace.c |   48 ++++++++++++++++++++++++--------------------
 1 files changed, 26 insertions(+), 22 deletions(-)

diff --git a/arch/mips/kernel/ftrace.c b/arch/mips/kernel/ftrace.c
index c4042ca..628e90b 100644
--- a/arch/mips/kernel/ftrace.c
+++ b/arch/mips/kernel/ftrace.c
@@ -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 */
-- 
1.7.0.4

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

[Wu Zhangjin]

From: Wu Zhangjin <wuzhangjin@gmail.com>

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

Note: This may not work when the kernel is compiled with -msym32.

Signed-off-by: Wu Zhangjin <wuzhangjin@gmail.com>
---
 arch/mips/kernel/ftrace.c |   17 ++++++++++++++---
 1 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/arch/mips/kernel/ftrace.c b/arch/mips/kernel/ftrace.c
index 628e90b..37f15b6 100644
--- a/arch/mips/kernel/ftrace.c
+++ b/arch/mips/kernel/ftrace.c
@@ -17,6 +17,17 @@
 #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 may not work when the kernel is compiled with -msym32.
+ */
+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 +89,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 +128,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 +199,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}
-- 
1.7.0.4

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

[David Daney]

On 05/12/2010 06:23 AM, Wu Zhangjin wrote:
> From: Wu Zhangjin<wuzhangjin@gmail.com>
>
> This patch adds an inline function in_module() to check which space the
> instruction pointer in, kernel space or module space.
>
> Note: This may not work when the kernel is compiled with -msym32.
>

The kernel is always compiled with -msym32, so the patch is a bit pointless.



> Signed-off-by: Wu Zhangjin<wuzhangjin@gmail.com>
> ---
>   arch/mips/kernel/ftrace.c |   17 ++++++++++++++---
>   1 files changed, 14 insertions(+), 3 deletions(-)
>
> diff --git a/arch/mips/kernel/ftrace.c b/arch/mips/kernel/ftrace.c
> index 628e90b..37f15b6 100644
> --- a/arch/mips/kernel/ftrace.c
> +++ b/arch/mips/kernel/ftrace.c
> @@ -17,6 +17,17 @@
>   #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 may not work when the kernel is compiled with -msym32.
> + */
> +static inline int in_module(unsigned long ip)
> +{
> +	return ip&  0x40000000;
> +}
> +

How about (untested):


static inline int in_module(unsigned long ip)
{
	return ip < _text || ip > _etext;
}


But why do we even care?  Can't we just probe the function prologue and 
determine from that what needs to be done?

David Daney

>   #ifdef CONFIG_DYNAMIC_FTRACE
>
>   #define JAL 0x0c000000		/* jump&  link: ip -->  ra, jump to target */
> @@ -78,7 +89,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 +128,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 +199,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}

Re: [PATCH 2/9] tracing: MIPS: mcount.S: Fixup of the 32bit support with gcc 4.5

[David Daney]

On 05/12/2010 06:23 AM, Wu Zhangjin wrote:
> From: Wu Zhangjin<wuzhangjin@gmail.com>
>
> 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
>
> Signed-off-by: Wu Zhangjin<wuzhangjin@gmail.com>

Would it be better to do?:

#define MCOUNT_RA_ADDRESS_REG $12

s/t0/MCOUNT_RA_ADDRESS_REG/g

David Daney

> ---
>   arch/mips/kernel/mcount.S |   10 +++++-----
>   1 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/arch/mips/kernel/mcount.S b/arch/mips/kernel/mcount.S
> index e256bf9..92d1540 100644
> --- a/arch/mips/kernel/mcount.S
> +++ b/arch/mips/kernel/mcount.S
> @@ -69,7 +69,7 @@ _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	$12, PT_R12(sp)	/* $12 saved the location of the return address(at) by -mmcount-ra-address */
>   #endif
>
>   	move	a0, ra		/* arg1: next ip, selfaddr */
> @@ -135,7 +135,7 @@ 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 */
> +	PTR_L	$12, PT_R12(sp)	/* load the original $12 from the stack */
>   #endif
>   #else
>   	MCOUNT_SAVE_REGS
> @@ -143,10 +143,10 @@ NESTED(ftrace_graph_caller, PT_SIZE, ra)
>   #endif
>
>   #ifdef KBUILD_MCOUNT_RA_ADDRESS
> -	bnez	t0, 1f		/* non-leaf func: t0 saved the location of the return address */
> +	bnez	$12, 1f		/* non-leaf func: $12 saved the location of the return address */
>   	 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 */
> +	PTR_LA	$12, PT_R1(sp)	/* leaf func: get the location of at(old ra) from our own stack */
> +1:	move	a0, $12		/* arg1: the location of the return address */
>   #else
>   	PTR_LA	a0, PT_R1(sp)	/* arg1:&AT ->  a0 */
>   #endif

Re: [PATCH 2/9] tracing: MIPS: mcount.S: Fixup of the 32bit support with gcc 4.5

[Wu Zhangjin]

On Wed, 2010-05-12 at 10:16 -0700, David Daney wrote:
> On 05/12/2010 06:23 AM, Wu Zhangjin wrote:
> > From: Wu Zhangjin<wuzhangjin@gmail.com>
> >
> > 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
> >
> > Signed-off-by: Wu Zhangjin<wuzhangjin@gmail.com>
> 
> Would it be better to do?:
> 
> #define MCOUNT_RA_ADDRESS_REG $12
> 
> s/t0/MCOUNT_RA_ADDRESS_REG/g

Good idea, will change it later.

Regards,
	Wu Zhangjin

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

[Wu Zhangjin]

On Wed, 2010-05-12 at 10:13 -0700, David Daney wrote:
> On 05/12/2010 06:23 AM, Wu Zhangjin wrote:
> > From: Wu Zhangjin<wuzhangjin@gmail.com>
> >
> > This patch adds an inline function in_module() to check which space the
> > instruction pointer in, kernel space or module space.
> >
> > Note: This may not work when the kernel is compiled with -msym32.
> >
> 
> The kernel is always compiled with -msym32, so the patch is a bit pointless.
> 
> 

In reality, with -msym32, it works well on my machine, but seems you and
some other people told me that it may not work when the kernel and
module space are the same with -msym32 and some other options. perhaps I
need to change the comments to just:

Note: This will not work when the kernel and module space are the same.

If the kernel and module space are the same, We just need to modify the
recording of the calling site to _mcount in scripts/recordmcount.pl and
tune the related address handling in
ftrace_make_nop()/ftrace_make_call().

But I have no such situation to test, how can I simply let the module
has the same address space as the kernel. without -mlong-calls? and
should we change arch/mips/kernel/vmlinux.lds.S and
arch/mips/kernel/module.c?

If the module space and kernel space are the same, we may remove the
long call from the module space to kernel space to speedup the function
call.

> >
> > diff --git a/arch/mips/kernel/ftrace.c b/arch/mips/kernel/ftrace.c
> > index 628e90b..37f15b6 100644
> > --- a/arch/mips/kernel/ftrace.c
> > +++ b/arch/mips/kernel/ftrace.c
> > @@ -17,6 +17,17 @@
> >   #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 may not work when the kernel is compiled with -msym32.
> > + */
> > +static inline int in_module(unsigned long ip)
> > +{
> > +	return ip&  0x40000000;
> > +}
> > +
> 
> How about (untested):
> 
> 
> static inline int in_module(unsigned long ip)
> {
> 	return ip < _text || ip > _etext;
> }
> 

It may work, thanks!

> 
> But why do we even care?  Can't we just probe the function prologue and 
> determine from that what needs to be done?

Yes, it should work via checking the instructions in the memory before
the ip but I think a lighter solution is better.

Thanks & Regards,
	Wu Zhangjin

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

[Ralf Baechle]

On Wed, May 12, 2010 at 10:13:01AM -0700, David Daney wrote:

> The kernel is always compiled with -msym32, so the patch is a bit pointless.

Not quite true.  Some systems only have enough memory for the exception
vectors in the low 512MB of physical address space, so these can't use
an -msym32 kernel.

My general impression is that hardware designers "design" address maps by
throwing darts over their shoulder after a few pints ;-)

  Ralf

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

[David Daney]

On 05/13/2010 09:13 AM, Ralf Baechle wrote:
> On Wed, May 12, 2010 at 10:13:01AM -0700, David Daney wrote:
>
>> The kernel is always compiled with -msym32, so the patch is a bit pointless.
>
> Not quite true.  Some systems only have enough memory for the exception
> vectors in the low 512MB of physical address space, so these can't use
> an -msym32 kernel.
>
> My general impression is that hardware designers "design" address maps by
> throwing darts over their shoulder after a few pints ;-)
>

Well it was mostly a rhetorical point.  Of course -msym32 is not 
universal, but it is more common than not I would say.  Because of that, 
we should take a few more minutes and figure out how to make ftrace work 
well with it.

David Daney

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

[Wu Zhangjin]

From: Wu Zhangjin <wuzhangjin@gmail.com>

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 <wuzhangjin@gmail.com>
---
 arch/mips/kernel/ftrace.c |   22 +++++++++++++++++++---
 1 files changed, 19 insertions(+), 3 deletions(-)

diff --git a/arch/mips/kernel/ftrace.c b/arch/mips/kernel/ftrace.c
index 628e90b..5a84a1f 100644
--- a/arch/mips/kernel/ftrace.c
+++ b/arch/mips/kernel/ftrace.c
@@ -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}
-- 
1.7.0.4

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

[Ralf Baechle]

Applied, thanks!

  Ralf