2013-04-03 19:15:35

by Tejun Heo

[permalink] [raw]
Subject: [PATCHSET v2] arch: unify task dump debug info

Hello,

Andrew, I think it's about ready and nobody seems to be against the
proposed changes. Can you please take these into -mm?

This v2 of this patchset. Changes from the last posting[1] are,

* CPU is now printed for both dump_stack() and show_regs() which s390
folks tell me should be enough to allow converting s390 to use the
generic version.

* s390 converted.

* DMI arch ID now uses the same ID string as the debug "DMI: "
printout during boot as suggested by Bjorn Helgaas.

* arc duplicate removal patch folded in.

* Build breakage on blackfin should be fixed now.

* Rebased on top of -mm.

There are multiple ways a task can be dumped - explicit call to
dump_stack(), triggering WARN() or BUG(), through sysrq-t and so on.
Most of what gets printed is upto each architecture and the current
state is not particularly pretty. Different pieces of information are
presented differently depending on which path the dump takes and which
architecture it's running on. This is messy for no good reason and
makes it exceedingly difficult to add or modify debug information to
task dumps.

In all archs except for s390, there's nothing arch-specific about the
printed debug information. This patchset updates all those archs to
use the same helpers to consistently print out the same debug
information.

An example WARN dump after this patchset.

WARNING: at /work/os/work/kernel/workqueue.c:4841 init_workqueues+0x35/0x505()
Modules linked in:
CPU: 0 PID: 1 Comm: swapper/0 Not tainted 3.9.0-rc1-work+ #3
Hardware name: empty empty/S3992, BIOS 080011 10/26/2007
0000000000000009 ffff88007c861e08 ffffffff81c614dc ffff88007c861e48
ffffffff8108f500 ffffffff82228240 0000000000000040 ffffffff8234a08e
0000000000000000 0000000000000000 0000000000000000 ffff88007c861e58
Call Trace:
[<ffffffff81c614dc>] dump_stack+0x19/0x1b
[<ffffffff8108f500>] warn_slowpath_common+0x70/0xa0
[<ffffffff8108f54a>] warn_slowpath_null+0x1a/0x20
[<ffffffff8234a0c3>] init_workqueues+0x35/0x505
...

And BUG dump.

kernel BUG at /work/os/work/kernel/workqueue.c:4841!
invalid opcode: 0000 [#1] PREEMPT SMP DEBUG_PAGEALLOC
Modules linked in:
CPU: 0 PID: 1 Comm: swapper/0 Not tainted 3.9.0-rc1-work+ #7
Hardware name: empty empty/S3992, BIOS 080011 10/26/2007
task: ffff88007c85e040 ti: ffff88007c860000 task.ti: ffff88007c860000
RIP: 0010:[<ffffffff8234a07e>] [<ffffffff8234a07e>] init_workqueues+0x4/0x6
RSP: 0000:ffff88007c861ec8 EFLAGS: 00010246
RAX: ffff88007c861fd8 RBX: ffffffff824466a8 RCX: 0000000000000001
RDX: 0000000000000046 RSI: 0000000000000001 RDI: ffffffff8234a07a
RBP: ffff88007c861ec8 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000001 R11: 0000000000000000 R12: ffffffff8234a07a
R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000
FS: 0000000000000000(0000) GS:ffff88007dc00000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b
CR2: ffff88015f7ff000 CR3: 00000000021f1000 CR4: 00000000000007f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
Stack:
ffff88007c861ef8 ffffffff81000312 ffffffff824466a8 ffff88007c85e650
0000000000000003 0000000000000000 ffff88007c861f38 ffffffff82335e5d
ffff88007c862080 ffffffff8223d8c0 ffff88007c862080 ffffffff81c47760
Call Trace:
[<ffffffff81000312>] do_one_initcall+0x122/0x170
[<ffffffff82335e5d>] kernel_init_freeable+0x9b/0x1c8
[<ffffffff81c47760>] ? rest_init+0x140/0x140
[<ffffffff81c4776e>] kernel_init+0xe/0xf0
[<ffffffff81c6be9c>] ret_from_fork+0x7c/0xb0
[<ffffffff81c47760>] ? rest_init+0x140/0x140
...

This patchset contains the following seven patches.

0001-x86-don-t-show-trace-beyond-show_stack-NULL-NULL.patch
0002-sparc32-make-show_stack-acquire-fp-if-_ksp-is-not-sp.patch
0003-dump_stack-consolidate-dump_stack-implementations-an.patch
0004-dmi-morph-dmi_dump_ids-into-dmi_format_ids-which-for.patch
0005-dump_stack-implement-arch-specific-hardware-descript.patch
0006-dump_stack-unify-debug-information-printed-by-show_r.patch
0007-arc-print-fatal-signals-reduce-duplicated-informatio.patch

0001-0002 update stack dumping functions in x86 and sparc32 in
preparation.

0003 makes all arches except blackfin use generic dump_stack().
blackfin still uses the generic helper to print the same info.

0004-0005 properly abstract DMI identifier printing in WARN() and
show_regs() so that all dumps print out the information. This enables
show_regs() to use the same debug info message.

0006 updates show_regs() of all arches to use a common generic helper
to print debug info.

0007 removes somem duplicate information from arc dumps.

While this patchset changes how debug info is printed on some archs,
the printed information is always superset of what used to be there.

This patchset makes task dump debug messages consistent and enables
adding more information. Workqueue is scheduled to add worker
information including the workqueue in use and work item specific
description.

This patchset is based on top of -mm as of 2013/04/03 and available in
the following git branch.

git://git.kernel.org/pub/scm/linux/kernel/git/tj/misc.git review-unify-dump

While this patch touches a lot of archs, it isn't too likely to cause
non-trivial conflicts with arch-specfic changes and would probably be
best to route together either through -mm.

x86 is tested but other archs are either only compile tested or not
tested at all. Changes to most archs are generally trivial.

diffstat follows. Thanks.

arch/alpha/kernel/process.c | 1
arch/alpha/kernel/traps.c | 7 ----
arch/arc/kernel/stacktrace.c | 7 ----
arch/arc/kernel/troubleshoot.c | 3 +
arch/arm/kernel/process.c | 8 +---
arch/arm/kernel/traps.c | 7 ----
arch/arm64/kernel/process.c | 7 ----
arch/arm64/kernel/traps.c | 7 ----
arch/avr32/kernel/process.c | 13 +------
arch/blackfin/kernel/dumpstack.c | 1
arch/blackfin/kernel/trace.c | 2 +
arch/c6x/kernel/traps.c | 10 ------
arch/cris/arch-v10/kernel/process.c | 3 +
arch/cris/arch-v32/kernel/process.c | 3 +
arch/cris/kernel/traps.c | 7 ----
arch/frv/kernel/traps.c | 14 --------
arch/h8300/kernel/process.c | 2 +
arch/h8300/kernel/traps.c | 7 ----
arch/hexagon/kernel/traps.c | 8 ----
arch/hexagon/kernel/vm_events.c | 2 +
arch/ia64/kernel/process.c | 12 +------
arch/ia64/kernel/setup.c | 1
arch/m32r/kernel/process.c | 2 +
arch/m32r/kernel/traps.c | 9 -----
arch/m68k/kernel/traps.c | 12 -------
arch/metag/kernel/process.c | 2 +
arch/metag/kernel/traps.c | 6 ---
arch/microblaze/kernel/process.c | 2 +
arch/microblaze/kernel/traps.c | 6 ---
arch/mips/kernel/traps.c | 15 ---------
arch/mn10300/kernel/process.c | 1
arch/mn10300/kernel/traps.c | 11 ------
arch/openrisc/kernel/process.c | 1
arch/openrisc/kernel/traps.c | 11 ------
arch/parisc/kernel/traps.c | 10 +-----
arch/powerpc/kernel/process.c | 14 +-------
arch/s390/kernel/dumpstack.c | 26 ---------------
arch/score/kernel/traps.c | 12 +------
arch/sh/kernel/dumpstack.c | 6 ---
arch/sh/kernel/process_32.c | 6 ---
arch/sh/kernel/process_64.c | 1
arch/sparc/kernel/process_32.c | 23 ++++---------
arch/sparc/kernel/process_64.c | 2 +
arch/sparc/kernel/traps_64.c | 7 ----
arch/tile/kernel/process.c | 3 -
arch/um/kernel/sysrq.c | 12 -------
arch/um/sys-ppc/sysrq.c | 2 +
arch/unicore32/kernel/process.c | 6 ---
arch/unicore32/kernel/traps.c | 6 ---
arch/x86/include/asm/bug.h | 3 -
arch/x86/kernel/dumpstack.c | 28 ++++++----------
arch/x86/kernel/dumpstack_32.c | 4 --
arch/x86/kernel/dumpstack_64.c | 6 ---
arch/x86/kernel/process.c | 24 --------------
arch/x86/kernel/process_32.c | 2 -
arch/x86/kernel/process_64.c | 1
arch/x86/kernel/setup.c | 1
arch/xtensa/kernel/traps.c | 10 +-----
drivers/firmware/dmi_scan.c | 57 ++++++++++++++++++++++++----------
include/linux/dmi.h | 2 +
include/linux/printk.h | 15 +++++++++
kernel/panic.c | 6 ---
kernel/printk.c | 60 ++++++++++++++++++++++++++++++++++++
kernel/signal.c | 3 -
lib/dump_stack.c | 11 ++++--
65 files changed, 200 insertions(+), 379 deletions(-)

--
tejun

[1] https://lkml.org/lkml/2013/3/29/354


2013-04-03 19:15:43

by Tejun Heo

[permalink] [raw]
Subject: [PATCH 1/7] x86: don't show trace beyond show_stack(NULL, NULL)

show_stack(current or NULL, NULL) is used to print the backtrace of
the current task. As trace beyond the function itself isn't of much
interest to anyone, don't show it by determining sp and bp in
show_stack()'s frame and passing them to show_stack_log_lvl().

This brings show_stack(NULL, NULL)'s behavior in line with
dump_stack().

Signed-off-by: Tejun Heo <[email protected]>
---
arch/x86/kernel/dumpstack.c | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kernel/dumpstack.c b/arch/x86/kernel/dumpstack.c
index c8797d5..dd1a7c3 100644
--- a/arch/x86/kernel/dumpstack.c
+++ b/arch/x86/kernel/dumpstack.c
@@ -176,7 +176,19 @@ void show_trace(struct task_struct *task, struct pt_regs *regs,

void show_stack(struct task_struct *task, unsigned long *sp)
{
- show_stack_log_lvl(task, NULL, sp, 0, "");
+ unsigned long bp = 0;
+ unsigned long stack;
+
+ /*
+ * Stack frames below this one aren't interesting. Don't show them
+ * if we're printing for %current.
+ */
+ if (!sp && (!task || task == current)) {
+ sp = &stack;
+ bp = stack_frame(current, NULL);
+ }
+
+ show_stack_log_lvl(task, NULL, sp, bp, "");
}

/*
--
1.8.1.4

2013-04-03 19:16:01

by Tejun Heo

[permalink] [raw]
Subject: [PATCH 2/7] sparc32: make show_stack() acquire %fp if @_ksp is not specified

show_stack(current or NULL, NULL) is used by arch-independent code to
dump backtrace of the current task; however, sparc32 show_stack()
doesn't implement it and wouldn't print any backtrace when NULL @_ksp
is specfied.

Make show_stack() acquire and use %fp if @tsk is NULL or current and
@_ksp is NULL. This makes %fp fetching in dump_stack() unnecessary.
Make it use NULL for @_ksp instead.

Only compile tested.

Signed-off-by: Tejun Heo <[email protected]>
Acked-by: David S. Miller <[email protected]>
---
arch/sparc/kernel/process_32.c | 16 +++++++---------
1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/arch/sparc/kernel/process_32.c b/arch/sparc/kernel/process_32.c
index 62eede1..0d57b24 100644
--- a/arch/sparc/kernel/process_32.c
+++ b/arch/sparc/kernel/process_32.c
@@ -153,11 +153,13 @@ void show_stack(struct task_struct *tsk, unsigned long *_ksp)
struct reg_window32 *rw;
int count = 0;

- if (tsk != NULL)
- task_base = (unsigned long) task_stack_page(tsk);
- else
- task_base = (unsigned long) current_thread_info();
+ if (!tsk)
+ tsk = current;

+ if (tsk == current && !_ksp)
+ __asm__ __volatile__("mov %%fp, %0" : "=r" (_ksp));
+
+ task_base = (unsigned long) task_stack_page(tsk);
fp = (unsigned long) _ksp;
do {
/* Bogus frame pointer? */
@@ -175,11 +177,7 @@ void show_stack(struct task_struct *tsk, unsigned long *_ksp)

void dump_stack(void)
{
- unsigned long *ksp;
-
- __asm__ __volatile__("mov %%fp, %0"
- : "=r" (ksp));
- show_stack(current, ksp);
+ show_stack(current, NULL);
}

EXPORT_SYMBOL(dump_stack);
--
1.8.1.4

2013-04-03 19:16:15

by Tejun Heo

[permalink] [raw]
Subject: [PATCH 3/7] dump_stack: consolidate dump_stack() implementations and unify their behaviors

Both dump_stack() and show_stack() are currently implemented by each
architecture. show_stack(NULL, NULL) dumps the backtrace for the
current task as does dump_stack(). On some archs, dump_stack() prints
extra information - pid, utsname and so on - in addition to the
backtrace while the two are identical on other archs.

The usages in arch-independent code of the two functions indicate
show_stack(NULL, NULL) should print out bare backtrace while
dump_stack() is used for debugging purposes when something went wrong,
so it does make sense to print additional information on the task
which triggered dump_stack().

There's no reason to require archs to implement two separate but
mostly identical functions. It leads to unnecessary subtle
differences among archs and makes it very tedius to add generic debug
information.

This patch expands the dummy fallback dump_stack() implementation in
lib/dump_stack.c such that it prints out debug information (taken from
x86) and invokes show_stack(NULL, NULL) and drops arch-specific
dump_stack() implementations in all archs except blackfin. Blackfin's
dump_stack() does something wonky that I don't understand.

Debug information can be printed separately by calling
dump_stack_print_info() so that arch-specific dump_stack()
implementation can still emit the same debug information. This is
used in blackfin.

This patch brings the following behavior changes.

* On some archs, an extra level in backtrace for show_stack() could be
printed. This is because the top frame was determined in
dump_stack() on those archs while generic dump_stack() can't do that
reliably. It can be compensated by inlining dump_stack() but not
sure whether that'd be necessary.

* Most archs didn't use to print debug info on dump_stack(). They do
now.

An example WARN dump follows.

WARNING: at /work/os/work/kernel/workqueue.c:4841 init_workqueues+0x35/0x505()
Hardware name: empty
Modules linked in:
CPU: 0 PID: 1 Comm: swapper/0 Not tainted 3.9.0-rc1-work+ #9
0000000000000009 ffff88007c861e08 ffffffff81c614dc ffff88007c861e48
ffffffff8108f50f ffffffff82228240 0000000000000040 ffffffff8234a03c
0000000000000000 0000000000000000 0000000000000000 ffff88007c861e58
Call Trace:
[<ffffffff81c614dc>] dump_stack+0x19/0x1b
[<ffffffff8108f50f>] warn_slowpath_common+0x7f/0xc0
[<ffffffff8108f56a>] warn_slowpath_null+0x1a/0x20
[<ffffffff8234a071>] init_workqueues+0x35/0x505
...

v2: CPU number added to the generic debug info as requested by s390
folks and dropped the s390 specific dump_stack(). This loses %ksp
from the debug message which the maintainers think isn't important
enough to keep the s390-specific dump_stack() implementation.

dump_stack_print_info() is moved to kernel/printk.c from
lib/dump_stack.c. Because linkage is per objecct file,
dump_stack_print_info() living in the same lib file as generic
dump_stack() means that archs which implement custom dump_stack()
- at this point, only blackfin - can't use dump_stack_print_info()
as that will bring in the generic version of dump_stack() too. v1
The v1 patch broke build on blackfin due to this issue. The build
breakage was reported by Fengguang Wu.

Signed-off-by: Tejun Heo <[email protected]>
Acked-by: David S. Miller <[email protected]>
Acked-by: Vineet Gupta <[email protected]>
Acked-by: Jesper Nilsson <[email protected]>
Acked-by: Vineet Gupta <[email protected]>
Cc: Martin Schwidefsky <[email protected]>
Cc: Heiko Carstens <[email protected]>
Cc: [email protected]
Cc: Mike Frysinger <[email protected]>
Cc: [email protected]
Cc: Fengguang Wu <[email protected]>
---
arch/alpha/kernel/traps.c | 7 -------
arch/arc/kernel/stacktrace.c | 7 -------
arch/arm/kernel/traps.c | 7 -------
arch/arm64/kernel/traps.c | 7 -------
arch/avr32/kernel/process.c | 8 --------
arch/blackfin/kernel/dumpstack.c | 1 +
arch/c6x/kernel/traps.c | 9 ---------
arch/cris/kernel/traps.c | 7 -------
arch/frv/kernel/traps.c | 11 -----------
arch/h8300/kernel/traps.c | 7 -------
arch/hexagon/kernel/traps.c | 8 --------
arch/ia64/kernel/process.c | 8 --------
arch/m32r/kernel/traps.c | 9 ---------
arch/m68k/kernel/traps.c | 12 ------------
arch/metag/kernel/traps.c | 6 ------
arch/microblaze/kernel/traps.c | 6 ------
arch/mips/kernel/traps.c | 13 -------------
arch/mn10300/kernel/traps.c | 11 -----------
arch/openrisc/kernel/traps.c | 11 -----------
arch/parisc/kernel/traps.c | 8 --------
arch/powerpc/kernel/process.c | 6 ------
arch/s390/kernel/dumpstack.c | 17 -----------------
arch/score/kernel/traps.c | 10 ----------
arch/sh/kernel/dumpstack.c | 6 ------
arch/sparc/kernel/process_32.c | 7 -------
arch/sparc/kernel/traps_64.c | 7 -------
arch/um/kernel/sysrq.c | 12 ------------
arch/unicore32/kernel/traps.c | 6 ------
arch/x86/kernel/dumpstack.c | 18 ------------------
arch/xtensa/kernel/traps.c | 8 --------
include/linux/printk.h | 5 +++++
kernel/printk.c | 18 ++++++++++++++++++
lib/dump_stack.c | 11 ++++++++---
33 files changed, 32 insertions(+), 262 deletions(-)

diff --git a/arch/alpha/kernel/traps.c b/arch/alpha/kernel/traps.c
index 4037461..affccb9 100644
--- a/arch/alpha/kernel/traps.c
+++ b/arch/alpha/kernel/traps.c
@@ -169,13 +169,6 @@ void show_stack(struct task_struct *task, unsigned long *sp)
dik_show_trace(sp);
}

-void dump_stack(void)
-{
- show_stack(NULL, NULL);
-}
-
-EXPORT_SYMBOL(dump_stack);
-
void
die_if_kernel(char * str, struct pt_regs *regs, long err, unsigned long *r9_15)
{
diff --git a/arch/arc/kernel/stacktrace.c b/arch/arc/kernel/stacktrace.c
index a63ff84..ca0207b 100644
--- a/arch/arc/kernel/stacktrace.c
+++ b/arch/arc/kernel/stacktrace.c
@@ -220,13 +220,6 @@ void show_stack(struct task_struct *tsk, unsigned long *sp)
show_stacktrace(tsk, NULL);
}

-/* Expected by Rest of kernel code */
-void dump_stack(void)
-{
- show_stacktrace(NULL, NULL);
-}
-EXPORT_SYMBOL(dump_stack);
-
/* Another API expected by schedular, shows up in "ps" as Wait Channel
* Ofcourse just returning schedule( ) would be pointless so unwind until
* the function is not in schedular code
diff --git a/arch/arm/kernel/traps.c b/arch/arm/kernel/traps.c
index 1c08911..18b32e8 100644
--- a/arch/arm/kernel/traps.c
+++ b/arch/arm/kernel/traps.c
@@ -204,13 +204,6 @@ static void dump_backtrace(struct pt_regs *regs, struct task_struct *tsk)
}
#endif

-void dump_stack(void)
-{
- dump_backtrace(NULL, NULL);
-}
-
-EXPORT_SYMBOL(dump_stack);
-
void show_stack(struct task_struct *tsk, unsigned long *sp)
{
dump_backtrace(NULL, tsk);
diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
index b3c5f62..61d7dd2 100644
--- a/arch/arm64/kernel/traps.c
+++ b/arch/arm64/kernel/traps.c
@@ -167,13 +167,6 @@ static void dump_backtrace(struct pt_regs *regs, struct task_struct *tsk)
}
}

-void dump_stack(void)
-{
- dump_backtrace(NULL, NULL);
-}
-
-EXPORT_SYMBOL(dump_stack);
-
void show_stack(struct task_struct *tsk, unsigned long *sp)
{
dump_backtrace(NULL, tsk);
diff --git a/arch/avr32/kernel/process.c b/arch/avr32/kernel/process.c
index fd78f58..c1cafdb 100644
--- a/arch/avr32/kernel/process.c
+++ b/arch/avr32/kernel/process.c
@@ -213,14 +213,6 @@ void show_stack(struct task_struct *tsk, unsigned long *stack)
show_stack_log_lvl(tsk, (unsigned long)stack, NULL, "");
}

-void dump_stack(void)
-{
- unsigned long stack;
-
- show_trace_log_lvl(current, &stack, NULL, "");
-}
-EXPORT_SYMBOL(dump_stack);
-
static const char *cpu_modes[] = {
"Application", "Supervisor", "Interrupt level 0", "Interrupt level 1",
"Interrupt level 2", "Interrupt level 3", "Exception", "NMI"
diff --git a/arch/blackfin/kernel/dumpstack.c b/arch/blackfin/kernel/dumpstack.c
index 5cfbaa2..95ba6d9 100644
--- a/arch/blackfin/kernel/dumpstack.c
+++ b/arch/blackfin/kernel/dumpstack.c
@@ -168,6 +168,7 @@ void dump_stack(void)
#endif
trace_buffer_save(tflags);
dump_bfin_trace_buffer();
+ dump_stack_print_info(KERN_DEFAULT);
show_stack(current, &stack);
trace_buffer_restore(tflags);
}
diff --git a/arch/c6x/kernel/traps.c b/arch/c6x/kernel/traps.c
index 1be74e5..d0b96ef 100644
--- a/arch/c6x/kernel/traps.c
+++ b/arch/c6x/kernel/traps.c
@@ -67,15 +67,6 @@ void show_regs(struct pt_regs *regs)
pr_err("A31: %08lx B31: %08lx\n", regs->a31, regs->b31);
}

-void dump_stack(void)
-{
- unsigned long stack;
-
- show_stack(current, &stack);
-}
-EXPORT_SYMBOL(dump_stack);
-
-
void die(char *str, struct pt_regs *fp, int nr)
{
console_verbose();
diff --git a/arch/cris/kernel/traps.c b/arch/cris/kernel/traps.c
index a11ad32..0ffda73 100644
--- a/arch/cris/kernel/traps.c
+++ b/arch/cris/kernel/traps.c
@@ -147,13 +147,6 @@ show_stack(void)
#endif

void
-dump_stack(void)
-{
- show_stack(NULL, NULL);
-}
-EXPORT_SYMBOL(dump_stack);
-
-void
set_nmi_handler(void (*handler)(struct pt_regs *))
{
nmi_handler = handler;
diff --git a/arch/frv/kernel/traps.c b/arch/frv/kernel/traps.c
index 5cfd142..cfcd802 100644
--- a/arch/frv/kernel/traps.c
+++ b/arch/frv/kernel/traps.c
@@ -466,17 +466,6 @@ asmlinkage void compound_exception(unsigned long esfr1,
BUG();
} /* end compound_exception() */

-/*****************************************************************************/
-/*
- * The architecture-independent backtrace generator
- */
-void dump_stack(void)
-{
- show_stack(NULL, NULL);
-}
-
-EXPORT_SYMBOL(dump_stack);
-
void show_stack(struct task_struct *task, unsigned long *sp)
{
}
diff --git a/arch/h8300/kernel/traps.c b/arch/h8300/kernel/traps.c
index 7833aa3..cfe494d 100644
--- a/arch/h8300/kernel/traps.c
+++ b/arch/h8300/kernel/traps.c
@@ -164,10 +164,3 @@ void show_trace_task(struct task_struct *tsk)
{
show_stack(tsk,(unsigned long *)tsk->thread.esp0);
}
-
-void dump_stack(void)
-{
- show_stack(NULL,NULL);
-}
-
-EXPORT_SYMBOL(dump_stack);
diff --git a/arch/hexagon/kernel/traps.c b/arch/hexagon/kernel/traps.c
index aaf53f8..7858663 100644
--- a/arch/hexagon/kernel/traps.c
+++ b/arch/hexagon/kernel/traps.c
@@ -195,14 +195,6 @@ void show_stack(struct task_struct *task, unsigned long *fp)
do_show_stack(task, fp, 0);
}

-void dump_stack(void)
-{
- unsigned long *fp;
- asm("%0 = r30" : "=r" (fp));
- show_stack(current, fp);
-}
-EXPORT_SYMBOL(dump_stack);
-
int die(const char *str, struct pt_regs *regs, long err)
{
static struct {
diff --git a/arch/ia64/kernel/process.c b/arch/ia64/kernel/process.c
index 6f7dc8b..87c7a26 100644
--- a/arch/ia64/kernel/process.c
+++ b/arch/ia64/kernel/process.c
@@ -96,14 +96,6 @@ show_stack (struct task_struct *task, unsigned long *sp)
}

void
-dump_stack (void)
-{
- show_stack(NULL, NULL);
-}
-
-EXPORT_SYMBOL(dump_stack);
-
-void
show_regs (struct pt_regs *regs)
{
unsigned long ip = regs->cr_iip + ia64_psr(regs)->ri;
diff --git a/arch/m32r/kernel/traps.c b/arch/m32r/kernel/traps.c
index 3bcb207..5623ea3 100644
--- a/arch/m32r/kernel/traps.c
+++ b/arch/m32r/kernel/traps.c
@@ -169,15 +169,6 @@ void show_stack(struct task_struct *task, unsigned long *sp)
show_trace(task, sp);
}

-void dump_stack(void)
-{
- unsigned long stack;
-
- show_trace(current, &stack);
-}
-
-EXPORT_SYMBOL(dump_stack);
-
static void show_registers(struct pt_regs *regs)
{
int i = 0;
diff --git a/arch/m68k/kernel/traps.c b/arch/m68k/kernel/traps.c
index f32ab22..88fcd8c 100644
--- a/arch/m68k/kernel/traps.c
+++ b/arch/m68k/kernel/traps.c
@@ -992,18 +992,6 @@ void show_stack(struct task_struct *task, unsigned long *stack)
}

/*
- * The architecture-independent backtrace generator
- */
-void dump_stack(void)
-{
- unsigned long stack;
-
- show_trace(&stack);
-}
-
-EXPORT_SYMBOL(dump_stack);
-
-/*
* The vector number returned in the frame pointer may also contain
* the "fs" (Fault Status) bits on ColdFire. These are in the bottom
* 2 bits, and upper 2 bits. So we need to mask out the real vector
diff --git a/arch/metag/kernel/traps.c b/arch/metag/kernel/traps.c
index 8961f24..2ceeaae 100644
--- a/arch/metag/kernel/traps.c
+++ b/arch/metag/kernel/traps.c
@@ -987,9 +987,3 @@ void show_stack(struct task_struct *tsk, unsigned long *sp)

show_trace(tsk, sp, NULL);
}
-
-void dump_stack(void)
-{
- show_stack(NULL, NULL);
-}
-EXPORT_SYMBOL(dump_stack);
diff --git a/arch/microblaze/kernel/traps.c b/arch/microblaze/kernel/traps.c
index 30e6b50..cb61953 100644
--- a/arch/microblaze/kernel/traps.c
+++ b/arch/microblaze/kernel/traps.c
@@ -75,9 +75,3 @@ void show_stack(struct task_struct *task, unsigned long *sp)

debug_show_held_locks(task);
}
-
-void dump_stack(void)
-{
- show_stack(NULL, NULL);
-}
-EXPORT_SYMBOL(dump_stack);
diff --git a/arch/mips/kernel/traps.c b/arch/mips/kernel/traps.c
index 59351e5..d2a31ac 100644
--- a/arch/mips/kernel/traps.c
+++ b/arch/mips/kernel/traps.c
@@ -206,19 +206,6 @@ void show_stack(struct task_struct *task, unsigned long *sp)
show_stacktrace(task, &regs);
}

-/*
- * The architecture-independent dump_stack generator
- */
-void dump_stack(void)
-{
- struct pt_regs regs;
-
- prepare_frametrace(&regs);
- show_backtrace(current, &regs);
-}
-
-EXPORT_SYMBOL(dump_stack);
-
static void show_code(unsigned int __user *pc)
{
long i;
diff --git a/arch/mn10300/kernel/traps.c b/arch/mn10300/kernel/traps.c
index b900e5a..a7a987c 100644
--- a/arch/mn10300/kernel/traps.c
+++ b/arch/mn10300/kernel/traps.c
@@ -294,17 +294,6 @@ void show_stack(struct task_struct *task, unsigned long *sp)
}

/*
- * the architecture-independent dump_stack generator
- */
-void dump_stack(void)
-{
- unsigned long stack;
-
- show_stack(current, &stack);
-}
-EXPORT_SYMBOL(dump_stack);
-
-/*
* dump the register file in the specified exception frame
*/
void show_registers_only(struct pt_regs *regs)
diff --git a/arch/openrisc/kernel/traps.c b/arch/openrisc/kernel/traps.c
index 5cce396..3d3f606 100644
--- a/arch/openrisc/kernel/traps.c
+++ b/arch/openrisc/kernel/traps.c
@@ -105,17 +105,6 @@ void show_trace_task(struct task_struct *tsk)
*/
}

