Commit e8e999cf authored by Namhyung Kim's avatar Namhyung Kim Committed by Ingo Molnar

x86, dumpstack: Correct stack dump info when frame pointer is available

Current stack dump code scans entire stack and check each entry
contains a pointer to kernel code. If CONFIG_FRAME_POINTER=y it
could mark whether the pointer is valid or not based on value of
the frame pointer. Invalid entries could be preceded by '?' sign.

However this was not going to happen because scan start point
was always higher than the frame pointer so that they could not
meet.

Commit 9c0729dc ("x86: Eliminate bp argument from the stack
tracing routines") delayed bp acquisition point, so the bp was
read in lower frame, thus all of the entries were marked
invalid.

This patch fixes this by reverting above commit while retaining
stack_frame() helper as suggested by Frederic Weisbecker.

End result looks like below:

before:

 [    3.508329] Call Trace:
 [    3.508551]  [<ffffffff814f35c9>] ? panic+0x91/0x199
 [    3.508662]  [<ffffffff814f3739>] ? printk+0x68/0x6a
 [    3.508770]  [<ffffffff81a981b2>] ? mount_block_root+0x257/0x26e
 [    3.508876]  [<ffffffff81a9821f>] ? mount_root+0x56/0x5a
 [    3.508975]  [<ffffffff81a98393>] ? prepare_namespace+0x170/0x1a9
 [    3.509216]  [<ffffffff81a9772b>] ? kernel_init+0x1d2/0x1e2
 [    3.509335]  [<ffffffff81003894>] ? kernel_thread_helper+0x4/0x10
 [    3.509442]  [<ffffffff814f6880>] ? restore_args+0x0/0x30
 [    3.509542]  [<ffffffff81a97559>] ? kernel_init+0x0/0x1e2
 [    3.509641]  [<ffffffff81003890>] ? kernel_thread_helper+0x0/0x10

after:

 [    3.522991] Call Trace:
 [    3.523351]  [<ffffffff814f35b9>] panic+0x91/0x199
 [    3.523468]  [<ffffffff814f3729>] ? printk+0x68/0x6a
 [    3.523576]  [<ffffffff81a981b2>] mount_block_root+0x257/0x26e
 [    3.523681]  [<ffffffff81a9821f>] mount_root+0x56/0x5a
 [    3.523780]  [<ffffffff81a98393>] prepare_namespace+0x170/0x1a9
 [    3.523885]  [<ffffffff81a9772b>] kernel_init+0x1d2/0x1e2
 [    3.523987]  [<ffffffff81003894>] kernel_thread_helper+0x4/0x10
 [    3.524228]  [<ffffffff814f6880>] ? restore_args+0x0/0x30
 [    3.524345]  [<ffffffff81a97559>] ? kernel_init+0x0/0x1e2
 [    3.524445]  [<ffffffff81003890>] ? kernel_thread_helper+0x0/0x10

 -v5:
   * fix build breakage with oprofile

 -v4:
   * use 0 instead of regs->bp
   * separate out printk changes

 -v3:
   * apply comment from Frederic
   * add a couple of printk fixes
Signed-off-by: default avatarNamhyung Kim <namhyung@gmail.com>
Acked-by: default avatarPeter Zijlstra <a.p.zijlstra@chello.nl>
Acked-by: default avatarFrederic Weisbecker <fweisbec@gmail.com>
Cc: Soren Sandmann <ssp@redhat.com>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Robert Richter <robert.richter@amd.com>
LKML-Reference: <1300416006-3163-1-git-send-email-namhyung@gmail.com>
Signed-off-by: default avatarIngo Molnar <mingo@elte.hu>
parent 2c76397b
...@@ -26,7 +26,7 @@ extern void die(const char *, struct pt_regs *,long); ...@@ -26,7 +26,7 @@ extern void die(const char *, struct pt_regs *,long);
extern int __must_check __die(const char *, struct pt_regs *, long); extern int __must_check __die(const char *, struct pt_regs *, long);
extern void show_registers(struct pt_regs *regs); extern void show_registers(struct pt_regs *regs);
extern void show_trace(struct task_struct *t, struct pt_regs *regs, extern void show_trace(struct task_struct *t, struct pt_regs *regs,
unsigned long *sp); unsigned long *sp, unsigned long bp);
extern void __show_regs(struct pt_regs *regs, int all); extern void __show_regs(struct pt_regs *regs, int all);
extern void show_regs(struct pt_regs *regs); extern void show_regs(struct pt_regs *regs);
extern unsigned long oops_begin(void); extern unsigned long oops_begin(void);
......
...@@ -47,7 +47,7 @@ struct stacktrace_ops { ...@@ -47,7 +47,7 @@ struct stacktrace_ops {
}; };
void dump_trace(struct task_struct *tsk, struct pt_regs *regs, void dump_trace(struct task_struct *tsk, struct pt_regs *regs,
unsigned long *stack, unsigned long *stack, unsigned long bp,
const struct stacktrace_ops *ops, void *data); const struct stacktrace_ops *ops, void *data);
#ifdef CONFIG_X86_32 #ifdef CONFIG_X86_32
...@@ -86,11 +86,11 @@ stack_frame(struct task_struct *task, struct pt_regs *regs) ...@@ -86,11 +86,11 @@ stack_frame(struct task_struct *task, struct pt_regs *regs)
extern void extern void
show_trace_log_lvl(struct task_struct *task, struct pt_regs *regs, show_trace_log_lvl(struct task_struct *task, struct pt_regs *regs,
unsigned long *stack, char *log_lvl); unsigned long *stack, unsigned long bp, char *log_lvl);
extern void extern void
show_stack_log_lvl(struct task_struct *task, struct pt_regs *regs, show_stack_log_lvl(struct task_struct *task, struct pt_regs *regs,
unsigned long *sp, char *log_lvl); unsigned long *sp, unsigned long bp, char *log_lvl);
extern unsigned int code_bytes; extern unsigned int code_bytes;
......
...@@ -1792,7 +1792,7 @@ perf_callchain_kernel(struct perf_callchain_entry *entry, struct pt_regs *regs) ...@@ -1792,7 +1792,7 @@ perf_callchain_kernel(struct perf_callchain_entry *entry, struct pt_regs *regs)
perf_callchain_store(entry, regs->ip); perf_callchain_store(entry, regs->ip);
dump_trace(NULL, regs, NULL, &backtrace_ops, entry); dump_trace(NULL, regs, NULL, 0, &backtrace_ops, entry);
} }
#ifdef CONFIG_COMPAT #ifdef CONFIG_COMPAT
......
...@@ -175,21 +175,21 @@ static const struct stacktrace_ops print_trace_ops = { ...@@ -175,21 +175,21 @@ static const struct stacktrace_ops print_trace_ops = {
void void
show_trace_log_lvl(struct task_struct *task, struct pt_regs *regs, show_trace_log_lvl(struct task_struct *task, struct pt_regs *regs,
unsigned long *stack, char *log_lvl) unsigned long *stack, unsigned long bp, char *log_lvl)
{ {
printk("%sCall Trace:\n", log_lvl); printk("%sCall Trace:\n", log_lvl);
dump_trace(task, regs, stack, &print_trace_ops, log_lvl); dump_trace(task, regs, stack, bp, &print_trace_ops, log_lvl);
} }
void show_trace(struct task_struct *task, struct pt_regs *regs, void show_trace(struct task_struct *task, struct pt_regs *regs,
unsigned long *stack) unsigned long *stack, unsigned long bp)
{ {
show_trace_log_lvl(task, regs, stack, ""); show_trace_log_lvl(task, regs, stack, bp, "");
} }
void show_stack(struct task_struct *task, unsigned long *sp) void show_stack(struct task_struct *task, unsigned long *sp)
{ {
show_stack_log_lvl(task, NULL, sp, ""); show_stack_log_lvl(task, NULL, sp, 0, "");
} }
/* /*
...@@ -197,14 +197,16 @@ void show_stack(struct task_struct *task, unsigned long *sp) ...@@ -197,14 +197,16 @@ void show_stack(struct task_struct *task, unsigned long *sp)
*/ */
void dump_stack(void) void dump_stack(void)
{ {
unsigned long bp;
unsigned long stack; unsigned long stack;
bp = stack_frame(current, NULL);
printk("Pid: %d, comm: %.20s %s %s %.*s\n", printk("Pid: %d, comm: %.20s %s %s %.*s\n",
current->pid, current->comm, print_tainted(), current->pid, current->comm, print_tainted(),
init_utsname()->release, init_utsname()->release,
(int)strcspn(init_utsname()->version, " "), (int)strcspn(init_utsname()->version, " "),
init_utsname()->version); init_utsname()->version);
show_trace(NULL, NULL, &stack); show_trace(NULL, NULL, &stack, bp);
} }
EXPORT_SYMBOL(dump_stack); EXPORT_SYMBOL(dump_stack);
......
...@@ -17,12 +17,11 @@ ...@@ -17,12 +17,11 @@
#include <asm/stacktrace.h> #include <asm/stacktrace.h>
void dump_trace(struct task_struct *task, void dump_trace(struct task_struct *task, struct pt_regs *regs,
struct pt_regs *regs, unsigned long *stack, unsigned long *stack, unsigned long bp,
const struct stacktrace_ops *ops, void *data) const struct stacktrace_ops *ops, void *data)
{ {
int graph = 0; int graph = 0;
unsigned long bp;
if (!task) if (!task)
task = current; task = current;
...@@ -35,7 +34,9 @@ void dump_trace(struct task_struct *task, ...@@ -35,7 +34,9 @@ void dump_trace(struct task_struct *task,
stack = (unsigned long *)task->thread.sp; stack = (unsigned long *)task->thread.sp;
} }
if (!bp)
bp = stack_frame(task, regs); bp = stack_frame(task, regs);
for (;;) { for (;;) {
struct thread_info *context; struct thread_info *context;
...@@ -55,7 +56,7 @@ EXPORT_SYMBOL(dump_trace); ...@@ -55,7 +56,7 @@ EXPORT_SYMBOL(dump_trace);
void void
show_stack_log_lvl(struct task_struct *task, struct pt_regs *regs, show_stack_log_lvl(struct task_struct *task, struct pt_regs *regs,
unsigned long *sp, char *log_lvl) unsigned long *sp, unsigned long bp, char *log_lvl)
{ {
unsigned long *stack; unsigned long *stack;
int i; int i;
...@@ -77,7 +78,7 @@ show_stack_log_lvl(struct task_struct *task, struct pt_regs *regs, ...@@ -77,7 +78,7 @@ show_stack_log_lvl(struct task_struct *task, struct pt_regs *regs,
touch_nmi_watchdog(); touch_nmi_watchdog();
} }
printk(KERN_CONT "\n"); printk(KERN_CONT "\n");
show_trace_log_lvl(task, regs, sp, log_lvl); show_trace_log_lvl(task, regs, sp, bp, log_lvl);
} }
...@@ -102,7 +103,7 @@ void show_registers(struct pt_regs *regs) ...@@ -102,7 +103,7 @@ void show_registers(struct pt_regs *regs)
u8 *ip; u8 *ip;
printk(KERN_EMERG "Stack:\n"); printk(KERN_EMERG "Stack:\n");
show_stack_log_lvl(NULL, regs, &regs->sp, KERN_EMERG); show_stack_log_lvl(NULL, regs, &regs->sp, 0, KERN_EMERG);
printk(KERN_EMERG "Code: "); printk(KERN_EMERG "Code: ");
......
...@@ -139,8 +139,8 @@ fixup_bp_irq_link(unsigned long bp, unsigned long *stack, ...@@ -139,8 +139,8 @@ fixup_bp_irq_link(unsigned long bp, unsigned long *stack,
* severe exception (double fault, nmi, stack fault, debug, mce) hardware stack * severe exception (double fault, nmi, stack fault, debug, mce) hardware stack
*/ */
void dump_trace(struct task_struct *task, void dump_trace(struct task_struct *task, struct pt_regs *regs,
struct pt_regs *regs, unsigned long *stack, unsigned long *stack, unsigned long bp,
const struct stacktrace_ops *ops, void *data) const struct stacktrace_ops *ops, void *data)
{ {
const unsigned cpu = get_cpu(); const unsigned cpu = get_cpu();
...@@ -150,7 +150,6 @@ void dump_trace(struct task_struct *task, ...@@ -150,7 +150,6 @@ void dump_trace(struct task_struct *task,
struct thread_info *tinfo; struct thread_info *tinfo;
int graph = 0; int graph = 0;
unsigned long dummy; unsigned long dummy;
unsigned long bp;
if (!task) if (!task)
task = current; task = current;
...@@ -161,6 +160,7 @@ void dump_trace(struct task_struct *task, ...@@ -161,6 +160,7 @@ void dump_trace(struct task_struct *task,
stack = (unsigned long *)task->thread.sp; stack = (unsigned long *)task->thread.sp;
} }
if (!bp)
bp = stack_frame(task, regs); bp = stack_frame(task, regs);
/* /*
* Print function call entries in all stacks, starting at the * Print function call entries in all stacks, starting at the
...@@ -225,7 +225,7 @@ EXPORT_SYMBOL(dump_trace); ...@@ -225,7 +225,7 @@ EXPORT_SYMBOL(dump_trace);
void void
show_stack_log_lvl(struct task_struct *task, struct pt_regs *regs, show_stack_log_lvl(struct task_struct *task, struct pt_regs *regs,
unsigned long *sp, char *log_lvl) unsigned long *sp, unsigned long bp, char *log_lvl)
{ {
unsigned long *irq_stack_end; unsigned long *irq_stack_end;
unsigned long *irq_stack; unsigned long *irq_stack;
...@@ -269,7 +269,7 @@ show_stack_log_lvl(struct task_struct *task, struct pt_regs *regs, ...@@ -269,7 +269,7 @@ show_stack_log_lvl(struct task_struct *task, struct pt_regs *regs,
preempt_enable(); preempt_enable();
printk(KERN_CONT "\n"); printk(KERN_CONT "\n");
show_trace_log_lvl(task, regs, sp, log_lvl); show_trace_log_lvl(task, regs, sp, bp, log_lvl);
} }
void show_registers(struct pt_regs *regs) void show_registers(struct pt_regs *regs)
...@@ -298,7 +298,7 @@ void show_registers(struct pt_regs *regs) ...@@ -298,7 +298,7 @@ void show_registers(struct pt_regs *regs)
printk(KERN_EMERG "Stack:\n"); printk(KERN_EMERG "Stack:\n");
show_stack_log_lvl(NULL, regs, (unsigned long *)sp, show_stack_log_lvl(NULL, regs, (unsigned long *)sp,
KERN_EMERG); 0, KERN_EMERG);
printk(KERN_EMERG "Code: "); printk(KERN_EMERG "Code: ");
......
...@@ -87,7 +87,7 @@ void exit_thread(void) ...@@ -87,7 +87,7 @@ void exit_thread(void)
void show_regs(struct pt_regs *regs) void show_regs(struct pt_regs *regs)
{ {
show_registers(regs); show_registers(regs);
show_trace(NULL, regs, (unsigned long *)kernel_stack_pointer(regs)); show_trace(NULL, regs, (unsigned long *)kernel_stack_pointer(regs), 0);
} }
void show_regs_common(void) void show_regs_common(void)
......
...@@ -73,7 +73,7 @@ static const struct stacktrace_ops save_stack_ops_nosched = { ...@@ -73,7 +73,7 @@ static const struct stacktrace_ops save_stack_ops_nosched = {
*/ */
void save_stack_trace(struct stack_trace *trace) void save_stack_trace(struct stack_trace *trace)
{ {
dump_trace(current, NULL, NULL, &save_stack_ops, trace); dump_trace(current, NULL, NULL, 0, &save_stack_ops, trace);
if (trace->nr_entries < trace->max_entries) if (trace->nr_entries < trace->max_entries)
trace->entries[trace->nr_entries++] = ULONG_MAX; trace->entries[trace->nr_entries++] = ULONG_MAX;
} }
...@@ -81,14 +81,14 @@ EXPORT_SYMBOL_GPL(save_stack_trace); ...@@ -81,14 +81,14 @@ EXPORT_SYMBOL_GPL(save_stack_trace);
void save_stack_trace_regs(struct stack_trace *trace, struct pt_regs *regs) void save_stack_trace_regs(struct stack_trace *trace, struct pt_regs *regs)
{ {
dump_trace(current, regs, NULL, &save_stack_ops, trace); dump_trace(current, regs, NULL, 0, &save_stack_ops, trace);
if (trace->nr_entries < trace->max_entries) if (trace->nr_entries < trace->max_entries)
trace->entries[trace->nr_entries++] = ULONG_MAX; trace->entries[trace->nr_entries++] = ULONG_MAX;
} }
void save_stack_trace_tsk(struct task_struct *tsk, struct stack_trace *trace) void save_stack_trace_tsk(struct task_struct *tsk, struct stack_trace *trace)
{ {
dump_trace(tsk, NULL, NULL, &save_stack_ops_nosched, trace); dump_trace(tsk, NULL, NULL, 0, &save_stack_ops_nosched, trace);
if (trace->nr_entries < trace->max_entries) if (trace->nr_entries < trace->max_entries)
trace->entries[trace->nr_entries++] = ULONG_MAX; trace->entries[trace->nr_entries++] = ULONG_MAX;
} }
......
...@@ -126,7 +126,7 @@ x86_backtrace(struct pt_regs * const regs, unsigned int depth) ...@@ -126,7 +126,7 @@ x86_backtrace(struct pt_regs * const regs, unsigned int depth)
if (!user_mode_vm(regs)) { if (!user_mode_vm(regs)) {
unsigned long stack = kernel_stack_pointer(regs); unsigned long stack = kernel_stack_pointer(regs);
if (depth) if (depth)
dump_trace(NULL, regs, (unsigned long *)stack, dump_trace(NULL, regs, (unsigned long *)stack, 0,
&backtrace_ops, &depth); &backtrace_ops, &depth);
return; return;
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment