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 9c0729dc8062 ("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
Signed-off-by: Namhyung Kim <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Soren Sandmann <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Arnaldo Carvalho de Melo <[email protected]>
---
v4:
* use 0 instead of regs->bp
* separate out printk changes
v3:
* apply comment from Frederic
* add a couple of printk fixes
arch/x86/include/asm/kdebug.h | 2 +-
arch/x86/include/asm/stacktrace.h | 6 +++---
arch/x86/kernel/cpu/perf_event.c | 2 +-
arch/x86/kernel/dumpstack.c | 14 ++++++++------
arch/x86/kernel/dumpstack_32.c | 15 ++++++++-------
arch/x86/kernel/dumpstack_64.c | 14 +++++++-------
arch/x86/kernel/process.c | 2 +-
arch/x86/kernel/stacktrace.c | 6 +++---
8 files changed, 32 insertions(+), 29 deletions(-)
diff --git a/arch/x86/include/asm/kdebug.h b/arch/x86/include/asm/kdebug.h
index ca242d35e873..e28ec43a0737 100644
--- a/arch/x86/include/asm/kdebug.h
+++ b/arch/x86/include/asm/kdebug.h
@@ -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 void show_registers(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);
extern unsigned long oops_begin(void);
diff --git a/arch/x86/include/asm/stacktrace.h b/arch/x86/include/asm/stacktrace.h
index 52b5c7ed3608..d7e89c83645d 100644
--- a/arch/x86/include/asm/stacktrace.h
+++ b/arch/x86/include/asm/stacktrace.h
@@ -47,7 +47,7 @@ struct stacktrace_ops {
};
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);
#ifdef CONFIG_X86_32
@@ -86,11 +86,11 @@ stack_frame(struct task_struct *task, struct pt_regs *regs)
extern void
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
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;
diff --git a/arch/x86/kernel/cpu/perf_event.c b/arch/x86/kernel/cpu/perf_event.c
index 9d977a2ea693..d19cdc2d602e 100644
--- a/arch/x86/kernel/cpu/perf_event.c
+++ b/arch/x86/kernel/cpu/perf_event.c
@@ -1792,7 +1792,7 @@ perf_callchain_kernel(struct perf_callchain_entry *entry, struct pt_regs *regs)
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
diff --git a/arch/x86/kernel/dumpstack.c b/arch/x86/kernel/dumpstack.c
index df20723a6a1b..c330160b6da3 100644
--- a/arch/x86/kernel/dumpstack.c
+++ b/arch/x86/kernel/dumpstack.c
@@ -175,21 +175,21 @@ static const struct stacktrace_ops print_trace_ops = {
void
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);
- 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,
- 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)
{
- 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)
*/
void dump_stack(void)
{
+ unsigned long bp;
unsigned long stack;
+ bp = stack_frame(current, NULL);
printk("Pid: %d, comm: %.20s %s %s %.*s\n",
current->pid, current->comm, print_tainted(),
init_utsname()->release,
(int)strcspn(init_utsname()->version, " "),
init_utsname()->version);
- show_trace(NULL, NULL, &stack);
+ show_trace(NULL, NULL, &stack, bp);
}
EXPORT_SYMBOL(dump_stack);
diff --git a/arch/x86/kernel/dumpstack_32.c b/arch/x86/kernel/dumpstack_32.c
index 74cc1eda384b..3b97a80ce329 100644
--- a/arch/x86/kernel/dumpstack_32.c
+++ b/arch/x86/kernel/dumpstack_32.c
@@ -17,12 +17,11 @@
#include <asm/stacktrace.h>
-void dump_trace(struct task_struct *task,
- struct pt_regs *regs, unsigned long *stack,
+void dump_trace(struct task_struct *task, struct pt_regs *regs,
+ unsigned long *stack, unsigned long bp,
const struct stacktrace_ops *ops, void *data)
{
int graph = 0;
- unsigned long bp;
if (!task)
task = current;
@@ -35,7 +34,9 @@ void dump_trace(struct task_struct *task,
stack = (unsigned long *)task->thread.sp;
}
- bp = stack_frame(task, regs);
+ if (!bp)
+ bp = stack_frame(task, regs);
+
for (;;) {
struct thread_info *context;
@@ -55,7 +56,7 @@ EXPORT_SYMBOL(dump_trace);
void
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;
int i;
@@ -77,7 +78,7 @@ show_stack_log_lvl(struct task_struct *task, struct pt_regs *regs,
touch_nmi_watchdog();
}
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)
u8 *ip;
printk(KERN_EMERG "Stack:\n");
- show_stack_log_lvl(NULL, regs, ®s->sp, KERN_EMERG);
+ show_stack_log_lvl(NULL, regs, ®s->sp, 0, KERN_EMERG);
printk(KERN_EMERG "Code: ");
diff --git a/arch/x86/kernel/dumpstack_64.c b/arch/x86/kernel/dumpstack_64.c
index a6b6fcf7f0ae..e71c98d3c0d2 100644
--- a/arch/x86/kernel/dumpstack_64.c
+++ b/arch/x86/kernel/dumpstack_64.c
@@ -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
*/
-void dump_trace(struct task_struct *task,
- struct pt_regs *regs, unsigned long *stack,
+void dump_trace(struct task_struct *task, struct pt_regs *regs,
+ unsigned long *stack, unsigned long bp,
const struct stacktrace_ops *ops, void *data)
{
const unsigned cpu = get_cpu();
@@ -150,7 +150,6 @@ void dump_trace(struct task_struct *task,
struct thread_info *tinfo;
int graph = 0;
unsigned long dummy;
- unsigned long bp;
if (!task)
task = current;
@@ -161,7 +160,8 @@ void dump_trace(struct task_struct *task,
stack = (unsigned long *)task->thread.sp;
}
- bp = stack_frame(task, regs);
+ if (!bp)
+ bp = stack_frame(task, regs);
/*
* Print function call entries in all stacks, starting at the
* current stack address. If the stacks consist of nested
@@ -225,7 +225,7 @@ EXPORT_SYMBOL(dump_trace);
void
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;
@@ -269,7 +269,7 @@ show_stack_log_lvl(struct task_struct *task, struct pt_regs *regs,
preempt_enable();
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)
@@ -298,7 +298,7 @@ void show_registers(struct pt_regs *regs)
printk(KERN_EMERG "Stack:\n");
show_stack_log_lvl(NULL, regs, (unsigned long *)sp,
- KERN_EMERG);
+ 0, KERN_EMERG);
printk(KERN_EMERG "Code: ");
diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
index ff4554198981..f4c6cc40168e 100644
--- a/arch/x86/kernel/process.c
+++ b/arch/x86/kernel/process.c
@@ -86,7 +86,7 @@ void exit_thread(void)
void show_regs(struct pt_regs *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)
diff --git a/arch/x86/kernel/stacktrace.c b/arch/x86/kernel/stacktrace.c
index 938c8e10a19a..6515733a289d 100644
--- a/arch/x86/kernel/stacktrace.c
+++ b/arch/x86/kernel/stacktrace.c
@@ -73,7 +73,7 @@ static const struct stacktrace_ops save_stack_ops_nosched = {
*/
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)
trace->entries[trace->nr_entries++] = ULONG_MAX;
}
@@ -81,14 +81,14 @@ EXPORT_SYMBOL_GPL(save_stack_trace);
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)
trace->entries[trace->nr_entries++] = ULONG_MAX;
}
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)
trace->entries[trace->nr_entries++] = ULONG_MAX;
}
--
1.7.4
Use KERN_CONT level if messages are continued from previous one.
Also remove a pair of angle brackets on EOE for consistency.
Signed-off-by: Namhyung Kim <[email protected]>
---
arch/x86/kernel/dumpstack_32.c | 8 ++++----
arch/x86/kernel/dumpstack_64.c | 10 +++++-----
2 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/arch/x86/kernel/dumpstack_32.c b/arch/x86/kernel/dumpstack_32.c
index 3b97a80ce329..c99f9ed013d5 100644
--- a/arch/x86/kernel/dumpstack_32.c
+++ b/arch/x86/kernel/dumpstack_32.c
@@ -116,16 +116,16 @@ void show_registers(struct pt_regs *regs)
for (i = 0; i < code_len; i++, ip++) {
if (ip < (u8 *)PAGE_OFFSET ||
probe_kernel_address(ip, c)) {
- printk(" Bad EIP value.");
+ printk(KERN_CONT " Bad EIP value.");
break;
}
if (ip == (u8 *)regs->ip)
- printk("<%02x> ", c);
+ printk(KERN_CONT "<%02x> ", c);
else
- printk("%02x ", c);
+ printk(KERN_CONT "%02x ", c);
}
}
- printk("\n");
+ printk(KERN_CONT "\n");
}
int is_valid_bugaddr(unsigned long ip)
diff --git a/arch/x86/kernel/dumpstack_64.c b/arch/x86/kernel/dumpstack_64.c
index e71c98d3c0d2..50aa303f6611 100644
--- a/arch/x86/kernel/dumpstack_64.c
+++ b/arch/x86/kernel/dumpstack_64.c
@@ -180,7 +180,7 @@ void dump_trace(struct task_struct *task, struct pt_regs *regs,
bp = ops->walk_stack(tinfo, stack, bp, ops,
data, estack_end, &graph);
- ops->stack(data, "<EOE>");
+ ops->stack(data, "EOE");
/*
* We link to the next stack via the
* second-to-last pointer (index -2 to end) in the
@@ -311,16 +311,16 @@ void show_registers(struct pt_regs *regs)
for (i = 0; i < code_len; i++, ip++) {
if (ip < (u8 *)PAGE_OFFSET ||
probe_kernel_address(ip, c)) {
- printk(" Bad RIP value.");
+ printk(KERN_CONT " Bad RIP value.");
break;
}
if (ip == (u8 *)regs->ip)
- printk("<%02x> ", c);
+ printk(KERN_CONT "<%02x> ", c);
else
- printk("%02x ", c);
+ printk(KERN_CONT "%02x ", c);
}
}
- printk("\n");
+ printk(KERN_CONT "\n");
}
int is_valid_bugaddr(unsigned long ip)
--
1.7.4
print_context_stack* and print_ftrace_graph_addr are misnomer.
They don't print anything by themselves and calls appropriate
callback routines. Actually save_stack_ops* use them just for
saving return addresses not for printing.
Rename them to walk_context_stack* will make more sense IMHO.
Signed-off-by: Namhyung Kim <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Arnaldo Carvalho de Melo <[email protected]>
Cc: Robert Richter <[email protected]>
---
arch/x86/include/asm/stacktrace.h | 4 ++--
arch/x86/kernel/cpu/perf_event.c | 2 +-
arch/x86/kernel/dumpstack.c | 18 +++++++++---------
arch/x86/kernel/stacktrace.c | 4 ++--
arch/x86/oprofile/backtrace.c | 2 +-
5 files changed, 15 insertions(+), 15 deletions(-)
diff --git a/arch/x86/include/asm/stacktrace.h b/arch/x86/include/asm/stacktrace.h
index d7e89c83645d..73fc8e2c4872 100644
--- a/arch/x86/include/asm/stacktrace.h
+++ b/arch/x86/include/asm/stacktrace.h
@@ -23,13 +23,13 @@ typedef unsigned long (*walk_stack_t)(struct thread_info *tinfo,
int *graph);
extern unsigned long
-print_context_stack(struct thread_info *tinfo,
+walk_context_stack(struct thread_info *tinfo,
unsigned long *stack, unsigned long bp,
const struct stacktrace_ops *ops, void *data,
unsigned long *end, int *graph);
extern unsigned long
-print_context_stack_bp(struct thread_info *tinfo,
+walk_context_stack_bp(struct thread_info *tinfo,
unsigned long *stack, unsigned long bp,
const struct stacktrace_ops *ops, void *data,
unsigned long *end, int *graph);
diff --git a/arch/x86/kernel/cpu/perf_event.c b/arch/x86/kernel/cpu/perf_event.c
index d19cdc2d602e..9237e83ccaa1 100644
--- a/arch/x86/kernel/cpu/perf_event.c
+++ b/arch/x86/kernel/cpu/perf_event.c
@@ -1779,7 +1779,7 @@ static const struct stacktrace_ops backtrace_ops = {
.warning_symbol = backtrace_warning_symbol,
.stack = backtrace_stack,
.address = backtrace_address,
- .walk_stack = print_context_stack_bp,
+ .walk_stack = walk_context_stack_bp,
};
void
diff --git a/arch/x86/kernel/dumpstack.c b/arch/x86/kernel/dumpstack.c
index c330160b6da3..38b74a5e59aba 100644
--- a/arch/x86/kernel/dumpstack.c
+++ b/arch/x86/kernel/dumpstack.c
@@ -33,7 +33,7 @@ void printk_address(unsigned long address, int reliable)
#ifdef CONFIG_FUNCTION_GRAPH_TRACER
static void
-print_ftrace_graph_addr(unsigned long addr, void *data,
+walk_ftrace_graph_addr(unsigned long addr, void *data,
const struct stacktrace_ops *ops,
struct thread_info *tinfo, int *graph)
{
@@ -56,7 +56,7 @@ print_ftrace_graph_addr(unsigned long addr, void *data,
}
#else
static inline void
-print_ftrace_graph_addr(unsigned long addr, void *data,
+walk_ftrace_graph_addr(unsigned long addr, void *data,
const struct stacktrace_ops *ops,
struct thread_info *tinfo, int *graph)
{ }
@@ -83,7 +83,7 @@ static inline int valid_stack_ptr(struct thread_info *tinfo,
}
unsigned long
-print_context_stack(struct thread_info *tinfo,
+walk_context_stack(struct thread_info *tinfo,
unsigned long *stack, unsigned long bp,
const struct stacktrace_ops *ops, void *data,
unsigned long *end, int *graph)
@@ -102,16 +102,16 @@ print_context_stack(struct thread_info *tinfo,
} else {
ops->address(data, addr, 0);
}
- print_ftrace_graph_addr(addr, data, ops, tinfo, graph);
+ walk_ftrace_graph_addr(addr, data, ops, tinfo, graph);
}
stack++;
}
return bp;
}
-EXPORT_SYMBOL_GPL(print_context_stack);
+EXPORT_SYMBOL_GPL(walk_context_stack);
unsigned long
-print_context_stack_bp(struct thread_info *tinfo,
+walk_context_stack_bp(struct thread_info *tinfo,
unsigned long *stack, unsigned long bp,
const struct stacktrace_ops *ops, void *data,
unsigned long *end, int *graph)
@@ -128,12 +128,12 @@ print_context_stack_bp(struct thread_info *tinfo,
ops->address(data, addr, 1);
frame = frame->next_frame;
ret_addr = &frame->return_address;
- print_ftrace_graph_addr(addr, data, ops, tinfo, graph);
+ walk_ftrace_graph_addr(addr, data, ops, tinfo, graph);
}
return (unsigned long)frame;
}
-EXPORT_SYMBOL_GPL(print_context_stack_bp);
+EXPORT_SYMBOL_GPL(walk_context_stack_bp);
static void
@@ -170,7 +170,7 @@ static const struct stacktrace_ops print_trace_ops = {
.warning_symbol = print_trace_warning_symbol,
.stack = print_trace_stack,
.address = print_trace_address,
- .walk_stack = print_context_stack,
+ .walk_stack = walk_context_stack,
};
void
diff --git a/arch/x86/kernel/stacktrace.c b/arch/x86/kernel/stacktrace.c
index 6515733a289d..2e44ee13637f 100644
--- a/arch/x86/kernel/stacktrace.c
+++ b/arch/x86/kernel/stacktrace.c
@@ -57,7 +57,7 @@ static const struct stacktrace_ops save_stack_ops = {
.warning_symbol = save_stack_warning_symbol,
.stack = save_stack_stack,
.address = save_stack_address,
- .walk_stack = print_context_stack,
+ .walk_stack = walk_context_stack,
};
static const struct stacktrace_ops save_stack_ops_nosched = {
@@ -65,7 +65,7 @@ static const struct stacktrace_ops save_stack_ops_nosched = {
.warning_symbol = save_stack_warning_symbol,
.stack = save_stack_stack,
.address = save_stack_address_nosched,
- .walk_stack = print_context_stack,
+ .walk_stack = walk_context_stack,
};
/*
diff --git a/arch/x86/oprofile/backtrace.c b/arch/x86/oprofile/backtrace.c
index 72cbec14d783..f4b9fbb8de2b 100644
--- a/arch/x86/oprofile/backtrace.c
+++ b/arch/x86/oprofile/backtrace.c
@@ -46,7 +46,7 @@ static struct stacktrace_ops backtrace_ops = {
.warning_symbol = backtrace_warning_symbol,
.stack = backtrace_stack,
.address = backtrace_address,
- .walk_stack = print_context_stack,
+ .walk_stack = walk_context_stack,
};
#ifdef CONFIG_COMPAT
--
1.7.4
We now have CONFIG_FRAME_POINTER=y by default, it would be better
use the frame pointer for the stack backtrace rather than scanning
whole stack blindly.
Signed-off-by: Namhyung Kim <[email protected]>
---
arch/x86/kernel/dumpstack.c | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/arch/x86/kernel/dumpstack.c b/arch/x86/kernel/dumpstack.c
index 38b74a5e59aba..56db27d36858 100644
--- a/arch/x86/kernel/dumpstack.c
+++ b/arch/x86/kernel/dumpstack.c
@@ -170,7 +170,11 @@ static const struct stacktrace_ops print_trace_ops = {
.warning_symbol = print_trace_warning_symbol,
.stack = print_trace_stack,
.address = print_trace_address,
+#ifdef CONFIG_FRAME_POINTER
+ .walk_stack = walk_context_stack_bp,
+#else
.walk_stack = walk_context_stack,
+#endif
};
void
--
1.7.4
Commit-ID: 074206a787e4dfdc4c290789ab6604c7f9e691ca
Gitweb: http://git.kernel.org/tip/074206a787e4dfdc4c290789ab6604c7f9e691ca
Author: Namhyung Kim <[email protected]>
AuthorDate: Tue, 8 Mar 2011 20:44:19 +0900
Committer: Thomas Gleixner <[email protected]>
CommitDate: Thu, 10 Mar 2011 23:20:30 +0100
x86, dumpstack: Correct stack dump info when frame pointer is available
Current stack dump code scans entire stack and checks each entry for 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 9c0729dc8062 ("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
Signed-off-by: Namhyung Kim <[email protected]>
Cc: Soren Sandmann <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Arnaldo Carvalho de Melo <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
LKML-Reference: <[email protected]>
Signed-off-by: Thomas Gleixner <[email protected]>
---
arch/x86/include/asm/kdebug.h | 2 +-
arch/x86/include/asm/stacktrace.h | 6 +++---
arch/x86/kernel/cpu/perf_event.c | 2 +-
arch/x86/kernel/dumpstack.c | 14 ++++++++------
arch/x86/kernel/dumpstack_32.c | 15 ++++++++-------
arch/x86/kernel/dumpstack_64.c | 14 +++++++-------
arch/x86/kernel/process.c | 2 +-
arch/x86/kernel/stacktrace.c | 6 +++---
8 files changed, 32 insertions(+), 29 deletions(-)
diff --git a/arch/x86/include/asm/kdebug.h b/arch/x86/include/asm/kdebug.h
index ca242d3..e28ec43 100644
--- a/arch/x86/include/asm/kdebug.h
+++ b/arch/x86/include/asm/kdebug.h
@@ -27,7 +27,7 @@ extern void 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_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);
extern unsigned long oops_begin(void);
diff --git a/arch/x86/include/asm/stacktrace.h b/arch/x86/include/asm/stacktrace.h
index 52b5c7e..d7e89c8 100644
--- a/arch/x86/include/asm/stacktrace.h
+++ b/arch/x86/include/asm/stacktrace.h
@@ -47,7 +47,7 @@ struct stacktrace_ops {
};
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);
#ifdef CONFIG_X86_32
@@ -86,11 +86,11 @@ stack_frame(struct task_struct *task, struct pt_regs *regs)
extern void
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
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;
diff --git a/arch/x86/kernel/cpu/perf_event.c b/arch/x86/kernel/cpu/perf_event.c
index 9d977a2..d19cdc2 100644
--- a/arch/x86/kernel/cpu/perf_event.c
+++ b/arch/x86/kernel/cpu/perf_event.c
@@ -1710,7 +1710,7 @@ perf_callchain_kernel(struct perf_callchain_entry *entry, struct pt_regs *regs)
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
diff --git a/arch/x86/kernel/dumpstack.c b/arch/x86/kernel/dumpstack.c
index df20723..c330160 100644
--- a/arch/x86/kernel/dumpstack.c
+++ b/arch/x86/kernel/dumpstack.c
@@ -175,21 +175,21 @@ static const struct stacktrace_ops print_trace_ops = {
void
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);
- 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,
- 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)
{
- 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)
*/
void dump_stack(void)
{
+ unsigned long bp;
unsigned long stack;
+ bp = stack_frame(current, NULL);
printk("Pid: %d, comm: %.20s %s %s %.*s\n",
current->pid, current->comm, print_tainted(),
init_utsname()->release,
(int)strcspn(init_utsname()->version, " "),
init_utsname()->version);
- show_trace(NULL, NULL, &stack);
+ show_trace(NULL, NULL, &stack, bp);
}
EXPORT_SYMBOL(dump_stack);
diff --git a/arch/x86/kernel/dumpstack_32.c b/arch/x86/kernel/dumpstack_32.c
index 74cc1ed..3b97a80 100644
--- a/arch/x86/kernel/dumpstack_32.c
+++ b/arch/x86/kernel/dumpstack_32.c
@@ -17,12 +17,11 @@
#include <asm/stacktrace.h>
-void dump_trace(struct task_struct *task,
- struct pt_regs *regs, unsigned long *stack,
+void dump_trace(struct task_struct *task, struct pt_regs *regs,
+ unsigned long *stack, unsigned long bp,
const struct stacktrace_ops *ops, void *data)
{
int graph = 0;
- unsigned long bp;
if (!task)
task = current;
@@ -35,7 +34,9 @@ void dump_trace(struct task_struct *task,
stack = (unsigned long *)task->thread.sp;
}
- bp = stack_frame(task, regs);
+ if (!bp)
+ bp = stack_frame(task, regs);
+
for (;;) {
struct thread_info *context;
@@ -55,7 +56,7 @@ EXPORT_SYMBOL(dump_trace);
void
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;
int i;
@@ -77,7 +78,7 @@ show_stack_log_lvl(struct task_struct *task, struct pt_regs *regs,
touch_nmi_watchdog();
}
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)
u8 *ip;
printk(KERN_EMERG "Stack:\n");
- show_stack_log_lvl(NULL, regs, ®s->sp, KERN_EMERG);
+ show_stack_log_lvl(NULL, regs, ®s->sp, 0, KERN_EMERG);
printk(KERN_EMERG "Code: ");
diff --git a/arch/x86/kernel/dumpstack_64.c b/arch/x86/kernel/dumpstack_64.c
index a6b6fcf..e71c98d 100644
--- a/arch/x86/kernel/dumpstack_64.c
+++ b/arch/x86/kernel/dumpstack_64.c
@@ -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
*/
-void dump_trace(struct task_struct *task,
- struct pt_regs *regs, unsigned long *stack,
+void dump_trace(struct task_struct *task, struct pt_regs *regs,
+ unsigned long *stack, unsigned long bp,
const struct stacktrace_ops *ops, void *data)
{
const unsigned cpu = get_cpu();
@@ -150,7 +150,6 @@ void dump_trace(struct task_struct *task,
struct thread_info *tinfo;
int graph = 0;
unsigned long dummy;
- unsigned long bp;
if (!task)
task = current;
@@ -161,7 +160,8 @@ void dump_trace(struct task_struct *task,
stack = (unsigned long *)task->thread.sp;
}
- bp = stack_frame(task, regs);
+ if (!bp)
+ bp = stack_frame(task, regs);
/*
* Print function call entries in all stacks, starting at the
* current stack address. If the stacks consist of nested
@@ -225,7 +225,7 @@ EXPORT_SYMBOL(dump_trace);
void
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;
@@ -269,7 +269,7 @@ show_stack_log_lvl(struct task_struct *task, struct pt_regs *regs,
preempt_enable();
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)
@@ -298,7 +298,7 @@ void show_registers(struct pt_regs *regs)
printk(KERN_EMERG "Stack:\n");
show_stack_log_lvl(NULL, regs, (unsigned long *)sp,
- KERN_EMERG);
+ 0, KERN_EMERG);
printk(KERN_EMERG "Code: ");
diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
index ff45541..f4c6cc4 100644
--- a/arch/x86/kernel/process.c
+++ b/arch/x86/kernel/process.c
@@ -87,7 +87,7 @@ void exit_thread(void)
void show_regs(struct pt_regs *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)
diff --git a/arch/x86/kernel/stacktrace.c b/arch/x86/kernel/stacktrace.c
index 938c8e1..6515733 100644
--- a/arch/x86/kernel/stacktrace.c
+++ b/arch/x86/kernel/stacktrace.c
@@ -73,7 +73,7 @@ static const struct stacktrace_ops save_stack_ops_nosched = {
*/
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)
trace->entries[trace->nr_entries++] = ULONG_MAX;
}
@@ -81,14 +81,14 @@ EXPORT_SYMBOL_GPL(save_stack_trace);
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)
trace->entries[trace->nr_entries++] = ULONG_MAX;
}
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)
trace->entries[trace->nr_entries++] = ULONG_MAX;
}
Commit-ID: 2b8689e03cd48bb832b933ce5b6335f30e02c6f9
Gitweb: http://git.kernel.org/tip/2b8689e03cd48bb832b933ce5b6335f30e02c6f9
Author: Namhyung Kim <[email protected]>
AuthorDate: Tue, 8 Mar 2011 20:44:20 +0900
Committer: Thomas Gleixner <[email protected]>
CommitDate: Thu, 10 Mar 2011 23:20:30 +0100
x86, dumpstack: Cleanup printks
Use KERN_CONT level if messages are continued from previous one.
Also remove a pair of angle brackets on EOE for consistency.
Signed-off-by: Namhyung Kim <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
LKML-Reference: <[email protected]>
Signed-off-by: Thomas Gleixner <[email protected]>
---
arch/x86/kernel/dumpstack_32.c | 8 ++++----
arch/x86/kernel/dumpstack_64.c | 10 +++++-----
2 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/arch/x86/kernel/dumpstack_32.c b/arch/x86/kernel/dumpstack_32.c
index 3b97a80..c99f9ed 100644
--- a/arch/x86/kernel/dumpstack_32.c
+++ b/arch/x86/kernel/dumpstack_32.c
@@ -116,16 +116,16 @@ void show_registers(struct pt_regs *regs)
for (i = 0; i < code_len; i++, ip++) {
if (ip < (u8 *)PAGE_OFFSET ||
probe_kernel_address(ip, c)) {
- printk(" Bad EIP value.");
+ printk(KERN_CONT " Bad EIP value.");
break;
}
if (ip == (u8 *)regs->ip)
- printk("<%02x> ", c);
+ printk(KERN_CONT "<%02x> ", c);
else
- printk("%02x ", c);
+ printk(KERN_CONT "%02x ", c);
}
}
- printk("\n");
+ printk(KERN_CONT "\n");
}
int is_valid_bugaddr(unsigned long ip)
diff --git a/arch/x86/kernel/dumpstack_64.c b/arch/x86/kernel/dumpstack_64.c
index e71c98d..50aa303 100644
--- a/arch/x86/kernel/dumpstack_64.c
+++ b/arch/x86/kernel/dumpstack_64.c
@@ -180,7 +180,7 @@ void dump_trace(struct task_struct *task, struct pt_regs *regs,
bp = ops->walk_stack(tinfo, stack, bp, ops,
data, estack_end, &graph);
- ops->stack(data, "<EOE>");
+ ops->stack(data, "EOE");
/*
* We link to the next stack via the
* second-to-last pointer (index -2 to end) in the
@@ -311,16 +311,16 @@ void show_registers(struct pt_regs *regs)
for (i = 0; i < code_len; i++, ip++) {
if (ip < (u8 *)PAGE_OFFSET ||
probe_kernel_address(ip, c)) {
- printk(" Bad RIP value.");
+ printk(KERN_CONT " Bad RIP value.");
break;
}
if (ip == (u8 *)regs->ip)
- printk("<%02x> ", c);
+ printk(KERN_CONT "<%02x> ", c);
else
- printk("%02x ", c);
+ printk(KERN_CONT "%02x ", c);
}
}
- printk("\n");
+ printk(KERN_CONT "\n");
}
int is_valid_bugaddr(unsigned long ip)
Commit-ID: 27f30f3e462f84e0a7c561d80e08d428e566db5e
Gitweb: http://git.kernel.org/tip/27f30f3e462f84e0a7c561d80e08d428e566db5e
Author: Namhyung Kim <[email protected]>
AuthorDate: Tue, 8 Mar 2011 20:44:21 +0900
Committer: Thomas Gleixner <[email protected]>
CommitDate: Thu, 10 Mar 2011 23:20:30 +0100
x86, dumpstack: Rename print_context_stack and friends
print_context_stack* and print_ftrace_graph_addr are misnomers. They
don't print anything by themselves and call appropriate callback
routines. Actually save_stack_ops* use them just for saving return
addresses not for printing.
Rename them to walk_context_stack* will make more sense IMHO.
Signed-off-by: Namhyung Kim <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Arnaldo Carvalho de Melo <[email protected]>
Cc: Robert Richter <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
LKML-Reference: <[email protected]>
Signed-off-by: Thomas Gleixner <[email protected]>
---
arch/x86/include/asm/stacktrace.h | 4 ++--
arch/x86/kernel/cpu/perf_event.c | 2 +-
arch/x86/kernel/dumpstack.c | 18 +++++++++---------
arch/x86/kernel/stacktrace.c | 4 ++--
arch/x86/oprofile/backtrace.c | 2 +-
5 files changed, 15 insertions(+), 15 deletions(-)
diff --git a/arch/x86/include/asm/stacktrace.h b/arch/x86/include/asm/stacktrace.h
index d7e89c8..73fc8e2 100644
--- a/arch/x86/include/asm/stacktrace.h
+++ b/arch/x86/include/asm/stacktrace.h
@@ -23,13 +23,13 @@ typedef unsigned long (*walk_stack_t)(struct thread_info *tinfo,
int *graph);
extern unsigned long
-print_context_stack(struct thread_info *tinfo,
+walk_context_stack(struct thread_info *tinfo,
unsigned long *stack, unsigned long bp,
const struct stacktrace_ops *ops, void *data,
unsigned long *end, int *graph);
extern unsigned long
-print_context_stack_bp(struct thread_info *tinfo,
+walk_context_stack_bp(struct thread_info *tinfo,
unsigned long *stack, unsigned long bp,
const struct stacktrace_ops *ops, void *data,
unsigned long *end, int *graph);
diff --git a/arch/x86/kernel/cpu/perf_event.c b/arch/x86/kernel/cpu/perf_event.c
index d19cdc2..9237e83 100644
--- a/arch/x86/kernel/cpu/perf_event.c
+++ b/arch/x86/kernel/cpu/perf_event.c
@@ -1697,7 +1697,7 @@ static const struct stacktrace_ops backtrace_ops = {
.warning_symbol = backtrace_warning_symbol,
.stack = backtrace_stack,
.address = backtrace_address,
- .walk_stack = print_context_stack_bp,
+ .walk_stack = walk_context_stack_bp,
};
void
diff --git a/arch/x86/kernel/dumpstack.c b/arch/x86/kernel/dumpstack.c
index c330160..38b74a5e 100644
--- a/arch/x86/kernel/dumpstack.c
+++ b/arch/x86/kernel/dumpstack.c
@@ -33,7 +33,7 @@ void printk_address(unsigned long address, int reliable)
#ifdef CONFIG_FUNCTION_GRAPH_TRACER
static void
-print_ftrace_graph_addr(unsigned long addr, void *data,
+walk_ftrace_graph_addr(unsigned long addr, void *data,
const struct stacktrace_ops *ops,
struct thread_info *tinfo, int *graph)
{
@@ -56,7 +56,7 @@ print_ftrace_graph_addr(unsigned long addr, void *data,
}
#else
static inline void
-print_ftrace_graph_addr(unsigned long addr, void *data,
+walk_ftrace_graph_addr(unsigned long addr, void *data,
const struct stacktrace_ops *ops,
struct thread_info *tinfo, int *graph)
{ }
@@ -83,7 +83,7 @@ static inline int valid_stack_ptr(struct thread_info *tinfo,
}
unsigned long
-print_context_stack(struct thread_info *tinfo,
+walk_context_stack(struct thread_info *tinfo,
unsigned long *stack, unsigned long bp,
const struct stacktrace_ops *ops, void *data,
unsigned long *end, int *graph)
@@ -102,16 +102,16 @@ print_context_stack(struct thread_info *tinfo,
} else {
ops->address(data, addr, 0);
}
- print_ftrace_graph_addr(addr, data, ops, tinfo, graph);
+ walk_ftrace_graph_addr(addr, data, ops, tinfo, graph);
}
stack++;
}
return bp;
}
-EXPORT_SYMBOL_GPL(print_context_stack);
+EXPORT_SYMBOL_GPL(walk_context_stack);
unsigned long
-print_context_stack_bp(struct thread_info *tinfo,
+walk_context_stack_bp(struct thread_info *tinfo,
unsigned long *stack, unsigned long bp,
const struct stacktrace_ops *ops, void *data,
unsigned long *end, int *graph)
@@ -128,12 +128,12 @@ print_context_stack_bp(struct thread_info *tinfo,
ops->address(data, addr, 1);
frame = frame->next_frame;
ret_addr = &frame->return_address;
- print_ftrace_graph_addr(addr, data, ops, tinfo, graph);
+ walk_ftrace_graph_addr(addr, data, ops, tinfo, graph);
}
return (unsigned long)frame;
}
-EXPORT_SYMBOL_GPL(print_context_stack_bp);
+EXPORT_SYMBOL_GPL(walk_context_stack_bp);
static void
@@ -170,7 +170,7 @@ static const struct stacktrace_ops print_trace_ops = {
.warning_symbol = print_trace_warning_symbol,
.stack = print_trace_stack,
.address = print_trace_address,
- .walk_stack = print_context_stack,
+ .walk_stack = walk_context_stack,
};
void
diff --git a/arch/x86/kernel/stacktrace.c b/arch/x86/kernel/stacktrace.c
index 6515733..2e44ee1 100644
--- a/arch/x86/kernel/stacktrace.c
+++ b/arch/x86/kernel/stacktrace.c
@@ -57,7 +57,7 @@ static const struct stacktrace_ops save_stack_ops = {
.warning_symbol = save_stack_warning_symbol,
.stack = save_stack_stack,
.address = save_stack_address,
- .walk_stack = print_context_stack,
+ .walk_stack = walk_context_stack,
};
static const struct stacktrace_ops save_stack_ops_nosched = {
@@ -65,7 +65,7 @@ static const struct stacktrace_ops save_stack_ops_nosched = {
.warning_symbol = save_stack_warning_symbol,
.stack = save_stack_stack,
.address = save_stack_address_nosched,
- .walk_stack = print_context_stack,
+ .walk_stack = walk_context_stack,
};
/*
diff --git a/arch/x86/oprofile/backtrace.c b/arch/x86/oprofile/backtrace.c
index 72cbec1..f4b9fbb 100644
--- a/arch/x86/oprofile/backtrace.c
+++ b/arch/x86/oprofile/backtrace.c
@@ -46,7 +46,7 @@ static struct stacktrace_ops backtrace_ops = {
.warning_symbol = backtrace_warning_symbol,
.stack = backtrace_stack,
.address = backtrace_address,
- .walk_stack = print_context_stack,
+ .walk_stack = walk_context_stack,
};
#ifdef CONFIG_COMPAT
Commit-ID: 2f8058ae197236f9d5641850ce27f67d8f3e0b39
Gitweb: http://git.kernel.org/tip/2f8058ae197236f9d5641850ce27f67d8f3e0b39
Author: Namhyung Kim <[email protected]>
AuthorDate: Tue, 8 Mar 2011 20:44:22 +0900
Committer: Thomas Gleixner <[email protected]>
CommitDate: Thu, 10 Mar 2011 23:20:30 +0100
x86, dumpstack: Use frame pointer during stack trace
If CONFIG_FRAME_POINTER is set then use the frame pointer for the
stack backtrace rather than scanning whole stack blindly.
Signed-off-by: Namhyung Kim <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
LKML-Reference: <[email protected]>
Signed-off-by: Thomas Gleixner <[email protected]>
---
arch/x86/kernel/dumpstack.c | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/arch/x86/kernel/dumpstack.c b/arch/x86/kernel/dumpstack.c
index 38b74a5e..56db27d 100644
--- a/arch/x86/kernel/dumpstack.c
+++ b/arch/x86/kernel/dumpstack.c
@@ -170,7 +170,11 @@ static const struct stacktrace_ops print_trace_ops = {
.warning_symbol = print_trace_warning_symbol,
.stack = print_trace_stack,
.address = print_trace_address,
+#ifdef CONFIG_FRAME_POINTER
+ .walk_stack = walk_context_stack_bp,
+#else
.walk_stack = walk_context_stack,
+#endif
};
void
On Thu, Mar 10, 2011 at 10:24:53PM +0000, tip-bot for Namhyung Kim wrote:
> Commit-ID: 074206a787e4dfdc4c290789ab6604c7f9e691ca
> Gitweb: http://git.kernel.org/tip/074206a787e4dfdc4c290789ab6604c7f9e691ca
> Author: Namhyung Kim <[email protected]>
> AuthorDate: Tue, 8 Mar 2011 20:44:19 +0900
> Committer: Thomas Gleixner <[email protected]>
> CommitDate: Thu, 10 Mar 2011 23:20:30 +0100
>
> x86, dumpstack: Correct stack dump info when frame pointer is available
>
> Current stack dump code scans entire stack and checks each entry for 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 9c0729dc8062 ("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
>
> Signed-off-by: Namhyung Kim <[email protected]>
> Cc: Soren Sandmann <[email protected]>
> Cc: Peter Zijlstra <[email protected]>
> Cc: Paul Mackerras <[email protected]>
> Cc: Arnaldo Carvalho de Melo <[email protected]>
> Cc: Frederic Weisbecker <[email protected]>
> LKML-Reference: <[email protected]>
> Signed-off-by: Thomas Gleixner <[email protected]>
> ---
Nice fix, thanks!
On Thu, Mar 10, 2011 at 10:26:07PM +0000, tip-bot for Namhyung Kim wrote:
> Commit-ID: 2f8058ae197236f9d5641850ce27f67d8f3e0b39
> Gitweb: http://git.kernel.org/tip/2f8058ae197236f9d5641850ce27f67d8f3e0b39
> Author: Namhyung Kim <[email protected]>
> AuthorDate: Tue, 8 Mar 2011 20:44:22 +0900
> Committer: Thomas Gleixner <[email protected]>
> CommitDate: Thu, 10 Mar 2011 23:20:30 +0100
>
> x86, dumpstack: Use frame pointer during stack trace
>
> If CONFIG_FRAME_POINTER is set then use the frame pointer for the
> stack backtrace rather than scanning whole stack blindly.
We don't do it blindly, we actually check the reliability with the
frame pointer.
I'm not sure this patch is a good idea. stack dumps need to stay very
robust and not exclusively rely on the frame pointer to be correct.
At least walking blindly the stack provides a best effort dump as a last
resort.
>
> Signed-off-by: Namhyung Kim <[email protected]>
> Cc: Frederic Weisbecker <[email protected]>
> LKML-Reference: <[email protected]>
> Signed-off-by: Thomas Gleixner <[email protected]>
> ---
> arch/x86/kernel/dumpstack.c | 4 ++++
> 1 files changed, 4 insertions(+), 0 deletions(-)
>
> diff --git a/arch/x86/kernel/dumpstack.c b/arch/x86/kernel/dumpstack.c
> index 38b74a5e..56db27d 100644
> --- a/arch/x86/kernel/dumpstack.c
> +++ b/arch/x86/kernel/dumpstack.c
> @@ -170,7 +170,11 @@ static const struct stacktrace_ops print_trace_ops = {
> .warning_symbol = print_trace_warning_symbol,
> .stack = print_trace_stack,
> .address = print_trace_address,
> +#ifdef CONFIG_FRAME_POINTER
> + .walk_stack = walk_context_stack_bp,
> +#else
> .walk_stack = walk_context_stack,
> +#endif
> };
On Thu, 10 Mar 2011, tip-bot for Namhyung Kim wrote:
> Commit-ID: 074206a787e4dfdc4c290789ab6604c7f9e691ca
> Gitweb: http://git.kernel.org/tip/074206a787e4dfdc4c290789ab6604c7f9e691ca
> Author: Namhyung Kim <[email protected]>
> AuthorDate: Tue, 8 Mar 2011 20:44:19 +0900
> Committer: Thomas Gleixner <[email protected]>
> CommitDate: Thu, 10 Mar 2011 23:20:30 +0100
>
> x86, dumpstack: Correct stack dump info when frame pointer is available
Dropped it due to:
arch/x86/oprofile/backtrace.c:130:8: error: too few arguments to function ‘dump_trace’
Thanks,
tglx
2011-03-11 (금), 00:02 +0100, Frederic Weisbecker:
> On Thu, Mar 10, 2011 at 10:26:07PM +0000, tip-bot for Namhyung Kim wrote:
> > Commit-ID: 2f8058ae197236f9d5641850ce27f67d8f3e0b39
> > Gitweb: http://git.kernel.org/tip/2f8058ae197236f9d5641850ce27f67d8f3e0b39
> > Author: Namhyung Kim <[email protected]>
> > AuthorDate: Tue, 8 Mar 2011 20:44:22 +0900
> > Committer: Thomas Gleixner <[email protected]>
> > CommitDate: Thu, 10 Mar 2011 23:20:30 +0100
> >
> > x86, dumpstack: Use frame pointer during stack trace
> >
> > If CONFIG_FRAME_POINTER is set then use the frame pointer for the
> > stack backtrace rather than scanning whole stack blindly.
>
> We don't do it blindly, we actually check the reliability with the
> frame pointer.
>
> I'm not sure this patch is a good idea. stack dumps need to stay very
> robust and not exclusively rely on the frame pointer to be correct.
> At least walking blindly the stack provides a best effort dump as a last
> resort.
>
Sounds reasonable. How about adding a boot param to control it then?
diff --git a/arch/x86/kernel/dumpstack.c b/arch/x86/kernel/dumpstack.c
index 809d027de28f..0d7efd90c588 100644
--- a/arch/x86/kernel/dumpstack.c
+++ b/arch/x86/kernel/dumpstack.c
@@ -177,6 +177,16 @@ static const struct stacktrace_ops print_trace_ops
#endif
};
+static int __init nofptrace_setup(char *s)
+{
+ struct stacktrace_ops *ops;
+
+ ops = (struct stacktrace_ops *) &print_trace_ops;
+ ops->walk_stack = walk_context_stack;
+ return 0;
+}
+early_param("nofptrace", nofptrace_setup);
+
void
show_trace_log_lvl(struct task_struct *task, struct pt_regs *regs,
unsigned long *stack, unsigned long bp, char *log_lvl)
--
Regards,
Namhyung Kim
2011/3/23 Namhyung Kim <[email protected]>:
> 2011-03-11 (금), 00:02 +0100, Frederic Weisbecker:
>> On Thu, Mar 10, 2011 at 10:26:07PM +0000, tip-bot for Namhyung Kim wrote:
>> > Commit-ID: 2f8058ae197236f9d5641850ce27f67d8f3e0b39
>> > Gitweb: http://git.kernel.org/tip/2f8058ae197236f9d5641850ce27f67d8f3e0b39
>> > Author: Namhyung Kim <[email protected]>
>> > AuthorDate: Tue, 8 Mar 2011 20:44:22 +0900
>> > Committer: Thomas Gleixner <[email protected]>
>> > CommitDate: Thu, 10 Mar 2011 23:20:30 +0100
>> >
>> > x86, dumpstack: Use frame pointer during stack trace
>> >
>> > If CONFIG_FRAME_POINTER is set then use the frame pointer for the
>> > stack backtrace rather than scanning whole stack blindly.
>>
>> We don't do it blindly, we actually check the reliability with the
>> frame pointer.
>>
>> I'm not sure this patch is a good idea. stack dumps need to stay very
>> robust and not exclusively rely on the frame pointer to be correct.
>> At least walking blindly the stack provides a best effort dump as a last
>> resort.
>>
>
> Sounds reasonable. How about adding a boot param to control it then?
Hmm, but I'm not sure what it would be useful for. Even if one is sure that his
crash will have the needed reliable addresses already, having
unreliable ones too in
the report are not a problem. Are they?
Besides, how can we read a stack dump report posted by someone and be sure
he did not miss an important part because he had this kernel parameter enabled
and his frame pointer broken, or our frame based stack dump walking broken as
it has been many times, like your fixed in your patchset.
One win I could find though is that it can help a stack dump to fit in
the screen....
>
> diff --git a/arch/x86/kernel/dumpstack.c b/arch/x86/kernel/dumpstack.c
> index 809d027de28f..0d7efd90c588 100644
> --- a/arch/x86/kernel/dumpstack.c
> +++ b/arch/x86/kernel/dumpstack.c
> @@ -177,6 +177,16 @@ static const struct stacktrace_ops print_trace_ops
> #endif
> };
>
> +static int __init nofptrace_setup(char *s)
> +{
> + struct stacktrace_ops *ops;
> +
> + ops = (struct stacktrace_ops *) &print_trace_ops;
> + ops->walk_stack = walk_context_stack;
> + return 0;
> +}
> +early_param("nofptrace", nofptrace_setup);
> +
> void
> show_trace_log_lvl(struct task_struct *task, struct pt_regs *regs,
> unsigned long *stack, unsigned long bp, char *log_lvl)
>
>
>
> --
> Regards,
> Namhyung Kim
>
>
>
* Frederic Weisbecker <[email protected]> wrote:
> 2011/3/23 Namhyung Kim <[email protected]>:
> > 2011-03-11 (금), 00:02 +0100, Frederic Weisbecker:
> >> On Thu, Mar 10, 2011 at 10:26:07PM +0000, tip-bot for Namhyung Kim wrote:
> >> > Commit-ID: 2f8058ae197236f9d5641850ce27f67d8f3e0b39
> >> > Gitweb: http://git.kernel.org/tip/2f8058ae197236f9d5641850ce27f67d8f3e0b39
> >> > Author: Namhyung Kim <[email protected]>
> >> > AuthorDate: Tue, 8 Mar 2011 20:44:22 +0900
> >> > Committer: Thomas Gleixner <[email protected]>
> >> > CommitDate: Thu, 10 Mar 2011 23:20:30 +0100
> >> >
> >> > x86, dumpstack: Use frame pointer during stack trace
> >> >
> >> > If CONFIG_FRAME_POINTER is set then use the frame pointer for the
> >> > stack backtrace rather than scanning whole stack blindly.
> >>
> >> We don't do it blindly, we actually check the reliability with the
> >> frame pointer.
> >>
> >> I'm not sure this patch is a good idea. stack dumps need to stay very
> >> robust and not exclusively rely on the frame pointer to be correct.
> >> At least walking blindly the stack provides a best effort dump as a last
> >> resort.
> >>
> >
> > Sounds reasonable. How about adding a boot param to control it then?
>
> Hmm, but I'm not sure what it would be useful for. Even if one is sure that
> his crash will have the needed reliable addresses already, having unreliable
> ones too in the report are not a problem. Are they?
Agreed, there's no point in such a boot parameter really.
The principles for printing backtraces are the following:
- Robustness comes first. We do not ever want to crash or miss important
information because the frame pointer chain broke in some rarely used
assembler code.
- Information. Backtraces like:
[ 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
are immensely useful, because firstly we see the 'real' backtrace:
[ 3.523351] [<ffffffff814f35b9>] panic+0x91/0x199
[ 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
Secondly, we also see the 'residual trace' of what is on the kernel stack:
[ 3.523468] [<ffffffff814f3729>] ? printk+0x68/0x6a
[ 3.524228] [<ffffffff814f6880>] ? restore_args+0x0/0x30
[ 3.524345] [<ffffffff81a97559>] ? kernel_init+0x0/0x1e2
[ 3.524445] [<ffffffff81003890>] ? kernel_thread_helper+0x0/0x10
Which is a poor man's kernel trace really. Here it tells us that a printk
executed shortly before the backtrace was generated. Info like this can
be useful when rare crashes are analyzed.
So hiding that information is not really productive. If then we could think about
making the visual output more expressive. For example:
Call Trace:
[<ffffffff814f35b9>] panic() # 0x091/0x199
[<ffffffff814f3729>] # ? printk+0x68/0x6a
[<ffffffff81a981b2>] mount_block_root() # 0x257/0x26e
[<ffffffff81a9821f>] mount_root() # 0x056/0x05a
[<ffffffff81a98393>] prepare_namespace() # 0x170/0x1a9
[<ffffffff81a9772b>] kernel_init() # 0x1d2/0x1e2
[<ffffffff81003894>] kernel_thread_helper() # 0x004/0x010
[<ffffffff814f6880>] # ? restore_args+0x0/0x30
[<ffffffff81a97559>] # ? kernel_init+0x0/0x1e2
[<ffffffff81003890>] # ? kernel_thread_helper+0x0/0x10
Would be a 'human readable' variant that tells us the real backtrace 'at a
glance' - perhaps in a bit clearer fashion - while still keeping the 'residual'
entries included as well.
The downside is that tools that parse backtraces out of the syslog might get
confused, so that aspect has to be investigated as well.
Thanks,
Ingo