* [Bug-fix] backtrace when HAVE_FUNCTION_TRACER is enable
@ 2012-08-09 13:53 Akhilesh Kumar
[not found] ` <CAD+V5YKZJHKONehvT+-GrKLP2+e0PiLiFTJWEojiDoNyLT3yGQ@mail.gmail.com>
0 siblings, 1 reply; 7+ messages in thread
From: Akhilesh Kumar @ 2012-08-09 13:53 UTC (permalink / raw)
To: ralf; +Cc: linux-mips
[-- Attachment #1: Type: text/plain, Size: 4060 bytes --]
Hi Ralf,
Sub:- Bug unable to retrive backtrace when HAVE_FUNCTION_TRACER is enable.
I send this bug and bug fix long back, I am resending this patch again for
review.
Please review below patch if you agree I will regenerate this patch and
with you.
====[ backtrace testing ]===========
Testing a backtrace from process context.
The following trace is a kernel self test and not a bug!
Testing a backtrace.
The following trace is a kernel self test and not a bug!
Call Trace:
[<80295134>] dump_stack+0x8/0x34
[<c0946060>] backtrace_regression_test+0x60/0x94 [sisc_backtrcae]
[<800004f0>] do_one_initcall+0xf0/0x1d0
[<80060954>] sys_init_module+0x19c8/0x1c60
[<8000a418>] stack_done+0x20/0x40
output befor patch when HAVE_FUNCTION_TRACER is enable
---------------------------------------------------------------------
#> insmod backtrace.ko
====[ backtrace testing ]===========
Testing a backtrace from process context.
The following trace is a kernel self test and not a bug!
Testing a backtrace.
The following trace is a kernel self test and not a bug!
Call Trace:
[<802e5164>] dump_stack+0x1c/0x50
[<802e5164>] dump_stack+0x1c/0x50
====[ end of backtrace testing ]====
------------------------------------------------------
above shows the wrong back trcae
output after patch when HAVE_FUNCTION_TRACER is enable
----------------------------------------------------------------------
====[ backtrace testing ]===========
Testing a backtrace from process context.
The following trace is a kernel self test and not a bug!
Testing a backtrace.
The following trace is a kernel self test and not a bug!
Call Trace:
[<802eb1a4>] dump_stack+0x20/0x54
[<c003405c>] backtrace_test_timer+0x5c/0x74 [sisc_backtrcae]
[<c00340dc>] init_module+0x68/0xa0 [sisc_backtrcae]
[<80000508>] do_one_initcall+0x108/0x1f0
[<8006d4c4>] sys_init_module+0x1a10/0x1c74
[<8000b038>] stack_done+0x20/0x40
------------------------------------------------------------------
get_frame_info() is used to fetch the frame information from the
function.
However,
1. this function just considers the first stack adjustment for frame
size.
2. On finding the save_lr instruction, it returns.
It doesn't handle the ftrace condition.
If Dynamic Frace "CONFIG_DYNAMIC_FTRACE" is enabled, the instrumentation
code is:
- jal <ftrace_caller>
- addiu sp,sp,-8
Thus, the current Frame Size of function is increased by 8 for Ftrace.
Signed-off-by: Akhilesh Kumar <akhilesh.lxr@gmail.com>
---
arch/mips/kernel/process.c | 67
++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 67 insertions(+), 0 deletions(-)
diff --git a/arch/mips/kernel/process.c b/arch/mips/kernel/process.c
index 7955409..df72738 100644
--- a/arch/mips/kernel/process.c
+++ b/arch/mips/kernel/process.c
@@ -290,12 +290,45 @@ static inline int is_sp_move_ins(union
mips_instruction *ip)
return 0;
}
+#ifdef CONFIG_DYNAMIC_FTRACE
+/*
+ * To create the jal <> instruction from mcount.
+ * taken from:
+ * - arch/mips/kernel/ftrace.c
+ */
+#define ADDR_MASK 0x03ffffff /* op_code|addr : 31...26|25 ....0 */
+#define JAL 0x0c000000 /* jump & link: ip --> ra, jump to target */
+#define INSN_JAL(addr) \
+ ((unsigned int)(JAL | (((addr) >> 2) & ADDR_MASK)))
+
+/*
+ * We assume jal <mcount>/<ftrace_caller> to be present in
+ * first JAL_MAX_OFFSET instructions.
+ * Increment this, if its otherwise
+ */
+#define JAL_MAX_OFFSET 16U
+#define MCOUNT_STACK_INST 0x27bdfff8 /* addiu sp,sp,-8 */
+
+/*
+ * If Dynamic Ftrace is enabled, ftrace_caller is the trace function.
+ * Otherwise its - mcount
+ */
+extern void ftrace_caller(void);
+#endif /* CONFIG_DYNAMIC_FTRACE */
+
static int get_frame_info(struct mips_frame_info *info)
{
union mips_instruction *ip = info->func;
unsigned max_insns = info->func_size / sizeof(union mips_instruction);
unsigned i;
+#ifdef CONFIG_DYNAMIC_FTRACE
+ unsigned max_prolog_inst = max_insns;
+ int jal_found = 0;
+ /* instruction corresponding to jal <_mcount>/<ftrace_caller> */
+ int jal_mcount = 0;
+#endif
+
info->pc_offset = -1;
info->frame_size = 0;
[-- Attachment #2: Type: text/html, Size: 4927 bytes --]
^ permalink raw reply related [flat|nested] 7+ messages in thread[parent not found: <CAD+V5YKZJHKONehvT+-GrKLP2+e0PiLiFTJWEojiDoNyLT3yGQ@mail.gmail.com>]
* Re: [Bug-fix] backtrace when HAVE_FUNCTION_TRACER is enable [not found] ` <CAD+V5YKZJHKONehvT+-GrKLP2+e0PiLiFTJWEojiDoNyLT3yGQ@mail.gmail.com> @ 2012-08-09 17:16 ` Akhilesh Kumar 2012-08-12 6:28 ` John Crispin 2012-08-13 15:44 ` Akhilesh Kumar 0 siblings, 2 replies; 7+ messages in thread From: Akhilesh Kumar @ 2012-08-09 17:16 UTC (permalink / raw) To: wuzhangjin, ralf; +Cc: linux-mips [-- Attachment #1: Type: text/plain, Size: 7764 bytes --] yes Zhangin please find the complete patch ==================================================================== diff --git a/arch/mips/kernel/process.c b/arch/mips/kernel/process.c index 7955409..df72738 100644 --- a/arch/mips/kernel/process.c +++ b/arch/mips/kernel/process.c @@ -290,12 +290,45 @@ static inline int is_sp_move_ins(union mips_instruction *ip) return 0; } +#ifdef CONFIG_DYNAMIC_FTRACE +/* + * To create the jal <> instruction from mcount. + * taken from: + * - arch/mips/kernel/ftrace.c + */ +#define ADDR_MASK 0x03ffffff /* op_code|addr : 31...26|25 ....0 */ +#define JAL 0x0c000000 /* jump & link: ip --> ra, jump to target */ +#define INSN_JAL(addr) \ + ((unsigned int)(JAL | (((addr) >> 2) & ADDR_MASK))) + +/* + * We assume jal <mcount>/<ftrace_caller> to be present in + * first JAL_MAX_OFFSET instructions. + * Increment this, if its otherwise + */ +#define JAL_MAX_OFFSET 16U +#define MCOUNT_STACK_INST 0x27bdfff8 /* addiu sp,sp,-8 */ + +/* + * If Dynamic Ftrace is enabled, ftrace_caller is the trace function. + * Otherwise its - mcount + */ +extern void ftrace_caller(void); +#endif /* CONFIG_DYNAMIC_FTRACE */ + static int get_frame_info(struct mips_frame_info *info) { union mips_instruction *ip = info->func; unsigned max_insns = info->func_size / sizeof(union mips_instruction); unsigned i; +#ifdef CONFIG_DYNAMIC_FTRACE + unsigned max_prolog_inst = max_insns; + int jal_found = 0; + /* instruction corresponding to jal <_mcount>/<ftrace_caller> */ + int jal_mcount = 0; +#endif + info->pc_offset = -1; info->frame_size = 0; @@ -306,6 +339,28 @@ static int get_frame_info(struct mips_frame_info *info) max_insns = 128U; /* unknown function size */ max_insns = min(128U, max_insns); +#ifdef CONFIG_DYNAMIC_FTRACE + max_prolog_inst = min(JAL_MAX_OFFSET, max_prolog_inst); + jal_mcount = INSN_JAL((unsigned)&ftrace_caller); + + for (i = 0; i < max_prolog_inst; i++, ip++) { + if ((*(int *)ip == jal_mcount) || + /* + * for dyn ftrace, the code initially has 0. + * so we check whether the next instruction is + * addiu sp,sp,-8 + */ + (!(*(int *)ip) && + (*(int *)(ip + 1) == MCOUNT_STACK_INST)) + ) { + jal_found = 1; + break; + } + } + /* restore the ip to start of function */ + ip = info->func; +#endif + for (i = 0; i < max_insns; i++, ip++) { if (is_jal_jalr_jr_ins(ip)) @@ -321,6 +376,18 @@ static int get_frame_info(struct mips_frame_info *info) break; } } +#ifdef CONFIG_DYNAMIC_FTRACE + /* + * to offset the effect of: + * addiu sp,sp,-8 + */ + if (jal_found) { + if (info->frame_size) + info->frame_size += 8; + if (info->pc_offset >= 0) + info->pc_offset += 8 / sizeof(long); + } +#endif if (info->frame_size && info->pc_offset >= 0) /* nested */ return 0; if (info->pc_offset < 0) /* leaf */ -- 1.7.8.4 ==================================================================== On Thu, Aug 9, 2012 at 9:41 PM, Wu Zhangjin <wuzhangjin@gmail.com> wrote: > Hi, Akhilesh > > Thanks very much for your work. > > Seems this patch has lost something, can you send a full one? > > Best Regards, > Wu Zhangjin > > On Thu, Aug 9, 2012 at 9:53 PM, Akhilesh Kumar <akhilesh.lxr@gmail.com> > wrote: > > Hi Ralf, > > > > > > Sub:- Bug unable to retrive backtrace when HAVE_FUNCTION_TRACER is > enable. > > I send this bug and bug fix long back, I am resending this patch again > for > > review. > > > > Please review below patch if you agree I will regenerate this patch and > with > > you. > > > > ====[ backtrace testing ]=========== > > Testing a backtrace from process context. > > The following trace is a kernel self test and not a bug! > > Testing a backtrace. > > The following trace is a kernel self test and not a bug! > > Call Trace: > > [<80295134>] dump_stack+0x8/0x34 > > [<c0946060>] backtrace_regression_test+0x60/0x94 [sisc_backtrcae] > > [<800004f0>] do_one_initcall+0xf0/0x1d0 > > [<80060954>] sys_init_module+0x19c8/0x1c60 > > [<8000a418>] stack_done+0x20/0x40 > > output befor patch when HAVE_FUNCTION_TRACER is enable > > --------------------------------------------------------------------- > > #> insmod backtrace.ko > > ====[ backtrace testing ]=========== > > Testing a backtrace from process context. > > The following trace is a kernel self test and not a bug! > > Testing a backtrace. > > The following trace is a kernel self test and not a bug! > > Call Trace: > > [<802e5164>] dump_stack+0x1c/0x50 > > [<802e5164>] dump_stack+0x1c/0x50 > > ====[ end of backtrace testing ]==== > > ------------------------------------------------------ > > above shows the wrong back trcae > > output after patch when HAVE_FUNCTION_TRACER is enable > > ---------------------------------------------------------------------- > > ====[ backtrace testing ]=========== > > Testing a backtrace from process context. > > The following trace is a kernel self test and not a bug! > > Testing a backtrace. > > The following trace is a kernel self test and not a bug! > > Call Trace: > > [<802eb1a4>] dump_stack+0x20/0x54 > > [<c003405c>] backtrace_test_timer+0x5c/0x74 [sisc_backtrcae] > > [<c00340dc>] init_module+0x68/0xa0 [sisc_backtrcae] > > [<80000508>] do_one_initcall+0x108/0x1f0 > > [<8006d4c4>] sys_init_module+0x1a10/0x1c74 > > [<8000b038>] stack_done+0x20/0x40 > > ------------------------------------------------------------------ > > get_frame_info() is used to fetch the frame information from the > > function. > > However, > > 1. this function just considers the first stack adjustment for frame > > size. > > 2. On finding the save_lr instruction, it returns. > > It doesn't handle the ftrace condition. > > If Dynamic Frace "CONFIG_DYNAMIC_FTRACE" is enabled, the instrumentation > > code is: > > - jal <ftrace_caller> > > - addiu sp,sp,-8 > > Thus, the current Frame Size of function is increased by 8 for Ftrace. > > Signed-off-by: Akhilesh Kumar <akhilesh.lxr@gmail.com> > > --- > > arch/mips/kernel/process.c | 67 > > ++++++++++++++++++++++++++++++++++++++++++++ > > 1 files changed, 67 insertions(+), 0 deletions(-) > > diff --git a/arch/mips/kernel/process.c b/arch/mips/kernel/process.c > > index 7955409..df72738 100644 > > --- a/arch/mips/kernel/process.c > > +++ b/arch/mips/kernel/process.c > > @@ -290,12 +290,45 @@ static inline int is_sp_move_ins(union > > mips_instruction *ip) > > return 0; > > } > > +#ifdef CONFIG_DYNAMIC_FTRACE > > +/* > > + * To create the jal <> instruction from mcount. > > + * taken from: > > + * - arch/mips/kernel/ftrace.c > > + */ > > +#define ADDR_MASK 0x03ffffff /* op_code|addr : 31...26|25 ....0 */ > > +#define JAL 0x0c000000 /* jump & link: ip --> ra, jump to target */ > > +#define INSN_JAL(addr) \ > > + ((unsigned int)(JAL | (((addr) >> 2) & ADDR_MASK))) > > + > > +/* > > + * We assume jal <mcount>/<ftrace_caller> to be present in > > + * first JAL_MAX_OFFSET instructions. > > + * Increment this, if its otherwise > > + */ > > +#define JAL_MAX_OFFSET 16U > > +#define MCOUNT_STACK_INST 0x27bdfff8 /* addiu sp,sp,-8 */ > > + > > +/* > > + * If Dynamic Ftrace is enabled, ftrace_caller is the trace function. > > + * Otherwise its - mcount > > + */ > > +extern void ftrace_caller(void); > > +#endif /* CONFIG_DYNAMIC_FTRACE */ > > + > > static int get_frame_info(struct mips_frame_info *info) > > { > > union mips_instruction *ip = info->func; > > unsigned max_insns = info->func_size / sizeof(union mips_instruction); > > unsigned i; > > +#ifdef CONFIG_DYNAMIC_FTRACE > > + unsigned max_prolog_inst = max_insns; > > + int jal_found = 0; > > + /* instruction corresponding to jal <_mcount>/<ftrace_caller> */ > > + int jal_mcount = 0; > > +#endif > > + > > info->pc_offset = -1; > > info->frame_size = 0; > [-- Attachment #2: Type: text/html, Size: 10288 bytes --] ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [Bug-fix] backtrace when HAVE_FUNCTION_TRACER is enable 2012-08-09 17:16 ` Akhilesh Kumar @ 2012-08-12 6:28 ` John Crispin 2012-08-13 15:44 ` Akhilesh Kumar 1 sibling, 0 replies; 7+ messages in thread From: John Crispin @ 2012-08-12 6:28 UTC (permalink / raw) To: linux-mips Hi, This patch is missing the Description and SoB line John On 09/08/12 19:16, Akhilesh Kumar wrote: > yes Zhangin > > please find the complete patch > ==================================================================== > diff --git a/arch/mips/kernel/process.c b/arch/mips/kernel/process.c > index 7955409..df72738 100644 > --- a/arch/mips/kernel/process.c > +++ b/arch/mips/kernel/process.c > @@ -290,12 +290,45 @@ static inline int is_sp_move_ins(union > mips_instruction *ip) > return 0; > } > > +#ifdef CONFIG_DYNAMIC_FTRACE > +/* > + * To create the jal <> instruction from mcount. > + * taken from: > + * - arch/mips/kernel/ftrace.c > + */ > +#define ADDR_MASK 0x03ffffff /* op_code|addr : 31...26|25 ....0 */ > +#define JAL 0x0c000000 /* jump & link: ip --> ra, jump to target */ > +#define INSN_JAL(addr) \ > + ((unsigned int)(JAL | (((addr) >> 2) & ADDR_MASK))) > + > +/* > + * We assume jal <mcount>/<ftrace_caller> to be present in > + * first JAL_MAX_OFFSET instructions. > + * Increment this, if its otherwise > + */ > +#define JAL_MAX_OFFSET 16U > +#define MCOUNT_STACK_INST 0x27bdfff8 /* addiu sp,sp,-8 */ > + > +/* > + * If Dynamic Ftrace is enabled, ftrace_caller is the trace function. > + * Otherwise its - mcount > + */ > +extern void ftrace_caller(void); > +#endif /* CONFIG_DYNAMIC_FTRACE */ > + > static int get_frame_info(struct mips_frame_info *info) > { > union mips_instruction *ip = info->func; > unsigned max_insns = info->func_size / sizeof(union mips_instruction); > unsigned i; > > +#ifdef CONFIG_DYNAMIC_FTRACE > + unsigned max_prolog_inst = max_insns; > + int jal_found = 0; > + /* instruction corresponding to jal <_mcount>/<ftrace_caller> */ > + int jal_mcount = 0; > +#endif > + > info->pc_offset = -1; > info->frame_size = 0; > > @@ -306,6 +339,28 @@ static int get_frame_info(struct mips_frame_info *info) > max_insns = 128U; /* unknown function size */ > max_insns = min(128U, max_insns); > > +#ifdef CONFIG_DYNAMIC_FTRACE > + max_prolog_inst = min(JAL_MAX_OFFSET, max_prolog_inst); > + jal_mcount = INSN_JAL((unsigned)&ftrace_caller); > + > + for (i = 0; i < max_prolog_inst; i++, ip++) { > + if ((*(int *)ip == jal_mcount) || > + /* > + * for dyn ftrace, the code initially has 0. > + * so we check whether the next instruction is > + * addiu sp,sp,-8 > + */ > + (!(*(int *)ip) && > + (*(int *)(ip + 1) == MCOUNT_STACK_INST)) > + ) { > + jal_found = 1; > + break; > + } > + } > + /* restore the ip to start of function */ > + ip = info->func; > +#endif > + > for (i = 0; i < max_insns; i++, ip++) { > > if (is_jal_jalr_jr_ins(ip)) > @@ -321,6 +376,18 @@ static int get_frame_info(struct mips_frame_info *info) > break; > } > } > +#ifdef CONFIG_DYNAMIC_FTRACE > + /* > + * to offset the effect of: > + * addiu sp,sp,-8 > + */ > + if (jal_found) { > + if (info->frame_size) > + info->frame_size += 8; > + if (info->pc_offset >= 0) > + info->pc_offset += 8 / sizeof(long); > + } > +#endif > if (info->frame_size && info->pc_offset >= 0) /* nested */ > return 0; > if (info->pc_offset < 0) /* leaf */ > -- > 1.7.8.4 > > ==================================================================== > > On Thu, Aug 9, 2012 at 9:41 PM, Wu Zhangjin <wuzhangjin@gmail.com > <mailto:wuzhangjin@gmail.com>> wrote: > > Hi, Akhilesh > > Thanks very much for your work. > > Seems this patch has lost something, can you send a full one? > > Best Regards, > Wu Zhangjin > > On Thu, Aug 9, 2012 at 9:53 PM, Akhilesh Kumar > <akhilesh.lxr@gmail.com <mailto:akhilesh.lxr@gmail.com>> wrote: > > Hi Ralf, > > > > > > Sub:- Bug unable to retrive backtrace when HAVE_FUNCTION_TRACER > is enable. > > I send this bug and bug fix long back, I am resending this patch > again for > > review. > > > > Please review below patch if you agree I will regenerate this > patch and with > > you. > > > > ====[ backtrace testing ]=========== > > Testing a backtrace from process context. > > The following trace is a kernel self test and not a bug! > > Testing a backtrace. > > The following trace is a kernel self test and not a bug! > > Call Trace: > > [<80295134>] dump_stack+0x8/0x34 > > [<c0946060>] backtrace_regression_test+0x60/0x94 [sisc_backtrcae] > > [<800004f0>] do_one_initcall+0xf0/0x1d0 > > [<80060954>] sys_init_module+0x19c8/0x1c60 > > [<8000a418>] stack_done+0x20/0x40 > > output befor patch when HAVE_FUNCTION_TRACER is enable > > --------------------------------------------------------------------- > > #> insmod backtrace.ko > > ====[ backtrace testing ]=========== > > Testing a backtrace from process context. > > The following trace is a kernel self test and not a bug! > > Testing a backtrace. > > The following trace is a kernel self test and not a bug! > > Call Trace: > > [<802e5164>] dump_stack+0x1c/0x50 > > [<802e5164>] dump_stack+0x1c/0x50 > > ====[ end of backtrace testing ]==== > > ------------------------------------------------------ > > above shows the wrong back trcae > > output after patch when HAVE_FUNCTION_TRACER is enable > > ---------------------------------------------------------------------- > > ====[ backtrace testing ]=========== > > Testing a backtrace from process context. > > The following trace is a kernel self test and not a bug! > > Testing a backtrace. > > The following trace is a kernel self test and not a bug! > > Call Trace: > > [<802eb1a4>] dump_stack+0x20/0x54 > > [<c003405c>] backtrace_test_timer+0x5c/0x74 [sisc_backtrcae] > > [<c00340dc>] init_module+0x68/0xa0 [sisc_backtrcae] > > [<80000508>] do_one_initcall+0x108/0x1f0 > > [<8006d4c4>] sys_init_module+0x1a10/0x1c74 > > [<8000b038>] stack_done+0x20/0x40 > > ------------------------------------------------------------------ > > get_frame_info() is used to fetch the frame information from the > > function. > > However, > > 1. this function just considers the first stack adjustment for frame > > size. > > 2. On finding the save_lr instruction, it returns. > > It doesn't handle the ftrace condition. > > If Dynamic Frace "CONFIG_DYNAMIC_FTRACE" is enabled, the > instrumentation > > code is: > > - jal <ftrace_caller> > > - addiu sp,sp,-8 > > Thus, the current Frame Size of function is increased by 8 for Ftrace. > > Signed-off-by: Akhilesh Kumar <akhilesh.lxr@gmail.com > <mailto:akhilesh.lxr@gmail.com>> > > --- > > arch/mips/kernel/process.c | 67 > > ++++++++++++++++++++++++++++++++++++++++++++ > > 1 files changed, 67 insertions(+), 0 deletions(-) > > diff --git a/arch/mips/kernel/process.c b/arch/mips/kernel/process.c > > index 7955409..df72738 100644 > > --- a/arch/mips/kernel/process.c > > +++ b/arch/mips/kernel/process.c > > @@ -290,12 +290,45 @@ static inline int is_sp_move_ins(union > > mips_instruction *ip) > > return 0; > > } > > +#ifdef CONFIG_DYNAMIC_FTRACE > > +/* > > + * To create the jal <> instruction from mcount. > > + * taken from: > > + * - arch/mips/kernel/ftrace.c > > + */ > > +#define ADDR_MASK 0x03ffffff /* op_code|addr : 31...26|25 > ....0 */ > > +#define JAL 0x0c000000 /* jump & link: ip --> ra, jump to > target */ > > +#define INSN_JAL(addr) \ > > + ((unsigned int)(JAL | (((addr) >> 2) & ADDR_MASK))) > > + > > +/* > > + * We assume jal <mcount>/<ftrace_caller> to be present in > > + * first JAL_MAX_OFFSET instructions. > > + * Increment this, if its otherwise > > + */ > > +#define JAL_MAX_OFFSET 16U > > +#define MCOUNT_STACK_INST 0x27bdfff8 /* addiu sp,sp,-8 */ > > + > > +/* > > + * If Dynamic Ftrace is enabled, ftrace_caller is the trace function. > > + * Otherwise its - mcount > > + */ > > +extern void ftrace_caller(void); > > +#endif /* CONFIG_DYNAMIC_FTRACE */ > > + > > static int get_frame_info(struct mips_frame_info *info) > > { > > union mips_instruction *ip = info->func; > > unsigned max_insns = info->func_size / sizeof(union > mips_instruction); > > unsigned i; > > +#ifdef CONFIG_DYNAMIC_FTRACE > > + unsigned max_prolog_inst = max_insns; > > + int jal_found = 0; > > + /* instruction corresponding to jal <_mcount>/<ftrace_caller> */ > > + int jal_mcount = 0; > > +#endif > > + > > info->pc_offset = -1; > > info->frame_size = 0; > > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Bug-fix] backtrace when HAVE_FUNCTION_TRACER is enable @ 2012-08-13 15:44 ` Akhilesh Kumar 0 siblings, 0 replies; 7+ messages in thread From: Akhilesh Kumar @ 2012-08-13 15:44 UTC (permalink / raw) To: ralf, Wu Zhangjin; +Cc: linux-mips [-- Attachment #1: Type: text/plain, Size: 14588 bytes --] Hi Ralf/John, Detail Description is added in the patch Thanks, Akhilesh >From 89004f88a1f8399be7a856b43b475f06c3d5bb30 Mon Sep 17 00:00:00 2001 From: Akhilesh Kumar <akhilesh.lxr@...com> Date: Fri, 4 May 2012 18:52:46 +0530 Subject: process_backtrace Patch fix the backtrcae problem using unwind_stack()) When ftrace is used, unwind_stack() is unable to retrieve the frame info and call-trace doesn't work. test module ---------------- static struct timer_list backtrace_timer; static void backtrace_test_timer(unsigned long data) { printk("Testing a backtrace from irq context.\n"); printk("The following trace is a kernel self test and not a bug!\n"); dump_stack(); } static int backtrace_regression_test(void) { printk("====[ backtrace testing ]===========\n"); printk("Testing a backtrace from process context.\n"); printk("The following trace is a kernel self test and not a bug!\n"); dump_stack(); init_timer(&backtrace_timer); backtrace_timer.function = backtrace_test_timer; mod_timer(&backtrace_timer, jiffies + 10); msleep(10); printk("====[ end of backtrace testing ]====\n"); return 0; } static void exitf(void) { } module_init(backtrace_regression_test); module_exit(exitf); MODULE_LICENSE("GPL"); --------------------------------------------------------------------- out put when HAVE_FUNCTION_TRACER is disable ====[ backtrace testing ]=========== Testing a backtrace from process context. The following trace is a kernel self test and not a bug! Testing a backtrace. The following trace is a kernel self test and not a bug! Call Trace: [<80295134>] dump_stack+0x8/0x34 [<c0946060>] backtrace_regression_test+0x60/0x94 [sisc_backtrcae] [<800004f0>] do_one_initcall+0xf0/0x1d0 [<80060954>] sys_init_module+0x19c8/0x1c60 [<8000a418>] stack_done+0x20/0x40 output befor patch when HAVE_FUNCTION_TRACER is enable --------------------------------------------------------------------- VDLinux#> insmod sisc_backtrcae.ko ====[ backtrace testing ]=========== Testing a backtrace from process context. The following trace is a kernel self test and not a bug! Testing a backtrace. The following trace is a kernel self test and not a bug! Call Trace: [<802e5164>] dump_stack+0x1c/0x50 [<802e5164>] dump_stack+0x1c/0x50 ====[ end of backtrace testing ]==== ------------------------------------------------------ above shows the wrong back trcae output after patch when HAVE_FUNCTION_TRACER is enable ---------------------------------------------------------------------- ====[ backtrace testing ]=========== Testing a backtrace from process context. The following trace is a kernel self test and not a bug! Testing a backtrace. The following trace is a kernel self test and not a bug! Call Trace: [<802eb1a4>] dump_stack+0x20/0x54 [<c003405c>] backtrace_test_timer+0x5c/0x74 [sisc_backtrcae] [<c00340dc>] init_module+0x68/0xa0 [sisc_backtrcae] [<80000508>] do_one_initcall+0x108/0x1f0 [<8006d4c4>] sys_init_module+0x1a10/0x1c74 [<8000b038>] stack_done+0x20/0x40 ------------------------------------------------------------------ get_frame_info() is used to fetch the frame information from the function. However, 1. this function just considers the first stack adjustment for frame size. 2. On finding the save_lr instruction, it returns. It doesn't handle the ftrace condition. If Dynamic Frace "CONFIG_DYNAMIC_FTRACE" is enabled, the instrumentation code is: - jal <ftrace_caller> - addiu sp,sp,-8 Thus, the current Frame Size of function is increased by 8 for Ftrace. Signed-off-by: Akhilesh Kumar <akhilesh.lxr@...com> --- arch/mips/kernel/process.c | 67 ++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 67 insertions(+), 0 deletions(-) diff --git a/arch/mips/kernel/process.c b/arch/mips/kernel/process.c index 7955409..df72738 100644 --- a/arch/mips/kernel/process.c +++ b/arch/mips/kernel/process.c @@ -290,12 +290,45 @@ static inline int is_sp_move_ins(union mips_instruction *ip) return 0; } +#ifdef CONFIG_DYNAMIC_FTRACE +/* + * To create the jal <> instruction from mcount. + * taken from: + * - arch/mips/kernel/ftrace.c + */ +#define ADDR_MASK 0x03ffffff /* op_code|addr : 31...26|25 ....0 */ +#define JAL 0x0c000000 /* jump & link: ip --> ra, jump to target */ +#define INSN_JAL(addr) \ + ((unsigned int)(JAL | (((addr) >> 2) & ADDR_MASK))) + +/* + * We assume jal <mcount>/<ftrace_caller> to be present in + * first JAL_MAX_OFFSET instructions. + * Increment this, if its otherwise + */ +#define JAL_MAX_OFFSET 16U +#define MCOUNT_STACK_INST 0x27bdfff8 /* addiu sp,sp,-8 */ + +/* + * If Dynamic Ftrace is enabled, ftrace_caller is the trace function. + * Otherwise its - mcount + */ +extern void ftrace_caller(void); +#endif /* CONFIG_DYNAMIC_FTRACE */ + static int get_frame_info(struct mips_frame_info *info) { union mips_instruction *ip = info->func; unsigned max_insns = info->func_size / sizeof(union mips_instruction); unsigned i; +#ifdef CONFIG_DYNAMIC_FTRACE + unsigned max_prolog_inst = max_insns; + int jal_found = 0; + /* instruction corresponding to jal <_mcount>/<ftrace_caller> */ + int jal_mcount = 0; +#endif + info->pc_offset = -1; info->frame_size = 0; @@ -306,6 +339,28 @@ static int get_frame_info(struct mips_frame_info *info) max_insns = 128U; /* unknown function size */ max_insns = min(128U, max_insns); +#ifdef CONFIG_DYNAMIC_FTRACE + max_prolog_inst = min(JAL_MAX_OFFSET, max_prolog_inst); + jal_mcount = INSN_JAL((unsigned)&ftrace_caller); + + for (i = 0; i < max_prolog_inst; i++, ip++) { + if ((*(int *)ip == jal_mcount) || + /* + * for dyn ftrace, the code initially has 0. + * so we check whether the next instruction is + * addiu sp,sp,-8 + */ + (!(*(int *)ip) && + (*(int *)(ip + 1) == MCOUNT_STACK_INST)) + ) { + jal_found = 1; + break; + } + } + /* restore the ip to start of function */ + ip = info->func; +#endif + for (i = 0; i < max_insns; i++, ip++) { if (is_jal_jalr_jr_ins(ip)) @@ -321,6 +376,18 @@ static int get_frame_info(struct mips_frame_info *info) break; } } +#ifdef CONFIG_DYNAMIC_FTRACE + /* + * to offset the effect of: + * addiu sp,sp,-8 + */ + if (jal_found) { + if (info->frame_size) + info->frame_size += 8; + if (info->pc_offset >= 0) + info->pc_offset += 8 / sizeof(long); + } +#endif if (info->frame_size && info->pc_offset >= 0) /* nested */ return 0; if (info->pc_offset < 0) /* leaf */ -- 1.7.8.4 On Thu, Aug 9, 2012 at 10:46 PM, Akhilesh Kumar <akhilesh.lxr@gmail.com>wrote: > yes Zhangin > > please find the complete patch > ==================================================================== > diff --git a/arch/mips/kernel/process.c b/arch/mips/kernel/process.c > index 7955409..df72738 100644 > --- a/arch/mips/kernel/process.c > +++ b/arch/mips/kernel/process.c > @@ -290,12 +290,45 @@ static inline int is_sp_move_ins(union > mips_instruction *ip) > return 0; > } > > +#ifdef CONFIG_DYNAMIC_FTRACE > +/* > + * To create the jal <> instruction from mcount. > + * taken from: > + * - arch/mips/kernel/ftrace.c > + */ > +#define ADDR_MASK 0x03ffffff /* op_code|addr : 31...26|25 ....0 */ > +#define JAL 0x0c000000 /* jump & link: ip --> ra, jump to target */ > +#define INSN_JAL(addr) \ > + ((unsigned int)(JAL | (((addr) >> 2) & ADDR_MASK))) > + > +/* > + * We assume jal <mcount>/<ftrace_caller> to be present in > + * first JAL_MAX_OFFSET instructions. > + * Increment this, if its otherwise > + */ > +#define JAL_MAX_OFFSET 16U > +#define MCOUNT_STACK_INST 0x27bdfff8 /* addiu sp,sp,-8 */ > + > +/* > + * If Dynamic Ftrace is enabled, ftrace_caller is the trace function. > + * Otherwise its - mcount > + */ > +extern void ftrace_caller(void); > +#endif /* CONFIG_DYNAMIC_FTRACE */ > + > static int get_frame_info(struct mips_frame_info *info) > { > union mips_instruction *ip = info->func; > unsigned max_insns = info->func_size / sizeof(union mips_instruction); > unsigned i; > > +#ifdef CONFIG_DYNAMIC_FTRACE > + unsigned max_prolog_inst = max_insns; > + int jal_found = 0; > + /* instruction corresponding to jal <_mcount>/<ftrace_caller> */ > + int jal_mcount = 0; > +#endif > + > info->pc_offset = -1; > info->frame_size = 0; > > @@ -306,6 +339,28 @@ static int get_frame_info(struct mips_frame_info > *info) > max_insns = 128U; /* unknown function size */ > max_insns = min(128U, max_insns); > > +#ifdef CONFIG_DYNAMIC_FTRACE > + max_prolog_inst = min(JAL_MAX_OFFSET, max_prolog_inst); > + jal_mcount = INSN_JAL((unsigned)&ftrace_caller); > + > + for (i = 0; i < max_prolog_inst; i++, ip++) { > + if ((*(int *)ip == jal_mcount) || > + /* > + * for dyn ftrace, the code initially has 0. > + * so we check whether the next instruction is > + * addiu sp,sp,-8 > + */ > + (!(*(int *)ip) && > + (*(int *)(ip + 1) == MCOUNT_STACK_INST)) > + ) { > + jal_found = 1; > + break; > + } > + } > + /* restore the ip to start of function */ > + ip = info->func; > +#endif > + > for (i = 0; i < max_insns; i++, ip++) { > > if (is_jal_jalr_jr_ins(ip)) > @@ -321,6 +376,18 @@ static int get_frame_info(struct mips_frame_info > *info) > break; > } > } > +#ifdef CONFIG_DYNAMIC_FTRACE > + /* > + * to offset the effect of: > + * addiu sp,sp,-8 > + */ > + if (jal_found) { > + if (info->frame_size) > + info->frame_size += 8; > + if (info->pc_offset >= 0) > + info->pc_offset += 8 / sizeof(long); > + } > +#endif > if (info->frame_size && info->pc_offset >= 0) /* nested */ > return 0; > if (info->pc_offset < 0) /* leaf */ > -- > 1.7.8.4 > > ==================================================================== > > > On Thu, Aug 9, 2012 at 9:41 PM, Wu Zhangjin <wuzhangjin@gmail.com> wrote: > >> Hi, Akhilesh >> >> Thanks very much for your work. >> >> Seems this patch has lost something, can you send a full one? >> >> Best Regards, >> Wu Zhangjin >> >> On Thu, Aug 9, 2012 at 9:53 PM, Akhilesh Kumar <akhilesh.lxr@gmail.com> >> wrote: >> > Hi Ralf, >> > >> > >> > Sub:- Bug unable to retrive backtrace when HAVE_FUNCTION_TRACER is >> enable. >> > I send this bug and bug fix long back, I am resending this patch again >> for >> > review. >> > >> > Please review below patch if you agree I will regenerate this patch and >> with >> > you. >> > >> > ====[ backtrace testing ]=========== >> > Testing a backtrace from process context. >> > The following trace is a kernel self test and not a bug! >> > Testing a backtrace. >> > The following trace is a kernel self test and not a bug! >> > Call Trace: >> > [<80295134>] dump_stack+0x8/0x34 >> > [<c0946060>] backtrace_regression_test+0x60/0x94 [sisc_backtrcae] >> > [<800004f0>] do_one_initcall+0xf0/0x1d0 >> > [<80060954>] sys_init_module+0x19c8/0x1c60 >> > [<8000a418>] stack_done+0x20/0x40 >> > output befor patch when HAVE_FUNCTION_TRACER is enable >> > --------------------------------------------------------------------- >> > #> insmod backtrace.ko >> > ====[ backtrace testing ]=========== >> > Testing a backtrace from process context. >> > The following trace is a kernel self test and not a bug! >> > Testing a backtrace. >> > The following trace is a kernel self test and not a bug! >> > Call Trace: >> > [<802e5164>] dump_stack+0x1c/0x50 >> > [<802e5164>] dump_stack+0x1c/0x50 >> > ====[ end of backtrace testing ]==== >> > ------------------------------------------------------ >> > above shows the wrong back trcae >> > output after patch when HAVE_FUNCTION_TRACER is enable >> > ---------------------------------------------------------------------- >> > ====[ backtrace testing ]=========== >> > Testing a backtrace from process context. >> > The following trace is a kernel self test and not a bug! >> > Testing a backtrace. >> > The following trace is a kernel self test and not a bug! >> > Call Trace: >> > [<802eb1a4>] dump_stack+0x20/0x54 >> > [<c003405c>] backtrace_test_timer+0x5c/0x74 [sisc_backtrcae] >> > [<c00340dc>] init_module+0x68/0xa0 [sisc_backtrcae] >> > [<80000508>] do_one_initcall+0x108/0x1f0 >> > [<8006d4c4>] sys_init_module+0x1a10/0x1c74 >> > [<8000b038>] stack_done+0x20/0x40 >> > ------------------------------------------------------------------ >> > get_frame_info() is used to fetch the frame information from the >> > function. >> > However, >> > 1. this function just considers the first stack adjustment for frame >> > size. >> > 2. On finding the save_lr instruction, it returns. >> > It doesn't handle the ftrace condition. >> > If Dynamic Frace "CONFIG_DYNAMIC_FTRACE" is enabled, the instrumentation >> > code is: >> > - jal <ftrace_caller> >> > - addiu sp,sp,-8 >> > Thus, the current Frame Size of function is increased by 8 for Ftrace. >> > Signed-off-by: Akhilesh Kumar <akhilesh.lxr@gmail.com> >> > --- >> > arch/mips/kernel/process.c | 67 >> > ++++++++++++++++++++++++++++++++++++++++++++ >> > 1 files changed, 67 insertions(+), 0 deletions(-) >> > diff --git a/arch/mips/kernel/process.c b/arch/mips/kernel/process.c >> > index 7955409..df72738 100644 >> > --- a/arch/mips/kernel/process.c >> > +++ b/arch/mips/kernel/process.c >> > @@ -290,12 +290,45 @@ static inline int is_sp_move_ins(union >> > mips_instruction *ip) >> > return 0; >> > } >> > +#ifdef CONFIG_DYNAMIC_FTRACE >> > +/* >> > + * To create the jal <> instruction from mcount. >> > + * taken from: >> > + * - arch/mips/kernel/ftrace.c >> > + */ >> > +#define ADDR_MASK 0x03ffffff /* op_code|addr : 31...26|25 ....0 */ >> > +#define JAL 0x0c000000 /* jump & link: ip --> ra, jump to target >> */ >> > +#define INSN_JAL(addr) \ >> > + ((unsigned int)(JAL | (((addr) >> 2) & ADDR_MASK))) >> > + >> > +/* >> > + * We assume jal <mcount>/<ftrace_caller> to be present in >> > + * first JAL_MAX_OFFSET instructions. >> > + * Increment this, if its otherwise >> > + */ >> > +#define JAL_MAX_OFFSET 16U >> > +#define MCOUNT_STACK_INST 0x27bdfff8 /* addiu sp,sp,-8 */ >> > + >> > +/* >> > + * If Dynamic Ftrace is enabled, ftrace_caller is the trace function. >> > + * Otherwise its - mcount >> > + */ >> > +extern void ftrace_caller(void); >> > +#endif /* CONFIG_DYNAMIC_FTRACE */ >> > + >> > static int get_frame_info(struct mips_frame_info *info) >> > { >> > union mips_instruction *ip = info->func; >> > unsigned max_insns = info->func_size / sizeof(union mips_instruction); >> > unsigned i; >> > +#ifdef CONFIG_DYNAMIC_FTRACE >> > + unsigned max_prolog_inst = max_insns; >> > + int jal_found = 0; >> > + /* instruction corresponding to jal <_mcount>/<ftrace_caller> */ >> > + int jal_mcount = 0; >> > +#endif >> > + >> > info->pc_offset = -1; >> > info->frame_size = 0; >> > > [-- Attachment #2: Type: text/html, Size: 20031 bytes --] ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [Bug-fix] backtrace when HAVE_FUNCTION_TRACER is enable @ 2012-08-13 15:44 ` Akhilesh Kumar 0 siblings, 0 replies; 7+ messages in thread From: Akhilesh Kumar @ 2012-08-13 15:44 UTC (permalink / raw) To: ralf, Wu Zhangjin; +Cc: linux-mips [-- Attachment #1: Type: text/plain, Size: 14587 bytes --] Hi Ralf/John, Detail Description is added in the patch Thanks, Akhilesh From 89004f88a1f8399be7a856b43b475f06c3d5bb30 Mon Sep 17 00:00:00 2001 From: Akhilesh Kumar <akhilesh.lxr@...com> Date: Fri, 4 May 2012 18:52:46 +0530 Subject: process_backtrace Patch fix the backtrcae problem using unwind_stack()) When ftrace is used, unwind_stack() is unable to retrieve the frame info and call-trace doesn't work. test module ---------------- static struct timer_list backtrace_timer; static void backtrace_test_timer(unsigned long data) { printk("Testing a backtrace from irq context.\n"); printk("The following trace is a kernel self test and not a bug!\n"); dump_stack(); } static int backtrace_regression_test(void) { printk("====[ backtrace testing ]===========\n"); printk("Testing a backtrace from process context.\n"); printk("The following trace is a kernel self test and not a bug!\n"); dump_stack(); init_timer(&backtrace_timer); backtrace_timer.function = backtrace_test_timer; mod_timer(&backtrace_timer, jiffies + 10); msleep(10); printk("====[ end of backtrace testing ]====\n"); return 0; } static void exitf(void) { } module_init(backtrace_regression_test); module_exit(exitf); MODULE_LICENSE("GPL"); --------------------------------------------------------------------- out put when HAVE_FUNCTION_TRACER is disable ====[ backtrace testing ]=========== Testing a backtrace from process context. The following trace is a kernel self test and not a bug! Testing a backtrace. The following trace is a kernel self test and not a bug! Call Trace: [<80295134>] dump_stack+0x8/0x34 [<c0946060>] backtrace_regression_test+0x60/0x94 [sisc_backtrcae] [<800004f0>] do_one_initcall+0xf0/0x1d0 [<80060954>] sys_init_module+0x19c8/0x1c60 [<8000a418>] stack_done+0x20/0x40 output befor patch when HAVE_FUNCTION_TRACER is enable --------------------------------------------------------------------- VDLinux#> insmod sisc_backtrcae.ko ====[ backtrace testing ]=========== Testing a backtrace from process context. The following trace is a kernel self test and not a bug! Testing a backtrace. The following trace is a kernel self test and not a bug! Call Trace: [<802e5164>] dump_stack+0x1c/0x50 [<802e5164>] dump_stack+0x1c/0x50 ====[ end of backtrace testing ]==== ------------------------------------------------------ above shows the wrong back trcae output after patch when HAVE_FUNCTION_TRACER is enable ---------------------------------------------------------------------- ====[ backtrace testing ]=========== Testing a backtrace from process context. The following trace is a kernel self test and not a bug! Testing a backtrace. The following trace is a kernel self test and not a bug! Call Trace: [<802eb1a4>] dump_stack+0x20/0x54 [<c003405c>] backtrace_test_timer+0x5c/0x74 [sisc_backtrcae] [<c00340dc>] init_module+0x68/0xa0 [sisc_backtrcae] [<80000508>] do_one_initcall+0x108/0x1f0 [<8006d4c4>] sys_init_module+0x1a10/0x1c74 [<8000b038>] stack_done+0x20/0x40 ------------------------------------------------------------------ get_frame_info() is used to fetch the frame information from the function. However, 1. this function just considers the first stack adjustment for frame size. 2. On finding the save_lr instruction, it returns. It doesn't handle the ftrace condition. If Dynamic Frace "CONFIG_DYNAMIC_FTRACE" is enabled, the instrumentation code is: - jal <ftrace_caller> - addiu sp,sp,-8 Thus, the current Frame Size of function is increased by 8 for Ftrace. Signed-off-by: Akhilesh Kumar <akhilesh.lxr@...com> --- arch/mips/kernel/process.c | 67 ++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 67 insertions(+), 0 deletions(-) diff --git a/arch/mips/kernel/process.c b/arch/mips/kernel/process.c index 7955409..df72738 100644 --- a/arch/mips/kernel/process.c +++ b/arch/mips/kernel/process.c @@ -290,12 +290,45 @@ static inline int is_sp_move_ins(union mips_instruction *ip) return 0; } +#ifdef CONFIG_DYNAMIC_FTRACE +/* + * To create the jal <> instruction from mcount. + * taken from: + * - arch/mips/kernel/ftrace.c + */ +#define ADDR_MASK 0x03ffffff /* op_code|addr : 31...26|25 ....0 */ +#define JAL 0x0c000000 /* jump & link: ip --> ra, jump to target */ +#define INSN_JAL(addr) \ + ((unsigned int)(JAL | (((addr) >> 2) & ADDR_MASK))) + +/* + * We assume jal <mcount>/<ftrace_caller> to be present in + * first JAL_MAX_OFFSET instructions. + * Increment this, if its otherwise + */ +#define JAL_MAX_OFFSET 16U +#define MCOUNT_STACK_INST 0x27bdfff8 /* addiu sp,sp,-8 */ + +/* + * If Dynamic Ftrace is enabled, ftrace_caller is the trace function. + * Otherwise its - mcount + */ +extern void ftrace_caller(void); +#endif /* CONFIG_DYNAMIC_FTRACE */ + static int get_frame_info(struct mips_frame_info *info) { union mips_instruction *ip = info->func; unsigned max_insns = info->func_size / sizeof(union mips_instruction); unsigned i; +#ifdef CONFIG_DYNAMIC_FTRACE + unsigned max_prolog_inst = max_insns; + int jal_found = 0; + /* instruction corresponding to jal <_mcount>/<ftrace_caller> */ + int jal_mcount = 0; +#endif + info->pc_offset = -1; info->frame_size = 0; @@ -306,6 +339,28 @@ static int get_frame_info(struct mips_frame_info *info) max_insns = 128U; /* unknown function size */ max_insns = min(128U, max_insns); +#ifdef CONFIG_DYNAMIC_FTRACE + max_prolog_inst = min(JAL_MAX_OFFSET, max_prolog_inst); + jal_mcount = INSN_JAL((unsigned)&ftrace_caller); + + for (i = 0; i < max_prolog_inst; i++, ip++) { + if ((*(int *)ip == jal_mcount) || + /* + * for dyn ftrace, the code initially has 0. + * so we check whether the next instruction is + * addiu sp,sp,-8 + */ + (!(*(int *)ip) && + (*(int *)(ip + 1) == MCOUNT_STACK_INST)) + ) { + jal_found = 1; + break; + } + } + /* restore the ip to start of function */ + ip = info->func; +#endif + for (i = 0; i < max_insns; i++, ip++) { if (is_jal_jalr_jr_ins(ip)) @@ -321,6 +376,18 @@ static int get_frame_info(struct mips_frame_info *info) break; } } +#ifdef CONFIG_DYNAMIC_FTRACE + /* + * to offset the effect of: + * addiu sp,sp,-8 + */ + if (jal_found) { + if (info->frame_size) + info->frame_size += 8; + if (info->pc_offset >= 0) + info->pc_offset += 8 / sizeof(long); + } +#endif if (info->frame_size && info->pc_offset >= 0) /* nested */ return 0; if (info->pc_offset < 0) /* leaf */ -- 1.7.8.4 On Thu, Aug 9, 2012 at 10:46 PM, Akhilesh Kumar <akhilesh.lxr@gmail.com>wrote: > yes Zhangin > > please find the complete patch > ==================================================================== > diff --git a/arch/mips/kernel/process.c b/arch/mips/kernel/process.c > index 7955409..df72738 100644 > --- a/arch/mips/kernel/process.c > +++ b/arch/mips/kernel/process.c > @@ -290,12 +290,45 @@ static inline int is_sp_move_ins(union > mips_instruction *ip) > return 0; > } > > +#ifdef CONFIG_DYNAMIC_FTRACE > +/* > + * To create the jal <> instruction from mcount. > + * taken from: > + * - arch/mips/kernel/ftrace.c > + */ > +#define ADDR_MASK 0x03ffffff /* op_code|addr : 31...26|25 ....0 */ > +#define JAL 0x0c000000 /* jump & link: ip --> ra, jump to target */ > +#define INSN_JAL(addr) \ > + ((unsigned int)(JAL | (((addr) >> 2) & ADDR_MASK))) > + > +/* > + * We assume jal <mcount>/<ftrace_caller> to be present in > + * first JAL_MAX_OFFSET instructions. > + * Increment this, if its otherwise > + */ > +#define JAL_MAX_OFFSET 16U > +#define MCOUNT_STACK_INST 0x27bdfff8 /* addiu sp,sp,-8 */ > + > +/* > + * If Dynamic Ftrace is enabled, ftrace_caller is the trace function. > + * Otherwise its - mcount > + */ > +extern void ftrace_caller(void); > +#endif /* CONFIG_DYNAMIC_FTRACE */ > + > static int get_frame_info(struct mips_frame_info *info) > { > union mips_instruction *ip = info->func; > unsigned max_insns = info->func_size / sizeof(union mips_instruction); > unsigned i; > > +#ifdef CONFIG_DYNAMIC_FTRACE > + unsigned max_prolog_inst = max_insns; > + int jal_found = 0; > + /* instruction corresponding to jal <_mcount>/<ftrace_caller> */ > + int jal_mcount = 0; > +#endif > + > info->pc_offset = -1; > info->frame_size = 0; > > @@ -306,6 +339,28 @@ static int get_frame_info(struct mips_frame_info > *info) > max_insns = 128U; /* unknown function size */ > max_insns = min(128U, max_insns); > > +#ifdef CONFIG_DYNAMIC_FTRACE > + max_prolog_inst = min(JAL_MAX_OFFSET, max_prolog_inst); > + jal_mcount = INSN_JAL((unsigned)&ftrace_caller); > + > + for (i = 0; i < max_prolog_inst; i++, ip++) { > + if ((*(int *)ip == jal_mcount) || > + /* > + * for dyn ftrace, the code initially has 0. > + * so we check whether the next instruction is > + * addiu sp,sp,-8 > + */ > + (!(*(int *)ip) && > + (*(int *)(ip + 1) == MCOUNT_STACK_INST)) > + ) { > + jal_found = 1; > + break; > + } > + } > + /* restore the ip to start of function */ > + ip = info->func; > +#endif > + > for (i = 0; i < max_insns; i++, ip++) { > > if (is_jal_jalr_jr_ins(ip)) > @@ -321,6 +376,18 @@ static int get_frame_info(struct mips_frame_info > *info) > break; > } > } > +#ifdef CONFIG_DYNAMIC_FTRACE > + /* > + * to offset the effect of: > + * addiu sp,sp,-8 > + */ > + if (jal_found) { > + if (info->frame_size) > + info->frame_size += 8; > + if (info->pc_offset >= 0) > + info->pc_offset += 8 / sizeof(long); > + } > +#endif > if (info->frame_size && info->pc_offset >= 0) /* nested */ > return 0; > if (info->pc_offset < 0) /* leaf */ > -- > 1.7.8.4 > > ==================================================================== > > > On Thu, Aug 9, 2012 at 9:41 PM, Wu Zhangjin <wuzhangjin@gmail.com> wrote: > >> Hi, Akhilesh >> >> Thanks very much for your work. >> >> Seems this patch has lost something, can you send a full one? >> >> Best Regards, >> Wu Zhangjin >> >> On Thu, Aug 9, 2012 at 9:53 PM, Akhilesh Kumar <akhilesh.lxr@gmail.com> >> wrote: >> > Hi Ralf, >> > >> > >> > Sub:- Bug unable to retrive backtrace when HAVE_FUNCTION_TRACER is >> enable. >> > I send this bug and bug fix long back, I am resending this patch again >> for >> > review. >> > >> > Please review below patch if you agree I will regenerate this patch and >> with >> > you. >> > >> > ====[ backtrace testing ]=========== >> > Testing a backtrace from process context. >> > The following trace is a kernel self test and not a bug! >> > Testing a backtrace. >> > The following trace is a kernel self test and not a bug! >> > Call Trace: >> > [<80295134>] dump_stack+0x8/0x34 >> > [<c0946060>] backtrace_regression_test+0x60/0x94 [sisc_backtrcae] >> > [<800004f0>] do_one_initcall+0xf0/0x1d0 >> > [<80060954>] sys_init_module+0x19c8/0x1c60 >> > [<8000a418>] stack_done+0x20/0x40 >> > output befor patch when HAVE_FUNCTION_TRACER is enable >> > --------------------------------------------------------------------- >> > #> insmod backtrace.ko >> > ====[ backtrace testing ]=========== >> > Testing a backtrace from process context. >> > The following trace is a kernel self test and not a bug! >> > Testing a backtrace. >> > The following trace is a kernel self test and not a bug! >> > Call Trace: >> > [<802e5164>] dump_stack+0x1c/0x50 >> > [<802e5164>] dump_stack+0x1c/0x50 >> > ====[ end of backtrace testing ]==== >> > ------------------------------------------------------ >> > above shows the wrong back trcae >> > output after patch when HAVE_FUNCTION_TRACER is enable >> > ---------------------------------------------------------------------- >> > ====[ backtrace testing ]=========== >> > Testing a backtrace from process context. >> > The following trace is a kernel self test and not a bug! >> > Testing a backtrace. >> > The following trace is a kernel self test and not a bug! >> > Call Trace: >> > [<802eb1a4>] dump_stack+0x20/0x54 >> > [<c003405c>] backtrace_test_timer+0x5c/0x74 [sisc_backtrcae] >> > [<c00340dc>] init_module+0x68/0xa0 [sisc_backtrcae] >> > [<80000508>] do_one_initcall+0x108/0x1f0 >> > [<8006d4c4>] sys_init_module+0x1a10/0x1c74 >> > [<8000b038>] stack_done+0x20/0x40 >> > ------------------------------------------------------------------ >> > get_frame_info() is used to fetch the frame information from the >> > function. >> > However, >> > 1. this function just considers the first stack adjustment for frame >> > size. >> > 2. On finding the save_lr instruction, it returns. >> > It doesn't handle the ftrace condition. >> > If Dynamic Frace "CONFIG_DYNAMIC_FTRACE" is enabled, the instrumentation >> > code is: >> > - jal <ftrace_caller> >> > - addiu sp,sp,-8 >> > Thus, the current Frame Size of function is increased by 8 for Ftrace. >> > Signed-off-by: Akhilesh Kumar <akhilesh.lxr@gmail.com> >> > --- >> > arch/mips/kernel/process.c | 67 >> > ++++++++++++++++++++++++++++++++++++++++++++ >> > 1 files changed, 67 insertions(+), 0 deletions(-) >> > diff --git a/arch/mips/kernel/process.c b/arch/mips/kernel/process.c >> > index 7955409..df72738 100644 >> > --- a/arch/mips/kernel/process.c >> > +++ b/arch/mips/kernel/process.c >> > @@ -290,12 +290,45 @@ static inline int is_sp_move_ins(union >> > mips_instruction *ip) >> > return 0; >> > } >> > +#ifdef CONFIG_DYNAMIC_FTRACE >> > +/* >> > + * To create the jal <> instruction from mcount. >> > + * taken from: >> > + * - arch/mips/kernel/ftrace.c >> > + */ >> > +#define ADDR_MASK 0x03ffffff /* op_code|addr : 31...26|25 ....0 */ >> > +#define JAL 0x0c000000 /* jump & link: ip --> ra, jump to target >> */ >> > +#define INSN_JAL(addr) \ >> > + ((unsigned int)(JAL | (((addr) >> 2) & ADDR_MASK))) >> > + >> > +/* >> > + * We assume jal <mcount>/<ftrace_caller> to be present in >> > + * first JAL_MAX_OFFSET instructions. >> > + * Increment this, if its otherwise >> > + */ >> > +#define JAL_MAX_OFFSET 16U >> > +#define MCOUNT_STACK_INST 0x27bdfff8 /* addiu sp,sp,-8 */ >> > + >> > +/* >> > + * If Dynamic Ftrace is enabled, ftrace_caller is the trace function. >> > + * Otherwise its - mcount >> > + */ >> > +extern void ftrace_caller(void); >> > +#endif /* CONFIG_DYNAMIC_FTRACE */ >> > + >> > static int get_frame_info(struct mips_frame_info *info) >> > { >> > union mips_instruction *ip = info->func; >> > unsigned max_insns = info->func_size / sizeof(union mips_instruction); >> > unsigned i; >> > +#ifdef CONFIG_DYNAMIC_FTRACE >> > + unsigned max_prolog_inst = max_insns; >> > + int jal_found = 0; >> > + /* instruction corresponding to jal <_mcount>/<ftrace_caller> */ >> > + int jal_mcount = 0; >> > +#endif >> > + >> > info->pc_offset = -1; >> > info->frame_size = 0; >> > > [-- Attachment #2: Type: text/html, Size: 20031 bytes --] ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [Bug-fix] backtrace when HAVE_FUNCTION_TRACER is enable 2012-08-13 15:44 ` Akhilesh Kumar (?) @ 2012-08-13 16:47 ` Ralf Baechle 2012-08-14 11:15 ` Ralf Baechle -1 siblings, 1 reply; 7+ messages in thread From: Ralf Baechle @ 2012-08-13 16:47 UTC (permalink / raw) To: Akhilesh Kumar; +Cc: Wu Zhangjin, linux-mips On Mon, Aug 13, 2012 at 09:14:38PM +0530, Akhilesh Kumar wrote: > Detail Description is added in the patch This patch is garbled and doesn't apply. You can find some hints on how to deal with patch-corrupting mail clients in: http://www.linux-mips.org/wiki/Mailing-patches Ralf ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Bug-fix] backtrace when HAVE_FUNCTION_TRACER is enable 2012-08-13 16:47 ` Ralf Baechle @ 2012-08-14 11:15 ` Ralf Baechle 0 siblings, 0 replies; 7+ messages in thread From: Ralf Baechle @ 2012-08-14 11:15 UTC (permalink / raw) To: Akhilesh Kumar; +Cc: Wu Zhangjin, linux-mips On Mon, Aug 13, 2012 at 06:47:17PM +0200, Ralf Baechle wrote: > > > Detail Description is added in the patch > > This patch is garbled and doesn't apply. You can find some hints on > how to deal with patch-corrupting mail clients in: > > http://www.linux-mips.org/wiki/Mailing-patches Also it appears to be incomplete. I see your patch plenty of macro definitions and function declarations but nothing appears to use them. Ralf ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2012-08-14 11:15 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-08-09 13:53 [Bug-fix] backtrace when HAVE_FUNCTION_TRACER is enable Akhilesh Kumar
[not found] ` <CAD+V5YKZJHKONehvT+-GrKLP2+e0PiLiFTJWEojiDoNyLT3yGQ@mail.gmail.com>
2012-08-09 17:16 ` Akhilesh Kumar
2012-08-12 6:28 ` John Crispin
2012-08-13 15:44 ` Akhilesh Kumar
2012-08-13 15:44 ` Akhilesh Kumar
2012-08-13 16:47 ` Ralf Baechle
2012-08-14 11:15 ` Ralf Baechle
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.