cheap lookup of symbol names on oops()

This is from the -atp (Aunt Tillie and Penelope) tree.

This patch adds a small function that looks up symbol names that correspond to given addresses by digging through the already existent ksyms table. It's invaluable for debugging on embedded systems - especially when testing modules - since ksymoops is a hassle to deal with in cross-build environments. We already have this info in the kernel so we might as well use it.

This patch adds use of the function for PPC and i386.

diff -Nru a/arch/i386/kernel/traps.c b/arch/i386/kernel/traps.c
@@ -201,8 +201,11 @@
 		esp = regs->esp;
 		ss = regs->xss & 0xffff;
 	}
-	printk("CPU:    %d\nEIP:    %04x:[<%08lx>]    %s\nEFLAGS: %08lx\n",
-		smp_processor_id(), 0xffff & regs->xcs, regs->eip, print_tainted(), regs->eflags);
+	printk("CPU:    %d\nEIP:    %04x:[<%08lx>]",
+	       smp_processor_id(), 0xffff & regs->xcs, regs->eip );
+	module_print_addr(" ", regs->eip, NULL);
+	printk(" %s\nEFLAGS: %08lx\n",
+	       print_tainted(), regs->eflags);
 	printk("eax: %08lx   ebx: %08lx   ecx: %08lx   edx: %08lx\n",
 		regs->eax, regs->ebx, regs->ecx, regs->edx);
 	printk("esi: %08lx   edi: %08lx   ebp: %08lx   esp: %08lx\n",

diff -Nru a/arch/i386/mm/fault.c b/arch/i386/mm/fault.c
@@ -323,7 +323,9 @@
 		printk(KERN_ALERT "Unable to handle kernel paging request");
 	printk(" at virtual address %08lx\n",address);
 	printk(" printing eip:\n");
-	printk("%08lx\n", regs->eip);
+	printk("%08lx", regs->eip);
+	module_print_addr(" ",regs->eip, NULL);
+	printk("\n");
 	asm("movl %%cr3,%0":"=r" (page));
 	page = ((unsigned long *) __va(page))[address >> 22];
 	printk(KERN_ALERT "*pde = %08lx\n", page);

diff -Nru a/arch/ppc/kernel/process.c b/arch/ppc/kernel/process.c
@@ -245,8 +245,11 @@
 {
 	int i;
 
-	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("NIP: %08lX");
+	module_print_addr(" ", regs->nip, NULL);
+	printk("\n");
+	printk("XER: %08lX LR: %08lX SP: %08lX REGS: %p TRAP: %04lx    %s\n",
+	       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",
 	       regs->msr, regs->msr&MSR_EE ? 1 : 0, regs->msr&MSR_PR ? 1 : 0,
 	       regs->msr & MSR_FP ? 1 : 0,regs->msr&MSR_ME ? 1 : 0,

diff -Nru a/include/linux/module.h b/include/linux/module.h
@@ -411,4 +411,10 @@
 #define SET_MODULE_OWNER(some_struct) do { } while (0)
 #endif
 
+#ifdef CONFIG_MODULES
+void module_print_addr(char *, unsigned long, char *);
+#else /* CONFIG_MODULES */
+#define module_print_addr(x,y,z) do { } while (0)
+#endif /* CONFIG_MODULES */
+
 #endif /* _LINUX_MODULE_H */

Wow! very usefull patch. O want it for 2.4 and 2.5, please.

But could you please fix up the indentation to match common kernel style?

I'm glad you found it useful.

I'm sorry, Aunt Tillie has a mind of her own about indentation. Patch below with spaces turned to tabs.

I have the bk patch, if needed.

} Wow! very usefull patch. O want it for 2.4 and 2.5, please. }
} But could you please fix up the indentation to match common kernel style?

diff -Nru a/arch/i386/kernel/traps.c b/arch/i386/kernel/traps.c
@@ -201,8 +201,11 @@
 		esp = regs->esp;
 		ss = regs->xss & 0xffff;
 	}
-	printk("CPU:    %d\nEIP:    %04x:[<%08lx>]    %s\nEFLAGS: %08lx\n",
-		smp_processor_id(), 0xffff & regs->xcs, regs->eip, print_tainted(), regs->eflags);
+	printk("CPU:    %d\nEIP:    %04x:[<%08lx>]",
+	       smp_processor_id(), 0xffff & regs->xcs, regs->eip );
+	module_print_addr(" ", regs->eip, NULL);
+	printk(" %s\nEFLAGS: %08lx\n",
+	       print_tainted(), regs->eflags);
 	printk("eax: %08lx   ebx: %08lx   ecx: %08lx   edx: %08lx\n",
 		regs->eax, regs->ebx, regs->ecx, regs->edx);
 	printk("esi: %08lx   edi: %08lx   ebp: %08lx   esp: %08lx\n",

diff -Nru a/arch/i386/mm/fault.c b/arch/i386/mm/fault.c
@@ -323,7 +323,9 @@
 		printk(KERN_ALERT "Unable to handle kernel paging request");
 	printk(" at virtual address %08lx\n",address);
 	printk(" printing eip:\n");
-	printk("%08lx\n", regs->eip);
+	printk("%08lx", regs->eip);
+	module_print_addr(" ",regs->eip, NULL);
+	printk("\n");
 	asm("movl %%cr3,%0":"=r" (page));
 	page = ((unsigned long *) __va(page))[address >> 22];
 	printk(KERN_ALERT "*pde = %08lx\n", page);

diff -Nru a/arch/ppc/kernel/process.c b/arch/ppc/kernel/process.c
@@ -245,8 +245,11 @@
 {
 	int i;
 
-	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("NIP: %08lX");
+	module_print_addr(" ", regs->nip, NULL);
+	printk("\n");
+	printk("XER: %08lX LR: %08lX SP: %08lX REGS: %p TRAP: %04lx    %s\n",
+	       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",
 	       regs->msr, regs->msr&MSR_EE ? 1 : 0, regs->msr&MSR_PR ? 1 : 0,
 	       regs->msr & MSR_FP ? 1 : 0,regs->msr&MSR_ME ? 1 : 0,

diff -Nru a/include/linux/module.h b/include/linux/module.h
@@ -411,4 +411,10 @@
 #define SET_MODULE_OWNER(some_struct) do { } while (0)
 #endif
 
+#ifdef CONFIG_MODULES
+void module_print_addr(char *, unsigned long, char *);
+#else /* CONFIG_MODULES */
+#define module_print_addr(x,y,z) do { } while (0)
+#endif /* CONFIG_MODULES */
+
 #endif /* _LINUX_MODULE_H */

Nearly all of the code scattered through arch/ppc uses the Cort indention style, it hasn't done any damage. I don't think a 50 line patch is going to drag Linux down into the mudpits because of an extra \n. If the indention is an issue it can be quickly changed when it's applied.

Aunt Tillie is, after all, a non-coder. She was just barely able to configure the kernel thanks to CML2. If Penelope hadn't been helped out by the creepy guy and the good looking guy she wouldn't even have started emacs.

As for myself, I think 50 locking primitives are spurious but a couple extra whitespace chars are just a minor issue :)

} It looks like she still has to ready the Documentation/CodingStyle. The } space after the ( and before the ) are spurious, and thes { belongs on } the same line as the if.

It looks like it has a few issues, can you verify the below patch?

I don't think it make sense to resolve the EIP unless it's a module, you'd liekly need the system.map/vmlinux anyways to get to the right function symbol name.

Furthmore it would be nice to print also for the stack trace, in case there are intra-modules, infact I guess I would prefer to keep trace of all the modules affected and to print the start address of each module affected instead of resolving to the nearest symbol, since as for the kernel .text, also the module .text isn't likely to be always exported.

In short I don't like very much your approch but for now the below will be ok, and it will work right most of the time (since from such symbol we'll be able to deduce the rest and it's likely all the modules addresses in the stack trace cames from the same module .o, but it's not guaranteed)

But as said above long term the right thing to do is to print the start address of each module affected in the trace (we just check for that during the oops to discard bogus values from the stack trace), not to try to resolve anything in the kernel. So I will reject the current patch as soon as the right way is implemented (then of course ksymoops will have to learn the right way too, instead of guessing from the current unreliable /proc/ksyms after reboot).

@@ -246,10 +246,11 @@ void free_module(struct module *, int ta
  * the need for user space tools.
  *  -- Cort <[email protected]>
  */
+#define PRINT_ADDR_LENGTH 60
 void module_print_addr(char *s1, unsigned long addr, char *s2)
 {
 	unsigned long best_match = 0; /* so far */
-	char best_match_string[60] = {0, }; /* so far */
+	char best_match_string[PRINT_ADDR_LENGTH];
 	struct module *mod;
 	struct module_symbol *sym;
 	int j;

@@ -265,24 +266,25 @@ void module_print_addr(char *s1, unsigne
 		return;
 	}
 
-	for (mod = module_list; mod; mod = mod->next)
-	{
+	for (mod = module_list; mod != &kernel_module; mod = mod->next) {
+		if (!mod_bound(addr, 0, mod))
+			continue;
 		for ( j = 0, sym = mod->syms; j < mod->nsyms; ++j, ++sym)
 		{
 			/* is this a better match than what we've
 			 * found so far? -- Cort */
-			if ( (sym->value < addr) &&
-			     ((addr - sym->value) < (addr - best_match)) )
+			if (sym->value <= addr &&
+			    addr - sym->value < addr - best_match)
 			{
 				best_match = sym->value;
 				/* kernelmodule.name is "" so we
 				 * have a special case -- Cort */
 				if ( mod->name[0] == 0 )
-					sprintf(best_match_string, "%s",
-						sym->name);
+					snprintf(best_match_string, PRINT_ADDR_LENGTH, "%s",
+						 sym->name);
 				else
-					sprintf(best_match_string, "%s:%s",
-						sym->name, mod->name);
+					snprintf(best_match_string, PRINT_ADDR_LENGTH, "%s:%s",
+						 sym->name, mod->name);
 			}
 		}
 	}

Andrea

None of the changes look like a problem. I'll absorb them and send another patch. One that fixes Ben's criticisms of spurious whitespace, even :)

Are you suggesting that it should print the start/end of the module in each trace in addition to the symbol names or instead of them?

I've found it valuable to have the EIP resolved. Even though the symbol name may not be perfect (only resolves exported names) it is valuable to see that the function crashed 0x1fe bytes after a given symbol name.

The version I use prints names for the stack trace but the formatting is abysmal. Making it clean and pretty on the screen would turn the "simple and cheap" function into a parsing monster. I'm happy to add it but it wouldn't be the innocuous little patch it is now.

} It looks like it has a few issues, can you verify the below patch? }
} I don't think it make sense to resolve the EIP unless it's a module, } you'd liekly need the system.map/vmlinux anyways to get to the right } function symbol name.
}
} Furthmore it would be nice to print also for the stack trace, in case } there are intra-modules, infact I guess I would prefer to keep trace of } all the modules affected and to print the start address of each module } affected instead of resolving to the nearest symbol, since as for the } kernel .text, also the module .text isn't likely to be always exported. }
} In short I don't like very much your approch but for now the below will } be ok, and it will work right most of the time (since from such symbol } we'll be able to deduce the rest and it's likely all the modules } addresses in the stack trace cames from the same module .o, but it's not } guaranteed)
}
} But as said above long term the right thing to do is to print the start } address of each module affected in the trace (we just check for that } during the oops to discard bogus values from the stack trace), not to } try to resolve anything in the kernel. So I will reject the current } patch as soon as the right way is implemented (then of course ksymoops } will have to learn the right way too, instead of guessing from the } current unreliable /proc/ksyms after reboot).
}
} --- symb/kernel/module.c.~1~ Thu Jul 25 20:21:19 2002
} +++ symb/kernel/module.c Thu Jul 25 20:48:20 2002
} @@ -246,10 +246,11 @@ void free_module(struct module *, int ta

}   * the need for user space tools.
}   *  -- Cort <[email protected]>
}   */
} +#define PRINT_ADDR_LENGTH 60
}  void module_print_addr(char *s1, unsigned long addr, char *s2)
}  {
}  	unsigned long best_match = 0; /* so far */
} -	char best_match_string[60] = {0, }; /* so far */
} +	char best_match_string[PRINT_ADDR_LENGTH];
}  	struct module *mod;
}  	struct module_symbol *sym;
}  	int j;

} @@ -265,24 +266,25 @@ void module_print_addr(char *s1, unsigne
} return;
} }
}
} - for (mod = module_list; mod; mod = mod->next)
} - {
} + for (mod = module_list; mod != &kernel_module; mod = mod->next) {
} + if (!mod_bound(addr, 0, mod))
} + continue;

}  		for ( j = 0, sym = mod->syms; j < mod->nsyms; ++j, ++sym)
}  		{
}  			/* is this a better match than what we've
}  			 * found so far? -- Cort */

} - if ( (sym->value < addr) &&
} - ((addr - sym->value) < (addr - best_match)) )
} + if (sym->value <= addr &&
} + addr - sym->value < addr - best_match)

}  			{
}  				best_match = sym->value;
}  				/* kernelmodule.name is "" so we
}  				 * have a special case -- Cort */
}  				if ( mod->name[0] == 0 )

} - sprintf(best_match_string, "%s",
} - sym->name);
} + snprintf(best_match_string, PRINT_ADDR_LENGTH, "%s",
} + sym->name);
} else
} - sprintf(best_match_string, "%s:%s",
} - sym->name, mod->name);
} + snprintf(best_match_string, PRINT_ADDR_LENGTH, "%s:%s",
} + sym->name, mod->name);

}  			}
}  		}
}  	}

}
}
}
} Andrea
} -
} To unsubscribe from this list: send the line "unsubscribe linux-kernel" in } the body of a message to [email protected]
} More majordomo info at http://vger.kernel.org/majordomo-info.html
} Please read the FAQ at http://www.tux.org/lkml/

the lack of <= was a bug, the buffer overflows on the stack with symbols

Not the end, just the start, and not in addition, but only the start address of each module without any symbol.

valuable for what? you need the system.map or the .o disassembly of the module anyways to take advantage of such symbol. I don't find it useful. Furthmore ksymoops will prefer to work with hex numbers rather than doing the reverse lookup to the number and then resolving to the right symbol (and not only ksymoops, me too while doing by hand and that's why I prefer hex there)

One thing is if you have ksymall ala kdb and you can resolve to something where you don't need the system.map to guess what happened, but without the ksymall you need the system.map or vmlinux anyways.

Andrea

What I meant was - "none of your changes seem to cause problems, and I'll absorb them and send another patch" and not "your changes don't fix any problems". Yes, I see that your changes address real problems.

Patch (with the indentation fixed) below.

} > None of the changes look like a problem. I'll absorb them and send another }
} the lack of <= was a bug, the buffer overflows on the stack with symbols } > 60 was a bug too even if less likely to trigger.
}
} > patch. One that fixes Ben's criticisms of spurious whitespace, even :) } >
} > Are you suggesting that it should print the start/end of the module in each } > trace in addition to the symbol names or instead of them?
}
} Not the end, just the start, and not in addition, but only the start } address of each module without any symbol.
}
} > I've found it valuable to have the EIP resolved. Even though the symbol } > name may not be perfect (only resolves exported names) it is valuable to } > see that the function crashed 0x1fe bytes after a given symbol name.

That's only true for kernel names. Function names that are not static in modules are given - and it's been useful for quickly finding which function in what module caused the trap.

I must misunderstand you since it seems you're suggesting that the symbol names aren't useful. That's the whole point of the patch - to give symbol names :)

} valuable for what? you need the system.map or the .o disassembly of the } module anyways to take advantage of such symbol. I don't find it useful. } Furthmore ksymoops will prefer to work with hex numbers rather than } doing the reverse lookup to the number and then resolving to the right } symbol (and not only ksymoops, me too while doing by hand and that's why } I prefer hex there)

On PPC I just tack the system.map.gz to the xmon debugger and lookup there. That doesn't help with module oops' since it doesn't enter the debugger. It's also an extra set of steps when looking at a symbol name and the actual address is much easier when looking at a target board.

} One thing is if you have ksymall ala kdb and you can resolve to } something where you don't need the system.map to guess what happened, } but without the ksymall you need the system.map or vmlinux anyways.

diff -Nru a/arch/i386/kernel/traps.c b/arch/i386/kernel/traps.c
@@ -201,8 +201,11 @@
 		esp = regs->esp;
 		ss = regs->xss & 0xffff;
 	}
-	printk("CPU:    %d\nEIP:    %04x:[<%08lx>]    %s\nEFLAGS: %08lx\n",
-		smp_processor_id(), 0xffff & regs->xcs, regs->eip, print_tainted(), regs->eflags);
+	printk("CPU:    %d\nEIP:    %04x:[<%08lx>]",
+	       smp_processor_id(), 0xffff & regs->xcs, regs->eip );
+	module_print_addr(" ", regs->eip, NULL);
+	printk(" %s\nEFLAGS: %08lx\n",
+	       print_tainted(), regs->eflags);
 	printk("eax: %08lx   ebx: %08lx   ecx: %08lx   edx: %08lx\n",
 		regs->eax, regs->ebx, regs->ecx, regs->edx);
 	printk("esi: %08lx   edi: %08lx   ebp: %08lx   esp: %08lx\n",

diff -Nru a/arch/i386/mm/fault.c b/arch/i386/mm/fault.c
@@ -323,7 +323,9 @@
 		printk(KERN_ALERT "Unable to handle kernel paging request");
 	printk(" at virtual address %08lx\n",address);
 	printk(" printing eip:\n");
-	printk("%08lx\n", regs->eip);
+	printk("%08lx", regs->eip);
+	module_print_addr(" ",regs->eip, NULL);
+	printk("\n");
 	asm("movl %%cr3,%0":"=r" (page));
 	page = ((unsigned long *) __va(page))[address >> 22];
 	printk(KERN_ALERT "*pde = %08lx\n", page);

diff -Nru a/arch/ppc/kernel/process.c b/arch/ppc/kernel/process.c
@@ -245,8 +245,11 @@
 {
 	int i;
 
-	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("NIP: %08lX");
+	module_print_addr(" ", regs->nip, NULL);
+	printk("\n");
+	printk("XER: %08lX LR: %08lX SP: %08lX REGS: %p TRAP: %04lx    %s\n",
+	       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",
 	       regs->msr, regs->msr&MSR_EE ? 1 : 0, regs->msr&MSR_PR ? 1 : 0,
 	       regs->msr & MSR_FP ? 1 : 0,regs->msr&MSR_ME ? 1 : 0,

diff -Nru a/include/linux/module.h b/include/linux/module.h
@@ -411,4 +411,10 @@
 #define SET_MODULE_OWNER(some_struct) do { } while (0)
 #endif
 
+#ifdef CONFIG_MODULES
+void module_print_addr(char *, unsigned long, char *);
+#else /* CONFIG_MODULES */
+#define module_print_addr(x,y,z) do { } while (0)
+#endif /* CONFIG_MODULES */
+
 #endif /* _LINUX_MODULE_H */

Hmm no, only functions that are explicitly exported through EXPORT_SYMBOL are given, they're the only one needed, if you were right the modules would be wasting an overkill of kernel not swappable ram for no good reason. This is true for both kernel and modules, no difference.

And even if only the non static ones would not be given still it would be an high probability to need the system.map to resolve it.

I appreciated it as a good start to get more reliable module reports, but I think it's not the best way to do it (but still it's way better than nothing! :).

I'm not trying to make it easier, I'm trying to make it possible at all.

Right now if I get an oops into a module from an user I cannot debug it period. I'm missing information that I cannot generate on my side. Every time he loads the module it will be at a different address (in particular with my tree where I try to allocate modules in multipages to get faster performance of the binary image at runtime even if they will use more ram), so unless he managed running ksymoops on the "after-oops" semi broken kernel, the oops will be totally useless.

Printing the start of each affected module into the oops will solve the problem and it will make possible for us to debug oopses that involves modules. Then to automate it we ""only"" need to teach ksymoops to parse those module-start informations.

Andrea

The patch works, though. I'm seeing lots of function names in ksyms -a and haven't done a single export_symbol in any module. Perhaps you mean only in the kernel. Do I misunderstand?

} Hmm no, only functions that are explicitly exported through } EXPORT_SYMBOL are given, they're the only one needed, if you were right } the modules would be wasting an overkill of kernel not swappable ram for } no good reason. This is true for both kernel and modules, no difference. }
} And even if only the non static ones would not be given still it would } be an high probability to need the system.map to resolve it.

That it certainly is!

} I appreciated it as a good start to get more reliable module reports, } but I think it's not the best way to do it (but still it's way better } than nothing! :).

Explaining to people what a System.map is, how to send it to me and trying to get them to be methodical enough to be sure they're sending the right one is a sure road to insanity. I took a daytrip down that road once.

} I'm not trying to make it easier, I'm trying to make it possible at all. }
} Right now if I get an oops into a module from an user I cannot debug it } period. I'm missing information that I cannot generate on my side. Every } time he loads the module it will be at a different address (in } particular with my tree where I try to allocate modules in multipages to } get faster performance of the binary image at runtime even if they will } use more ram), so unless he managed running ksymoops on the "after-oops" } semi broken kernel, the oops will be totally useless.

It's hard in a cross-build environment to get ksymoops to be useful, though. If you have suggestions, I have the will.

} Printing the start of each affected module into the oops will solve the } problem and it will make possible for us to debug oopses that involves } modules. Then to automate it we ""only"" need to teach ksymoops to } parse those module-start informations.

It's really weird that the patch works in a generic manner for you, maybe our userspace side is different than, or maybe it's a difference on ppc where you're not trimming the symbol list to the exported functions?

See:

andrea@dualathlon:~> ksyms -a | grep bttv

cc8e8400  bttv_get_cardinfo_Rsmp_48320158   [bttv]
cc8e8440  bttv_get_id_Rsmp_294b5550         [bttv]
cc8e8480  bttv_gpio_enable_Rsmp_11dc4b6d    [bttv]
cc8e8500  bttv_read_gpio_Rsmp_bcf2d2fb      [bttv]
cc8e8550  bttv_write_gpio_Rsmp_8ecf4acc     [bttv]
cc8e85d0  bttv_get_gpio_queue_Rsmp_a6788968  [bttv]
cc8e0000 __insmod_bttv_O/lib/modules/2.4.19-rc3aa2/kernel/drivers/media/video/bttv.o_M3D3ED28E_V132115 [bttv]
cc8e0080  __insmod_bttv_S.text_L35810       [bttv]
cc8e8d0c  __insmod_bttv_S.rodata_L340       [bttv]
cc8eb5a0  __insmod_bttv_S.data_L14172       [bttv]
cc8eed00  __insmod_bttv_S.bss_L5920         [bttv]

andrea@dualathlon:~/devel/kernel/2.4.19rc3aa2> egrep 'bttv_get_cardinfo|bttv_get_id|bttv_gpio_enable|bttv_read_gpio|bttv_write_gpio|bttv_get_gpio_queue' drivers/media/video/* drivers/media/video/bttv-if.c:EXPORT_SYMBOL(bttv_get_cardinfo);
drivers/media/video/bttv-if.c:EXPORT_SYMBOL(bttv_get_id);
drivers/media/video/bttv-if.c:EXPORT_SYMBOL(bttv_gpio_enable);
drivers/media/video/bttv-if.c:EXPORT_SYMBOL(bttv_read_gpio);
drivers/media/video/bttv-if.c:EXPORT_SYMBOL(bttv_write_gpio);
drivers/media/video/bttv-if.c:EXPORT_SYMBOL(bttv_get_gpio_queue);
drivers/media/video/bttv-if.c:int bttv_get_cardinfo(unsigned int card, int *type, int *cardid)
drivers/media/video/bttv-if.c:int bttv_get_id(unsigned int card)
drivers/media/video/bttv-if.c: printk("bttv_get_id is obsolete, use bttv_get_cardinfo instead\n");
drivers/media/video/bttv-if.c:int bttv_gpio_enable(unsigned int card, unsigned long mask, unsigned long data)
drivers/media/video/bttv-if.c:int bttv_read_gpio(unsigned int card, unsigned long *data)
drivers/media/video/bttv-if.c:int bttv_write_gpio(unsigned int card, unsigned long mask, unsigned long data) drivers/media/video/bttv-if.c:wait_queue_head_t* bttv_get_gpio_queue(unsigned int card)
drivers/media/video/bttv.h:extern int bttv_get_cardinfo(unsigned int card, int *type, int *cardid);
drivers/media/video/bttv.h:/* obsolete, use bttv_get_cardinfo instead */
drivers/media/video/bttv.h:extern int bttv_get_id(unsigned int card);
drivers/media/video/bttv.h:extern int bttv_gpio_enable(unsigned int card,
drivers/media/video/bttv.h:extern int bttv_read_gpio(unsigned int card, unsigned long *data);
drivers/media/video/bttv.h:extern int bttv_write_gpio(unsigned int card,
drivers/media/video/bttv.h: (wake_up_interruptible) and following call to the function bttv_read_gpio
drivers/media/video/bttv.h:extern wait_queue_head_t* bttv_get_gpio_queue(unsigned int card);
andrea@dualathlon:~/devel/kernel/2.4.19rc3aa2> egrep 'EXPORT_SYMBOL.*bttv' drivers/media/video/* drivers/media/video/bttv-if.c:EXPORT_SYMBOL(bttv_get_cardinfo);
drivers/media/video/bttv-if.c:EXPORT_SYMBOL(bttv_get_id);
drivers/media/video/bttv-if.c:EXPORT_SYMBOL(bttv_gpio_enable);
drivers/media/video/bttv-if.c:EXPORT_SYMBOL(bttv_read_gpio);
drivers/media/video/bttv-if.c:EXPORT_SYMBOL(bttv_write_gpio);
drivers/media/video/bttv-if.c:EXPORT_SYMBOL(bttv_get_gpio_queue);
andrea@dualathlon:~/devel/kernel/2.4.19rc3aa2> egrep 'bttv_bit_setsda' drivers/media/video/*
drivers/media/video/bttv-if.c:void bttv_bit_setsda(void *data, int state)

drivers/media/video/bttv-if.c:  setsda:  bttv_bit_setsda,
drivers/media/video/bttv-if.c:  bttv_bit_setsda(btv,1);

drivers/media/video/bttv.h:extern void bttv_bit_setsda(void *data, int state); andrea@dualathlon:~/devel/kernel/2.4.19rc3aa2> egrep 'bttv_bit_setsda' /proc/ksyms andrea@dualathlon:~/devel/kernel/2.4.19rc3aa2>

I guess it worked for you because you've single file .c modules, that interface with each other, so there every extern function is going to be exported.

modutils could build the whole symbol table if asked to, so with a change of insmod you could make your patch to work in a generic manner for all module symbols too, but it would be a waste of ram like kksymoops.

:) In that case kksymoops is probably the best. OTOH for shipped binary images this problem doesn't exist assuming we can regognize the image from the oops (hence the idea to show uname -a into the oops).

That's more a ksymoops problem then, the symbol resolution is really trivial to implement in a generic manner.

Andrea

I wrote the critter on my x86 laptop and dualathlon desktop. I only did additional testing on PPC. Both give me bunches of symbols.

Stock redhat 7.2 gives me hundreds of module symbols when I load up a bunch of modules (via ksyms -a or cat /proc/ksyms). A cross-build of stock modutils-2.4.16 for x86->ppc gives me the same.

} It's really weird that the patch works in a generic manner for you, maybe our } userspace side is different than, or maybe it's a difference on ppc where } you're not trimming the symbol list to the exported functions?

I'm testing with multi .c modules. I haven't tested with a single .c module though.

} I guess it worked for you because you've single file .c modules, that interface } with each other, so there every extern function is going to be exported. }
} modutils could build the whole symbol table if asked to, so with a change of } insmod you could make your patch to work in a generic manner for all } module symbols too, but it would be a waste of ram like kksymoops.

I implemented what I need to track down oopses with modules. ksymoops should learn about it too. This will also allow us to recognize immediatly the kernel image used.

here an example of oops in a module with the patch applied (only 1 module is affected so only 1 module is listed). I checked that 0xca40306e-0xca403060 gives the exact offset to lookup in the objdump -d of the module object.

this patch will solve all the issues in being able to track down module oopses and kernel image without introducing any waste of ram (nitpick:
except 40 bytes of ram). For user compiled kernels, if the user isn't capable of saving System.map and vmlinux kksymoops remains a viable alternative, but I don't feel it needed for pre-compiled kernel images provided a compile-time database exists (also considering kksymoops doesn't obviate the need of the vmlinux, really one can dump the asm off the bzImage or even more simply from /dev/ram but saving vmlinux is simpler than to ask the user to do that).

last thing: this will work well till 320 modules simultaneously loaded on 32bit archs, or 640 for 64bit archs. More modules won't hurt but they may not showup in the trace, elarging the limit is trivial, it will only waste some more byte. I also considered another implementation that basically used an array of "module idx" rather than the bitmap of all the modules. That had the advantage of requiring ram only in function of the modules present in the trace, but I didn't like to make too much assumption on the number of modules in the trace, and with 1 module in the trace I can track 32 real modules with the below implementation. So there was not an huge saving, possibly using char instead of ints would been viable but the below looks simpler too.

Unable to handle kernel NULL pointer dereference at virtual address 00000000
 printing eip:
ca40306e
*pde = 00000000
Oops: 0002
Linux version 2.4.19-rc3aa2 (andrea@dualathlon) (gcc version 3.1.1 20020618 (prerelease)) #17 SMP Fri Jul 26 23:25:11 CEST 2002
CPU:    1
EIP:    0010:[<ca40306e>]    Tainted: P
EFLAGS: 00010246
eax: ca403060   ebx: ffffffea   ecx: c0493d10   edx: c11ec080
esi: 00000000   edi: 00000000   ebp: ca403000   esp: ca433f10
ds: 0018   es: 0018   ss: 0018
Process insmod (pid: 1134, stackpage=ca433000)
Stack: 00000000 00000000 ffffffea c0120817 ca403060 08086ab0 000000e0 00000000
       08086b15 000000c5 00000060 00000060 00000004 c4359440 ca402000 ca438000
       00000060 c0492600 ca403060 00000140 00000000 00000000 00000000 00000000
Call Trace:    [<c0120817>] [<ca403060>] [<ca403060>] [<c01090e7>]
Module Oops Tracking:
        [(oops:<ca403060>:<ca403140>)]
Code: c7 05 00 00 00 00 00 00 00 00 83 0d 14 30 40 ca 08 e8 4c 7e

And here it is the patch:

diff -urNp ref/arch/i386/kernel/traps.c oops-x/arch/i386/kernel/traps.c
@@ -103,16 +103,18 @@ static inline int kernel_text_address(un
 {
 	int retval = 0;
 	struct module *mod;
+	int nr;
 
 	if (addr >= (unsigned long) &_stext &&
 	    addr <= (unsigned long) &_etext)
 		return 1;
 
-	for (mod = module_list; mod != &kernel_module; mod = mod->next) {
+	for (nr = 0, mod = module_list; mod != &kernel_module; nr++, 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(nr);
 			retval = 1;
 			break;
 		}

@@ -201,6 +203,8 @@ void show_registers(struct pt_regs *regs
 	unsigned long esp;
 	unsigned short ss;
 
+	printk(linux_banner);
+
 	esp = (unsigned long) (&regs->esp);
 	ss = __KERNEL_DS;
 	if (regs->xcs & 3) {

@@ -208,6 +212,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 +232,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;
 

diff -urNp ref/include/linux/kernel.h oops-x/include/linux/kernel.h
@@ -109,6 +109,8 @@ extern const char *print_tainted(void);
 
 extern void dump_stack(void);
 
+extern char *linux_banner;
+
 #if DEBUG
 #define pr_debug(fmt,arg...) \
 	printk(KERN_DEBUG fmt,##arg)

diff -urNp ref/include/linux/module.h oops-x/include/linux/module.h
@@ -411,4 +411,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(int);
+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 ref/init/main.c oops-x/init/main.c
@@ -81,7 +81,6 @@ extern int irda_device_init(void);
 #endif
 
 extern char _stext, _etext;
-extern char *linux_banner;
 
 static int init(void *);
 

Andrea

I don't see how this is significantly different from the patch I sent apart from removing some useful debugging. Is there something I'm missing?

The patch I sent prints out the offset from the symbol name nearest (below) the EIP and the offset from that symbol. I found that really useful and was what I use the patch for. Is there a reason you don't want that? I know you had some cases where all symbols weren't listed for you but many of us do have those symbols we'd like to see.

that's why, because of those cases. I thought you had those cases too right? And for ksymoops it's much simpler to avoid the reverse lookup.

I think either you go kksymoops, or you go my way + module tracking aware ksymoops (only one "k"), the intermediate approch is probably a nice way but handy only until a module tracking aware ksymoops exists.

Furhtmore your patch doesn't really provide all the info need, you only take care of the module where the EIP lives, you don't handle the case of multiple module addresses in the stack trace. And printing lots of symbols for the stack trace too (potentially to reverse all the time too) can overflow the screen. Infact I also considered to print modules tracking info without \n in between even if it can overflow to save screen ram space. Maybe that's worhwhile, OTOH I wouldn't expect more than a few lines usually and the \n makes it more readable but I may change it to use better the screen space.

Andrea

I haven't found those cases, nope. Even with them, it is nice to resolve to the nearest symbol when we can. Printing the beginning and end of the module that you put in there is useful, though.

Any names for the stack trace are a mess, since formatting that is hard without scrolling off the screen and making the oops useless. My patch was just a first cut to get a symbol name and module name for the EIP. I think that's a good start and it seems your version actually removes some of the very simple functionality that I wanted to be able to use.

Do I follow what you're doing correctly or do I have it confused?

} that's why, because of those cases. I thought you had those cases too } right? And for ksymoops it's much simpler to avoid the reverse lookup. }
} I think either you go kksymoops, or you go my way + module tracking } aware ksymoops (only one "k"), the intermediate approch is probably a } nice way but handy only until a module tracking aware ksymoops exists. }
} Furhtmore your patch doesn't really provide all the info need, you } only take care of the module where the EIP lives, you don't handle the } case of multiple module addresses in the stack trace. And printing lots } of symbols for the stack trace too (potentially to reverse all the time } too) can overflow the screen. Infact I also considered to print modules } tracking info without \n in between even if it can overflow to save } screen ram space. Maybe that's worhwhile, OTOH I wouldn't expect more } than a few lines usually and the \n makes it more readable but I may } change it to use better the screen space.

You follow it. The reason I'm dropping functionality is that I want the functionality in userspace, not because your patch in-kernel was wasting any resources, but because from userspace the functionality will do the *right* thing always without me having to check if the symbol happened to be right because it was exported (and still no idea why you've more symbols than me in ksyms for modules). Also rarely I can get away with just the EIP. So in short the symbol isn't going to help because I'd need to recheck with ksymoops anyways and I need the stack trace too potentially part of other modules, so I prefer to provide via the oops only the numeric information, but *all* of it :) to also reduce the oops size. If your main object is to skip the ksymoops step, then I think even in such case you want to go to a full complete kksymoops instead of the halfway approch. I think ksymoops + module oops tracking patch is more than enough for the bugreports (modulo dprobes/lkcd etc...). For developement using a true debugger to get stack traces and inspect ram (i.e. uml or kgdb or kdb ) is probably superior to any other oops functionalty anyways (so if your object is to avoid the ksymoops step during developement I would suggest a real debugger that will save even more time).

I like the strict module oops tracking in particular for pre-compiled kernels where we don't even need to ask the user the system.map/vmlinux in order to debug it, and so where it would be a total loss of global ram resources to load in kernel ram of every machine all the symbols of the whole kernel and modules. Furthmore this adds a critical needed feature to have a chance to debug module oopses, so it's a must-have.

Nevertheless adding your ksym lookup for the EIP wouldn't hurt and it wouldn't waste lines (columns doesn't matter near the EIP), so we could merge the two things together, but as said above I don't feel the need of it as far as ksymoops learns about resolving the eip through the module information now included in the oops.

I updated the patch to reduce the number of lines of the oops, this is incremental with the previous patch.

@@ -1271,26 +1271,29 @@ static void __module_oops_tracking_print
 
 	for (mod = module_list; mod != &kernel_module; mod = mod->next) {
 		if (!nr--)
-			printk("	[(%s:<%p>:<%p>)]\n",
+			printk(" [(%s:<%p>:<%p>)]",
 			       mod->name,
 			       (char *) mod + mod->size_of_struct,
 			       (char *) mod + mod->size);
 	}
-	
 }
 
 void module_oops_tracking_print(void)
 {
 	int nr;
+	int i = 0;
 
-	printk("Module Oops Tracking:\n");
+	printk("Modules:");
 	nr = find_first_bit(module_oops_tracking, MODULE_OOPS_TRACKING_NR_BITS);
 	for (;;) {
 		if (nr == MODULE_OOPS_TRACKING_NR_BITS)
-			return;
+			break;
+		if (i && !(i++ % 2))
+			printk("\n        ");
 		__module_oops_tracking_print(nr);
 		nr = find_next_bit(module_oops_tracking, MODULE_OOPS_TRACKING_NR_BITS, nr+1);
 	}
+	printk("\n");
 }
 
 #else		/* CONFIG_MODULES */

Andrea

All I need is the nearest symbol name and that's it. The patch was pretty simple so it could be easily merged. It added only a few 100's of bytes on x86 and below 1k on PPC. It would be handy to have this in a kernel rather than require some user utility that requires I make it work in a cross-build environment, copy the 'insmod -m' map output to my host machine, run the user program on the system.map and the user utilities for every boot, crash, debug cycle.

The patch is tiny and simple and I hoped to keep it that way. If I want to dig into the system and seriously debug rather than follow a known problem I can use xmon on PPC, kgdb or more often than not a real JTAG debugger.

How about this - we print the module beginning _and_ the EIP symbol if we can find one. You get what you want and I get what I want (I won't even add a line of output to get what I want).

To solve the ksyms mystery can you send me what versions of modutils, what architecture, what distribution and anything else you think might be useful? I'd like to know what cases this fails in.

} You follow it. The reason I'm dropping functionality is that I want the } functionality in userspace, not because your patch in-kernel was wasting } any resources, but because from userspace the functionality will do the } *right* thing always without me having to check if the symbol happened } to be right because it was exported (and still no idea why you've more } symbols than me in ksyms for modules). Also rarely I can get away with } just the EIP. So in short the symbol isn't going to help because I'd } need to recheck with ksymoops anyways and I need the stack trace too } potentially part of other modules, so I prefer to provide via the oops } only the numeric information, but *all* of it :) to also reduce the oops } size. If your main object is to skip the ksymoops step, then I think } even in such case you want to go to a full complete kksymoops instead of } the halfway approch. I think ksymoops + module oops tracking patch is } more than enough for the bugreports (modulo dprobes/lkcd etc...). For } developement using a true debugger to get stack traces and inspect ram } (i.e. uml or kgdb or kdb ) is probably superior to any other oops } functionalty anyways (so if your object is to avoid the ksymoops step } during developement I would suggest a real debugger that will save even } more time).
}
} I like the strict module oops tracking in particular for pre-compiled } kernels where we don't even need to ask the user the system.map/vmlinux } in order to debug it, and so where it would be a total loss of global } ram resources to load in kernel ram of every machine all the symbols of } the whole kernel and modules. Furthmore this adds a critical needed } feature to have a chance to debug module oopses, so it's a must-have. }
} Nevertheless adding your ksym lookup for the EIP wouldn't hurt and it } wouldn't waste lines (columns doesn't matter near the EIP), so we could } merge the two things together, but as said above I don't feel the need } of it as far as ksymoops learns about resolving the eip through the } module information now included in the oops.
}
} I updated the patch to reduce the number of lines of the oops, this is } incremental with the previous patch.
}
} --- 2.4.19rc3aa2/kernel/module.c.~1~ Sat Jul 27 00:48:38 2002
} +++ 2.4.19rc3aa2/kernel/module.c Sat Jul 27 03:44:16 2002
} @@ -1271,26 +1271,29 @@ static void __module_oops_tracking_print }
} for (mod = module_list; mod != &kernel_module; mod = mod->next) {
} if (!nr--)
} - printk(" [(%s:<%p>:<%p>)]\n",
} + printk(" [(%s:<%p>:<%p>)]",

}  			       mod->name,
}  			       (char *) mod + mod->size_of_struct,
}  			       (char *) mod + mod->size);
}  	}
} -	
}  }
}  
}  void module_oops_tracking_print(void)
}  {
}  	int nr;

} + int i = 0;
}
} - printk("Module Oops Tracking:\n");
} + printk("Modules:");

}  	nr = find_first_bit(module_oops_tracking, MODULE_OOPS_TRACKING_NR_BITS);
}  	for (;;) {
}  		if (nr == MODULE_OOPS_TRACKING_NR_BITS)

} - return;
} + break;
} + if (i && !(i++ % 2))
} + printk("\n ");

}  		__module_oops_tracking_print(nr);
}  		nr = find_next_bit(module_oops_tracking, MODULE_OOPS_TRACKING_NR_BITS, nr+1);
}  	}

} + printk("\n");
} }
}
} #else /* CONFIG_MODULES */
}
}
} Andrea

