2008-10-04 21:12:33

by Alexander van Heukelum

[permalink] [raw]
Subject: [PATCH] dumpstack: x86: various small unification steps

Hi Ingo,

Here is the first round of unification of dumpstack_32.c
and dumpstack_64.c. The first patch can also be seen as
a clean-up of the traps.c-unification as I forgot to move
one function at the time. Anyhow, the series depends on
the traps unification.

B.T.W., I could reproduce the spontaneous reboot with the
traps unification with glibc. The change GATE_INTERRUPT ->
GATE_TRAP fixed the crash there. Usually I test only with
a small klibc-based userspace, and there the reboot does
not happen. I guess the int 0x80 interface is less picky.
(Just to say that I do test the changes a bit ;) )

Example dumps, after the changes:

------------[ cut here ]------------
Kernel BUG at ffffffff802aa012 [verbose debug info unavailable]
invalid opcode: 0000 [#1]
CPU 0
Modules linked in:
Pid: 21, comm: sh Not tainted 2.6.27-rc8-tip #50
RIP: 0010:[<ffffffff802aa012>] [<ffffffff802aa012>]
sysrq_key_table_key2index+0x12/0x30
RSP: 0000:ffff88000793dea8 EFLAGS: 00000046
RAX: 000000000000000b RBX: 0000000000000002 RCX: 0000000000000001
RDX: 00000000000003f9 RSI: 0000000000000001 RDI: 0000000000000040
RBP: 0000000000000000 R08: 0000000000000005 R09: 0000000000000000
R10: ffff88000793c000 R11: ffffffff8029ce50 R12: 0000000000000296
R13: 0000000000000040 R14: 0000000000000000 R15: 0000000000000007
FS: 0000000000000000(0000) GS:ffffffff80309000(0000)
knlGS:0000000000000000
CS: 0010 DS: 002b ES: 002b CR0: 000000008005003b
CR2: 00000000f7f5a170 CR3: 0000000007942000 CR4: 00000000000006a0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 0000000000000000 DR7: 0000000000000000
Process sh (pid: 21, threadinfo ffff88000793c000, task ffff880007943830)
Stack:
ffffffff802aa035 ffffffff802aa121 ffff88000783c300 0000000000000002
fffffffffffffffb 00000000f7f59170 00000000f7f59170 0000000000000000
0000000000000000 ffffffff80291833 ffff88000780c980 ffffffff8028b573
0000000000000000 ffff88000783c480 ffff88000793df50 ffffffff80260f8d
ffff88000783c480 0000000000000002 fffffffffffffff7 ffffffff80261463
0000000006010a74 0000000000000000 0000000000000000 0000000000000001
0000000000000000 0000000000000000 0000000000000000 ffffffff8021e292
0000000000000000 0000000000000000 0000000000000000 0000000000000000
Call Trace:
[<ffffffff802aa035>] __sysrq_get_key_op+0x5/0x20
[<ffffffff802aa121>] __handle_sysrq+0x41/0x130
[<ffffffff80291833>] write_sysrq_trigger+0x33/0x40
[<ffffffff8028b573>] proc_reg_write+0x33/0x50
[<ffffffff80260f8d>] vfs_write+0xad/0xe0
[<ffffffff80261463>] sys_write+0x53/0x90
[<ffffffff8021e292>] ia32_sysret+0x0/0xa
Code: 01 00 00 00 85 d2 75 0c 31 c0 83 3d 18 f5 05 00 00 0f 95 c0 f3 c3 0f 1f 00
83 ff 40 74 0d 8d 47 d0 83 f8 09 89 c1 77 07 89 c8 c3 <0f> 0b eb fe 8d 47 9f 8d
57 a9 b9 ff ff ff ff 83 f8 1a 0f 42 ca
RIP [<ffffffff802aa012>] sysrq_key_table_key2index+0x12/0x30
RSP <ffff88000793dea8>
---[ end trace 61c257d34db6f0d4 ]---

------------[ cut here ]------------
Kernel BUG at c01768a6 [verbose debug info unavailable]
invalid opcode: 0000 [#1]
Modules linked in:

Pid: 21, comm: sh Not tainted (2.6.27-rc8-tip #51)
EIP: 0060:[<c01768a6>] EFLAGS: 00000046 CPU: 0
EIP is at sysrq_key_table_key2index+0x7/0x27
EAX: 00000040 EBX: 00000002 ECX: 000003f9 EDX: 00000040
ESI: 00000296 EDI: 00000000 EBP: 00000040 ESP: c7917f3c
DS: 007b ES: 007b FS: 0000 GS: 0000 SS: 0068
Process sh (pid: 21, ti=c7917000 task=c790e330 task.ti=c7917000)
Stack:
c01768cb c0176952 c019f80c 00000000 00000007 00000002 c01654fb fffffffb
c79131a0 c016551e c7807004 c0161208 c7917fa0 b80c0170 c01611d9 c79131a0
c7917fa0 c0147f65 c7917fa0 c79131a0 fffffff7 08055574 c7917000 c01482b6
c7917fa0 00000000 00000000 00000000 00000001 08056a20 c0112afa 00000001
Call Trace:
[<c01768cb>] __sysrq_get_key_op+0x5/0x16
[<c0176952>] __handle_sysrq+0x35/0xe4
[<c01654fb>] write_sysrq_trigger+0x0/0x29
[<c016551e>] write_sysrq_trigger+0x23/0x29
[<c0161208>] proc_reg_write+0x2f/0x40
[<c01611d9>] proc_reg_write+0x0/0x40
[<c0147f65>] vfs_write+0x6e/0x8d
[<c01482b6>] sys_write+0x3c/0x63
[<c0112afa>] syscall_call+0x7/0xb
Code: f0 5b 5e 5f 5d c3 90 90 90 83 3d 84 6b 1b c0 00 b8 01 00 00 00 75 0c 31 c0
83 3d 88 6b 1b c0 00 0f 95 c0 c3 83 f8 40 89 c2 75 04 <0f> 0b eb fe 8d 40 d0 83
f8 09 89 c1 76 0f 8d 42 9f 83 c9 ff 83
EIP: [<c01768a6>] sysrq_key_table_key2index+0x7/0x27 SS:ESP 0068:c7917f3c
---[ end trace dabd939c3e498718 ]---

Greetings,
Alexander


2008-10-04 21:12:19

by Alexander van Heukelum

[permalink] [raw]
Subject: [PATCH] dumpstack: x86: move die_nmi to dumpstack_32.c

For some reason die_nmi is still defined in traps.c for
i386, but is found in dumpstack_64.c for x86_64. Move it
to dumpstack_32.c

Signed-off-by: Alexander van Heukelum <[email protected]>
---
arch/x86/kernel/dumpstack_32.c | 36 ++++++++++++++++++++++++++++++++++++
arch/x86/kernel/traps.c | 37 -------------------------------------
2 files changed, 36 insertions(+), 37 deletions(-)

diff --git a/arch/x86/kernel/dumpstack_32.c b/arch/x86/kernel/dumpstack_32.c
index 7378c0c..fd9b439 100644
--- a/arch/x86/kernel/dumpstack_32.c
+++ b/arch/x86/kernel/dumpstack_32.c
@@ -406,6 +406,42 @@ void die(const char *str, struct pt_regs *regs, long err)
oops_end(flags, regs, SIGSEGV);
}

+static DEFINE_SPINLOCK(nmi_print_lock);
+
+void notrace __kprobes
+die_nmi(char *str, struct pt_regs *regs, int do_panic)
+{
+ if (notify_die(DIE_NMIWATCHDOG, str, regs, 0, 2, SIGINT) == NOTIFY_STOP)
+ return;
+
+ spin_lock(&nmi_print_lock);
+ /*
+ * We are in trouble anyway, lets at least try
+ * to get a message out:
+ */
+ bust_spinlocks(1);
+ printk(KERN_EMERG "%s", str);
+ printk(" on CPU%d, ip %08lx, registers:\n",
+ smp_processor_id(), regs->ip);
+ show_registers(regs);
+ if (do_panic)
+ panic("Non maskable interrupt");
+ console_silent();
+ spin_unlock(&nmi_print_lock);
+ bust_spinlocks(0);
+
+ /*
+ * If we are in kernel we are probably nested up pretty bad
+ * and might aswell get out now while we still can:
+ */
+ if (!user_mode_vm(regs)) {
+ current->thread.trap_no = 2;
+ crash_kexec(regs);
+ }
+
+ do_exit(SIGSEGV);
+}
+
static int __init kstack_setup(char *s)
{
kstack_depth_to_print = simple_strtoul(s, NULL, 0);
diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
index 54e08d2..3e825da 100644
--- a/arch/x86/kernel/traps.c
+++ b/arch/x86/kernel/traps.c
@@ -429,43 +429,6 @@ unknown_nmi_error(unsigned char reason, struct pt_regs *regs)
printk(KERN_EMERG "Dazed and confused, but trying to continue\n");
}

-#ifdef CONFIG_X86_32
-static DEFINE_SPINLOCK(nmi_print_lock);
-
-void notrace __kprobes die_nmi(char *str, struct pt_regs *regs, int do_panic)
-{
- if (notify_die(DIE_NMIWATCHDOG, str, regs, 0, 2, SIGINT) == NOTIFY_STOP)
- return;
-
- spin_lock(&nmi_print_lock);
- /*
- * We are in trouble anyway, lets at least try
- * to get a message out:
- */
- bust_spinlocks(1);
- printk(KERN_EMERG "%s", str);
- printk(" on CPU%d, ip %08lx, registers:\n",
- smp_processor_id(), regs->ip);
- show_registers(regs);
- if (do_panic)
- panic("Non maskable interrupt");
- console_silent();
- spin_unlock(&nmi_print_lock);
- bust_spinlocks(0);
-
- /*
- * If we are in kernel we are probably nested up pretty bad
- * and might aswell get out now while we still can:
- */
- if (!user_mode_vm(regs)) {
- current->thread.trap_no = 2;
- crash_kexec(regs);
- }
-
- do_exit(SIGSEGV);
-}
-#endif
-
static notrace __kprobes void default_do_nmi(struct pt_regs *regs)
{
unsigned char reason = 0;
--
1.5.4.3

2008-10-04 21:17:18

by Alexander van Heukelum

[permalink] [raw]
Subject: [PATCH] dumpstack: x86: make printk_address equal

- x86_64: use %p to print an address
- make i386-version the same as the above

The result should be the same on x86_64; on i386 the
output only changes if CONFIG_KALLSYMS is turned off,
in which case the address is printed twice.

Signed-off-by: Alexander van Heukelum <[email protected]>
---
arch/x86/kernel/dumpstack_32.c | 27 ++-------------------------
arch/x86/kernel/dumpstack_64.c | 4 ++--
2 files changed, 4 insertions(+), 27 deletions(-)

diff --git a/arch/x86/kernel/dumpstack_32.c b/arch/x86/kernel/dumpstack_32.c
index fd9b439..62f71c8 100644
--- a/arch/x86/kernel/dumpstack_32.c
+++ b/arch/x86/kernel/dumpstack_32.c
@@ -23,31 +23,8 @@ static int die_counter;

void printk_address(unsigned long address, int reliable)
{
-#ifdef CONFIG_KALLSYMS
- unsigned long offset = 0;
- unsigned long symsize;
- const char *symname;
- char *modname;
- char *delim = ":";
- char namebuf[KSYM_NAME_LEN];
- char reliab[4] = "";
-
- symname = kallsyms_lookup(address, &symsize, &offset,
- &modname, namebuf);
- if (!symname) {
- printk(" [<%08lx>]\n", address);
- return;
- }
- if (!reliable)
- strcpy(reliab, "? ");
-
- if (!modname)
- modname = delim = "";
- printk(" [<%08lx>] %s%s%s%s%s+0x%lx/0x%lx\n",
- address, reliab, delim, modname, delim, symname, offset, symsize);
-#else
- printk(" [<%08lx>]\n", address);
-#endif
+ printk(" [<%p>] %s%pS\n", (void *) address,
+ reliable ? "" : "? ", (void *) address);
}

static inline int valid_stack_ptr(struct thread_info *tinfo,
diff --git a/arch/x86/kernel/dumpstack_64.c b/arch/x86/kernel/dumpstack_64.c
index 11f22d2..f215f1c 100644
--- a/arch/x86/kernel/dumpstack_64.c
+++ b/arch/x86/kernel/dumpstack_64.c
@@ -23,8 +23,8 @@ static int die_counter;

void printk_address(unsigned long address, int reliable)
{
- printk(" [<%016lx>] %s%pS\n",
- address, reliable ? "" : "? ", (void *) address);
+ printk(" [<%p>] %s%pS\n", (void *) address,
+ reliable ? "" : "? ", (void *) address);
}

static unsigned long *in_exception_stack(unsigned cpu, unsigned long stack,
--
1.5.4.3

2008-10-04 21:17:31

by Alexander van Heukelum

[permalink] [raw]
Subject: [PATCH] dumpstack: x86: various small unification steps

- define STACKSLOTS_PER_LINE and use it
- define get_bp macro to hide the %%ebp/%%rbp difference
- i386: check task==NULL in dump_trace, like x86_64
- i386: show_trace(NULL, ...) uses current automatically
- x86_64: use [#%d] for die_counter, like i386
- whitespace and comments

Signed-off-by: Alexander van Heukelum <[email protected]>
---
arch/x86/kernel/dumpstack_32.c | 23 +++++++++++------------
arch/x86/kernel/dumpstack_64.c | 15 +++++++++------
2 files changed, 20 insertions(+), 18 deletions(-)

diff --git a/arch/x86/kernel/dumpstack_32.c b/arch/x86/kernel/dumpstack_32.c
index d5347f1..8d13ebe 100644
--- a/arch/x86/kernel/dumpstack_32.c
+++ b/arch/x86/kernel/dumpstack_32.c
@@ -16,8 +16,11 @@

#include <asm/stacktrace.h>

+#define STACKSLOTS_PER_LINE 8
+#define get_bp(bp) asm("movl %%ebp, %0" : "=r" (bp) :)
+
int panic_on_unrecovered_nmi;
-int kstack_depth_to_print = 24;
+int kstack_depth_to_print = 3 * STACKSLOTS_PER_LINE;
static unsigned int code_bytes = 64;
static int die_counter;

@@ -82,7 +85,7 @@ void dump_trace(struct task_struct *task, struct pt_regs *regs,
if (!stack) {
unsigned long dummy;
stack = &dummy;
- if (task != current)
+ if (task && task != current)
stack = (unsigned long *)task->thread.sp;
}

@@ -90,7 +93,7 @@ void dump_trace(struct task_struct *task, struct pt_regs *regs,
if (!bp) {
if (task == current) {
/* Grab bp right from our regs */
- asm("movl %%ebp, %0" : "=r" (bp) :);
+ get_bp(bp);
} else {
/* bp is the last reg pushed by switch_to */
bp = *(unsigned long *) task->thread.sp;
@@ -167,7 +170,7 @@ void show_trace(struct task_struct *task, struct pt_regs *regs,

static void
show_stack_log_lvl(struct task_struct *task, struct pt_regs *regs,
- unsigned long *sp, unsigned long bp, char *log_lvl)
+ unsigned long *sp, unsigned long bp, char *log_lvl)
{
unsigned long *stack;
int i;
@@ -183,7 +186,7 @@ show_stack_log_lvl(struct task_struct *task, struct pt_regs *regs,
for (i = 0; i < kstack_depth_to_print; i++) {
if (kstack_end(stack))
break;
- if (i && ((i % 8) == 0))
+ if (i && ((i % STACKSLOTS_PER_LINE) == 0))
printk("\n%s", log_lvl);
printk(" %08lx", *stack++);
touch_nmi_watchdog();
@@ -207,7 +210,7 @@ void dump_stack(void)

#ifdef CONFIG_FRAME_POINTER
if (!bp)
- asm("movl %%ebp, %0" : "=r" (bp):);
+ get_bp(bp);
#endif

printk("Pid: %d, comm: %.20s %s %s %.*s\n",
@@ -215,8 +218,7 @@ void dump_stack(void)
init_utsname()->release,
(int)strcspn(init_utsname()->version, " "),
init_utsname()->version);
-
- show_trace(current, NULL, &stack, bp);
+ show_trace(NULL, NULL, &stack, bp);
}

EXPORT_SYMBOL(dump_stack);
@@ -249,7 +251,7 @@ void show_registers(struct pt_regs *regs)

ip = (u8 *)regs->ip - code_prologue;
if (ip < (u8 *)PAGE_OFFSET || probe_kernel_address(ip, c)) {
- /* try starting at EIP */
+ /* try starting at IP */
ip = (u8 *)regs->ip;
code_len = code_len - code_prologue + 1;
}
@@ -320,13 +322,10 @@ void __kprobes oops_end(unsigned long flags, struct pt_regs *regs, int signr)

if (in_nmi())
panic("Fatal exception in non-maskable interrupt");
-
if (in_interrupt())
panic("Fatal exception in interrupt");
-
if (panic_on_oops)
panic("Fatal exception");
-
oops_exit();
do_exit(signr);
}
diff --git a/arch/x86/kernel/dumpstack_64.c b/arch/x86/kernel/dumpstack_64.c
index 6ac80f0..ddc8a38 100644
--- a/arch/x86/kernel/dumpstack_64.c
+++ b/arch/x86/kernel/dumpstack_64.c
@@ -16,8 +16,11 @@

#include <asm/stacktrace.h>

+#define STACKSLOTS_PER_LINE 4
+#define get_bp(bp) asm("movl %%rbp, %0" : "=r" (bp) :)
+
int panic_on_unrecovered_nmi;
-int kstack_depth_to_print = 12;
+int kstack_depth_to_print = 3 * STACKSLOTS_PER_LINE;
static unsigned int code_bytes = 64;
static int die_counter;

@@ -177,7 +180,7 @@ void dump_trace(struct task_struct *task, struct pt_regs *regs,
if (!bp) {
if (task == current) {
/* Grab bp right from our regs */
- asm("movq %%rbp, %0" : "=r" (bp) : );
+ get_bp(bp);
} else {
/* bp is the last reg pushed by switch_to */
bp = *(unsigned long *) task->thread.sp;
@@ -329,7 +332,7 @@ show_stack_log_lvl(struct task_struct *task, struct pt_regs *regs,
if (((long) stack & (THREAD_SIZE-1)) == 0)
break;
}
- if (i && ((i % 4) == 0))
+ if (i && ((i % STACKSLOTS_PER_LINE) == 0))
printk("\n%s", log_lvl);
printk(" %016lx", *stack++);
touch_nmi_watchdog();
@@ -353,7 +356,7 @@ void dump_stack(void)

#ifdef CONFIG_FRAME_POINTER
if (!bp)
- asm("movq %%rbp, %0" : "=r" (bp) : );
+ get_bp(bp);
#endif

printk("Pid: %d, comm: %.20s %s %s %.*s\n",
@@ -396,7 +399,7 @@ void show_registers(struct pt_regs *regs)

ip = (u8 *)regs->ip - code_prologue;
if (ip < (u8 *)PAGE_OFFSET || probe_kernel_address(ip, c)) {
- /* try starting at RIP */
+ /* try starting at IP */
ip = (u8 *)regs->ip;
code_len = code_len - code_prologue + 1;
}
@@ -477,7 +480,7 @@ void __kprobes oops_end(unsigned long flags, struct pt_regs *regs, int signr)

int __kprobes __die(const char *str, struct pt_regs *regs, long err)
{
- printk(KERN_EMERG "%s: %04lx [%u] ", str, err & 0xffff, ++die_counter);
+ printk(KERN_EMERG "%s: %04lx [#%d] ", str, err & 0xffff, ++die_counter);
#ifdef CONFIG_PREEMPT
printk("PREEMPT ");
#endif
--
1.5.4.3

2008-10-04 21:22:18

by Alexander van Heukelum

[permalink] [raw]
Subject: [PATCH] dumptrace: x86: consistently include loglevel, print stack switch

- i386 and x86_64: always printk the 'data' parameter
- i386: announce stack switch (irq -> normal)
- i386: check if there is a stack switch before announcing it

There is a warning that 'context' might come out corrupt in early
boot. If this is true it should be fixed, not worked around.

Signed-off-by: Alexander van Heukelum <[email protected]>
---
arch/x86/kernel/dumpstack_32.c | 17 ++++++-----------
arch/x86/kernel/dumpstack_64.c | 9 +++++++--
2 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/arch/x86/kernel/dumpstack_32.c b/arch/x86/kernel/dumpstack_32.c
index 09cd37c..851b692 100644
--- a/arch/x86/kernel/dumpstack_32.c
+++ b/arch/x86/kernel/dumpstack_32.c
@@ -104,16 +104,12 @@ void dump_trace(struct task_struct *task, struct pt_regs *regs,
context = (struct thread_info *)
((unsigned long)stack & (~(THREAD_SIZE - 1)));
bp = print_context_stack(context, stack, bp, ops, data, NULL);
- /*
- * Should be after the line below, but somewhere
- * in early boot context comes out corrupted and we
- * can't reference it:
- */
- if (ops->stack(data, "IRQ") < 0)
- break;
+
stack = (unsigned long *)context->previous_esp;
if (!stack)
break;
+ if (ops->stack(data, "IRQ") < 0)
+ break;
touch_nmi_watchdog();
}
}
@@ -134,6 +130,7 @@ static void print_trace_warning(void *data, char *msg)

static int print_trace_stack(void *data, char *name)
{
+ printk("%s <%s> ", (char *)data, name);
return 0;
}

@@ -142,11 +139,9 @@ static int print_trace_stack(void *data, char *name)
*/
static void print_trace_address(void *data, unsigned long addr, int reliable)
{
- printk("%s [<%08lx>] ", (char *)data, addr);
- if (!reliable)
- printk("? ");
- print_symbol("%s\n", addr);
touch_nmi_watchdog();
+ printk(data);
+ printk_address(addr, reliable);
}

static const struct stacktrace_ops print_trace_ops = {
diff --git a/arch/x86/kernel/dumpstack_64.c b/arch/x86/kernel/dumpstack_64.c
index 9e40357..029c141 100644
--- a/arch/x86/kernel/dumpstack_64.c
+++ b/arch/x86/kernel/dumpstack_64.c
@@ -247,24 +247,29 @@ EXPORT_SYMBOL(dump_trace);
static void
print_trace_warning_symbol(void *data, char *msg, unsigned long symbol)
{
+ printk(data);
print_symbol(msg, symbol);
printk("\n");
}

static void print_trace_warning(void *data, char *msg)
{
- printk("%s\n", msg);
+ printk("%s%s\n", (char *)data, msg);
}

static int print_trace_stack(void *data, char *name)
{
- printk(" <%s> ", name);
+ printk("%s <%s> ", (char *)data, name);
return 0;
}

+/*
+ * Print one address/symbol entries per line.
+ */
static void print_trace_address(void *data, unsigned long addr, int reliable)
{
touch_nmi_watchdog();
+ printk(data);
printk_address(addr, reliable);
}

--
1.5.4.3

2008-10-04 21:22:31

by Alexander van Heukelum

[permalink] [raw]
Subject: [PATCH] dumpstack: x86: use log_lvl and unify trace formatting

From: Alexander van Heukelum <[email protected]>

- x86: Write log_lvl strings if available
- start raw stack dumps on new line
- i386: Remove extra indentation for raw stack dumps

Signed-off-by: Alexander van Heukelum <[email protected]>
---
arch/x86/kernel/dumpstack_32.c | 18 +++++++++---------
arch/x86/kernel/dumpstack_64.c | 8 ++++----
2 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/arch/x86/kernel/dumpstack_32.c b/arch/x86/kernel/dumpstack_32.c
index 851b692..96d78c9 100644
--- a/arch/x86/kernel/dumpstack_32.c
+++ b/arch/x86/kernel/dumpstack_32.c
@@ -155,8 +155,8 @@ static void
show_trace_log_lvl(struct task_struct *task, struct pt_regs *regs,
unsigned long *stack, unsigned long bp, char *log_lvl)
{
+ printk("%sCall Trace:\n", log_lvl);
dump_trace(task, regs, stack, bp, &print_trace_ops, log_lvl);
- printk("%s =======================\n", log_lvl);
}

void show_trace(struct task_struct *task, struct pt_regs *regs,
@@ -184,17 +184,16 @@ show_stack_log_lvl(struct task_struct *task, struct pt_regs *regs,
if (kstack_end(stack))
break;
if (i && ((i % 8) == 0))
- printk("\n%s ", log_lvl);
- printk("%08lx ", *stack++);
+ printk("\n%s", log_lvl);
+ printk(" %08lx", *stack++);
+ touch_nmi_watchdog();
}
- printk("\n%sCall Trace:\n", log_lvl);
-
+ printk("\n");
show_trace_log_lvl(task, regs, sp, bp, log_lvl);
}

void show_stack(struct task_struct *task, unsigned long *sp)
{
- printk(" ");
show_stack_log_lvl(task, NULL, sp, 0, "");
}

@@ -229,7 +228,7 @@ void show_registers(struct pt_regs *regs)
print_modules();
__show_regs(regs, 0);

- printk(KERN_EMERG "Process %.*s (pid: %d, ti=%p task=%p task.ti=%p)",
+ printk(KERN_EMERG "Process %.*s (pid: %d, ti=%p task=%p task.ti=%p)\n",
TASK_COMM_LEN, current->comm, task_pid_nr(current),
current_thread_info(), current, task_thread_info(current));
/*
@@ -242,8 +241,9 @@ void show_registers(struct pt_regs *regs)
unsigned char c;
u8 *ip;

- printk("\n" KERN_EMERG "Stack: ");
- show_stack_log_lvl(NULL, regs, &regs->sp, 0, KERN_EMERG);
+ printk(KERN_EMERG "Stack:\n");
+ show_stack_log_lvl(NULL, regs, &regs->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 029c141..6ac80f0 100644
--- a/arch/x86/kernel/dumpstack_64.c
+++ b/arch/x86/kernel/dumpstack_64.c
@@ -284,7 +284,7 @@ static void
show_trace_log_lvl(struct task_struct *task, struct pt_regs *regs,
unsigned long *stack, unsigned long bp, char *log_lvl)
{
- printk("Call Trace:\n");
+ printk("%sCall Trace:\n", log_lvl);
dump_trace(task, regs, stack, bp, &print_trace_ops, log_lvl);
}

@@ -330,7 +330,7 @@ show_stack_log_lvl(struct task_struct *task, struct pt_regs *regs,
break;
}
if (i && ((i % 4) == 0))
- printk("\n");
+ printk("\n%s", log_lvl);
printk(" %016lx", *stack++);
touch_nmi_watchdog();
}
@@ -388,9 +388,9 @@ void show_registers(struct pt_regs *regs)
unsigned char c;
u8 *ip;

- printk("Stack: ");
+ printk(KERN_EMERG "Stack:\n");
show_stack_log_lvl(NULL, regs, (unsigned long *)sp,
- regs->bp, "");
+ regs->bp, KERN_EMERG);

printk(KERN_EMERG "Code: ");

--
1.5.4.3

2008-10-04 21:32:18

by Alexander van Heukelum

[permalink] [raw]
Subject: [PATCH] dumpstack: x86: add "end" parameter to valid_stack_ptr and print_context_stack

- Add "end" parameter to valid_stack_ptr and print_context_stack
- use sizeof(long) as the size of a word on the stack

Signed-off-by: Alexander van Heukelum <[email protected]>
---
arch/x86/kernel/dumpstack_32.c | 19 +++++++++++++------
arch/x86/kernel/dumpstack_64.c | 2 +-
2 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/arch/x86/kernel/dumpstack_32.c b/arch/x86/kernel/dumpstack_32.c
index 62f71c8..09cd37c 100644
--- a/arch/x86/kernel/dumpstack_32.c
+++ b/arch/x86/kernel/dumpstack_32.c
@@ -28,10 +28,16 @@ void printk_address(unsigned long address, int reliable)
}

static inline int valid_stack_ptr(struct thread_info *tinfo,
- void *p, unsigned int size)
+ void *p, unsigned int size, void *end)
{
void *t = tinfo;
- return p > t && p <= t + THREAD_SIZE - size;
+ if (end) {
+ if (p < end && p >= (end-THREAD_SIZE))
+ return 1;
+ else
+ return 0;
+ }
+ return p > t && p < t + THREAD_SIZE - size;
}

/* The form of the top of the frame on the stack */
@@ -43,16 +49,17 @@ struct stack_frame {
static inline unsigned long
print_context_stack(struct thread_info *tinfo,
unsigned long *stack, unsigned long bp,
- const struct stacktrace_ops *ops, void *data)
+ const struct stacktrace_ops *ops, void *data,
+ unsigned long *end)
{
struct stack_frame *frame = (struct stack_frame *)bp;

- while (valid_stack_ptr(tinfo, stack, sizeof(*stack))) {
+ while (valid_stack_ptr(tinfo, stack, sizeof(*stack), end)) {
unsigned long addr;

addr = *stack;
if (__kernel_text_address(addr)) {
- if ((unsigned long) stack == bp + 4) {
+ if ((unsigned long) stack == bp + sizeof(long)) {
ops->address(data, addr, 1);
frame = frame->next_frame;
bp = (unsigned long) frame;
@@ -96,7 +103,7 @@ void dump_trace(struct task_struct *task, struct pt_regs *regs,

context = (struct thread_info *)
((unsigned long)stack & (~(THREAD_SIZE - 1)));
- bp = print_context_stack(context, stack, bp, ops, data);
+ bp = print_context_stack(context, stack, bp, ops, data, NULL);
/*
* Should be after the line below, but somewhere
* in early boot context comes out corrupted and we
diff --git a/arch/x86/kernel/dumpstack_64.c b/arch/x86/kernel/dumpstack_64.c
index f215f1c..9e40357 100644
--- a/arch/x86/kernel/dumpstack_64.c
+++ b/arch/x86/kernel/dumpstack_64.c
@@ -141,7 +141,7 @@ print_context_stack(struct thread_info *tinfo,

addr = *stack;
if (__kernel_text_address(addr)) {
- if ((unsigned long) stack == bp + 8) {
+ if ((unsigned long) stack == bp + sizeof(long)) {
ops->address(data, addr, 1);
frame = frame->next_frame;
bp = (unsigned long) frame;
--
1.5.4.3

2008-10-04 21:32:31

by Alexander van Heukelum

[permalink] [raw]
Subject: [PATCH] dumpstack: i386: make kstack= an early boot-param and add oops=panic

- make kstack= and early_param
- add oops=panic, setting panic_on_oops

Signed-off-by: Alexander van Heukelum <[email protected]>
---
arch/x86/kernel/dumpstack_32.c | 17 ++++++++++++++---
1 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kernel/dumpstack_32.c b/arch/x86/kernel/dumpstack_32.c
index 96d78c9..d5347f1 100644
--- a/arch/x86/kernel/dumpstack_32.c
+++ b/arch/x86/kernel/dumpstack_32.c
@@ -421,13 +421,24 @@ die_nmi(char *str, struct pt_regs *regs, int do_panic)
do_exit(SIGSEGV);
}

+static int __init oops_setup(char *s)
+{
+ if (!s)
+ return -EINVAL;
+ if (!strcmp(s, "panic"))
+ panic_on_oops = 1;
+ return 0;
+}
+early_param("oops", oops_setup);
+
static int __init kstack_setup(char *s)
{
+ if (!s)
+ return -EINVAL;
kstack_depth_to_print = simple_strtoul(s, NULL, 0);
-
- return 1;
+ return 0;
}
-__setup("kstack=", kstack_setup);
+early_param("kstack", kstack_setup);

static int __init code_bytes_setup(char *s)
{
--
1.5.4.3

2008-10-05 09:35:40

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH] dumpstack: x86: various small unification steps


* Alexander van Heukelum <[email protected]> wrote:

> Hi Ingo,
>
> Here is the first round of unification of dumpstack_32.c
> and dumpstack_64.c. The first patch can also be seen as
> a clean-up of the traps.c-unification as I forgot to move
> one function at the time. Anyhow, the series depends on
> the traps unification.

great - i've created tip/x86/dumpstack for this and applied your patches
there. (that branch embedds tip/x86/core which already embedds
tip/x86/traps)

> B.T.W., I could reproduce the spontaneous reboot with the
> traps unification with glibc. The change GATE_INTERRUPT ->
> GATE_TRAP fixed the crash there. Usually I test only with
> a small klibc-based userspace, and there the reboot does
> not happen. I guess the int 0x80 interface is less picky.
> (Just to say that I do test the changes a bit ;) )

it also passed -tip testing still then so the changes are fine.

Generally if you see your commits show up and stay in tip/master it
means they get tested with a newly built random kernel about once every
two minutes or so.

regarding klibc, that's interesting: is that the in-kernel klibc from
hpa? Which tree are you using to pull that into tip/master? (unless i
misunderstood what you are doing)

Ingo

2008-10-05 09:46:42

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH] dumpstack: x86: various small unification steps


* Ingo Molnar <[email protected]> wrote:

> great - i've created tip/x86/dumpstack for this and applied your
> patches there. (that branch embedds tip/x86/core which already embedds
> tip/x86/traps)

-tip testing found a 64-bit build failure:

{standard input}: Assembler messages:
{standard input}:720: Error: Incorrect register `%rbx' used with `l' suffix
{standard input}:1340: Error: Incorrect register `%r12' used with `l' suffix

reproducer config attached.

Ingo


Attachments:
(No filename) (470.00 B)
config-Sun_Oct__5_11_31_29_CEST_2008.bad (54.88 kB)
Download all attachments
Subject: Re: [PATCH] dumpstack: x86: various small unification steps

After "dumpstack: x86: various small unification steps", the
assembler gives the following compile error. The error is in
dumpstack_64.c.

{standard input}: Assembler messages:
{standard input}:720: Error: Incorrect register `%rbx' used with `l' suffix
{standard input}:1340: Error: Incorrect register `%r12' used with `l' suffix

Indeed the suffix in get_bp() was wrong.

Signed-off-by: Alexander van Heukelum <[email protected]>

---

On Sun, Oct 05, 2008 at 11:46:15AM +0200, Ingo Molnar wrote:
> * Ingo Molnar <[email protected]> wrote:
>
> > great - i've created tip/x86/dumpstack for this and applied your
> > patches there. (that branch embedds tip/x86/core which already embedds
> > tip/x86/traps)
>
> -tip testing found a 64-bit build failure:
>
> {standard input}: Assembler messages:
> {standard input}:720: Error: Incorrect register `%rbx' used with `l' suffix
> {standard input}:1340: Error: Incorrect register `%r12' used with `l' suffix
>
> reproducer config attached.
>
> Ingo

Hi Ingo,

"dumpstack: x86: various small unification steps" contained
another copy/paste/forgot-to-edit bug. I should have gone
to bed instead of rushing the patch set out.

Greetings,
Alexander

diff --git a/arch/x86/kernel/dumpstack_64.c b/arch/x86/kernel/dumpstack_64.c
index 13379a9..086cc81 100644
--- a/arch/x86/kernel/dumpstack_64.c
+++ b/arch/x86/kernel/dumpstack_64.c
@@ -17,7 +17,7 @@
#include <asm/stacktrace.h>

#define STACKSLOTS_PER_LINE 4
-#define get_bp(bp) asm("movl %%rbp, %0" : "=r" (bp) :)
+#define get_bp(bp) asm("movq %%rbp, %0" : "=r" (bp) :)

int panic_on_unrecovered_nmi;
int kstack_depth_to_print = 3 * STACKSLOTS_PER_LINE;

2008-10-05 11:40:38

by Alexander van Heukelum

[permalink] [raw]
Subject: Re: [PATCH] dumpstack: x86: various small unification steps

On Sun, 5 Oct 2008 11:35:23 +0200, "Ingo Molnar" <[email protected]> said:
> [...]
>
> regarding klibc, that's interesting: is that the in-kernel klibc from
> hpa? Which tree are you using to pull that into tip/master? (unless i
> misunderstood what you are doing)

Hi,

I just use hpa's git://git.kernel.org/pub/scm/libs/klibc/klibc.git,
and compile it 'stand-alone' (you need to create a symlink 'linux',
pointing to a configured kernel tree first.)

I put the klibc-library and all the binaries in the kernel-built-in
initramfs and add an 'init', which contains:

#! /bin/sh
mkdir proc
mount -t proc proc /proc
mkdir sys
mount -t sysfs sysfs /sys
mkdir debugfs
mount -t debugfs debugfs /debugfs
bin/sh -i
reboot

This way even the simplest test-kernel gets into userspace and
you can examine the contents of proc and sysfs.

As hpa is listening too... I have a feature request for klibc
to auto-generate a file that can be fed to the kernel's
INITRAMFS_SOURCE, containing all binaries and an init like
the one I gave above. Especially because the hash added to
the shared library name keeps changing :-/.

Adding a (staticly linked) binary for testing is then an
easy way to run a test in userspace. And usually I do that
using qemu.

Greetings,
Alexander

> Ingo
--
Alexander van Heukelum
[email protected]

--
http://www.fastmail.fm - One of many happy users:
http://www.fastmail.fm/docs/quotes.html

2008-10-05 16:43:17

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH] dumpstack: x86: various small unification steps


* Alexander van Heukelum <[email protected]> wrote:

> After "dumpstack: x86: various small unification steps", the
> assembler gives the following compile error. The error is in
> dumpstack_64.c.
>
> {standard input}: Assembler messages:
> {standard input}:720: Error: Incorrect register `%rbx' used with `l' suffix
> {standard input}:1340: Error: Incorrect register `%r12' used with `l' suffix
>
> Indeed the suffix in get_bp() was wrong.
>
> Signed-off-by: Alexander van Heukelum <[email protected]>

applied to tip/x86/dumpstack, thanks. I've again started testing it.

Ingo

2008-10-06 09:35:50

by Louis Rilling

[permalink] [raw]
Subject: Re: [PATCH] dumpstack: x86: various small unification steps

On Sat, Oct 04, 2008 at 11:12:46PM +0200, Alexander van Heukelum wrote:
> - define STACKSLOTS_PER_LINE and use it
> - define get_bp macro to hide the %%ebp/%%rbp difference
> - i386: check task==NULL in dump_trace, like x86_64
> - i386: show_trace(NULL, ...) uses current automatically
> - x86_64: use [#%d] for die_counter, like i386
> - whitespace and comments
>
[...]

> diff --git a/arch/x86/kernel/dumpstack_64.c b/arch/x86/kernel/dumpstack_64.c
> index 6ac80f0..ddc8a38 100644
> --- a/arch/x86/kernel/dumpstack_64.c
> +++ b/arch/x86/kernel/dumpstack_64.c
> @@ -16,8 +16,11 @@
>
> #include <asm/stacktrace.h>
>
> +#define STACKSLOTS_PER_LINE 4
> +#define get_bp(bp) asm("movl %%rbp, %0" : "=r" (bp) :)
^^^^
Should be movq.

Louis

> int panic_on_unrecovered_nmi;
> -int kstack_depth_to_print = 12;
> +int kstack_depth_to_print = 3 * STACKSLOTS_PER_LINE;
> static unsigned int code_bytes = 64;
> static int die_counter;
>
> @@ -177,7 +180,7 @@ void dump_trace(struct task_struct *task, struct pt_regs *regs,
> if (!bp) {
> if (task == current) {
> /* Grab bp right from our regs */
> - asm("movq %%rbp, %0" : "=r" (bp) : );
> + get_bp(bp);
> } else {
> /* bp is the last reg pushed by switch_to */
> bp = *(unsigned long *) task->thread.sp;
> @@ -329,7 +332,7 @@ show_stack_log_lvl(struct task_struct *task, struct pt_regs *regs,
> if (((long) stack & (THREAD_SIZE-1)) == 0)
> break;
> }
> - if (i && ((i % 4) == 0))
> + if (i && ((i % STACKSLOTS_PER_LINE) == 0))
> printk("\n%s", log_lvl);
> printk(" %016lx", *stack++);
> touch_nmi_watchdog();
> @@ -353,7 +356,7 @@ void dump_stack(void)
>
> #ifdef CONFIG_FRAME_POINTER
> if (!bp)
> - asm("movq %%rbp, %0" : "=r" (bp) : );
> + get_bp(bp);
> #endif
>
> printk("Pid: %d, comm: %.20s %s %s %.*s\n",
> @@ -396,7 +399,7 @@ void show_registers(struct pt_regs *regs)
>
> ip = (u8 *)regs->ip - code_prologue;
> if (ip < (u8 *)PAGE_OFFSET || probe_kernel_address(ip, c)) {
> - /* try starting at RIP */
> + /* try starting at IP */
> ip = (u8 *)regs->ip;
> code_len = code_len - code_prologue + 1;
> }
> @@ -477,7 +480,7 @@ void __kprobes oops_end(unsigned long flags, struct pt_regs *regs, int signr)
>
> int __kprobes __die(const char *str, struct pt_regs *regs, long err)
> {
> - printk(KERN_EMERG "%s: %04lx [%u] ", str, err & 0xffff, ++die_counter);
> + printk(KERN_EMERG "%s: %04lx [#%d] ", str, err & 0xffff, ++die_counter);
> #ifdef CONFIG_PREEMPT
> printk("PREEMPT ");
> #endif
> --
> 1.5.4.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/

--
Dr Louis Rilling Kerlabs
Skype: louis.rilling Batiment Germanium
Phone: (+33|0) 6 80 89 08 23 80 avenue des Buttes de Coesmes
http://www.kerlabs.com/ 35700 Rennes


Attachments:
(No filename) (2.95 kB)
signature.asc (189.00 B)
Digital signature
Download all attachments

2008-10-06 09:43:52

by Alexander van Heukelum

[permalink] [raw]
Subject: Re: [PATCH] dumpstack: x86: various small unification steps

On Mon, 6 Oct 2008 11:35:29 +0200, "Louis Rilling"
<[email protected]> said:
> On Sat, Oct 04, 2008 at 11:12:46PM +0200, Alexander van Heukelum wrote:
> > - define STACKSLOTS_PER_LINE and use it
> > - define get_bp macro to hide the %%ebp/%%rbp difference
> > - i386: check task==NULL in dump_trace, like x86_64
> > - i386: show_trace(NULL, ...) uses current automatically
> > - x86_64: use [#%d] for die_counter, like i386
> > - whitespace and comments
> >
> [...]
>
> > diff --git a/arch/x86/kernel/dumpstack_64.c b/arch/x86/kernel/dumpstack_64.c
> > index 6ac80f0..ddc8a38 100644
> > --- a/arch/x86/kernel/dumpstack_64.c
> > +++ b/arch/x86/kernel/dumpstack_64.c
> > @@ -16,8 +16,11 @@
> >
> > #include <asm/stacktrace.h>
> >
> > +#define STACKSLOTS_PER_LINE 4
> > +#define get_bp(bp) asm("movl %%rbp, %0" : "=r" (bp) :)
> ^^^^
> Should be movq.

Hi Louis,

You're right. A fix has been sent and applied already, though.

Thanks,
Alexander

> Louis
>
> > int panic_on_unrecovered_nmi;
> > -int kstack_depth_to_print = 12;
> > +int kstack_depth_to_print = 3 * STACKSLOTS_PER_LINE;
> > static unsigned int code_bytes = 64;
> > static int die_counter;
> >
> > @@ -177,7 +180,7 @@ void dump_trace(struct task_struct *task, struct pt_regs *regs,
> > if (!bp) {
> > if (task == current) {
> > /* Grab bp right from our regs */
> > - asm("movq %%rbp, %0" : "=r" (bp) : );
> > + get_bp(bp);
> > } else {
> > /* bp is the last reg pushed by switch_to */
> > bp = *(unsigned long *) task->thread.sp;
> > @@ -329,7 +332,7 @@ show_stack_log_lvl(struct task_struct *task, struct pt_regs *regs,
> > if (((long) stack & (THREAD_SIZE-1)) == 0)
> > break;
> > }
> > - if (i && ((i % 4) == 0))
> > + if (i && ((i % STACKSLOTS_PER_LINE) == 0))
> > printk("\n%s", log_lvl);
> > printk(" %016lx", *stack++);
> > touch_nmi_watchdog();
> > @@ -353,7 +356,7 @@ void dump_stack(void)
> >
> > #ifdef CONFIG_FRAME_POINTER
> > if (!bp)
> > - asm("movq %%rbp, %0" : "=r" (bp) : );
> > + get_bp(bp);
> > #endif
> >
> > printk("Pid: %d, comm: %.20s %s %s %.*s\n",
> > @@ -396,7 +399,7 @@ void show_registers(struct pt_regs *regs)
> >
> > ip = (u8 *)regs->ip - code_prologue;
> > if (ip < (u8 *)PAGE_OFFSET || probe_kernel_address(ip, c)) {
> > - /* try starting at RIP */
> > + /* try starting at IP */
> > ip = (u8 *)regs->ip;
> > code_len = code_len - code_prologue + 1;
> > }
> > @@ -477,7 +480,7 @@ void __kprobes oops_end(unsigned long flags, struct pt_regs *regs, int signr)
> >
> > int __kprobes __die(const char *str, struct pt_regs *regs, long err)
> > {
> > - printk(KERN_EMERG "%s: %04lx [%u] ", str, err & 0xffff, ++die_counter);
> > + printk(KERN_EMERG "%s: %04lx [#%d] ", str, err & 0xffff, ++die_counter);
> > #ifdef CONFIG_PREEMPT
> > printk("PREEMPT ");
> > #endif
> > --
> > 1.5.4.3
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> > the body of a message to [email protected]
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> > Please read the FAQ at http://www.tux.org/lkml/
>
> --
> Dr Louis Rilling Kerlabs
> Skype: louis.rilling Batiment Germanium
> Phone: (+33|0) 6 80 89 08 23 80 avenue des Buttes de Coesmes
> http://www.kerlabs.com/ 35700 Rennes
--
Alexander van Heukelum
[email protected]

--
http://www.fastmail.fm - IMAP accessible web-mail