-/*
- * The architecture-independent backtrace generator
- */
-void dump_stack(void)
-{
- unsigned long stack;
-
- show_stack(current, &stack);
-}
-EXPORT_SYMBOL(dump_stack);
-
void show_registers(struct pt_regs *regs)
{
int i;
diff --git a/arch/parisc/kernel/traps.c b/arch/parisc/kernel/traps.c
index aeb8f8f..e64cf5f 100644
--- a/arch/parisc/kernel/traps.c
+++ b/arch/parisc/kernel/traps.c
@@ -158,14 +158,6 @@ void show_regs(struct pt_regs *regs)
}
}

-
-void dump_stack(void)
-{
- show_stack(NULL, NULL);
-}
-
-EXPORT_SYMBOL(dump_stack);
-
static void do_show_stack(struct unwind_frame_info *info)
{
int i = 1;
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index 59dd545..2e6f83a 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -1360,12 +1360,6 @@ void show_stack(struct task_struct *tsk, unsigned long *stack)
} while (count++ < kstack_depth_to_print);
}

-void dump_stack(void)
-{
- show_stack(current, NULL);
-}
-EXPORT_SYMBOL(dump_stack);
-
#ifdef CONFIG_PPC64
/* Called with hard IRQs off */
void __ppc64_runlatch_on(void)
diff --git a/arch/s390/kernel/dumpstack.c b/arch/s390/kernel/dumpstack.c
index 03dce39..2f1f639 100644
--- a/arch/s390/kernel/dumpstack.c
+++ b/arch/s390/kernel/dumpstack.c
@@ -129,23 +129,6 @@ static void show_last_breaking_event(struct pt_regs *regs)
#endif
}

-/*
- * The architecture-independent dump_stack generator
- */
-void dump_stack(void)
-{
- printk("CPU: %d %s %s %.*s\n",
- task_thread_info(current)->cpu, print_tainted(),
- init_utsname()->release,
- (int)strcspn(init_utsname()->version, " "),
- init_utsname()->version);
- printk("Process %s (pid: %d, task: %p, ksp: %p)\n",
- current->comm, current->pid, current,
- (void *) current->thread.ksp);
- show_stack(NULL, NULL);
-}
-EXPORT_SYMBOL(dump_stack);
-
static inline int mask_bits(struct pt_regs *regs, unsigned long bits)
{
return (regs->psw.mask & bits) / ((~bits + 1) & bits);
diff --git a/arch/score/kernel/traps.c b/arch/score/kernel/traps.c
index 0e46fb1..a38f435 100644
--- a/arch/score/kernel/traps.c
+++ b/arch/score/kernel/traps.c
@@ -149,16 +149,6 @@ static void show_registers(struct pt_regs *regs)
printk(KERN_NOTICE "\n");
}

-/*
- * The architecture-independent dump_stack generator
- */
-void dump_stack(void)
-{
- show_stack(current_thread_info()->task,
- (long *) get_irq_regs()->regs[0]);
-}
-EXPORT_SYMBOL(dump_stack);
-
void __die(const char *str, struct pt_regs *regs, const char *file,
const char *func, unsigned long line)
{
diff --git a/arch/sh/kernel/dumpstack.c b/arch/sh/kernel/dumpstack.c
index 7617dc4..b959f55 100644
--- a/arch/sh/kernel/dumpstack.c
+++ b/arch/sh/kernel/dumpstack.c
@@ -158,9 +158,3 @@ void show_stack(struct task_struct *tsk, unsigned long *sp)
(unsigned long)task_stack_page(tsk));
show_trace(tsk, sp, NULL);
}
-
-void dump_stack(void)
-{
- show_stack(NULL, NULL);
-}
-EXPORT_SYMBOL(dump_stack);
diff --git a/arch/sparc/kernel/process_32.c b/arch/sparc/kernel/process_32.c
index 0d57b24..fe99fbd 100644
--- a/arch/sparc/kernel/process_32.c
+++ b/arch/sparc/kernel/process_32.c
@@ -175,13 +175,6 @@ void show_stack(struct task_struct *tsk, unsigned long *_ksp)
printk("\n");
}

-void dump_stack(void)
-{
- show_stack(current, NULL);
-}
-
-EXPORT_SYMBOL(dump_stack);
-
/*
* Note: sparc64 has a pretty intricated thread_saved_pc, check it out.
*/
diff --git a/arch/sparc/kernel/traps_64.c b/arch/sparc/kernel/traps_64.c
index 8d38ca9..b3f833a 100644
--- a/arch/sparc/kernel/traps_64.c
+++ b/arch/sparc/kernel/traps_64.c
@@ -2350,13 +2350,6 @@ void show_stack(struct task_struct *tsk, unsigned long *_ksp)
} while (++count < 16);
}

-void dump_stack(void)
-{
- show_stack(current, NULL);
-}
-
-EXPORT_SYMBOL(dump_stack);
-
static inline struct reg_window *kernel_stack_up(struct reg_window *rw)
{
unsigned long fp = rw->ins[6];
diff --git a/arch/um/kernel/sysrq.c b/arch/um/kernel/sysrq.c
index e562ff8..7d101a2 100644
--- a/arch/um/kernel/sysrq.c
+++ b/arch/um/kernel/sysrq.c
@@ -35,18 +35,6 @@ void show_trace(struct task_struct *task, unsigned long * stack)
}
#endif

-/*
- * stack dumps generator - this is used by arch-independent code.
- * And this is identical to i386 currently.
- */
-void dump_stack(void)
-{
- unsigned long stack;
-
- show_trace(current, &stack);
-}
-EXPORT_SYMBOL(dump_stack);
-
/*Stolen from arch/i386/kernel/traps.c */
static const int kstack_depth_to_print = 24;

diff --git a/arch/unicore32/kernel/traps.c b/arch/unicore32/kernel/traps.c
index 0870b68..c54e324 100644
--- a/arch/unicore32/kernel/traps.c
+++ b/arch/unicore32/kernel/traps.c
@@ -170,12 +170,6 @@ static void dump_backtrace(struct pt_regs *regs, struct task_struct *tsk)
c_backtrace(fp, mode);
}

-void dump_stack(void)
-{
- dump_backtrace(NULL, NULL);
-}
-EXPORT_SYMBOL(dump_stack);
-
void show_stack(struct task_struct *tsk, unsigned long *sp)
{
dump_backtrace(NULL, tsk);
diff --git a/arch/x86/kernel/dumpstack.c b/arch/x86/kernel/dumpstack.c
index dd1a7c3..deb6421 100644
--- a/arch/x86/kernel/dumpstack.c
+++ b/arch/x86/kernel/dumpstack.c
@@ -191,24 +191,6 @@ void show_stack(struct task_struct *task, unsigned long *sp)
show_stack_log_lvl(task, NULL, sp, bp, "");
}

-/*
- * The architecture-independent dump_stack generator
- */
-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, bp);
-}
-EXPORT_SYMBOL(dump_stack);
-
static arch_spinlock_t die_lock = __ARCH_SPIN_LOCK_UNLOCKED;
static int die_owner = -1;
static unsigned int die_nest_count;
diff --git a/arch/xtensa/kernel/traps.c b/arch/xtensa/kernel/traps.c
index 923db5c..384b7c7 100644
--- a/arch/xtensa/kernel/traps.c
+++ b/arch/xtensa/kernel/traps.c
@@ -481,14 +481,6 @@ void show_stack(struct task_struct *task, unsigned long *sp)
show_trace(task, stack);
}

-void dump_stack(void)
-{
- show_stack(current, NULL);
-}
-
-EXPORT_SYMBOL(dump_stack);
-
-
void show_code(unsigned int *pc)
{
long i;
diff --git a/include/linux/printk.h b/include/linux/printk.h
index 4890fe6..7ce1f87 100644
--- a/include/linux/printk.h
+++ b/include/linux/printk.h
@@ -145,6 +145,7 @@ extern void wake_up_klogd(void);

void log_buf_kexec_setup(void);
void __init setup_log_buf(int early);
+void dump_stack_print_info(const char *log_lvl);
#else
static inline __printf(1, 0)
int vprintk(const char *s, va_list args)
@@ -182,6 +183,10 @@ static inline void log_buf_kexec_setup(void)
static inline void setup_log_buf(int early)
{
}
+
+static inline void dump_stack_print_info(const char *log_lvl)
+{
+}
#endif

extern void dump_stack(void) __cold;
diff --git a/kernel/printk.c b/kernel/printk.c
index 021053d..a00a285 100644
--- a/kernel/printk.c
+++ b/kernel/printk.c
@@ -44,6 +44,7 @@
#include <linux/rculist.h>
#include <linux/poll.h>
#include <linux/irq_work.h>
+#include <linux/utsname.h>

#include <asm/uaccess.h>

@@ -2850,4 +2851,21 @@ void kmsg_dump_rewind(struct kmsg_dumper *dumper)
raw_spin_unlock_irqrestore(&logbuf_lock, flags);
}
EXPORT_SYMBOL_GPL(kmsg_dump_rewind);
+
+/**
+ * dump_stack_print_info - print generic debug info for dump_stack()
+ * @log_lvl: log level
+ *
+ * Arch-specific dump_stack() implementations can use this function to
+ * print out the same debug information as the generic dump_stack().
+ */
+void dump_stack_print_info(const char *log_lvl)
+{
+ printk("%sCPU: %d PID: %d Comm: %.20s %s %s %.*s\n",
+ log_lvl, raw_smp_processor_id(), current->pid, current->comm,
+ print_tainted(), init_utsname()->release,
+ (int)strcspn(init_utsname()->version, " "),
+ init_utsname()->version);
+}
+
#endif
diff --git a/lib/dump_stack.c b/lib/dump_stack.c
index 42f4f55..53bad09 100644
--- a/lib/dump_stack.c
+++ b/lib/dump_stack.c
@@ -5,11 +5,16 @@

#include <linux/kernel.h>
#include <linux/export.h>
+#include <linux/sched.h>

+/**
+ * dump_stack - dump the current task information and its stack trace
+ *
+ * Architectures can override this implementation by implementing its own.
+ */
void dump_stack(void)
{
- printk(KERN_NOTICE
- "This architecture does not implement dump_stack()\n");
+ dump_stack_print_info(KERN_DEFAULT);
+ show_stack(NULL, NULL);
}
-
EXPORT_SYMBOL(dump_stack);
--
1.8.1.4

2013-04-03 19:16:24

by Tejun Heo

[permalink] [raw]
Subject: [PATCH 4/7] dmi: morph dmi_dump_ids() into dmi_format_ids() which formats into a buffer

We're gonna use DMI identification for other purposes too. Morph
dmi_dump_ids() which is used to print DMI identification as a debug
message during boot into dmi_format_ids() which formats the same
information sans the leading "DMI:" tag into a string buffer.

dmi_present() is updated to format the information into
dmi_ids_string[] using the new function and print it with "DMI:"
prefix. dmi_ids_string[] will be used for another purpose by a future
patch.

Signed-off-by: Tejun Heo <[email protected]>
Cc: Bjorn Helgaas <[email protected]>
---
drivers/firmware/dmi_scan.c | 44 +++++++++++++++++++++++++++-----------------
1 file changed, 27 insertions(+), 17 deletions(-)

diff --git a/drivers/firmware/dmi_scan.c b/drivers/firmware/dmi_scan.c
index 40e940d..5022307 100644
--- a/drivers/firmware/dmi_scan.c
+++ b/drivers/firmware/dmi_scan.c
@@ -22,6 +22,9 @@ static u16 __initdata dmi_ver;
*/
static int dmi_initialized;

+/* DMI system identification string used during boot */
+static char dmi_ids_string[128] __initdata;
+
static const char * __init dmi_string_nosave(const struct dmi_header *dm, u8 s)
{
const u8 *bp = ((u8 *) dm) + dm->length;
@@ -376,38 +379,44 @@ static void __init dmi_decode(const struct dmi_header *dm, void *dummy)
}
}

-static void __init print_filtered(const char *info)
+static int __init print_filtered(char *buf, size_t len, const char *info)
{
+ int c = 0;
const char *p;

if (!info)
- return;
+ return c;

for (p = info; *p; p++)
if (isprint(*p))
- printk(KERN_CONT "%c", *p);
+ c += scnprintf(buf + c, len - c, "%c", *p);
else
- printk(KERN_CONT "\\x%02x", *p & 0xff);
+ c += scnprintf(buf + c, len - c, "\\x%02x", *p & 0xff);
+ return c;
}

-static void __init dmi_dump_ids(void)
+static void __init dmi_format_ids(char *buf, size_t len)
{
+ int c = 0;
const char *board; /* Board Name is optional */

- printk(KERN_DEBUG "DMI: ");
- print_filtered(dmi_get_system_info(DMI_SYS_VENDOR));
- printk(KERN_CONT " ");
- print_filtered(dmi_get_system_info(DMI_PRODUCT_NAME));
+ c += print_filtered(buf + c, len - c,
+ dmi_get_system_info(DMI_SYS_VENDOR));
+ c += scnprintf(buf + c, len - c, " ");
+ c += print_filtered(buf + c, len - c,
+ dmi_get_system_info(DMI_PRODUCT_NAME));
+
board = dmi_get_system_info(DMI_BOARD_NAME);
if (board) {
- printk(KERN_CONT "/");
- print_filtered(board);
+ c += scnprintf(buf + c, len - c, "/");
+ c += print_filtered(buf + c, len - c, board);
}
- printk(KERN_CONT ", BIOS ");
- print_filtered(dmi_get_system_info(DMI_BIOS_VERSION));
- printk(KERN_CONT " ");
- print_filtered(dmi_get_system_info(DMI_BIOS_DATE));
- printk(KERN_CONT "\n");
+ c += scnprintf(buf + c, len - c, ", BIOS ");
+ c += print_filtered(buf + c, len - c,
+ dmi_get_system_info(DMI_BIOS_VERSION));
+ c += scnprintf(buf + c, len - c, " ");
+ c += print_filtered(buf + c, len - c,
+ dmi_get_system_info(DMI_BIOS_DATE));
}

static int __init dmi_present(const u8 *buf)
@@ -454,7 +463,8 @@ static int __init dmi_present(const u8 *buf)
pr_info("Legacy DMI %d.%d present.\n",
dmi_ver >> 8, dmi_ver & 0xFF);
}
- dmi_dump_ids();
+ dmi_format_ids(dmi_ids_string, sizeof(dmi_ids_string));
+ printk(KERN_DEBUG "DMI: %s\n", dmi_ids_string);
return 0;
}
}
--
1.8.1.4

2013-04-03 19:16:28

by Tejun Heo

[permalink] [raw]
Subject: [PATCH 5/7] dump_stack: implement arch-specific hardware description in task dumps

x86 and ia64 can acquire extra hardware identification information
from DMI and print it along with task dumps; however, the usage isn't
consistent.

* x86 show_regs() collects vendor, product and board strings and print
them out with PID, comm and utsname. Some of the information is
printed again later in the same dump.

* warn_slowpath_common() explicitly accesses the DMI board and prints
it out with "Hardware name:" label. This applies to both x86 and
ia64 but is irrelevant on all other archs.

* ia64 doesn't show DMI information on other non-WARN dumps.

This patch introduces arch-specific hardware description used by
dump_stack(). It can be set by calling dump_stack_set_arch_desc()
during boot and, if exists, printed out in a separate line with
"Hardware name:" label.

dmi_set_dump_stack_arch_desc() is added which sets arch-specific
description from DMI data. It uses dmi_ids_string[] which is set from
dmi_present() used for DMI debug message. It is superset of the
information x86 show_regs() is using. The function is called from x86
and ia64 boot code right after dmi_scan_machine().

This makes the explicit DMI handling in warn_slowpath_common()
unnecessary. Removed.

show_regs() isn't yet converted to use generic debug information
printing and this patch doesn't remove the duplicate DMI handling in
x86 show_regs(). The next patch will unify show_regs() handling and
remove the duplication.

An example WARN dump follows.

WARNING: at /work/os/work/kernel/workqueue.c:4841 init_workqueues+0x35/0x505()
Modules linked in:
CPU: 0 PID: 1 Comm: swapper/0 Not tainted 3.9.0-rc1-work+ #3
Hardware name: empty empty/S3992, BIOS 080011 10/26/2007
0000000000000009 ffff88007c861e08 ffffffff81c614dc ffff88007c861e48
ffffffff8108f500 ffffffff82228240 0000000000000040 ffffffff8234a08e
0000000000000000 0000000000000000 0000000000000000 ffff88007c861e58
Call Trace:
[<ffffffff81c614dc>] dump_stack+0x19/0x1b
[<ffffffff8108f500>] warn_slowpath_common+0x70/0xa0
[<ffffffff8108f54a>] warn_slowpath_null+0x1a/0x20
[<ffffffff8234a0c3>] init_workqueues+0x35/0x505
...

v2: Use the same string as the debug message from dmi_present() which
also contains BIOS information. Move hardware name into its own
line as warn_slowpath_common() did. This change was suggested by
Bjorn Helgaas.

Signed-off-by: Tejun Heo <[email protected]>
Cc: Bjorn Helgaas <[email protected]>
---
arch/ia64/kernel/setup.c | 1 +
arch/x86/kernel/setup.c | 1 +
drivers/firmware/dmi_scan.c | 13 +++++++++++++
include/linux/dmi.h | 2 ++
include/linux/printk.h | 5 +++++
kernel/panic.c | 6 ------
kernel/printk.c | 26 ++++++++++++++++++++++++++
7 files changed, 48 insertions(+), 6 deletions(-)

diff --git a/arch/ia64/kernel/setup.c b/arch/ia64/kernel/setup.c
index 2029cc0..13bfdd2 100644
--- a/arch/ia64/kernel/setup.c
+++ b/arch/ia64/kernel/setup.c
@@ -1063,6 +1063,7 @@ check_bugs (void)
static int __init run_dmi_scan(void)
{
dmi_scan_machine();
+ dmi_set_dump_stack_arch_desc();
return 0;
}
core_initcall(run_dmi_scan);
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 90d8cc9..91b9e7c 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -970,6 +970,7 @@ void __init setup_arch(char **cmdline_p)
efi_init();

dmi_scan_machine();
+ dmi_set_dump_stack_arch_desc();

/*
* VMware detection requires dmi to be available, so this
diff --git a/drivers/firmware/dmi_scan.c b/drivers/firmware/dmi_scan.c
index 5022307..b95159b 100644
--- a/drivers/firmware/dmi_scan.c
+++ b/drivers/firmware/dmi_scan.c
@@ -525,6 +525,19 @@ void __init dmi_scan_machine(void)
}

/**
+ * dmi_set_dump_stack_arch_desc - set arch description for dump_stack()
+ *
+ * Invoke dump_stack_set_arch_desc() with DMI system information so that
+ * DMI identifiers are printed out on task dumps. Arch boot code should
+ * call this function after dmi_scan_machine() if it wants to print out DMI
+ * identifiers on task dumps.
+ */
+void __init dmi_set_dump_stack_arch_desc(void)
+{
+ dump_stack_set_arch_desc("%s", dmi_ids_string);
+}
+
+/**
* dmi_matches - check if dmi_system_id structure matches system DMI data
* @dmi: pointer to the dmi_system_id structure to check
*/
diff --git a/include/linux/dmi.h b/include/linux/dmi.h
index f156cca..b6eb7a0 100644
--- a/include/linux/dmi.h
+++ b/include/linux/dmi.h
@@ -99,6 +99,7 @@ extern const char * dmi_get_system_info(int field);
extern const struct dmi_device * dmi_find_device(int type, const char *name,
const struct dmi_device *from);
extern void dmi_scan_machine(void);
+extern void dmi_set_dump_stack_arch_desc(void);
extern bool dmi_get_date(int field, int *yearp, int *monthp, int *dayp);
extern int dmi_name_in_vendors(const char *str);
extern int dmi_name_in_serial(const char *str);
@@ -114,6 +115,7 @@ static inline const char * dmi_get_system_info(int field) { return NULL; }
static inline const struct dmi_device * dmi_find_device(int type, const char *name,
const struct dmi_device *from) { return NULL; }
static inline void dmi_scan_machine(void) { return; }
+static inline void dmi_set_dump_stack_arch_desc(void) { }
static inline bool dmi_get_date(int field, int *yearp, int *monthp, int *dayp)
{
if (yearp)
diff --git a/include/linux/printk.h b/include/linux/printk.h
index 7ce1f87..47827c0 100644
--- a/include/linux/printk.h
+++ b/include/linux/printk.h
@@ -145,6 +145,7 @@ extern void wake_up_klogd(void);

void log_buf_kexec_setup(void);
void __init setup_log_buf(int early);
+void dump_stack_set_arch_desc(const char *fmt, ...);
void dump_stack_print_info(const char *log_lvl);
#else
static inline __printf(1, 0)
@@ -184,6 +185,10 @@ static inline void setup_log_buf(int early)
{
}

+static inline void dump_stack_set_arch_desc(const char *fmt, ...)
+{
+}
+
static inline void dump_stack_print_info(const char *log_lvl)
{
}
diff --git a/kernel/panic.c b/kernel/panic.c
index 7c57cc9..167ec09 100644
--- a/kernel/panic.c
+++ b/kernel/panic.c
@@ -22,7 +22,6 @@
#include <linux/sysrq.h>
#include <linux/init.h>
#include <linux/nmi.h>
-#include <linux/dmi.h>

#define PANIC_TIMER_STEP 100
#define PANIC_BLINK_SPD 18
@@ -400,13 +399,8 @@ struct slowpath_args {
static void warn_slowpath_common(const char *file, int line, void *caller,
unsigned taint, struct slowpath_args *args)
{
- const char *board;
-
printk(KERN_WARNING "------------[ cut here ]------------\n");
printk(KERN_WARNING "WARNING: at %s:%d %pS()\n", file, line, caller);
- board = dmi_get_system_info(DMI_PRODUCT_NAME);
- if (board)
- printk(KERN_WARNING "Hardware name: %s\n", board);

if (args)
vprintk(args->fmt, args->args);
diff --git a/kernel/printk.c b/kernel/printk.c
index a00a285..f404b56 100644
--- a/kernel/printk.c
+++ b/kernel/printk.c
@@ -2852,6 +2852,28 @@ void kmsg_dump_rewind(struct kmsg_dumper *dumper)
}
EXPORT_SYMBOL_GPL(kmsg_dump_rewind);

+static char dump_stack_arch_desc_str[128];
+
+/**
+ * dump_stack_set_arch_desc - set arch-specific str to show with task dumps
+ * @fmt: printf-style format string
+ * @...: arguments for the format string
+ *
+ * The configured string will be printed right after utsname during task
+ * dumps. Usually used to add arch-specific system identifiers. If an
+ * arch wants to make use of such an ID string, it should initialize this
+ * as soon as possible during boot.
+ */
+void __init dump_stack_set_arch_desc(const char *fmt, ...)
+{
+ va_list args;
+
+ va_start(args, fmt);
+ vsnprintf(dump_stack_arch_desc_str, sizeof(dump_stack_arch_desc_str),
+ fmt, args);
+ va_end(args);
+}
+
/**
* dump_stack_print_info - print generic debug info for dump_stack()
* @log_lvl: log level
@@ -2866,6 +2888,10 @@ void dump_stack_print_info(const char *log_lvl)
print_tainted(), init_utsname()->release,
(int)strcspn(init_utsname()->version, " "),
init_utsname()->version);
+
+ if (dump_stack_arch_desc_str[0] != '\0')
+ printk("%sHardware name: %s\n",
+ log_lvl, dump_stack_arch_desc_str);
}

#endif
--
1.8.1.4

2013-04-03 19:16:39

by Tejun Heo

[permalink] [raw]
Subject: [PATCH 6/7] dump_stack: unify debug information printed by show_regs()

show_regs() is inherently arch-dependent but it does make sense to
print generic debug information and some archs already do albeit in
slightly different forms. This patch introduces a generic function to
print debug information from show_regs() so that different archs print
out the same information and it's much easier to modify what's
printed.

show_regs_print_info() prints out the same debug info as dump_stack()
does plus task and thread_info pointers.

* Archs which didn't print debug info now do.

alpha, arc, blackfin, c6x, cris, frv, h8300, hexagon, ia64, m32r,
metag, microblaze, mn10300, openrisc, parisc, score, sh64, sparc,
um, xtensa

* Already prints debug info. Replaced with show_regs_print_info().
The printed information is superset of what used to be there.

arm, arm64, avr32, mips, powerpc, sh32, tile, unicore32, x86

* s390 is special in that it used to print arch-specific information
along with generic debug info. Heiko and Martin think that the
arch-specific extra isn't worth keeping s390 specfic implementation.
Converted to use the generic version.

Note that now all archs print the debug info before actual register
dumps.

An example BUG() dump follows.

kernel BUG at /work/os/work/kernel/workqueue.c:4841!
invalid opcode: 0000 [#1] PREEMPT SMP DEBUG_PAGEALLOC
Modules linked in:
CPU: 0 PID: 1 Comm: swapper/0 Not tainted 3.9.0-rc1-work+ #7
Hardware name: empty empty/S3992, BIOS 080011 10/26/2007
task: ffff88007c85e040 ti: ffff88007c860000 task.ti: ffff88007c860000
RIP: 0010:[<ffffffff8234a07e>] [<ffffffff8234a07e>] init_workqueues+0x4/0x6
RSP: 0000:ffff88007c861ec8 EFLAGS: 00010246
RAX: ffff88007c861fd8 RBX: ffffffff824466a8 RCX: 0000000000000001
RDX: 0000000000000046 RSI: 0000000000000001 RDI: ffffffff8234a07a
RBP: ffff88007c861ec8 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000001 R11: 0000000000000000 R12: ffffffff8234a07a
R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000
FS: 0000000000000000(0000) GS:ffff88007dc00000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b
CR2: ffff88015f7ff000 CR3: 00000000021f1000 CR4: 00000000000007f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
Stack:
ffff88007c861ef8 ffffffff81000312 ffffffff824466a8 ffff88007c85e650
0000000000000003 0000000000000000 ffff88007c861f38 ffffffff82335e5d
ffff88007c862080 ffffffff8223d8c0 ffff88007c862080 ffffffff81c47760
Call Trace:
[<ffffffff81000312>] do_one_initcall+0x122/0x170
[<ffffffff82335e5d>] kernel_init_freeable+0x9b/0x1c8
[<ffffffff81c47760>] ? rest_init+0x140/0x140
[<ffffffff81c4776e>] kernel_init+0xe/0xf0
[<ffffffff81c6be9c>] ret_from_fork+0x7c/0xb0
[<ffffffff81c47760>] ? rest_init+0x140/0x140
...

v2: Typo fix in x86-32.

v3: CPU number dropped from show_regs_print_info() as
dump_stack_print_info() has been updated to print it. s390
specific implementation dropped as requested by s390 maintainers.

Signed-off-by: Tejun Heo <[email protected]>
Acked-by: David S. Miller <[email protected]>
Acked-by: Jesper Nilsson <[email protected]>
Cc: Heiko Carstens <[email protected]>
Cc: Martin Schwidefsky <[email protected]>
---
arch/alpha/kernel/process.c | 1 +
arch/arc/kernel/troubleshoot.c | 1 +
arch/arm/kernel/process.c | 8 ++------
arch/arm64/kernel/process.c | 7 +------
arch/avr32/kernel/process.c | 5 ++---
arch/blackfin/kernel/trace.c | 2 ++
arch/c6x/kernel/traps.c | 1 +
arch/cris/arch-v10/kernel/process.c | 3 +++
arch/cris/arch-v32/kernel/process.c | 3 +++
arch/frv/kernel/traps.c | 3 +--
arch/h8300/kernel/process.c | 2 ++
arch/hexagon/kernel/vm_events.c | 2 ++
arch/ia64/kernel/process.c | 4 ++--
arch/m32r/kernel/process.c | 2 ++
arch/metag/kernel/process.c | 2 ++
arch/microblaze/kernel/process.c | 2 ++
arch/mips/kernel/traps.c | 2 +-
arch/mn10300/kernel/process.c | 1 +
arch/openrisc/kernel/process.c | 1 +
arch/parisc/kernel/traps.c | 2 ++
arch/powerpc/kernel/process.c | 8 ++------
arch/s390/kernel/dumpstack.c | 9 +--------
arch/score/kernel/traps.c | 2 ++
arch/sh/kernel/process_32.c | 6 +-----
arch/sh/kernel/process_64.c | 1 +
arch/sparc/kernel/process_32.c | 2 ++
arch/sparc/kernel/process_64.c | 2 ++
arch/tile/kernel/process.c | 3 +--
arch/um/sys-ppc/sysrq.c | 2 ++
arch/unicore32/kernel/process.c | 6 +-----
arch/x86/include/asm/bug.h | 3 ---
arch/x86/kernel/dumpstack_32.c | 4 +---
arch/x86/kernel/dumpstack_64.c | 6 +-----
arch/x86/kernel/process.c | 24 ------------------------
arch/x86/kernel/process_32.c | 2 --
arch/x86/kernel/process_64.c | 1 -
arch/xtensa/kernel/traps.c | 2 ++
include/linux/printk.h | 5 +++++
kernel/printk.c | 16 ++++++++++++++++
39 files changed, 74 insertions(+), 84 deletions(-)

diff --git a/arch/alpha/kernel/process.c b/arch/alpha/kernel/process.c
index 63d27fb..7085cb8 100644
--- a/arch/alpha/kernel/process.c
+++ b/arch/alpha/kernel/process.c
@@ -194,6 +194,7 @@ machine_power_off(void)
void
show_regs(struct pt_regs *regs)
{
+ show_regs_print_info(KERN_DEFAULT);
dik_show_regs(regs, NULL);
}

diff --git a/arch/arc/kernel/troubleshoot.c b/arch/arc/kernel/troubleshoot.c
index dd7c0b7..158bbeb 100644
--- a/arch/arc/kernel/troubleshoot.c
+++ b/arch/arc/kernel/troubleshoot.c
@@ -167,6 +167,7 @@ void show_regs(struct pt_regs *regs)
return;

print_task_path_n_nm(tsk, buf);
+ show_regs_print_info(KERN_INFO);

if (current->thread.cause_code)
show_ecr_verbose(regs);
diff --git a/arch/arm/kernel/process.c b/arch/arm/kernel/process.c
index cbd0f51..7925de3 100644
--- a/arch/arm/kernel/process.c
+++ b/arch/arm/kernel/process.c
@@ -273,11 +273,8 @@ void __show_regs(struct pt_regs *regs)
unsigned long flags;
char buf[64];

- printk("CPU: %d %s (%s %.*s)\n",
- raw_smp_processor_id(), print_tainted(),
- init_utsname()->release,
- (int)strcspn(init_utsname()->version, " "),
- init_utsname()->version);
+ show_regs_print_info(KERN_DEFAULT);
+
print_symbol("PC is at %s\n", instruction_pointer(regs));
print_symbol("LR is at %s\n", regs->ARM_lr);
printk("pc : [<%08lx>] lr : [<%08lx>] psr: %08lx\n"
@@ -332,7 +329,6 @@ void __show_regs(struct pt_regs *regs)
void show_regs(struct pt_regs * regs)
{
printk("\n");
- printk("Pid: %d, comm: %20s\n", task_pid_nr(current), current->comm);
__show_regs(regs);
dump_stack();
}
diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
index 0337cdb..4f812bb 100644
--- a/arch/arm64/kernel/process.c
+++ b/arch/arm64/kernel/process.c
@@ -178,11 +178,7 @@ void __show_regs(struct pt_regs *regs)
{
int i;

- printk("CPU: %d %s (%s %.*s)\n",
- raw_smp_processor_id(), print_tainted(),
- init_utsname()->release,
- (int)strcspn(init_utsname()->version, " "),
- init_utsname()->version);
+ show_regs_print_info(KERN_DEFAULT);
print_symbol("PC is at %s\n", instruction_pointer(regs));
print_symbol("LR is at %s\n", regs->regs[30]);
printk("pc : [<%016llx>] lr : [<%016llx>] pstate: %08llx\n",
@@ -199,7 +195,6 @@ void __show_regs(struct pt_regs *regs)
void show_regs(struct pt_regs * regs)
{
printk("\n");
- printk("Pid: %d, comm: %20s\n", task_pid_nr(current), current->comm);
__show_regs(regs);
}

diff --git a/arch/avr32/kernel/process.c b/arch/avr32/kernel/process.c
index c1cafdb..63c9e8c 100644
--- a/arch/avr32/kernel/process.c
+++ b/arch/avr32/kernel/process.c
@@ -224,6 +224,8 @@ void show_regs_log_lvl(struct pt_regs *regs, const char *log_lvl)
unsigned long lr = regs->lr;
unsigned long mode = (regs->sr & MODE_MASK) >> MODE_SHIFT;

+ show_regs_print_info(log_lvl);
+
if (!user_mode(regs)) {
sp = (unsigned long)regs + FRAME_SIZE_FULL;

@@ -261,9 +263,6 @@ void show_regs_log_lvl(struct pt_regs *regs, const char *log_lvl)
regs->sr & SR_I0M ? '0' : '.',
regs->sr & SR_GM ? 'G' : 'g');
printk("%sCPU Mode: %s\n", log_lvl, cpu_modes[mode]);
- printk("%sProcess: %s [%d] (task: %p thread: %p)\n",
- log_lvl, current->comm, current->pid, current,
- task_thread_info(current));
}

void show_regs(struct pt_regs *regs)
diff --git a/arch/blackfin/kernel/trace.c b/arch/blackfin/kernel/trace.c
index f7f7a18..c36efa0 100644
--- a/arch/blackfin/kernel/trace.c
+++ b/arch/blackfin/kernel/trace.c
@@ -853,6 +853,8 @@ void show_regs(struct pt_regs *fp)
unsigned char in_atomic = (bfin_read_IPEND() & 0x10) || in_atomic();

pr_notice("\n");
+ show_regs_print_info(KERN_NOTICE);
+
if (CPUID != bfin_cpuid())
pr_notice("Compiled for cpu family 0x%04x (Rev %d), "
"but running on:0x%04x (Rev %d)\n",
diff --git a/arch/c6x/kernel/traps.c b/arch/c6x/kernel/traps.c
index d0b96ef..dcc2c2f 100644
--- a/arch/c6x/kernel/traps.c
+++ b/arch/c6x/kernel/traps.c
@@ -31,6 +31,7 @@ void __init trap_init(void)
void show_regs(struct pt_regs *regs)
{
pr_err("\n");
+ show_regs_print_info(KERN_ERR);
pr_err("PC: %08lx SP: %08lx\n", regs->pc, regs->sp);
pr_err("Status: %08lx ORIG_A4: %08lx\n", regs->csr, regs->orig_a4);
pr_err("A0: %08lx B0: %08lx\n", regs->a0, regs->b0);
diff --git a/arch/cris/arch-v10/kernel/process.c b/arch/cris/arch-v10/kernel/process.c
index b101875..9087bbf 100644
--- a/arch/cris/arch-v10/kernel/process.c
+++ b/arch/cris/arch-v10/kernel/process.c
@@ -175,6 +175,9 @@ unsigned long get_wchan(struct task_struct *p)
void show_regs(struct pt_regs * regs)
{
unsigned long usp = rdusp();
+
+ show_regs_print_info(KERN_DEFAULT);
+
printk("IRP: %08lx SRP: %08lx DCCR: %08lx USP: %08lx MOF: %08lx\n",
regs->irp, regs->srp, regs->dccr, usp, regs->mof );
printk(" r0: %08lx r1: %08lx r2: %08lx r3: %08lx\n",
diff --git a/arch/cris/arch-v32/kernel/process.c b/arch/cris/arch-v32/kernel/process.c
index 2b23ef0..09fd16c 100644
--- a/arch/cris/arch-v32/kernel/process.c
+++ b/arch/cris/arch-v32/kernel/process.c
@@ -170,6 +170,9 @@ get_wchan(struct task_struct *p)
void show_regs(struct pt_regs * regs)
{
unsigned long usp = rdusp();
+
+ show_regs_print_info(KERN_DEFAULT);
+
printk("ERP: %08lx SRP: %08lx CCS: %08lx USP: %08lx MOF: %08lx\n",
regs->erp, regs->srp, regs->ccs, usp, regs->mof);

diff --git a/arch/frv/kernel/traps.c b/arch/frv/kernel/traps.c
index cfcd802..4bff48c 100644
--- a/arch/frv/kernel/traps.c
+++ b/arch/frv/kernel/traps.c
@@ -497,6 +497,7 @@ void show_regs(struct pt_regs *regs)
int loop;

printk("\n");
+ show_regs_print_info(KERN_DEFAULT);

printk("Frame: @%08lx [%s]\n",
(unsigned long) regs,
@@ -511,8 +512,6 @@ void show_regs(struct pt_regs *regs)
else
printk(" | ");
}
-
- printk("Process %s (pid: %d)\n", current->comm, current->pid);
}

void die_if_kernel(const char *str, ...)
diff --git a/arch/h8300/kernel/process.c b/arch/h8300/kernel/process.c
index b609f63..8f235a3 100644
--- a/arch/h8300/kernel/process.c
+++ b/arch/h8300/kernel/process.c
@@ -110,6 +110,8 @@ void machine_power_off(void)

void show_regs(struct pt_regs * regs)
{
+ show_regs_print_info(KERN_DEFAULT);
+
printk("\nPC: %08lx Status: %02x",
regs->pc, regs->ccr);
printk("\nORIG_ER0: %08lx ER0: %08lx ER1: %08lx",
diff --git a/arch/hexagon/kernel/vm_events.c b/arch/hexagon/kernel/vm_events.c
index 3e44530..741aaa9 100644
--- a/arch/hexagon/kernel/vm_events.c
+++ b/arch/hexagon/kernel/vm_events.c
@@ -33,6 +33,8 @@
*/
void show_regs(struct pt_regs *regs)
{
+ show_regs_print_info(KERN_EMERG);
+
printk(KERN_EMERG "restart_r0: \t0x%08lx syscall_nr: %ld\n",
regs->restart_r0, regs->syscall_nr);
printk(KERN_EMERG "preds: \t\t0x%08lx\n", regs->preds);
diff --git a/arch/ia64/kernel/process.c b/arch/ia64/kernel/process.c
index 87c7a26..a79eead 100644
--- a/arch/ia64/kernel/process.c
+++ b/arch/ia64/kernel/process.c
@@ -101,8 +101,8 @@ show_regs (struct pt_regs *regs)
unsigned long ip = regs->cr_iip + ia64_psr(regs)->ri;

print_modules();
- printk("\nPid: %d, CPU %d, comm: %20s\n", task_pid_nr(current),
- smp_processor_id(), current->comm);
+ printk("\n");
+ show_regs_print_info(KERN_DEFAULT);
printk("psr : %016lx ifs : %016lx ip : [<%016lx>] %s (%s)\n",
regs->cr_ipsr, regs->cr_ifs, ip, print_tainted(),
init_utsname()->release);
diff --git a/arch/m32r/kernel/process.c b/arch/m32r/kernel/process.c
index bde899e..892a1d9 100644
--- a/arch/m32r/kernel/process.c
+++ b/arch/m32r/kernel/process.c
@@ -91,6 +91,8 @@ void machine_power_off(void)
void show_regs(struct pt_regs * regs)
{
printk("\n");
+ show_regs_print_info(KERN_DEFAULT);
+
printk("BPC[%08lx]:PSW[%08lx]:LR [%08lx]:FP [%08lx]\n", \
regs->bpc, regs->psw, regs->lr, regs->fp);
printk("BBPC[%08lx]:BBPSW[%08lx]:SPU[%08lx]:SPI[%08lx]\n", \
diff --git a/arch/metag/kernel/process.c b/arch/metag/kernel/process.c
index c6efe62..cfe4e7f 100644
--- a/arch/metag/kernel/process.c
+++ b/arch/metag/kernel/process.c
@@ -152,6 +152,8 @@ void show_regs(struct pt_regs *regs)
"D1.7 "
};

+ show_regs_print_info(KERN_INFO);
+
pr_info(" pt_regs @ %p\n", regs);
pr_info(" SaveMask = 0x%04hx\n", regs->ctx.SaveMask);
pr_info(" Flags = 0x%04hx (%c%c%c%c)\n", regs->ctx.Flags,
diff --git a/arch/microblaze/kernel/process.c b/arch/microblaze/kernel/process.c
index fa0ea60..d55700c 100644
--- a/arch/microblaze/kernel/process.c
+++ b/arch/microblaze/kernel/process.c
@@ -20,6 +20,8 @@

void show_regs(struct pt_regs *regs)
{
+ show_regs_print_info(KERN_INFO);
+
pr_info(" Registers dump: mode=%X\r\n", regs->pt_mode);
pr_info(" r1=%08lX, r2=%08lX, r3=%08lX, r4=%08lX\n",
regs->r1, regs->r2, regs->r3, regs->r4);
diff --git a/arch/mips/kernel/traps.c b/arch/mips/kernel/traps.c
index d2a31ac..af1d99c 100644
--- a/arch/mips/kernel/traps.c
+++ b/arch/mips/kernel/traps.c
@@ -231,7 +231,7 @@ static void __show_regs(const struct pt_regs *regs)
unsigned int cause = regs->cp0_cause;
int i;

- printk("Cpu %d\n", smp_processor_id());
+ show_regs_print_info(KERN_DEFAULT);

/*
* Saved main processor registers
diff --git a/arch/mn10300/kernel/process.c b/arch/mn10300/kernel/process.c
index 84f4e97..1223770 100644
--- a/arch/mn10300/kernel/process.c
+++ b/arch/mn10300/kernel/process.c
@@ -155,6 +155,7 @@ void machine_power_off(void)

void show_regs(struct pt_regs *regs)
{
+ show_regs_print_info(KERN_DEFAULT);
}

/*
diff --git a/arch/openrisc/kernel/process.c b/arch/openrisc/kernel/process.c
index 00c233b..386af25 100644
--- a/arch/openrisc/kernel/process.c
+++ b/arch/openrisc/kernel/process.c
@@ -90,6 +90,7 @@ void show_regs(struct pt_regs *regs)
{
extern void show_registers(struct pt_regs *regs);

+ show_regs_print_info(KERN_DEFAULT);
/* __PHX__ cleanup this mess */
show_registers(regs);
}
diff --git a/arch/parisc/kernel/traps.c b/arch/parisc/kernel/traps.c
index e64cf5f..f702bff 100644
--- a/arch/parisc/kernel/traps.c
+++ b/arch/parisc/kernel/traps.c
@@ -126,6 +126,8 @@ void show_regs(struct pt_regs *regs)
user = user_mode(regs);
level = user ? KERN_DEBUG : KERN_CRIT;

+ show_regs_print_info(level);
+
print_gr(level, regs);

for (i = 0; i < 8; i += 4)
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index 2e6f83a..fd1b300 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -829,6 +829,8 @@ void show_regs(struct pt_regs * regs)
{
int i, trap;

+ show_regs_print_info(KERN_DEFAULT);
+
printk("NIP: "REG" LR: "REG" CTR: "REG"\n",
regs->nip, regs->link, regs->ctr);
printk("REGS: %p TRAP: %04lx %s (%s)\n",
@@ -848,12 +850,6 @@ void show_regs(struct pt_regs * regs)
#else
printk("DAR: "REG", DSISR: %08lx\n", regs->dar, regs->dsisr);
#endif
- printk("TASK = %p[%d] '%s' THREAD: %p",
- current, task_pid_nr(current), current->comm, task_thread_info(current));
-
-#ifdef CONFIG_SMP
- printk(" CPU: %d", raw_smp_processor_id());
-#endif /* CONFIG_SMP */

for (i = 0; i < 32; i++) {
if ((i % REGS_PER_LINE) == 0)
diff --git a/arch/s390/kernel/dumpstack.c b/arch/s390/kernel/dumpstack.c
index 2f1f639..2982974 100644
--- a/arch/s390/kernel/dumpstack.c
+++ b/arch/s390/kernel/dumpstack.c
@@ -166,14 +166,7 @@ void show_registers(struct pt_regs *regs)

void show_regs(struct pt_regs *regs)
{
- printk("CPU: %d %s %s %.*s\n",
- task_thread_info(current)->cpu, print_tainted(),
- init_utsname()->release,
- (int)strcspn(init_utsname()->version, " "),
- init_utsname()->version);
- printk("Process %s (pid: %d, task: %p, ksp: %p)\n",
- current->comm, current->pid, current,
- (void *) current->thread.ksp);
+ show_regs_print_info(KERN_DEFAULT);
show_registers(regs);
/* Show stack backtrace if pt_regs is from kernel mode */
if (!user_mode(regs))
diff --git a/arch/score/kernel/traps.c b/arch/score/kernel/traps.c
index a38f435..1517a7d 100644
--- a/arch/score/kernel/traps.c
+++ b/arch/score/kernel/traps.c
@@ -117,6 +117,8 @@ static void show_code(unsigned int *pc)
*/
void show_regs(struct pt_regs *regs)
{
+ show_regs_print_info(KERN_DEFAULT);
+
printk("r0 : %08lx %08lx %08lx %08lx %08lx %08lx %08lx %08lx\n",
regs->regs[0], regs->regs[1], regs->regs[2], regs->regs[3],
regs->regs[4], regs->regs[5], regs->regs[6], regs->regs[7]);
diff --git a/arch/sh/kernel/process_32.c b/arch/sh/kernel/process_32.c
index 73eb66f..ebd3933 100644
--- a/arch/sh/kernel/process_32.c
+++ b/arch/sh/kernel/process_32.c
@@ -32,11 +32,7 @@
void show_regs(struct pt_regs * regs)
{
printk("\n");
- printk("Pid : %d, Comm: \t\t%s\n", task_pid_nr(current), current->comm);
- printk("CPU : %d \t\t%s (%s %.*s)\n\n",
- smp_processor_id(), print_tainted(), init_utsname()->release,
- (int)strcspn(init_utsname()->version, " "),
- init_utsname()->version);
+ show_regs_print_info(KERN_DEFAULT);

print_symbol("PC is at %s\n", instruction_pointer(regs));
print_symbol("PR is at %s\n", regs->pr);
diff --git a/arch/sh/kernel/process_64.c b/arch/sh/kernel/process_64.c
index e611c85..174d124 100644
--- a/arch/sh/kernel/process_64.c
+++ b/arch/sh/kernel/process_64.c
@@ -40,6 +40,7 @@ void show_regs(struct pt_regs *regs)
unsigned long long ah, al, bh, bl, ch, cl;

printk("\n");
+ show_regs_print_info(KERN_DEFAULT);

ah = (regs->pc) >> 32;
al = (regs->pc) & 0xffffffff;
diff --git a/arch/sparc/kernel/process_32.c b/arch/sparc/kernel/process_32.c
index fe99fbd..9e78f9a 100644
--- a/arch/sparc/kernel/process_32.c
+++ b/arch/sparc/kernel/process_32.c
@@ -123,6 +123,8 @@ void show_regs(struct pt_regs *r)
{
struct reg_window32 *rw = (struct reg_window32 *) r->u_regs[14];

+ show_regs_print_info(KERN_DEFAULT);
+
printk("PSR: %08lx PC: %08lx NPC: %08lx Y: %08lx %s\n",
r->psr, r->pc, r->npc, r->y, print_tainted());
printk("PC: <%pS>\n", (void *) r->pc);
diff --git a/arch/sparc/kernel/process_64.c b/arch/sparc/kernel/process_64.c
index cdb80b2..2415a95 100644
--- a/arch/sparc/kernel/process_64.c
+++ b/arch/sparc/kernel/process_64.c
@@ -186,6 +186,8 @@ static void show_regwindow(struct pt_regs *regs)

void show_regs(struct pt_regs *regs)
{
+ show_regs_print_info(KERN_DEFAULT);
+
printk("TSTATE: %016lx TPC: %016lx TNPC: %016lx Y: %08x %s\n", regs->tstate,
regs->tpc, regs->tnpc, regs->y, print_tainted());
printk("TPC: <%pS>\n", (void *) regs->tpc);
diff --git a/arch/tile/kernel/process.c b/arch/tile/kernel/process.c
index caf93ae..302691d 100644
--- a/arch/tile/kernel/process.c
+++ b/arch/tile/kernel/process.c
@@ -620,8 +620,7 @@ void show_regs(struct pt_regs *regs)
int i;

pr_err("\n");
- pr_err(" Pid: %d, comm: %20s, CPU: %d\n",
- tsk->pid, tsk->comm, smp_processor_id());
+ show_regs_print_info(KERN_ERR);
#ifdef __tilegx__
for (i = 0; i < 51; i += 3)
pr_err(" r%-2d: "REGFMT" r%-2d: "REGFMT" r%-2d: "REGFMT"\n",
diff --git a/arch/um/sys-ppc/sysrq.c b/arch/um/sys-ppc/sysrq.c
index f889449..1ff1ad7 100644
--- a/arch/um/sys-ppc/sysrq.c
+++ b/arch/um/sys-ppc/sysrq.c
@@ -11,6 +11,8 @@
void show_regs(struct pt_regs_subarch *regs)
{
printk("\n");
+ show_regs_print_info(KERN_DEFAULT);
+
printk("show_regs(): insert regs here.\n");
#if 0
printk("\n");
diff --git a/arch/unicore32/kernel/process.c b/arch/unicore32/kernel/process.c
index 872d7e2..c412484 100644
--- a/arch/unicore32/kernel/process.c
+++ b/arch/unicore32/kernel/process.c
@@ -159,11 +159,7 @@ void __show_regs(struct pt_regs *regs)
unsigned long flags;
char buf[64];

- printk(KERN_DEFAULT "CPU: %d %s (%s %.*s)\n",
- raw_smp_processor_id(), print_tainted(),
- init_utsname()->release,
- (int)strcspn(init_utsname()->version, " "),
- init_utsname()->version);
+ show_regs_print_info(KERN_DEFAULT);
print_symbol("PC is at %s\n", instruction_pointer(regs));
print_symbol("LR is at %s\n", regs->UCreg_lr);
printk(KERN_DEFAULT "pc : [<%08lx>] lr : [<%08lx>] psr: %08lx\n"
diff --git a/arch/x86/include/asm/bug.h b/arch/x86/include/asm/bug.h
index 11e1152..2f03ff0 100644
--- a/arch/x86/include/asm/bug.h
+++ b/arch/x86/include/asm/bug.h
@@ -37,7 +37,4 @@ do { \

#include <asm-generic/bug.h>

-
-extern void show_regs_common(void);
-
#endif /* _ASM_X86_BUG_H */
diff --git a/arch/x86/kernel/dumpstack_32.c b/arch/x86/kernel/dumpstack_32.c
index 1038a41..f2a1770 100644
--- a/arch/x86/kernel/dumpstack_32.c
+++ b/arch/x86/kernel/dumpstack_32.c
@@ -86,11 +86,9 @@ void show_regs(struct pt_regs *regs)
{
int i;

+ show_regs_print_info(KERN_EMERG);
__show_regs(regs, !user_mode_vm(regs));

- pr_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));
/*
* When in-kernel, we also print out the stack and code at the
* time of the fault..
diff --git a/arch/x86/kernel/dumpstack_64.c b/arch/x86/kernel/dumpstack_64.c
index b653675..addb207 100644
--- a/arch/x86/kernel/dumpstack_64.c
+++ b/arch/x86/kernel/dumpstack_64.c
@@ -249,14 +249,10 @@ void show_regs(struct pt_regs *regs)
{
int i;
unsigned long sp;
- const int cpu = smp_processor_id();
- struct task_struct *cur = current;

sp = regs->sp;
- printk("CPU %d ", cpu);
+ show_regs_print_info(KERN_DEFAULT);
__show_regs(regs, 1);
- printk(KERN_DEFAULT "Process %s (pid: %d, threadinfo %p, task %p)\n",
- cur->comm, cur->pid, task_thread_info(cur), cur);

/*
* When in-kernel, we also print out the stack and code at the
diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
index 14ae100..7646378 100644
--- a/arch/x86/kernel/process.c
+++ b/arch/x86/kernel/process.c
@@ -121,30 +121,6 @@ void exit_thread(void)
drop_fpu(me);
}

-void show_regs_common(void)
-{
- const char *vendor, *product, *board;
-
- vendor = dmi_get_system_info(DMI_SYS_VENDOR);
- if (!vendor)
- vendor = "";
- product = dmi_get_system_info(DMI_PRODUCT_NAME);
- if (!product)
- product = "";
-
- /* Board Name is optional */
- board = dmi_get_system_info(DMI_BOARD_NAME);
-
- printk(KERN_DEFAULT "Pid: %d, comm: %.20s %s %s %.*s %s %s%s%s\n",
- current->pid, current->comm, print_tainted(),
- init_utsname()->release,
- (int)strcspn(init_utsname()->version, " "),
- init_utsname()->version,
- vendor, product,
- board ? "/" : "",
- board ? board : "");
-}
-
void flush_thread(void)
{
struct task_struct *tsk = current;
diff --git a/arch/x86/kernel/process_32.c b/arch/x86/kernel/process_32.c
index b5a8905..7305f7d 100644
--- a/arch/x86/kernel/process_32.c
+++ b/arch/x86/kernel/process_32.c
@@ -84,8 +84,6 @@ void __show_regs(struct pt_regs *regs, int all)
savesegment(gs, gs);
}

- show_regs_common();
-
printk(KERN_DEFAULT "EIP: %04x:[<%08lx>] EFLAGS: %08lx CPU: %d\n",
(u16)regs->cs, regs->ip, regs->flags,
smp_processor_id());
diff --git a/arch/x86/kernel/process_64.c b/arch/x86/kernel/process_64.c
index 0f49677..355ae06 100644
--- a/arch/x86/kernel/process_64.c
+++ b/arch/x86/kernel/process_64.c
@@ -62,7 +62,6 @@ void __show_regs(struct pt_regs *regs, int all)
unsigned int fsindex, gsindex;
unsigned int ds, cs, es;

- show_regs_common();
printk(KERN_DEFAULT "RIP: %04lx:[<%016lx>] ", regs->cs & 0xffff, regs->ip);
printk_address(regs->ip, 1);
printk(KERN_DEFAULT "RSP: %04lx:%016lx EFLAGS: %08lx\n", regs->ss,
diff --git a/arch/xtensa/kernel/traps.c b/arch/xtensa/kernel/traps.c
index 384b7c7..458186d 100644
--- a/arch/xtensa/kernel/traps.c
+++ b/arch/xtensa/kernel/traps.c
@@ -383,6 +383,8 @@ void show_regs(struct pt_regs * regs)
{
int i, wmask;

+ show_regs_print_info(KERN_DEFAULT);
+
wmask = regs->wmask & ~1;

for (i = 0; i < 16; i++) {
diff --git a/include/linux/printk.h b/include/linux/printk.h
index 47827c0..6af944a 100644
--- a/include/linux/printk.h
+++ b/include/linux/printk.h
@@ -147,6 +147,7 @@ void log_buf_kexec_setup(void);
void __init setup_log_buf(int early);
void dump_stack_set_arch_desc(const char *fmt, ...);
void dump_stack_print_info(const char *log_lvl);
+void show_regs_print_info(const char *log_lvl);
#else
static inline __printf(1, 0)
int vprintk(const char *s, va_list args)
@@ -192,6 +193,10 @@ static inline void dump_stack_set_arch_desc(const char *fmt, ...)
static inline void dump_stack_print_info(const char *log_lvl)
{
}
+
+static inline void show_regs_print_info(const char *log_lvl)
+{
+}
#endif

extern void dump_stack(void) __cold;
diff --git a/kernel/printk.c b/kernel/printk.c
index f404b56..48bad34 100644
--- a/kernel/printk.c
+++ b/kernel/printk.c
@@ -2894,4 +2894,20 @@ void dump_stack_print_info(const char *log_lvl)
log_lvl, dump_stack_arch_desc_str);
}

+/**
+ * show_regs_print_info - print generic debug info for show_regs()
+ * @log_lvl: log level
+ *
+ * show_regs() implementations can use this function to print out generic
+ * debug information.
+ */
+void show_regs_print_info(const char *log_lvl)
+{
+ dump_stack_print_info(log_lvl);
+
+ printk("%stask: %p ti: %p task.ti: %p\n",
+ log_lvl, current, current_thread_info(),
+ task_thread_info(current));
+}
+
#endif
--
1.8.1.4

2013-04-03 19:16:47

by Tejun Heo

[permalink] [raw]
Subject: [PATCH 7/7] arc, print-fatal-signals: reduce duplicated information

From: Vineet Gupta <[email protected]>

After the recent generic debug info on dump_stack() and friends, arc
is printing duplicate information on debug dumps.

[ARCLinux]$ ./crash
crash/50: potentially unexpected fatal signal 11. <-- [1]
/sbin/crash, TGID 50 <-- [2]
Pid: 50, comm: crash Not tainted 3.9.0-rc4+ #132 <-- [3]
...

Remove them.

tj: Updated patch desc.

Signed-off-by: Vineet Gupta <[email protected]>
Signed-off-by: Tejun Heo <[email protected]>
---
arch/arc/kernel/troubleshoot.c | 2 +-
kernel/signal.c | 3 +--
2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/arch/arc/kernel/troubleshoot.c b/arch/arc/kernel/troubleshoot.c
index 158bbeb..39bbf58 100644
--- a/arch/arc/kernel/troubleshoot.c
+++ b/arch/arc/kernel/troubleshoot.c
@@ -75,7 +75,7 @@ void print_task_path_n_nm(struct task_struct *tsk, char *buf)
}

done:
- pr_info("%s, TGID %u\n", path_nm, tsk->tgid);
+ pr_info("Path: %s\n", path_nm);
}
EXPORT_SYMBOL(print_task_path_n_nm);

diff --git a/kernel/signal.c b/kernel/signal.c
index bb7ca79..92139dc 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -1163,8 +1163,7 @@ static int send_signal(int sig, struct siginfo *info, struct task_struct *t,
static void print_fatal_signal(int signr)
{
struct pt_regs *regs = signal_pt_regs();
- printk(KERN_INFO "%s/%d: potentially unexpected fatal signal %d.\n",
- current->comm, task_pid_nr(current), signr);
+ printk(KERN_INFO "potentially unexpected fatal signal %d.\n", signr);

#if defined(__i386__) && !defined(__arch_um__)
printk(KERN_INFO "code at %08lx: ", regs->ip);
--
1.8.1.4

2013-04-04 07:13:28

by Martin Schwidefsky

[permalink] [raw]
Subject: Re: [PATCH 3/7] dump_stack: consolidate dump_stack() implementations and unify their behaviors

On Wed, 3 Apr 2013 12:14:53 -0700
Tejun Heo <[email protected]> wrote:

> v2: CPU number added to the generic debug info as requested by s390
> folks and dropped the s390 specific dump_stack(). This loses %ksp
> from the debug message which the maintainers think isn't important
> enough to keep the s390-specific dump_stack() implementation.
>
> dump_stack_print_info() is moved to kernel/printk.c from
> lib/dump_stack.c. Because linkage is per objecct file,
> dump_stack_print_info() living in the same lib file as generic
> dump_stack() means that archs which implement custom dump_stack()
> - at this point, only blackfin - can't use dump_stack_print_info()
> as that will bring in the generic version of dump_stack() too. v1
> The v1 patch broke build on blackfin due to this issue. The build
> breakage was reported by Fengguang Wu.

For the s390 changes:
Acked-by: Martin Schwidefsky <[email protected]>

--
blue skies,
Martin.

"Reality continues to ruin my life." - Calvin.

2013-04-08 15:31:19

by Tejun Heo

[permalink] [raw]
Subject: Re: [PATCHSET v2] arch: unify task dump debug info

On Wed, Apr 03, 2013 at 12:14:50PM -0700, Tejun Heo wrote:
> Andrew, I think it's about ready and nobody seems to be against the
> proposed changes. Can you please take these into -mm?

Andrew, ping?

--
tejun

2013-04-08 16:08:21

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH 1/7] x86: don't show trace beyond show_stack(NULL, NULL)


* Tejun Heo <[email protected]> wrote:

> show_stack(current or NULL, NULL) is used to print the backtrace of
> the current task. As trace beyond the function itself isn't of much
> interest to anyone, don't show it by determining sp and bp in
> show_stack()'s frame and passing them to show_stack_log_lvl().
>
> This brings show_stack(NULL, NULL)'s behavior in line with
> dump_stack().
>
> Signed-off-by: Tejun Heo <[email protected]>
> ---
> arch/x86/kernel/dumpstack.c | 14 +++++++++++++-
> 1 file changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/kernel/dumpstack.c b/arch/x86/kernel/dumpstack.c
> index c8797d5..dd1a7c3 100644
> --- a/arch/x86/kernel/dumpstack.c
> +++ b/arch/x86/kernel/dumpstack.c
> @@ -176,7 +176,19 @@ void show_trace(struct task_struct *task, struct pt_regs *regs,
>
> void show_stack(struct task_struct *task, unsigned long *sp)
> {
> - show_stack_log_lvl(task, NULL, sp, 0, "");
> + unsigned long bp = 0;
> + unsigned long stack;
> +
> + /*
> + * Stack frames below this one aren't interesting. Don't show them
> + * if we're printing for %current.
> + */
> + if (!sp && (!task || task == current)) {
> + sp = &stack;
> + bp = stack_frame(current, NULL);
> + }
> +
> + show_stack_log_lvl(task, NULL, sp, bp, "");

Hm, show_regs() has a similar problem AFAICS.

Would be nice to create an __always_inline helper for all this, and to use it for
show_regs() as well.

Then the !bp special case in dump_trace() could be removed, and !bp be made an
invalid variant.

Thanks,

Ingo

2013-04-08 17:57:44

by Tejun Heo

[permalink] [raw]
Subject: Re: [PATCH 1/7] x86: don't show trace beyond show_stack(NULL, NULL)

Hello, Ingo.

On Mon, Apr 08, 2013 at 06:08:11PM +0200, Ingo Molnar wrote:
> > void show_stack(struct task_struct *task, unsigned long *sp)
> > {
> > - show_stack_log_lvl(task, NULL, sp, 0, "");
> > + unsigned long bp = 0;
> > + unsigned long stack;
> > +
> > + /*
> > + * Stack frames below this one aren't interesting. Don't show them
> > + * if we're printing for %current.
> > + */
> > + if (!sp && (!task || task == current)) {
> > + sp = &stack;
> > + bp = stack_frame(current, NULL);
> > + }
> > +
> > + show_stack_log_lvl(task, NULL, sp, bp, "");
>
> Hm, show_regs() has a similar problem AFAICS.

Doesn't seem so. show_regs() have pt_regs which gets passed to
dump_trace() and then used to determine the frame being dumped.
e.g. BUG() takes pt_regs from the faulting frame and thus doesn't show
anything beyond it.

kernel BUG at /work/os/work/kernel/workqueue.c:4841!
invalid opcode: 0000 [#1] PREEMPT SMP DEBUG_PAGEALLOC
Modules linked in:
CPU: 0 PID: 1 Comm: swapper/0 Not tainted 3.9.0-rc1-work+ #7
Hardware name: empty empty/S3992, BIOS 080011 10/26/2007
task: ffff88007c85e040 ti: ffff88007c860000 task.ti: ffff88007c860000
RIP: 0010:[<ffffffff8234a07e>] [<ffffffff8234a07e>] init_workqueues+0x4/0x6
RSP: 0000:ffff88007c861ec8 EFLAGS: 00010246
RAX: ffff88007c861fd8 RBX: ffffffff824466a8 RCX: 0000000000000001
RDX: 0000000000000046 RSI: 0000000000000001 RDI: ffffffff8234a07a
RBP: ffff88007c861ec8 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000001 R11: 0000000000000000 R12: ffffffff8234a07a
R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000
FS: 0000000000000000(0000) GS:ffff88007dc00000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b
CR2: ffff88015f7ff000 CR3: 00000000021f1000 CR4: 00000000000007f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
Stack:
ffff88007c861ef8 ffffffff81000312 ffffffff824466a8 ffff88007c85e650
0000000000000003 0000000000000000 ffff88007c861f38 ffffffff82335e5d
ffff88007c862080 ffffffff8223d8c0 ffff88007c862080 ffffffff81c47760
Call Trace:
[<ffffffff81000312>] do_one_initcall+0x122/0x170
[<ffffffff82335e5d>] kernel_init_freeable+0x9b/0x1c8
[<ffffffff81c47760>] ? rest_init+0x140/0x140
[<ffffffff81c4776e>] kernel_init+0xe/0xf0
[<ffffffff81c6be9c>] ret_from_fork+0x7c/0xb0
[<ffffffff81c47760>] ? rest_init+0x140/0x140
...


Thanks.

--
tejun

2013-04-10 10:35:17

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH 1/7] x86: don't show trace beyond show_stack(NULL, NULL)


* Tejun Heo <[email protected]> wrote:

> Hello, Ingo.
>
> On Mon, Apr 08, 2013 at 06:08:11PM +0200, Ingo Molnar wrote:
> > > void show_stack(struct task_struct *task, unsigned long *sp)
> > > {
> > > - show_stack_log_lvl(task, NULL, sp, 0, "");
> > > + unsigned long bp = 0;
> > > + unsigned long stack;
> > > +
> > > + /*
> > > + * Stack frames below this one aren't interesting. Don't show them
> > > + * if we're printing for %current.
> > > + */
> > > + if (!sp && (!task || task == current)) {
> > > + sp = &stack;
> > > + bp = stack_frame(current, NULL);
> > > + }
> > > +
> > > + show_stack_log_lvl(task, NULL, sp, bp, "");
> >
> > Hm, show_regs() has a similar problem AFAICS.
>
> Doesn't seem so. show_regs() have pt_regs which gets passed to
> dump_trace() and then used to determine the frame being dumped.
> e.g. BUG() takes pt_regs from the faulting frame and thus doesn't show
> anything beyond it.

Ok - but my other observation probably holds, that the bp == 0 special case in
dump_trace() should be changed to a [printk()-ed] warning or so? No user will pass
in bp == 0 legitimately, AFAICS.

Thanks,

Ingo

2013-04-10 18:54:25

by Tejun Heo

[permalink] [raw]
Subject: Re: [PATCH 1/7] x86: don't show trace beyond show_stack(NULL, NULL)

On Wed, Apr 10, 2013 at 12:35:08PM +0200, Ingo Molnar wrote:
> Ok - but my other observation probably holds, that the bp == 0 special case in
> dump_trace() should be changed to a [printk()-ed] warning or so? No user will pass
> in bp == 0 legitimately, AFAICS.

Yeah, seems so. I'll verify and add a warning message if both @bp and
@regs aren't specified in dump_trace().

Thanks.

--
tejun

2013-04-10 20:10:31

by Tejun Heo

[permalink] [raw]
Subject: [PATCH UPDATED 1/7] x86: don't show trace into stacktrace machinery

>From 1abcf30c5fe1ddf166bedd482b1fc7dbed164a87 Mon Sep 17 00:00:00 2001
From: Tejun Heo <[email protected]>
Date: Wed, 10 Apr 2013 13:06:06 -0700

When dumping stacktrace, frames of the stacktrace code itself aren't
interesting. Implement dump_trace_current_frame() helper which can be
used by (eventual) users of dump_trace() to determine the stack and
frame pointers of the current frame so that frames beyond that point
are ignored by dump_trace().

show_stack() and save_stack_trace[_tsk]() are updated to use the
helper and now ignore frames beyond their own. This brings
show_stack(NULL, NULL)'s behavior in line with dump_stack().

Also add dump_trace_warn_current_frame() which is used by dump_trace()
implementations to whine about callers which dump %current but don't
specify the frame to dump to catch mistakes.

The original patch just updated show_stack(). Applying it to other
users and addition of warning are suggested by Ingo.

Signed-off-by: Tejun Heo <[email protected]>
Cc: Ingo Molnar <[email protected]>
---
save_stack_trace*() were specifying NULL %bp as well. Introduced a
helper and make both dump_stack() and save_stack_trace*() use it and
added a warning message in dump_trace().

git branch also updated. This change doesn't affect other patches in
the series other than minor offset changes.

Thanks.

arch/x86/include/asm/stacktrace.h | 35 +++++++++++++++++++++++++++++++++++
arch/x86/kernel/dumpstack.c | 11 ++++++++++-
arch/x86/kernel/dumpstack_32.c | 2 ++
arch/x86/kernel/dumpstack_64.c | 2 ++
arch/x86/kernel/stacktrace.c | 12 ++++++++++--
5 files changed, 59 insertions(+), 3 deletions(-)

diff --git a/arch/x86/include/asm/stacktrace.h b/arch/x86/include/asm/stacktrace.h
index 70bbe39..1abe898 100644
--- a/arch/x86/include/asm/stacktrace.h
+++ b/arch/x86/include/asm/stacktrace.h
@@ -73,14 +73,49 @@ stack_frame(struct task_struct *task, struct pt_regs *regs)
/* bp is the last reg pushed by switch_to */
return *(unsigned long *)task->thread.sp;
}
+
+/* sanity check helper for dump_trace(), see dump_trace_current_frame() */
+static inline void
+dump_trace_warn_current_frame(struct task_struct *task, struct pt_regs *regs,
+ unsigned long bp)
+{
+ if ((!task || task == current) && !regs && !bp)
+ printk(KERN_WARNING "dump_trace: %pf didn't specify neither frame nor regs for %%current\n",
+ __builtin_return_address(0));
+}
#else
static inline unsigned long
stack_frame(struct task_struct *task, struct pt_regs *regs)
{
return 0;
}
+
+static inline void
+dump_trace_warn_current_frame(struct task_struct *task, struct pt_regs *regs,
+ unsigned long bp)
+{ }
#endif

+/**
+ * dump_trace_current_frame - stack and frame pointers for the current frame
+ * @sp: output argument for the stack pointer
+ * @bp: output argument for the frame pointer
+ *
+ * When dumping %current, dump_trace() wants its caller to specify the top
+ * frame so that it doesn't end up showing unncessary traces into stack
+ * dumping machinery. This helper can be used to determine @sp and @bp to
+ * pass to dump_trace() when dumping %current. This functions is
+ * __always_inline so that it records the frame of the caller.
+ */
+static __always_inline
+void dump_trace_current_frame(unsigned long **sp, unsigned long *bp)
+{
+ unsigned long stack;
+
+ *sp = &stack;
+ *bp = stack_frame(current, NULL);
+}
+
extern void
show_trace_log_lvl(struct task_struct *task, struct pt_regs *regs,
unsigned long *stack, unsigned long bp, char *log_lvl);
diff --git a/arch/x86/kernel/dumpstack.c b/arch/x86/kernel/dumpstack.c
index c8797d5..6e5e3ab 100644
--- a/arch/x86/kernel/dumpstack.c
+++ b/arch/x86/kernel/dumpstack.c
@@ -176,7 +176,16 @@ void show_trace(struct task_struct *task, struct pt_regs *regs,

void show_stack(struct task_struct *task, unsigned long *sp)
{
- show_stack_log_lvl(task, NULL, sp, 0, "");
+ unsigned long bp = 0;
+
+ /*
+ * Stack frames below this one aren't interesting. Don't show them
+ * if we're printing for %current.
+ */
+ if (!sp && (!task || task == current))
+ dump_trace_current_frame(&sp, &bp);
+
+ show_stack_log_lvl(task, NULL, sp, bp, "");
}

/*
diff --git a/arch/x86/kernel/dumpstack_32.c b/arch/x86/kernel/dumpstack_32.c
index 1038a41..8cc7c4a 100644
--- a/arch/x86/kernel/dumpstack_32.c
+++ b/arch/x86/kernel/dumpstack_32.c
@@ -23,6 +23,8 @@ void dump_trace(struct task_struct *task, struct pt_regs *regs,
{
int graph = 0;

+ dump_trace_warn_current_frame(task, regs, bp);
+
if (!task)
task = current;

diff --git a/arch/x86/kernel/dumpstack_64.c b/arch/x86/kernel/dumpstack_64.c
index b653675..349a8a9 100644
--- a/arch/x86/kernel/dumpstack_64.c
+++ b/arch/x86/kernel/dumpstack_64.c
@@ -123,6 +123,8 @@ void dump_trace(struct task_struct *task, struct pt_regs *regs,
int graph = 0;
unsigned long dummy;

+ dump_trace_warn_current_frame(task, regs, bp);
+
if (!task)
task = current;

diff --git a/arch/x86/kernel/stacktrace.c b/arch/x86/kernel/stacktrace.c
index fdd0c64..0908a50 100644
--- a/arch/x86/kernel/stacktrace.c
+++ b/arch/x86/kernel/stacktrace.c
@@ -60,7 +60,10 @@ static const struct stacktrace_ops save_stack_ops_nosched = {
*/
void save_stack_trace(struct stack_trace *trace)
{
- dump_trace(current, NULL, NULL, 0, &save_stack_ops, trace);
+ unsigned long *sp, bp;
+
+ dump_trace_current_frame(&sp, &bp);
+ dump_trace(current, NULL, sp, bp, &save_stack_ops, trace);
if (trace->nr_entries < trace->max_entries)
trace->entries[trace->nr_entries++] = ULONG_MAX;
}
@@ -75,7 +78,12 @@ void save_stack_trace_regs(struct pt_regs *regs, struct stack_trace *trace)

void save_stack_trace_tsk(struct task_struct *tsk, struct stack_trace *trace)
{
- dump_trace(tsk, NULL, NULL, 0, &save_stack_ops_nosched, trace);
+ unsigned long *sp = NULL, bp = 0;
+
+ if (!tsk || tsk == current)
+ dump_trace_current_frame(&sp, &bp);
+
+ dump_trace(tsk, NULL, sp, bp, &save_stack_ops_nosched, trace);
if (trace->nr_entries < trace->max_entries)
trace->entries[trace->nr_entries++] = ULONG_MAX;
}
--
1.8.1.4

2013-04-10 20:22:26

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH UPDATED 1/7] x86: don't show trace into stacktrace machinery

On Wed, Apr 10, 2013 at 01:10:20PM -0700, Tejun Heo wrote:
> +/* sanity check helper for dump_trace(), see dump_trace_current_frame() */
> +static inline void
> +dump_trace_warn_current_frame(struct task_struct *task, struct pt_regs *regs,
> + unsigned long bp)
> +{
> + if ((!task || task == current) && !regs && !bp)
> + printk(KERN_WARNING "dump_trace: %pf didn't specify neither frame nor regs for %%current\n",

This is double negation and is generally wrong in English. I think you
want to say:

"%pf specified neither frame nor regs for ..."

or

"%pf didn't specify either frame or regs for ..."

Or maybe even passive and shorter:

"%pf: No frame or regs specified for %%current."

and so on.

--
Regards/Gruss,
Boris.

Sent from a fat crate under my desk. Formatting is fine.
--

2013-04-10 21:09:21

by Tejun Heo

[permalink] [raw]
Subject: Re: [PATCH UPDATED 1/7] x86: don't show trace into stacktrace machinery

On Wed, Apr 10, 2013 at 10:22:20PM +0200, Borislav Petkov wrote:
> This is double negation and is generally wrong in English. I think you
> want to say:
>
> "%pf specified neither frame nor regs for ..."
>
> or
>
> "%pf didn't specify either frame or regs for ..."
>
> Or maybe even passive and shorter:
>
> "%pf: No frame or regs specified for %%current."
>
> and so on.

Crap, right, will post an updated version soon.

Thanks!

--
tejun

2013-04-10 21:18:27

by Tejun Heo

[permalink] [raw]
Subject: [PATCH UPDATED v2 1/7] x86: don't show trace into stacktrace machinery

>From d2a454b1b1b93e8a8ecc3639f6641b100dbddfe5 Mon Sep 17 00:00:00 2001
From: Tejun Heo <[email protected]>
Date: Wed, 10 Apr 2013 14:16:51 -0700

When dumping stacktrace, frames of the stacktrace code itself aren't
interesting. Implement dump_trace_current_frame() helper which can be
used by (eventual) users of dump_trace() to determine the stack and
frame pointers of the current frame so that frames beyond that point
are ignored by dump_trace().

show_stack() and save_stack_trace[_tsk]() are updated to use the
helper and now ignore frames beyond their own. This brings
show_stack(NULL, NULL)'s behavior in line with dump_stack().

Also add dump_trace_warn_current_frame() which is used by dump_trace()
implementations to whine about callers which dump %current but don't
specify the frame to dump to catch mistakes.

The original patch just updated show_stack(). Applying it to other
users and addition of warning are suggested by Ingo.

v2: Removed double negation in the warning message as suggested by
Borislav.

Signed-off-by: Tejun Heo <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Borislav Petkov <[email protected]>
---
arch/x86/include/asm/stacktrace.h | 35 +++++++++++++++++++++++++++++++++++
arch/x86/kernel/dumpstack.c | 11 ++++++++++-
arch/x86/kernel/dumpstack_32.c | 2 ++
arch/x86/kernel/dumpstack_64.c | 2 ++
arch/x86/kernel/stacktrace.c | 12 ++++++++++--
5 files changed, 59 insertions(+), 3 deletions(-)

diff --git a/arch/x86/include/asm/stacktrace.h b/arch/x86/include/asm/stacktrace.h
index 70bbe39..c610ab1 100644
--- a/arch/x86/include/asm/stacktrace.h
+++ b/arch/x86/include/asm/stacktrace.h
@@ -73,14 +73,49 @@ stack_frame(struct task_struct *task, struct pt_regs *regs)
/* bp is the last reg pushed by switch_to */
return *(unsigned long *)task->thread.sp;
}
+
+/* sanity check helper for dump_trace(), see dump_trace_current_frame() */
+static inline void
+dump_trace_warn_current_frame(struct task_struct *task, struct pt_regs *regs,
+ unsigned long bp)
+{
+ if ((!task || task == current) && !regs && !bp)
+ printk(KERN_WARNING "dump_trace: %pf specified neither frame nor regs for %%current\n",
+ __builtin_return_address(0));
+}
#else
static inline unsigned long
stack_frame(struct task_struct *task, struct pt_regs *regs)
{
return 0;
}
+
+static inline void
+dump_trace_warn_current_frame(struct task_struct *task, struct pt_regs *regs,
+ unsigned long bp)
+{ }
#endif

+/**
+ * dump_trace_current_frame - stack and frame pointers for the current frame
+ * @sp: output argument for the stack pointer
+ * @bp: output argument for the frame pointer
+ *
+ * When dumping %current, dump_trace() wants its caller to specify the top
+ * frame so that it doesn't end up showing unncessary traces into stack
+ * dumping machinery. This helper can be used to determine @sp and @bp to
+ * pass to dump_trace() when dumping %current. This functions is
+ * __always_inline so that it records the frame of the caller.
+ */
+static __always_inline
+void dump_trace_current_frame(unsigned long **sp, unsigned long *bp)
+{
+ unsigned long stack;
+
+ *sp = &stack;
+ *bp = stack_frame(current, NULL);
+}
+
extern void
show_trace_log_lvl(struct task_struct *task, struct pt_regs *regs,
unsigned long *stack, unsigned long bp, char *log_lvl);
diff --git a/arch/x86/kernel/dumpstack.c b/arch/x86/kernel/dumpstack.c
index c8797d5..6e5e3ab 100644
--- a/arch/x86/kernel/dumpstack.c
+++ b/arch/x86/kernel/dumpstack.c
@@ -176,7 +176,16 @@ void show_trace(struct task_struct *task, struct pt_regs *regs,

void show_stack(struct task_struct *task, unsigned long *sp)
{
- show_stack_log_lvl(task, NULL, sp, 0, "");
+ unsigned long bp = 0;
+
+ /*
+ * Stack frames below this one aren't interesting. Don't show them
+ * if we're printing for %current.
+ */
+ if (!sp && (!task || task == current))
+ dump_trace_current_frame(&sp, &bp);
+
+ show_stack_log_lvl(task, NULL, sp, bp, "");
}

/*
diff --git a/arch/x86/kernel/dumpstack_32.c b/arch/x86/kernel/dumpstack_32.c
index 1038a41..8cc7c4a 100644
--- a/arch/x86/kernel/dumpstack_32.c
+++ b/arch/x86/kernel/dumpstack_32.c
@@ -23,6 +23,8 @@ void dump_trace(struct task_struct *task, struct pt_regs *regs,
{
int graph = 0;

+ dump_trace_warn_current_frame(task, regs, bp);
+
if (!task)
task = current;

diff --git a/arch/x86/kernel/dumpstack_64.c b/arch/x86/kernel/dumpstack_64.c
index b653675..349a8a9 100644
--- a/arch/x86/kernel/dumpstack_64.c
+++ b/arch/x86/kernel/dumpstack_64.c
@@ -123,6 +123,8 @@ void dump_trace(struct task_struct *task, struct pt_regs *regs,
int graph = 0;
unsigned long dummy;

+ dump_trace_warn_current_frame(task, regs, bp);
+
if (!task)
task = current;

diff --git a/arch/x86/kernel/stacktrace.c b/arch/x86/kernel/stacktrace.c
index fdd0c64..0908a50 100644
--- a/arch/x86/kernel/stacktrace.c
+++ b/arch/x86/kernel/stacktrace.c
@@ -60,7 +60,10 @@ static const struct stacktrace_ops save_stack_ops_nosched = {
*/
void save_stack_trace(struct stack_trace *trace)
{
- dump_trace(current, NULL, NULL, 0, &save_stack_ops, trace);
+ unsigned long *sp, bp;
+
+ dump_trace_current_frame(&sp, &bp);
+ dump_trace(current, NULL, sp, bp, &save_stack_ops, trace);
if (trace->nr_entries < trace->max_entries)
trace->entries[trace->nr_entries++] = ULONG_MAX;
}
@@ -75,7 +78,12 @@ void save_stack_trace_regs(struct pt_regs *regs, struct stack_trace *trace)

void save_stack_trace_tsk(struct task_struct *tsk, struct stack_trace *trace)
{
- dump_trace(tsk, NULL, NULL, 0, &save_stack_ops_nosched, trace);
+ unsigned long *sp = NULL, bp = 0;
+
+ if (!tsk || tsk == current)
+ dump_trace_current_frame(&sp, &bp);
+
+ dump_trace(tsk, NULL, sp, bp, &save_stack_ops_nosched, trace);
if (trace->nr_entries < trace->max_entries)
trace->entries[trace->nr_entries++] = ULONG_MAX;
}
--
1.8.1.4

2013-04-11 18:48:00

by Tejun Heo

[permalink] [raw]
Subject: Re: [PATCHSET v2] arch: unify task dump debug info

On Mon, Apr 08, 2013 at 08:31:07AM -0700, Tejun Heo wrote:
> Andrew, ping?

Ping #2. Workqueue conversion of writeback in the block tree needs
these patches to avoid losing debug information over the conversion,
so it'd be great if this can be scheduled for 3.10.

Thanks.

--
tejun

2013-04-12 05:42:13

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCHSET v2] arch: unify task dump debug info


* Tejun Heo <[email protected]> wrote:

> On Mon, Apr 08, 2013 at 08:31:07AM -0700, Tejun Heo wrote:
> > Andrew, ping?
>
> Ping #2. Workqueue conversion of writeback in the block tree needs
> these patches to avoid losing debug information over the conversion,
> so it'd be great if this can be scheduled for 3.10.

The x86 bits look good to me, assuming it's tested, etc.

Acked-by: Ingo Molnar <[email protected]>

Thanks,

Ingo

2013-04-12 20:37:22

by Chris Metcalf

[permalink] [raw]
Subject: Re: [PATCH 6/7] dump_stack: unify debug information printed by show_regs()

On 4/3/2013 3:14 PM, Tejun Heo wrote:
> show_regs() is inherently arch-dependent but it does make sense to
> print generic debug information and some archs already do albeit in
> slightly different forms. This patch introduces a generic function to
> print debug information from show_regs() so that different archs print
> out the same information and it's much easier to modify what's
> printed.
>
> show_regs_print_info() prints out the same debug info as dump_stack()
> does plus task and thread_info pointers.
>
> * Archs which didn't print debug info now do.
>
> alpha, arc, blackfin, c6x, cris, frv, h8300, hexagon, ia64, m32r,
> metag, microblaze, mn10300, openrisc, parisc, score, sh64, sparc,
> um, xtensa
>
> * Already prints debug info. Replaced with show_regs_print_info().
> The printed information is superset of what used to be there.
>
> arm, arm64, avr32, mips, powerpc, sh32, tile, unicore32, x86

Acked-by: Chris Metcalf <[email protected]> [for tile]

--
Chris Metcalf, Tilera Corp.
http://www.tilera.com

2013-04-12 20:39:06

by Chris Metcalf

[permalink] [raw]
Subject: Re: [PATCH 3/7] dump_stack: consolidate dump_stack() implementations and unify their behaviors

On 4/3/2013 3:14 PM, Tejun Heo wrote:
> This patch expands the dummy fallback dump_stack() implementation in
> lib/dump_stack.c such that it prints out debug information (taken from
> x86) and invokes show_stack(NULL, NULL) and drops arch-specific
> dump_stack() implementations in all archs except blackfin. Blackfin's
> dump_stack() does something wonky that I don't understand.

arch/tile has a dump_stack() entry point in assembly (see arch/tile/kernel/entry.S) that passes to _dump_stack(), which passes to dump_stack_regs(), which passes to tile_show_stack(). Similarly, show_stack() passes to tile_show_stack(), which does lots of work to cross into userspace and continue showing the stack if possible, print symbol info for both kernel and userspace, etc. I don't know if that's all supported fully in Tejun's generic version.

Would it make sense for me to look at this after it's merged up into -next and see what tile needs? Unfortunately I'm taking off for a week's vacation so won't be able to do much until after the 23rd.

--
Chris Metcalf, Tilera Corp.
http://www.tilera.com

2013-04-12 22:00:09

by Tejun Heo

[permalink] [raw]
Subject: Re: [PATCH 3/7] dump_stack: consolidate dump_stack() implementations and unify their behaviors

Hello,

On Fri, Apr 12, 2013 at 04:39:01PM -0400, Chris Metcalf wrote:
> On 4/3/2013 3:14 PM, Tejun Heo wrote:
> > This patch expands the dummy fallback dump_stack() implementation in
> > lib/dump_stack.c such that it prints out debug information (taken from
> > x86) and invokes show_stack(NULL, NULL) and drops arch-specific
> > dump_stack() implementations in all archs except blackfin. Blackfin's
> > dump_stack() does something wonky that I don't understand.
>
> arch/tile has a dump_stack() entry point in assembly (see
> arch/tile/kernel/entry.S) that passes to _dump_stack(), which passes
> to dump_stack_regs(), which passes to tile_show_stack(). Similarly,
> show_stack() passes to tile_show_stack(), which does lots of work to
> cross into userspace and continue showing the stack if possible,
> print symbol info for both kernel and userspace, etc. I don't know
> if that's all supported fully in Tejun's generic version.

I see. In that case, this patchset shouldn't change anything for tile
as the generic dump_stack() is linked iff arch implementation doesn't
exist.

> Would it make sense for me to look at this after it's merged up into
> -next and see what tile needs? Unfortunately I'm taking off for a
> week's vacation so won't be able to do much until after the 23rd.

It'd be nice to get it inline with other archs but I don't think
updating tile to print the common debug info is a priority, so no
problem with dealing with it later.

Thanks.

Thanks.

--
tejun

2013-04-13 01:14:11

by Richard Kuo

[permalink] [raw]
Subject: Re: [PATCHSET v2] arch: unify task dump debug info

> On Mon, Apr 08, 2013 at 08:31:07AM -0700, Tejun Heo wrote:
>> Andrew, ping?
>
> Ping #2. Workqueue conversion of writeback in the block tree needs
> these patches to avoid losing debug information over the conversion,
> so it'd be great if this can be scheduled for 3.10.
>
> Thanks.
>
> --
> tejun
>

Sorry for the late reply; wasn't able to test this until today.

Hexagon could use the same "don't print into stacktrace machinery", but I
can add that to my tree.

So for the Hexagon bits:

Acked-by: Richard Kuo <[email protected]>


--
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

2013-04-18 14:35:27

by Russell King - ARM Linux

[permalink] [raw]
Subject: Re: [PATCHSET v2] arch: unify task dump debug info

On Wed, Apr 03, 2013 at 12:14:50PM -0700, Tejun Heo wrote:
> Andrew, I think it's about ready and nobody seems to be against the
> proposed changes. Can you please take these into -mm?
>
> This v2 of this patchset. Changes from the last posting[1] are,

So, I've just tried to apply this patch to my current testing tree,
and it fails because I'm not using -mm... That means I can't put
these in my nightly testing tree.