* [PATCH 3/5] dump_stack: consolidate dump_stack() implementations and unify their behaviors [not found] ` <20130330022706.6O_4ksyBEnkdW6XmrL4YUSD_Q4kA0Y95Y7NBinv2q54@z> @ 2013-03-30 2:27 ` Tejun Heo 2013-03-30 3:30 ` David Miller ` (4 more replies) 0 siblings, 5 replies; 8+ messages in thread From: Tejun Heo @ 2013-03-30 2:27 UTC (permalink / raw) To: linux-arch, linux-kernel, akpm, mingo, x86 Cc: rth, linux, msalter, starvik, dhowells, tony.luck, benh, takata, geert, james.hogan, monstr, ralf, jonas, rkuo, schwidefsky, liqin.chen, davem, lethal, vgupta, chris, cmetcalf, ysato, gxt, jdike, Tejun Heo, Heiko Carstens, linux-s390, Mike Frysinger, uclinux-dist-devel 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 and s390. Blackfin's dump_stack() does something wonky that I don't understand and s390 prints its own debug information which includes fields which aren't accessible from arch-indepdent code. 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:4840 init_workqueues+0x35/0x505() Hardware name: empty Modules linked in: Pid: 1, comm: swapper/0 Not tainted 3.9.0-rc1-work+ #17 0000000000000009 ffff88007c861e08 ffffffff81c61545 ffff88007c861e48 ffffffff8108f50f ffffffff82228240 0000000000000040 ffffffff8234a03c 0000000000000000 0000000000000000 0000000000000000 ffff88007c861e58 Call Trace: [<ffffffff81c61545>] dump_stack+0x19/0x1b [<ffffffff8108f50f>] warn_slowpath_common+0x7f/0xc0 [<ffffffff8108f56a>] warn_slowpath_null+0x1a/0x20 [<ffffffff8234a071>] init_workqueues+0x35/0x505 ... Signed-off-by: Tejun Heo <tj@kernel.org> Cc: Martin Schwidefsky <schwidefsky@de.ibm.com> Cc: Heiko Carstens <heiko.carstens@de.ibm.com> Cc: linux-s390@vger.kernel.org Cc: Mike Frysinger <vapier@gentoo.org> Cc: uclinux-dist-devel@blackfin.uclinux.org --- 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/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 | 1 + lib/dump_stack.c | 28 +++++++++++++++++++++++++--- 31 files changed, 27 insertions(+), 245 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 be5e2dd..cc2171b 100644 --- a/arch/hexagon/kernel/traps.c +++ b/arch/hexagon/kernel/traps.c @@ -191,14 +191,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 e34f565..29fb256 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 a200b5b..93f9b26 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, ®s); } -/* - * The architecture-independent dump_stack generator - */ -void dump_stack(void) -{ - struct pt_regs regs; - - prepare_frametrace(®s); - show_backtrace(current, ®s); -} - -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/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 822171f..952c1b2 100644 --- a/include/linux/printk.h +++ b/include/linux/printk.h @@ -177,6 +177,7 @@ static inline void setup_log_buf(int early) } #endif +extern void dump_stack_print_info(const char *log_lvl); extern void dump_stack(void) __cold; #ifndef pr_fmt diff --git a/lib/dump_stack.c b/lib/dump_stack.c index 42f4f55..5a67cfc 100644 --- a/lib/dump_stack.c +++ b/lib/dump_stack.c @@ -4,12 +4,34 @@ */ #include <linux/kernel.h> +#include <linux/sched.h> +#include <linux/utsname.h> #include <linux/export.h> -void dump_stack(void) +/** + * 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(KERN_NOTICE - "This architecture does not implement dump_stack()\n"); + printk("%sPid: %d, comm: %.20s %s %s %.*s\n", + log_lvl, current->pid, current->comm, print_tainted(), + init_utsname()->release, + (int)strcspn(init_utsname()->version, " "), + init_utsname()->version); } +/** + * dump_stack - dump the current task information and its stack trace + * + * Architectures can override this implementation by implementing its own. + */ +void dump_stack(void) +{ + dump_stack_print_info(KERN_DEFAULT); + show_stack(NULL, NULL); +} EXPORT_SYMBOL(dump_stack); -- 1.8.1.4 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 3/5] dump_stack: consolidate dump_stack() implementations and unify their behaviors 2013-03-30 2:27 ` [PATCH 3/5] dump_stack: consolidate dump_stack() implementations and unify their behaviors Tejun Heo @ 2013-03-30 3:30 ` David Miller 2013-03-30 5:27 ` Vineet Gupta ` (3 subsequent siblings) 4 siblings, 0 replies; 8+ messages in thread From: David Miller @ 2013-03-30 3:30 UTC (permalink / raw) To: tj Cc: linux-arch, linux-kernel, akpm, mingo, x86, rth, linux, msalter, starvik, dhowells, tony.luck, benh, takata, geert, james.hogan, monstr, ralf, jonas, rkuo, schwidefsky, liqin.chen, lethal, vgupta, chris, cmetcalf, ysato, gxt, jdike, heiko.carstens, linux-s390, vapier, uclinux-dist-devel From: Tejun Heo <tj@kernel.org> Date: Fri, 29 Mar 2013 19:27:06 -0700 > 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 and s390. > Blackfin's dump_stack() does something wonky that I don't understand > and s390 prints its own debug information which includes fields which > aren't accessible from arch-indepdent code. > > 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:4840 init_workqueues+0x35/0x505() > Hardware name: empty > Modules linked in: > Pid: 1, comm: swapper/0 Not tainted 3.9.0-rc1-work+ #17 > 0000000000000009 ffff88007c861e08 ffffffff81c61545 ffff88007c861e48 > ffffffff8108f50f ffffffff82228240 0000000000000040 ffffffff8234a03c > 0000000000000000 0000000000000000 0000000000000000 ffff88007c861e58 > Call Trace: > [<ffffffff81c61545>] dump_stack+0x19/0x1b > [<ffffffff8108f50f>] warn_slowpath_common+0x7f/0xc0 > [<ffffffff8108f56a>] warn_slowpath_null+0x1a/0x20 > [<ffffffff8234a071>] init_workqueues+0x35/0x505 > ... > > Signed-off-by: Tejun Heo <tj@kernel.org> For sparc bits: Acked-by: David S. Miller <davem@davemloft.net> ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 3/5] dump_stack: consolidate dump_stack() implementations and unify their behaviors 2013-03-30 2:27 ` [PATCH 3/5] dump_stack: consolidate dump_stack() implementations and unify their behaviors Tejun Heo 2013-03-30 3:30 ` David Miller @ 2013-03-30 5:27 ` Vineet Gupta 2013-03-30 11:45 ` Heiko Carstens ` (2 subsequent siblings) 4 siblings, 0 replies; 8+ messages in thread From: Vineet Gupta @ 2013-03-30 5:27 UTC (permalink / raw) To: Tejun Heo Cc: linux-arch, linux-kernel, akpm, mingo, x86, rth, linux, msalter, starvik, dhowells, tony.luck, benh, takata, geert, james.hogan, monstr, ralf, jonas, rkuo, schwidefsky, liqin.chen, davem, lethal, chris, cmetcalf, ysato, gxt, jdike, Heiko Carstens, linux-s390, Mike Frysinger, uclinux-dist-devel On 03/30/2013 07:57 AM, Tejun Heo wrote: > 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 and s390. > Blackfin's dump_stack() does something wonky that I don't understand > and s390 prints its own debug information which includes fields which > aren't accessible from arch-indepdent code. > > 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:4840 init_workqueues+0x35/0x505() > Hardware name: empty > Modules linked in: > Pid: 1, comm: swapper/0 Not tainted 3.9.0-rc1-work+ #17 > 0000000000000009 ffff88007c861e08 ffffffff81c61545 ffff88007c861e48 > ffffffff8108f50f ffffffff82228240 0000000000000040 ffffffff8234a03c > 0000000000000000 0000000000000000 0000000000000000 ffff88007c861e58 > Call Trace: > [<ffffffff81c61545>] dump_stack+0x19/0x1b > [<ffffffff8108f50f>] warn_slowpath_common+0x7f/0xc0 > [<ffffffff8108f56a>] warn_slowpath_null+0x1a/0x20 > [<ffffffff8234a071>] init_workqueues+0x35/0x505 > ... > > Signed-off-by: Tejun Heo <tj@kernel.org> For arch/arc bits Acked-by: Vineet Gupta <vgupta@synopsys.com> Thx, -Vineet ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 3/5] dump_stack: consolidate dump_stack() implementations and unify their behaviors 2013-03-30 2:27 ` [PATCH 3/5] dump_stack: consolidate dump_stack() implementations and unify their behaviors Tejun Heo 2013-03-30 3:30 ` David Miller 2013-03-30 5:27 ` Vineet Gupta @ 2013-03-30 11:45 ` Heiko Carstens 2013-04-02 6:48 ` Martin Schwidefsky 2013-04-02 9:22 ` Jesper Nilsson 2013-04-16 14:59 ` James Hogan 4 siblings, 1 reply; 8+ messages in thread From: Heiko Carstens @ 2013-03-30 11:45 UTC (permalink / raw) To: Tejun Heo Cc: linux-arch, linux-kernel, akpm, mingo, x86, rth, linux, msalter, starvik, dhowells, tony.luck, benh, takata, geert, james.hogan, monstr, ralf, jonas, rkuo, schwidefsky, liqin.chen, davem, lethal, vgupta, chris, cmetcalf, ysato, gxt, jdike, linux-s390, Mike Frysinger, uclinux-dist-devel On Fri, Mar 29, 2013 at 07:27:06PM -0700, Tejun Heo wrote: > 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. [...] > 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 and s390. > Blackfin's dump_stack() does something wonky that I don't understand > and s390 prints its own debug information which includes fields which > aren't accessible from arch-indepdent code. The couple of extra fields aren't worth an s390 version of dump_stack() that differs from all other architectures. Please drop the s390 version as well. Thanks! ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 3/5] dump_stack: consolidate dump_stack() implementations and unify their behaviors 2013-03-30 11:45 ` Heiko Carstens @ 2013-04-02 6:48 ` Martin Schwidefsky 2013-04-02 18:26 ` Tejun Heo 0 siblings, 1 reply; 8+ messages in thread From: Martin Schwidefsky @ 2013-04-02 6:48 UTC (permalink / raw) To: Heiko Carstens Cc: Tejun Heo, linux-arch, linux-kernel, akpm, mingo, x86, rth, linux, msalter, starvik, dhowells, tony.luck, benh, takata, geert, james.hogan, monstr, ralf, jonas, rkuo, liqin.chen, davem, lethal, vgupta, chris, cmetcalf, ysato, gxt, jdike, linux-s390, Mike Frysinger, uclinux-dist-devel On Sat, 30 Mar 2013 12:45:35 +0100 Heiko Carstens <heiko.carstens@de.ibm.com> wrote: > On Fri, Mar 29, 2013 at 07:27:06PM -0700, Tejun Heo wrote: > > 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. > > [...] > > > 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 and s390. > > Blackfin's dump_stack() does something wonky that I don't understand > > and s390 prints its own debug information which includes fields which > > aren't accessible from arch-indepdent code. > > The couple of extra fields aren't worth an s390 version of dump_stack() > that differs from all other architectures. > Please drop the s390 version as well. Thanks! Hmm, we would lose task_thread_info(current)->cpu and current->thread.ksp, I don't care much about the kernel stack pointer which has been stored at the last call to __switch_to but the cpu number might be interesting. Could we add the cpu number to the generic dump_stack version? -- blue skies, Martin. "Reality continues to ruin my life." - Calvin. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 3/5] dump_stack: consolidate dump_stack() implementations and unify their behaviors 2013-04-02 6:48 ` Martin Schwidefsky @ 2013-04-02 18:26 ` Tejun Heo 0 siblings, 0 replies; 8+ messages in thread From: Tejun Heo @ 2013-04-02 18:26 UTC (permalink / raw) To: Martin Schwidefsky Cc: Heiko Carstens, linux-arch, linux-kernel, akpm, mingo, x86, rth, linux, msalter, starvik, dhowells, tony.luck, benh, takata, geert, james.hogan, monstr, ralf, jonas, rkuo, liqin.chen, davem, lethal, vgupta, chris, cmetcalf, ysato, gxt, jdike, linux-s390, Mike Frysinger, uclinux-dist-devel Hello, Martin. On Tue, Apr 02, 2013 at 08:48:08AM +0200, Martin Schwidefsky wrote: > > The couple of extra fields aren't worth an s390 version of dump_stack() > > that differs from all other architectures. > > Please drop the s390 version as well. Thanks! > > Hmm, we would lose task_thread_info(current)->cpu and current->thread.ksp, > I don't care much about the kernel stack pointer which has been stored at > the last call to __switch_to but the cpu number might be interesting. > Could we add the cpu number to the generic dump_stack version? Yeah, I can move CPU from show_regs() to dump_stack(). It actually makes sense. I'll do that and convert s390 together with other archs. Thanks! -- tejun ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 3/5] dump_stack: consolidate dump_stack() implementations and unify their behaviors 2013-03-30 2:27 ` [PATCH 3/5] dump_stack: consolidate dump_stack() implementations and unify their behaviors Tejun Heo ` (2 preceding siblings ...) 2013-03-30 11:45 ` Heiko Carstens @ 2013-04-02 9:22 ` Jesper Nilsson 2013-04-16 14:59 ` James Hogan 4 siblings, 0 replies; 8+ messages in thread From: Jesper Nilsson @ 2013-04-02 9:22 UTC (permalink / raw) To: Tejun Heo Cc: linux-arch, linux-kernel, akpm, mingo, x86, rth, linux, msalter, starvik, dhowells, tony.luck, benh, takata, geert, james.hogan, monstr, ralf, jonas, rkuo, schwidefsky, liqin.chen, davem, lethal, vgupta, chris, cmetcalf, ysato, gxt, jdike, Heiko Carstens, linux-s390, Mike Frysinger, uclinux-dist-devel On Fri, Mar 29, 2013 at 07:27:06PM -0700, Tejun Heo wrote: > 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 and s390. > Blackfin's dump_stack() does something wonky that I don't understand > and s390 prints its own debug information which includes fields which > aren't accessible from arch-indepdent code. > > 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:4840 init_workqueues+0x35/0x505() > Hardware name: empty > Modules linked in: > Pid: 1, comm: swapper/0 Not tainted 3.9.0-rc1-work+ #17 > 0000000000000009 ffff88007c861e08 ffffffff81c61545 ffff88007c861e48 > ffffffff8108f50f ffffffff82228240 0000000000000040 ffffffff8234a03c > 0000000000000000 0000000000000000 0000000000000000 ffff88007c861e58 > Call Trace: > [<ffffffff81c61545>] dump_stack+0x19/0x1b > [<ffffffff8108f50f>] warn_slowpath_common+0x7f/0xc0 > [<ffffffff8108f56a>] warn_slowpath_null+0x1a/0x20 > [<ffffffff8234a071>] init_workqueues+0x35/0x505 > ... > > Signed-off-by: Tejun Heo <tj@kernel.org> > Cc: Martin Schwidefsky <schwidefsky@de.ibm.com> > Cc: Heiko Carstens <heiko.carstens@de.ibm.com> > Cc: linux-s390@vger.kernel.org > Cc: Mike Frysinger <vapier@gentoo.org> > Cc: uclinux-dist-devel@blackfin.uclinux.org For the CRIS bits: Acked-by: Jesper Nilsson <jesper.nilsson@axis.com> /^JN - Jesper Nilsson -- Jesper Nilsson -- jesper.nilsson@axis.com ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 3/5] dump_stack: consolidate dump_stack() implementations and unify their behaviors 2013-03-30 2:27 ` [PATCH 3/5] dump_stack: consolidate dump_stack() implementations and unify their behaviors Tejun Heo ` (3 preceding siblings ...) 2013-04-02 9:22 ` Jesper Nilsson @ 2013-04-16 14:59 ` James Hogan 4 siblings, 0 replies; 8+ messages in thread From: James Hogan @ 2013-04-16 14:59 UTC (permalink / raw) To: Tejun Heo Cc: linux-arch, linux-kernel, akpm, mingo, x86, rth, linux, msalter, starvik, dhowells, tony.luck, benh, takata, geert, monstr, ralf, jonas, rkuo, schwidefsky, liqin.chen, davem, lethal, vgupta, chris, cmetcalf, ysato, gxt, jdike, Heiko Carstens, linux-s390, Mike Frysinger, uclinux-dist-devel On 30/03/13 02:27, Tejun Heo wrote: > 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 and s390. > Blackfin's dump_stack() does something wonky that I don't understand > and s390 prints its own debug information which includes fields which > aren't accessible from arch-indepdent code. > > 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:4840 init_workqueues+0x35/0x505() > Hardware name: empty > Modules linked in: > Pid: 1, comm: swapper/0 Not tainted 3.9.0-rc1-work+ #17 > 0000000000000009 ffff88007c861e08 ffffffff81c61545 ffff88007c861e48 > ffffffff8108f50f ffffffff82228240 0000000000000040 ffffffff8234a03c > 0000000000000000 0000000000000000 0000000000000000 ffff88007c861e58 > Call Trace: > [<ffffffff81c61545>] dump_stack+0x19/0x1b > [<ffffffff8108f50f>] warn_slowpath_common+0x7f/0xc0 > [<ffffffff8108f56a>] warn_slowpath_null+0x1a/0x20 > [<ffffffff8234a071>] init_workqueues+0x35/0x505 > ... > > Signed-off-by: Tejun Heo <tj@kernel.org> > Cc: Martin Schwidefsky <schwidefsky@de.ibm.com> > Cc: Heiko Carstens <heiko.carstens@de.ibm.com> > Cc: linux-s390@vger.kernel.org > Cc: Mike Frysinger <vapier@gentoo.org> > Cc: uclinux-dist-devel@blackfin.uclinux.org For metag: Acked-by: James Hogan <james.hogan@imgtec.com> ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2013-04-16 14:59 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <1364610428-2074-1-git-send-email-tj@kernel.org>
[not found] ` <20130330022706.6O_4ksyBEnkdW6XmrL4YUSD_Q4kA0Y95Y7NBinv2q54@z>
2013-03-30 2:27 ` [PATCH 3/5] dump_stack: consolidate dump_stack() implementations and unify their behaviors Tejun Heo
2013-03-30 3:30 ` David Miller
2013-03-30 5:27 ` Vineet Gupta
2013-03-30 11:45 ` Heiko Carstens
2013-04-02 6:48 ` Martin Schwidefsky
2013-04-02 18:26 ` Tejun Heo
2013-04-02 9:22 ` Jesper Nilsson
2013-04-16 14:59 ` James Hogan
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox