tracing: MIPS: add misc fixups and cleanups
v6 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: 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 v6 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.
--- arch/mips/kernel/mcount.S | 2 -- 1 files changed, 0 insertions(+), 2 deletions(-)
arch/mips/kernel/mcount.S +0 -2
@@ -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
PATCH v6 2/9 tracing: MIPS: mcount.S: Fixup of the 32bit support with gcc 4.5
In reply to Wu ZhangjinFrom: 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
--- arch/mips/kernel/mcount.S | 10 +++++----- 1 files changed, 5 insertions(+), 5 deletions(-)
arch/mips/kernel/mcount.S +5 -5
@@ -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
Signature · 1 line
1.7.0.4
Re: [PATCH 2/9] tracing: MIPS: mcount.S: Fixup of the 32bit support with gcc 4.5
Quoted 8 lines · 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
_mcountfunction. 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.
Quoted 4 lines
[1] Gcc doc: MIPS Options
http://gcc.gnu.org/onlinedocs/gcc/MIPS-Options.htmlSigned-off-by Wu Zhangjin<[email protected]>
Would it be better to do?:
#define MCOUNT_RA_ADDRESS_REG $12
s/t0/MCOUNT_RA_ADDRESS_REG/g
David Daney
Quoted 40 lines
--- arch/mips/kernel/mcount.S | 10 +++++----- 1 files changed, 5 insertions(+), 5 deletions(-)arch/mips/kernel/mcount.S +5
-5@@ -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
Quoted 12 lines · David Daney
Quoted 11 lines · 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
_mcountfunction. 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
Quoted 7 lines
Quoted 1 line
Signed-off-by Wu Zhangjin<[email protected]>Would it be better to do?:
#define
MCOUNT_RA_ADDRESS_REG$12s/t0/MCOUNT_RA_ADDRESS_REG/g
Good idea, will change it later.
Regards,
Wu Zhangjin
PATCH v6 3/9 tracing: MIPS: mcount.S: cleanup the arguments of prepare_ftrace_return
In reply to Wu ZhangjinFrom: Wu Zhangjin <[email protected]>
The old arguments handling for prepare_ftrace_return is awlful, this patch cleans it up.
--- arch/mips/kernel/mcount.S | 36 ++++++++++++++++++++++-------------- 1 files changed, 22 insertions(+), 14 deletions(-)
arch/mips/kernel/mcount.S +22 -14
@@ -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)
Signature · 1 line
1.7.0.4
PATCH v6 4/9 tracing: MIPS: mcount.S: cleanup of the comments
In reply to Wu ZhangjinFrom: Wu Zhangjin <[email protected]>
This patch cleans the comments up.
--- arch/mips/kernel/mcount.S | 11 ++++++----- 1 files changed, 6 insertions(+), 5 deletions(-)
arch/mips/kernel/mcount.S +6 -5
@@ -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 $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
Signature · 1 line
1.7.0.4
PATCH v6 5/9 tracing: MIPS: Fixup of the 32bit support with -mmcount-ra-address
In reply to Wu ZhangjinFrom: 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.
--- arch/mips/kernel/ftrace.c | 16 ++++++++++++++-- 1 files changed, 14 insertions(+), 2 deletions(-)
arch/mips/kernel/ftrace.c +14 -2
@@ -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
PATCH v6 6/9 tracing: MIPS: cleanup of the instructions
In reply to Wu ZhangjinFrom: 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
--- arch/mips/kernel/ftrace.c | 19 +++++++++++-------- 1 files changed, 11 insertions(+), 8 deletions(-)
arch/mips/kernel/ftrace.c +11 -8
@@ -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
PATCH v6 7/9 tracing: MIPS: Reduce the overhead of dynamic Function Tracer
In reply to Wu ZhangjinFrom: 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.
--- arch/mips/kernel/ftrace.c | 93 +++++++++++++++++++++++--------------------- 1 files changed, 49 insertions(+), 44 deletions(-)
arch/mips/kernel/ftrace.c +49 -44
@@ -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
PATCH v6 8/9 tracing: MIPS: cleanup of function graph tracer
In reply to Wu ZhangjinFrom: Wu Zhangjin <[email protected]>
This patch cleans up the comments and ftrace_get_parent_addr() of function graph tracer.
--- arch/mips/kernel/ftrace.c | 48 ++++++++++++++++++++++++-------------------- 1 files changed, 26 insertions(+), 22 deletions(-)
arch/mips/kernel/ftrace.c +26 -22
@@ -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 (¤t->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
PATCH v6 9/9 tracing: MIPS: cleanup of the address space checking
In reply to Wu ZhangjinFrom: 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 may not work when the kernel is compiled with -msym32.
--- arch/mips/kernel/ftrace.c | 17 ++++++++++++++--- 1 files changed, 14 insertions(+), 3 deletions(-)
arch/mips/kernel/ftrace.c +14 -3
@@ -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}
Signature · 1 line
1.7.0.4
Re: [PATCH 9/9] tracing: MIPS: cleanup of the address space checking
Quoted 7 lines · 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 may not work when the kernel is compiled with -msym32.
The kernel is always compiled with -msym32, so the patch is a bit pointless.
Quoted 14 lines
Signed-off-by Wu Zhangjin<[email protected]>--- arch/mips/kernel/ftrace.c | 17 ++++++++++++++--- 1 files changed, 14 insertions(+), 3 deletions(-)arch/mips/kernel/ftrace.c +1
-0@@ -17,6 +17,17 @@#include <asm/cacheflush.h> #include <asm/uasm.h> +/*
arch/mips/kernel/ftrace.c line 30
Quoted 10 lines
arch/mips/kernel/ftrace.c +10
-0+* 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
Quoted 30 lines
arch/mips/kernel/ftrace.c +3
-3#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 9/9] tracing: MIPS: cleanup of the address space checking
Quoted 12 lines · David Daney
Quoted 7 lines · 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 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.
Quoted 24 lines
Quoted 10 lines
arch/mips/kernel/ftrace.c +1
-0@@ -17,6 +17,17 @@#include <asm/cacheflush.h> #include <asm/uasm.h> +/* arch/mips/kernel/ftrace.c line 30
Quoted 10 lines
arch/mips/kernel/ftrace.c +10
-0+* 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):
Quoted 5 lines
static inline int
in_module(unsigned long ip){ return ip < _text || ip > _etext; }
It may work, thanks!
Quoted 3 lines
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
In reply to David DaneyQuoted 1 line · David Daney
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
Quoted 11 lines · Ralf Baechle
Quoted 1 line · David Daney
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