ok, the prepare stage will be a bit more complicated but it seems a worthwhile change, thanks.

well I assume it's not going to happen in 2.4 anyways and the module information is really just necessary (we are lacking this needed information for years now). Once the text won't be contigous anymore to free init sections for modules too the same needed info on the presistent text could be provided in a different manner. If you need anything in the oops now to make life easier to ksymoops later just let me know.

Andrea

here an updated patch that moves the bitflag from the dedicated array to the mod->flags per your suggesiton.

The output now looks like this (I also compressed the oops_id info so that it doesn't overflow 80 cols, given I saved 1 suprios return this overall only adds an overhead of 1 line to the oops when there are modules and zero overhead of no module is involved). The new provided information is invaluable in may situations and it can be easily parsed by ksymoops in a reliable manner.

Unable to handle kernel NULL pointer dereference at virtual address 00000000
 printing eip:
c39d906e
*pde = 00000000
Oops: 0002
2.4.19-rc3aa2 #3 SMP Sat Jul 27 05:06:19 CEST 2002
CPU:    0
EIP:    0010:[<c39d906e>]    Tainted: P 
EFLAGS: 00010246
eax: c39d9060   ebx: ffffffea   ecx: c034b350   edx: c10ad8a0
esi: 00000000   edi: 00000000   ebp: c39d9000   esp: c39dbf10
ds: 0018   es: 0018   ss: 0018
Process insmod (pid: 929, stackpage=c39db000)
Stack: 00000000 00000000 ffffffea c011f917 c39d9060 080862d0 000000e0 00000000 
       08086335 000000c5 00000060 00000060 00000004 c22caa40 c39d8000 c39dc000 
       00000060 c2260000 c39d9060 00000140 00000000 00000000 00000000 00000000 
Call Trace:    [<c011f917>] [<c39d9060>] [<c39d9060>] [<c01090e7>]
Modules: [(oops:<c39d9060>:<c39d9140>)]
Code: c7 05 00 00 00 00 00 00 00 00 83 0d 14 90 9d c3 08 e8 4c 1e 

diff -urNp z/arch/i386/kernel/traps.c 2.4.19rc3aa2/arch/i386/kernel/traps.c
@@ -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;
 		}

