linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC] Smarter stack traces using the frame pointer
@ 2003-11-04 22:13 Adam Litke
  2003-11-05  2:21 ` Stephen Rothwell
  2003-11-10 17:46 ` Adam Litke
  0 siblings, 2 replies; 8+ messages in thread
From: Adam Litke @ 2003-11-04 22:13 UTC (permalink / raw)
  To: mbligh; +Cc: linux-kernel, lse-tech

I was working on this for the mjb tree but I thought I'd throw it out
here in case anyone else finds it interesting.  This simple change to
the stack trace code uses the frame pointer to do the trace instead of
assuming any kernel address on the stack is a return address.  It makes
for much more readable stack traces.

diff -purN linux-2.6.0-test9-virgin/arch/i386/kernel/traps.c linux-2.6.0-test9-stack/arch/i386/kernel/traps.c
--- linux-2.6.0-test9-virgin/arch/i386/kernel/traps.c	2003-11-03 10:34:18.000000000 -0800
+++ linux-2.6.0-test9-stack/arch/i386/kernel/traps.c	2003-11-04 13:51:20.000000000 -0800
@@ -93,17 +93,36 @@ asmlinkage void machine_check(void);
 
 static int kstack_depth_to_print = 24;
 
-void show_trace(struct task_struct *task, unsigned long * stack)
+#ifdef CONFIG_FRAME_POINTER
+void show_trace_fp(struct task_struct *task)
+{
+	unsigned long addr, ebp;
+
+	if (task == current) {
+		/* Grab ebp right from our regs */
+		asm ("movl %%ebp, %0" : "=r" (ebp) : );
+	} else {
+		/* ebp is the last reg pushed by switch_to */
+		ebp = *(unsigned long *) task->thread.esp;
+	}
+	
+	while (ebp) {
+		addr = *(unsigned long *) (ebp + 4);
+		if (!kernel_text_address(addr))
+			return;
+		printk(" [<%08lx>] ", addr);
+		print_symbol("%s\n", addr);
+		ebp = *(unsigned long *) ebp;
+	}
+}
+#else
+void show_trace_guess(unsigned long * stack)
 {
 	unsigned long addr;
 
 	if (!stack)
 		stack = (unsigned long*)&stack;
 
-	printk("Call Trace:");
-#ifdef CONFIG_KALLSYMS
-	printk("\n");
-#endif
 	while (!kstack_end(stack)) {
 		addr = *stack++;
 		if (kernel_text_address(addr)) {
@@ -111,6 +130,20 @@ void show_trace(struct task_struct *task
 			print_symbol("%s\n", addr);
 		}
 	}
+}
+#endif
+
+void show_trace(struct task_struct *task, unsigned long * stack)
+{
+	printk("Call Trace:");
+#ifdef CONFIG_KALLSYMS
+	printk("\n");
+#endif
+#ifdef CONFIG_FRAME_POINTER
+	show_trace_fp(task);
+#else
+	show_trace_guess(stack);
+#endif
 	printk("\n");
 } 
-- 
Adam Litke (agl at us.ibm.com)
IBM Linux Technology Center
(503) 578 - 3283 t/l 775 - 3283


^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [RFC] Smarter stack traces using the frame pointer
@ 2003-11-06 20:08 Dan Kegel
  0 siblings, 0 replies; 8+ messages in thread
From: Dan Kegel @ 2003-11-06 20:08 UTC (permalink / raw)
  To: linux-kernel

For what it's worth, on ppc I've made stack traces
slightly smarter, too.  I think the ppc stack dump
code always used the frame
pointer, but it still output a bunch of garbage at the
end of the stack dump.  As a byproduct of porting the
module tracking patch to ppc, I had to check each
address printed out by the stack dumper, and fairly
often they're not legal text addresses.  I print a ? after
the questionable ones now.  No big deal, but it is
*slightly* smarter.  Here's the bit in question:

--- patch-backup/arch/ppc/kernel/process.c	Mon Oct 13 17:32:04 2003
+++ arch/ppc/kernel/process.c	Tue Oct 14 00:05:29 2003
@@ -514,7 +564,11 @@
  			break;
  		if (cnt++ % 7 == 0)
  			printk("\n");
-		printk("%08lX ", i);
+		if (kernel_text_address(i)) {	/* mostly just want to mark modules */
+			printk("%08lX ", i);
+		} else {			/* FIXME: should we ignore fishy adrs like x86? */
+			printk("%08lX? ", i);	/* Or append a '?' (ksymoops doesn't mind) */
+		}
  		if (cnt > 32) break;
  		if (__get_user(sp, (unsigned long **)sp))
  			break;


And here's the whole module tracking patch, including my port of it to ppc
(the paths have the first component removed, sorry):

# Purpose: add module info to oops log
# Source http://www.kernel.org/pub/linux/kernel/people/andrea/kernels/v2.4/2.4.23pre6aa3/90_module-oops-tracking-3
# See also http://marc.theaimsgroup.com/?l=linux-kernel&m=102772338115172&w=2
# ppc port added by dank@kegel.com Oct 2003

diff -urNp z/arch/i386/kernel/traps.c 2.4.19rc3aa2/arch/i386/kernel/traps.c
--- arch/i386/kernel/traps.c	Sat Jul 27 03:42:39 2002
+++ arch/i386/kernel/traps.c	Sat Jul 27 03:41:56 2002
@@ -113,6 +113,7 @@ static inline int kernel_text_address(un
  		 * module area. Of course it'd be better to test only
  		 * for the .text subset... */
  		if (mod_bound(addr, 0, mod)) {
+			module_oops_tracking_mark(mod);
  			retval = 1;
  			break;
  		}
@@ -208,6 +209,8 @@ void show_registers(struct pt_regs *regs
  		esp = regs->esp;
  		ss = regs->xss & 0xffff;
  	}
+	module_oops_tracking_init();
+	kernel_text_address(regs->eip);
  	printk("CPU:    %d\nEIP:    %04x:[<%08lx>]    %s\nEFLAGS: %08lx\n",
  		smp_processor_id(), 0xffff & regs->xcs, regs->eip, print_tainted(), regs->eflags);
  	printk("eax: %08lx   ebx: %08lx   ecx: %08lx   edx: %08lx\n",
@@ -226,8 +229,9 @@ void show_registers(struct pt_regs *regs

  		printk("\nStack: ");
  		show_stack((unsigned long*)esp);
+		module_oops_tracking_print();

-		printk("\nCode: ");
+		printk("Code: ");
  		if(regs->eip < PAGE_OFFSET)
  			goto bad;

@@ -288,7 +292,8 @@ void die(const char * str, struct pt_reg
  	spin_lock_irq(&die_lock);
  	bust_spinlocks(1);
  	handle_BUG(regs);
-	printk("%s: %04lx\n", str, err & 0xffff);
+	printk("%s: %04lx ", str, err & 0xffff);
+	printk(oops_id);
  	show_registers(regs);
  	bust_spinlocks(0);
  	spin_unlock_irq(&die_lock);
diff -urNp z/include/linux/kernel.h 2.4.19rc3aa2/include/linux/kernel.h
--- include/linux/kernel.h	Sat Jul 27 03:42:39 2002
+++ include/linux/kernel.h	Sat Jul 27 04:57:04 2002
@@ -109,6 +109,8 @@ extern const char *print_tainted(void);

  extern void dump_stack(void);

+extern char *oops_id;
+
  #if DEBUG
  #define pr_debug(fmt,arg...) \
  	printk(KERN_DEBUG fmt,##arg)
diff -urNp z/include/linux/module.h 2.4.19rc3aa2/include/linux/module.h
--- include/linux/module.h	Sat Jul 27 03:42:39 2002
+++ include/linux/module.h	Sat Jul 27 04:57:40 2002
@@ -110,6 +110,7 @@ struct module_info
  #define MOD_USED_ONCE		16
  #define MOD_JUST_FREED		32
  #define MOD_INITIALIZING	64
+#define MOD_OOPS_PRINT		128UL

  /* Values for query_module's which.  */

@@ -411,4 +412,14 @@ __attribute__((section("__ksymtab"))) =	
  #define SET_MODULE_OWNER(some_struct) do { } while (0)
  #endif

+#ifdef CONFIG_MODULES
+extern void module_oops_tracking_init(void);
+extern void module_oops_tracking_mark(struct module *);
+extern void module_oops_tracking_print(void);
+#else
+#define module_oops_tracking_init() do { } while (0)
+#define module_oops_tracking_mark(x) do { } while (0)
+#define module_oops_tracking_print() do { } while (0)
+#endif
+
  #endif /* _LINUX_MODULE_H */
diff -urNp z/init/version.c 2.4.19rc3aa2/init/version.c
--- init/version.c	Sat Jul 27 03:42:39 2002
+++ init/version.c	Sat Jul 27 05:03:14 2002
@@ -24,3 +24,5 @@ struct new_utsname system_utsname = {
  const char *linux_banner =
  	"Linux version " UTS_RELEASE " (" LINUX_COMPILE_BY "@"
  	LINUX_COMPILE_HOST ") (" LINUX_COMPILER ") " UTS_VERSION "\n";
+
+const char *oops_id = UTS_RELEASE " " UTS_VERSION "\n";
diff -urNp z/kernel/module.c 2.4.19rc3aa2/kernel/module.c
--- kernel/module.c	Sat Jul 27 03:42:39 2002
+++ kernel/module.c	Sat Jul 27 02:38:24 2002
@@ -1242,6 +1242,50 @@ struct seq_operations ksyms_op = {
  	show:	s_show
  };

+/*
+ * The module tracking logic assumes the module list doesn't
+ * change under the oops. In case of a race we could get not
+ * the exact information about the affected modules, but it's
+ * almost impossible to trigger and it's a not interesting case.
+ * This code runs protected by the die_lock spinlock, the arch/
+ * caller takes care of it.
+ */
+void module_oops_tracking_init(void)
+{
+	struct module * mod;
+
+	for (mod = module_list; mod != &kernel_module; mod = mod->next)
+		mod->flags &= ~MOD_OOPS_PRINT;
+}
+
+void module_oops_tracking_mark(struct module * mod)
+{
+	mod->flags |= MOD_OOPS_PRINT;
+}
+
+void module_oops_tracking_print(void)
+{
+	struct module *mod;
+	int i = 0;
+
+	for (mod = module_list; mod != &kernel_module; mod = mod->next) {
+		if (!(mod->flags & MOD_OOPS_PRINT))
+			continue;
+		if (!i)
+			printk("Modules:");
+		if (i && !(i % 2))
+			printk("\n        ");
+		i++;
+
+		printk(" [(%s:<%p>:<%p>)]",
+		       mod->name,
+		       (char *) mod + mod->size_of_struct,
+		       (char *) mod + mod->size);
+	}
+	if (i)
+		printk("\n");
+}
+
  #else		/* CONFIG_MODULES */

  /* Dummy syscalls for people who don't want modules */
--- patch-backup/arch/ppc/kernel/process.c	Mon Oct 13 17:32:04 2003
+++ arch/ppc/kernel/process.c	Tue Oct 14 00:05:29 2003
@@ -60,6 +60,53 @@
  #undef SHOW_TASK_SWITCHES
  #undef CHECK_STACK

+/*
+ * If the address is either in the .text section of the
+ * kernel, or in the vmalloc'ed module regions, it *may*
+ * be the address of a calling routine
+ */
+
+extern char _etext[], _stext[];
+
+#ifdef CONFIG_MODULES
+#include <linux/module.h>
+
+extern struct module *module_list;
+extern struct module kernel_module;
+
+static inline int kernel_text_address(unsigned long addr)
+{
+	int retval = 0;
+	struct module *mod;
+
+	if (addr >= (unsigned long) &_stext &&
+	    addr <= (unsigned long) &_etext)
+		return 1;
+
+	for (mod = module_list; mod != &kernel_module; mod = mod->next) {
+		/* mod_bound tests for addr being inside the vmalloc'ed
+		 * module area. Of course it'd be better to test only
+		 * for the .text subset... */
+		if (mod_bound(addr, 0, mod)) {
+			module_oops_tracking_mark(mod);
+			retval = 1;
+			break;
+		}
+	}
+
+	return retval;
+}
+
+#else
+
+static inline int kernel_text_address(unsigned long addr)
+{
+	return (addr >= (unsigned long) &_stext &&
+		addr <= (unsigned long) &_etext);
+}
+
+#endif
+
  #if defined(CHECK_STACK)
  unsigned long
  kernel_stack_top(struct task_struct *tsk)
@@ -243,6 +290,8 @@
  {
  	int i;

+	module_oops_tracking_init();
+	kernel_text_address(regs->nip);
  	printk("NIP: %08lX XER: %08lX LR: %08lX SP: %08lX REGS: %p TRAP: %04lx    %s\n",
  	       regs->nip, regs->xer, regs->link, regs->gpr[1], regs,regs->trap, print_tainted());
  	printk("MSR: %08lx EE: %01x PR: %01x FP: %01x ME: %01x IR/DR: %01x%01x\n",
@@ -303,6 +352,7 @@
  	}
  out:
  	print_backtrace((unsigned long *)regs->gpr[1]);
+	module_oops_tracking_print();
  }

  void exit_thread(void)
@@ -514,7 +564,11 @@
  			break;
  		if (cnt++ % 7 == 0)
  			printk("\n");
-		printk("%08lX ", i);
+		if (kernel_text_address(i)) {	/* mostly just want to mark modules */
+			printk("%08lX ", i);
+		} else {			/* FIXME: should we ignore fishy adrs like x86? */
+			printk("%08lX? ", i);	/* Or append a '?' (ksymoops doesn't mind) */
+		}
  		if (cnt > 32) break;
  		if (__get_user(sp, (unsigned long **)sp))
  			break;
--- arch/ppc/kernel/traps.c.old	Mon Oct 13 17:23:40 2003
+++ arch/ppc/kernel/traps.c	Mon Oct 13 17:25:27 2003
@@ -97,7 +97,8 @@
  	set_backlight_enable(1);
  	set_backlight_level(BACKLIGHT_MAX);
  #endif
-	printk("Oops: %s, sig: %ld\n", str, err);
+	printk("Oops: %s, sig: %ld ", str, err);
+	printk(oops_id);
  	show_regs(fp);
  #ifdef CONFIG_BOOTX_TEXT
  	force_printk_to_btext = 0;


^ permalink raw reply	[flat|nested] 8+ messages in thread
[parent not found: <OhH5.6Y2.13@gated-at.bofh.it>]

end of thread, other threads:[~2003-11-10 18:06 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2003-11-04 22:13 [RFC] Smarter stack traces using the frame pointer Adam Litke
2003-11-05  2:21 ` Stephen Rothwell
2003-11-05 19:56   ` Martin J. Bligh
2003-11-06 11:51     ` Stephen Rothwell
2003-11-06 15:21       ` Martin J. Bligh
2003-11-10 17:46 ` Adam Litke
2003-11-06 20:08 Dan Kegel
     [not found] <OhH5.6Y2.13@gated-at.bofh.it>
     [not found] ` <QouH.36G.7@gated-at.bofh.it>
2003-11-10 18:06   ` Andi Kleen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).