* [PATCH] Fix show_regs to give a backtrace @ 2011-08-24 17:21 Laura Abbott 2011-08-24 17:21 ` [PATCH 1/2] arm: add dump_stack_regs to dump stack from registers Laura Abbott 2011-08-24 17:21 ` [PATCH 2/2] process: change from __backtrace to dump_stack_regs in show_regs Laura Abbott 0 siblings, 2 replies; 6+ messages in thread From: Laura Abbott @ 2011-08-24 17:21 UTC (permalink / raw) To: linux-arm-kernel Currently, show_regs will not give a backtrace unless frame pointer is enabled. show_regs calls __backtrace which does not support the arm unwind tables and returns nothing if CONFIG_FRAME_POINTER is not enabled. Having a useful backtrace when dumping registers is helpful, so fix show_regs to call into dump_backtrace which properly handles both the CONFIG_FRAME_POINTER and the CONFIG_ARM_UNWIND case for backtraces. ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/2] arm: add dump_stack_regs to dump stack from registers 2011-08-24 17:21 [PATCH] Fix show_regs to give a backtrace Laura Abbott @ 2011-08-24 17:21 ` Laura Abbott 2011-08-24 17:21 ` [PATCH 2/2] process: change from __backtrace to dump_stack_regs in show_regs Laura Abbott 1 sibling, 0 replies; 6+ messages in thread From: Laura Abbott @ 2011-08-24 17:21 UTC (permalink / raw) To: linux-arm-kernel Currently, there is no API to be able to dump a stack from registers. Use dump_backtrace to walk the stack with a given set of registers. Signed-off-by: Laura Abbott <lauraa@codeaurora.org> --- arch/arm/include/asm/stacktrace.h | 4 ++++ arch/arm/kernel/traps.c | 7 +++++++ 2 files changed, 11 insertions(+), 0 deletions(-) diff --git a/arch/arm/include/asm/stacktrace.h b/arch/arm/include/asm/stacktrace.h index 4d0a164..614b017 100644 --- a/arch/arm/include/asm/stacktrace.h +++ b/arch/arm/include/asm/stacktrace.h @@ -1,6 +1,8 @@ #ifndef __ASM_STACKTRACE_H #define __ASM_STACKTRACE_H +#include <asm/system.h> + struct stackframe { unsigned long fp; unsigned long sp; @@ -12,4 +14,6 @@ extern int unwind_frame(struct stackframe *frame); extern void walk_stackframe(struct stackframe *frame, int (*fn)(struct stackframe *, void *), void *data); +void dump_stack_regs(struct pt_regs *regs); + #endif /* __ASM_STACKTRACE_H */ diff --git a/arch/arm/kernel/traps.c b/arch/arm/kernel/traps.c index bc9f9da..983ba41 100644 --- a/arch/arm/kernel/traps.c +++ b/arch/arm/kernel/traps.c @@ -209,6 +209,13 @@ void dump_stack(void) EXPORT_SYMBOL(dump_stack); + +void dump_stack_regs(struct pt_regs *regs) +{ + dump_backtrace(regs, NULL); +} +EXPORT_SYMBOL(dump_stack_regs); + void show_stack(struct task_struct *tsk, unsigned long *sp) { dump_backtrace(NULL, tsk); -- 1.7.3.3 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] process: change from __backtrace to dump_stack_regs in show_regs 2011-08-24 17:21 [PATCH] Fix show_regs to give a backtrace Laura Abbott 2011-08-24 17:21 ` [PATCH 1/2] arm: add dump_stack_regs to dump stack from registers Laura Abbott @ 2011-08-24 17:21 ` Laura Abbott 2011-08-24 18:57 ` Nicolas Pitre 1 sibling, 1 reply; 6+ messages in thread From: Laura Abbott @ 2011-08-24 17:21 UTC (permalink / raw) To: linux-arm-kernel Currently, show_regs calls __backtrace which does nothing if CONIFG_FRAME_POINTER is not set. Switch to dump_stack_regs to show the backtrace from a given set of registers. Signed-off-by: Laura Abbott <lauraa@codeaurora.org> --- arch/arm/kernel/process.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/arch/arm/kernel/process.c b/arch/arm/kernel/process.c index 1a347f4..6f11213 100644 --- a/arch/arm/kernel/process.c +++ b/arch/arm/kernel/process.c @@ -319,7 +319,7 @@ void show_regs(struct pt_regs * regs) printk("\n"); printk("Pid: %d, comm: %20s\n", task_pid_nr(current), current->comm); __show_regs(regs); - __backtrace(); + dump_stack_regs(regs); } ATOMIC_NOTIFIER_HEAD(thread_notify_head); -- 1.7.3.3 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] process: change from __backtrace to dump_stack_regs in show_regs 2011-08-24 17:21 ` [PATCH 2/2] process: change from __backtrace to dump_stack_regs in show_regs Laura Abbott @ 2011-08-24 18:57 ` Nicolas Pitre 2011-08-24 20:58 ` Laura Abbott 0 siblings, 1 reply; 6+ messages in thread From: Nicolas Pitre @ 2011-08-24 18:57 UTC (permalink / raw) To: linux-arm-kernel On Wed, 24 Aug 2011, Laura Abbott wrote: > Currently, show_regs calls __backtrace which does > nothing if CONIFG_FRAME_POINTER is not set. Switch to s/CONIFG/CONFIG/ > dump_stack_regs to show the backtrace from a given > set of registers. > > Signed-off-by: Laura Abbott <lauraa@codeaurora.org> > --- > arch/arm/kernel/process.c | 2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/arch/arm/kernel/process.c b/arch/arm/kernel/process.c > index 1a347f4..6f11213 100644 > --- a/arch/arm/kernel/process.c > +++ b/arch/arm/kernel/process.c > @@ -319,7 +319,7 @@ void show_regs(struct pt_regs * regs) > printk("\n"); > printk("Pid: %d, comm: %20s\n", task_pid_nr(current), current->comm); > __show_regs(regs); > - __backtrace(); > + dump_stack_regs(regs); This is not equivalent. A call to __backtrace() will display a call trace leading to this show_regs(). Given that you defined dump_stack_regs(regs) as dump_backtrace(regs, NULL), the trace will instead contain calls leading up to the point where those regs were saved. A better way to achieve what you want would be: - remove the __backtrace entry point from lib/backtrace.S - define __backtrace() in kernel/traps.c as a call to dump_backtrace(NULL, NULL) which will have the same effect as the assembly code for __backtrace(). - realize that dump_stack() is already doing exactly that, therefore... - use dump_stack() in show_regs() which is the only caller of __backtrace and get rid of __backtrace entirely. Obviously the above can be collapsed in one patch. Nicolas ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2/2] process: change from __backtrace to dump_stack_regs in show_regs 2011-08-24 18:57 ` Nicolas Pitre @ 2011-08-24 20:58 ` Laura Abbott 2011-08-24 23:27 ` Nicolas Pitre 0 siblings, 1 reply; 6+ messages in thread From: Laura Abbott @ 2011-08-24 20:58 UTC (permalink / raw) To: linux-arm-kernel On Wed, August 24, 2011 11:57 am, Nicolas Pitre wrote: > On Wed, 24 Aug 2011, Laura Abbott wrote: > >> Currently, show_regs calls __backtrace which does >> nothing if CONIFG_FRAME_POINTER is not set. Switch to > > s/CONIFG/CONFIG/ > >> dump_stack_regs to show the backtrace from a given >> set of registers. >> >> Signed-off-by: Laura Abbott <lauraa@codeaurora.org> >> --- >> arch/arm/kernel/process.c | 2 +- >> 1 files changed, 1 insertions(+), 1 deletions(-) >> >> diff --git a/arch/arm/kernel/process.c b/arch/arm/kernel/process.c >> index 1a347f4..6f11213 100644 >> --- a/arch/arm/kernel/process.c >> +++ b/arch/arm/kernel/process.c >> @@ -319,7 +319,7 @@ void show_regs(struct pt_regs * regs) >> printk("\n"); >> printk("Pid: %d, comm: %20s\n", task_pid_nr(current), current->comm); >> __show_regs(regs); >> - __backtrace(); >> + dump_stack_regs(regs); > > This is not equivalent. A call to __backtrace() will display a call > trace leading to this show_regs(). Given that you defined > dump_stack_regs(regs) as dump_backtrace(regs, NULL), the trace will > instead contain calls leading up to the point where those regs were > saved. > Do other architectures show the call stack to show_regs? From my reading of other code (x86, ppc) it seems like if show_regs generates a call stack it generates a call stack based off of the saved registers unless I'm drastically misunderstanding the code. Generating a call stack based off of not saved registers seems inconsistent with the rest of show_regs which shows saved state. > A better way to achieve what you want would be: > > - remove the __backtrace entry point from lib/backtrace.S > > - define __backtrace() in kernel/traps.c as a call to > dump_backtrace(NULL, NULL) which will have the same effect as the > assembly code for __backtrace(). > > - realize that dump_stack() is already doing exactly that, therefore... > > - use dump_stack() in show_regs() which is the only caller of > __backtrace and get rid of __backtrace entirely. > > Obviously the above can be collapsed in one patch. > > > Nicolas > Laura -- Sent by an employee of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum. ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2/2] process: change from __backtrace to dump_stack_regs in show_regs 2011-08-24 20:58 ` Laura Abbott @ 2011-08-24 23:27 ` Nicolas Pitre 0 siblings, 0 replies; 6+ messages in thread From: Nicolas Pitre @ 2011-08-24 23:27 UTC (permalink / raw) To: linux-arm-kernel On Wed, 24 Aug 2011, Laura Abbott wrote: > > On Wed, August 24, 2011 11:57 am, Nicolas Pitre wrote: > > On Wed, 24 Aug 2011, Laura Abbott wrote: > > > >> Currently, show_regs calls __backtrace which does > >> nothing if CONIFG_FRAME_POINTER is not set. Switch to > > > > s/CONIFG/CONFIG/ > > > >> dump_stack_regs to show the backtrace from a given > >> set of registers. > >> > >> Signed-off-by: Laura Abbott <lauraa@codeaurora.org> > >> --- > >> arch/arm/kernel/process.c | 2 +- > >> 1 files changed, 1 insertions(+), 1 deletions(-) > >> > >> diff --git a/arch/arm/kernel/process.c b/arch/arm/kernel/process.c > >> index 1a347f4..6f11213 100644 > >> --- a/arch/arm/kernel/process.c > >> +++ b/arch/arm/kernel/process.c > >> @@ -319,7 +319,7 @@ void show_regs(struct pt_regs * regs) > >> printk("\n"); > >> printk("Pid: %d, comm: %20s\n", task_pid_nr(current), current->comm); > >> __show_regs(regs); > >> - __backtrace(); > >> + dump_stack_regs(regs); > > > > This is not equivalent. A call to __backtrace() will display a call > > trace leading to this show_regs(). Given that you defined > > dump_stack_regs(regs) as dump_backtrace(regs, NULL), the trace will > > instead contain calls leading up to the point where those regs were > > saved. > > > > Do other architectures show the call stack to show_regs? From my reading > of other code (x86, ppc) it seems like if show_regs generates a call stack > it generates a call stack based off of the saved registers unless I'm > drastically misunderstanding the code. Generating a call stack based off > of not saved registers seems inconsistent with the rest of show_regs which > shows saved state. Well, that might be true, and I'm not disputing that. However this change in behavior wasn't mentioned in your patch log. I'd still recommend making an initial patch the way I outlined so a kernel using unwind tables would produce the same trace results as a kernel with frame pointers always did. That part is pretty much uncontroversial. Then it might be good to have a second patch changing the actual trace content, and discussing the merits of such a change in the patch log. This issue should be independent of the backtrace mechanism used. Nicolas ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2011-08-24 23:27 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2011-08-24 17:21 [PATCH] Fix show_regs to give a backtrace Laura Abbott 2011-08-24 17:21 ` [PATCH 1/2] arm: add dump_stack_regs to dump stack from registers Laura Abbott 2011-08-24 17:21 ` [PATCH 2/2] process: change from __backtrace to dump_stack_regs in show_regs Laura Abbott 2011-08-24 18:57 ` Nicolas Pitre 2011-08-24 20:58 ` Laura Abbott 2011-08-24 23:27 ` Nicolas Pitre
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.