@@ -201,6 +202,8 @@ void show_registers(struct pt_regs *regs
 	unsigned long esp;
 	unsigned short ss;
 
+	printk(oops_id);
+
 	esp = (unsigned long) (&regs->esp);
 	ss = __KERNEL_DS;
 	if (regs->xcs & 3) {

@@ -208,6 +211,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 +231,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;
 

diff -urNp z/include/linux/kernel.h 2.4.19rc3aa2/include/linux/kernel.h
@@ -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
@@ -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		128
 
 /* 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
@@ -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";

Andrea

Kernel version without any identifying string cannot be picked up by ksymoops. Can you append the kernel version to the die message? That means changing all arch/*/traps.c but it saves a line on the console.

Oops: 0002 2.4.19-rc3aa2 #3 SMP Sat Jul 27 05:06:19 CEST 2002

		mod->flags &= ~MOD_OOPS_PRINT;

ok, it saves one more line (will be in next -aa too, for rc3aa2 it's too late 8).

diff -urNp z/arch/i386/kernel/traps.c 2.4.19rc3aa2/arch/i386/kernel/traps.c
@@ -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
@@ -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
@@ -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		128
 
 /* 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
@@ -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";

Andrea

I am still not convinced that this isn't the better approach overall; yes, the full ksymall table takes a few kb in memory, which can be avoided if one has a full vmlinux / System.map archive (as it would in theory be possible for a distribution for all shipped kernels), but having the fully decoded Oops - or at least, as far as possible - would certainly be more useful in the general case, and for self-compiled kernels.

Sincerely,
Lars Marowsky-Brée <[email protected]>

Immortality is an adequate definition of high availability for me.
	--- Gregory F. Pfister

depends how deep you need to go into the oops. I would say I find myself going into the asm of the vmlinux 9 times out of 10 (basically every time I cannot guess the problem by just looking at the oops without doing any real debugging). so yes kksymoops may help once in a while, but all other times I would probably be faster asking gdb to disasm a certain fixed hex address (that will give me the symbol too of course :).

So I don't feel the need of kksymoops, at least given I can find out what kernel the user was running, that will be possible by printing the uname -a information in the oops too. So with a "featured oops" with uname -a + module start info + database we'll have all we need to decode whatever oops usually without significant slowdown compared to kksymoops, but nevertheless if we're ok to waste some hundred kbyte of ram (and mbytes of space on disk for the numerous modules) also kksymoops will be fine. But nevertheless kksymoops should be a config option, so when disabled my "featured oops" wish still apply :)

Andrea

If you're willing to teach all our users how to use ksymoops ... ;)

Rik

Bravely reimplemented by the knights who say "NIH".

http://www.surriel.com/		http://distro.conectiva.com/

I don't even need to ask them the system.map if I compiled the kernel myself.

Andrea

Andrea is comeing from the background that he has an archive with all binary images and all vmlinux ever shipped available. So if he gets such an Oops with the start address of all loaded modules and a unique id of which kernel was used, a specially modified ksymoops can reliably decode the Oops.

This is a desireable feature in any case and just should be done.

However, it doesn't help people with self-compiled kernels; while some support contracts have a notice attached that such kernels invalidate all contracts, certifications and maintenance ;), we all know that such kernels are fairly common place; and the support staff would need to keep track of every single kernel image ever approved for customer use.

This is where kksymoops and Cort's patch would come in very handy and I do strongly believe that they should be included in the default kernel, or at least in the vendor ones, and their should be a strong suggestion to enable this feature for all self-compiled kernels.

It is good to see that such patches are at least being considered for inclusion now; the mainstream-kernel has an amazing lack of debugging features ;-) (Yes, I recall the discussions and the arguments about this topic since 1.0.x)

Sincerely,
Lars Marowsky-Brée <[email protected]>

Immortality is an adequate definition of high availability for me.
	--- Gregory F. Pfister

A story:

I'm not an experienced kernel hacker -- most of my questions probably belong on the kernelnewbies list. I am also, however, not afraid to muck around, which is why I run Slackware and a completely self-configured kernel. It just so happens that I run Linux on my laptop, and thus my kernel is reiserfs-only (I didn't even build ext2 as a module, and I'm not going to count iso9660fs).

About four months ago, I decided to try my very first 2.5 kernel ever, 2.5.7. [As many of you know -- just as I do now -- Al Viro did some cleanups which caused an oops when your rootfs was reiserfs.] I downloaded, I configured, I built, and I lilo'ed. And when I booted 2.5.7 for the first time, I got my very first kernel oops -- woo!

First thing I noted was that, since my rootfs didn't even get mounted at all, let alone read-write, I had no logfiles to work with. So I painstakingly copied the large pile of hex values down on paper and booted back to 2.5.7. Lucky for me, the oops happened at boot time, so there were no modules to be mucked with :) Having been on lkml for several months thus far, I knew exactly what to do -- run ksymoops. So I went to /usr/src/linux-2.5.7/scripts/ksymoops/, and the only file there was a README telling me that ksymoops was no longer distributed as part of the kernel, and that I needed to download it.

Great.

I downloaded ksymoops, and it wouldn't compile. I got some bizarre errors about undefined symbols (relating to libbfd), and googling for those errors gave people having those same problems, yet no responses.
After much grief I somehow finally determined that I needed new copies of libiberty and libbfd installed.
I downloaded and installed the new versions of libiberty and libbfd and was finally able to build ksymoops.

Once ksymoops was built, I carefully reproduced the oops I had transcribed to paper into a file and ran
ksymoops against it. At long last, I was rewarded with my decoded oops output, informing me that the crash had occurred in reiserfs code. I posted an e-mail to the list and was immediatly replied to that this was a known problem and that a patch was already available :P

The Moral Of The Story

It took me several hours to get my oops decoded. Most people don't have several hours to waste tracking down subtle bugs in userspace libraries and programs. Requiring users to run ksymoops, if a working alternative (such as this patch) exists, is NOT a good idea.

--
Stevie-O

Real programmers use COPY CON PROGRAM.EXE

It's an absolute must-have feature. Too many users send in undecoded or wrongly decoded oopses and we end up spending too much time trying to teach users how to decode oopses.

Add to that the fact that symbols from modules aren't always looked up allright.

Having the kernel print out the function symbol and offset into the function would save us a lot of time and trouble in tracking down problems.

regards,

Rik

Bravely reimplemented by the knights who say "NIH".

http://www.surriel.com/		http://distro.conectiva.com/

Cool indeed! I wan't it too. Even if it doesn't give me the precise information about where the stuff hung, it's allmost always sufficient for me to see where it crashed around... And far more convenient then RS232 console ore whatever in esp. since it's sufficent in 90% of the cases I tend to trigger during "experiments".

I would much rather have hex data. It makes all the parsing tools connected to the serial port that much easier. If instead of hacking the kernel you bang out a little bit of expect you can do it all on the host driving the embedded box, and find the file names, and open them in an editor at the right function, and do a parallel lookup in bugzilla for matching oops logs...

You don't lose the hex data. The patch just prints data that the kernel already has. For some situations this patch does match the needs of developers better.

} I would much rather have hex data. It makes all the parsing tools } connected to the serial port that much easier. If instead of hacking the } kernel you bang out a little bit of expect you can do it all on the host } driving the embedded box, and find the file names, and open them in an } editor at the right function, and do a parallel lookup in bugzilla for } matching oops logs...

I recently had an oops on my laptop with a rh kernel with I think this stuff in (it decoded the trace to symbols, etc).

The only problem was that the machine paniced at the end, with the register dump off the top of the screen. The only thing visible was the trace.

As useful as it was to have the trace, the oops was useless because the rest of the information was missing.

So why didn't I scroll up? Because shift-pgup needs a task queue to run, which doesn't happen after a panic, just like you can't ctrl-alt-del to reboot (but you can alt-sysrq-b)...

Personally, I'd much rather have the standard oops output and be able to scribble down the numbers, and then look them up than to have some snazzy bit of code in the kernel do it for me and scroll the useful information off the screen.

(And thanks for reminding me about the oops...)

Russell King ([email protected])                The developer of ARM Linux
             http://www.arm.linux.org.uk/personal/aboutme.html

I've had the stack backtrace cause cascading oops' that scroll until I power a machine off. It's a crash - things aren't always going to work out. Arguments like that can be made to remove nearly the entire oops message.

The patch as it is now doesn't even add a line of text to the x86 oops message. It does add a line to the PPC oops message though. I understand, and agree with, the need for brevity in a oops.

Would you be happy with the lookup function as a config option rather than the default?

} I recently had an oops on my laptop with a rh kernel with I think this } stuff in (it decoded the trace to symbols, etc).
}
} The only problem was that the machine paniced at the end, with the } register dump off the top of the screen. The only thing visible was } the trace.
}
} As useful as it was to have the trace, the oops was useless because } the rest of the information was missing.
}
} So why didn't I scroll up? Because shift-pgup needs a task queue to } run, which doesn't happen after a panic, just like you can't ctrl-alt-del } to reboot (but you can alt-sysrq-b)...
}
} Personally, I'd much rather have the standard oops output and be able } to scribble down the numbers, and then look them up than to have some } snazzy bit of code in the kernel do it for me and scroll the useful } information off the screen.
}
} (And thanks for reminding me about the oops...)
}
} --
} Russell King ([email protected]) The developer of ARM Linux
} http://www.arm.linux.org.uk/personal/aboutme.html

I'd prefer having both. The hex data is useful for developers, but seems pretty much unusable for normal users and sysadmins.

And we _do_ want those people to be able to give us useful bug reports...

regards,

Rik

Bravely reimplemented by the knights who say "NIH".

http://www.surriel.com/		http://distro.conectiva.com/