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
--- a/arch/i386/kernel/traps.c Thu Jul 25 01:11:24 2002
+++ b/arch/i386/kernel/traps.c Thu Jul 25 01:11:24 2002
@@ -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
--- a/arch/i386/mm/fault.c Thu Jul 25 01:11:24 2002
+++ b/arch/i386/mm/fault.c Thu Jul 25 01:11:24 2002
@@ -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
--- a/arch/ppc/kernel/process.c Thu Jul 25 01:11:24 2002
+++ b/arch/ppc/kernel/process.c Thu Jul 25 01:11:24 2002
@@ -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
--- a/include/linux/module.h Thu Jul 25 01:11:24 2002
+++ b/include/linux/module.h Thu Jul 25 01:11:24 2002
@@ -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 */
diff -Nru a/kernel/module.c b/kernel/module.c
--- a/kernel/module.c Thu Jul 25 01:11:24 2002
+++ b/kernel/module.c Thu Jul 25 01:11:24 2002
@@ -238,6 +238,63 @@
struct module *find_module(const char *name);
void free_module(struct module *, int tag_freed);
+/*
+ * Lookup an address in all modules (including the kernel) ksyms
+ * and print any corresponding symbol name. Useful for quick
+ * recognition of faulting addresses during panics without
+ * the need for user space tools.
+ * -- Cort <[email protected]>
+ */
+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 */
+ struct module *mod;
+ struct module_symbol *sym;
+ int j;
+
+ /* user addresses - just print user and return -- Cort */
+ if ( addr < PAGE_OFFSET )
+ {
+ if ( s1 )
+ printk("%s", s1);
+ printk("(user)");
+ if ( s2 )
+ printk("%s", s2);
+ return;
+ }
+
+ for (mod = module_list; mod; mod = mod->next)
+ {
+ 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)) )
+ {
+ 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);
+ else
+ sprintf(best_match_string, "%s:%s",
+ sym->name, mod->name);
+ }
+ }
+ }
+
+ if ( best_match )
+ {
+ if ( s1 )
+ printk("%s", s1);
+ printk("(%s + 0x%lx)", best_match_string, addr - best_match);
+ if ( s2 )
+ printk("%s", s2);
+ }
+}
/*
* Called at boot time
On Thu, Jul 25, 2002 at 11:00:33AM -0600, Cort Dougan wrote:
> 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.
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
--- a/arch/i386/kernel/traps.c Thu Jul 25 11:19:18 2002
+++ b/arch/i386/kernel/traps.c Thu Jul 25 11:19:18 2002
@@ -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
--- a/arch/i386/mm/fault.c Thu Jul 25 11:19:18 2002
+++ b/arch/i386/mm/fault.c Thu Jul 25 11:19:18 2002
@@ -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
--- a/arch/ppc/kernel/process.c Thu Jul 25 11:19:18 2002
+++ b/arch/ppc/kernel/process.c Thu Jul 25 11:19:18 2002
@@ -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
--- a/include/linux/module.h Thu Jul 25 11:19:18 2002
+++ b/include/linux/module.h Thu Jul 25 11:19:18 2002
@@ -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 */
diff -Nru a/kernel/module.c b/kernel/module.c
--- a/kernel/module.c Thu Jul 25 11:19:18 2002
+++ b/kernel/module.c Thu Jul 25 11:19:18 2002
@@ -238,6 +238,63 @@
struct module *find_module(const char *name);
void free_module(struct module *, int tag_freed);
+/*
+ * Lookup an address in all modules (including the kernel) ksyms
+ * and print any corresponding symbol name. Useful for quick
+ * recognition of faulting addresses during panics without
+ * the need for user space tools.
+ * -- Cort <[email protected]>
+ */
+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 */
+ struct module *mod;
+ struct module_symbol *sym;
+ int j;
+
+ /* user addresses - just print user and return -- Cort */
+ if ( addr < PAGE_OFFSET )
+ {
+ if ( s1 )
+ printk("%s", s1);
+ printk("(user)");
+ if ( s2 )
+ printk("%s", s2);
+ return;
+ }
+
+ for (mod = module_list; mod; mod = mod->next)
+ {
+ 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)) )
+ {
+ 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);
+ else
+ sprintf(best_match_string, "%s:%s",
+ sym->name, mod->name);
+ }
+ }
+ }
+
+ if ( best_match )
+ {
+ if ( s1 )
+ printk("%s", s1);
+ printk("(%s + 0x%lx)", best_match_string, addr - best_match);
+ if ( s2 )
+ printk("%s", s2);
+ }
+}
/*
* Called at boot time
On Thu, Jul 25, 2002 at 11:21:42AM -0600, Cort Dougan wrote:
> 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.
> + if ( addr < PAGE_OFFSET )
> + {
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.
-ben
On Thu, Jul 25, 2002 at 11:21:42AM -0600, Cort Dougan wrote:
> 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.
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
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.
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/
On Thu, Jul 25, 2002 at 02:27:16PM -0600, Cort Dougan wrote:
> 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.
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
--- a/arch/i386/kernel/traps.c Thu Jul 25 15:02:22 2002
+++ b/arch/i386/kernel/traps.c Thu Jul 25 15:02:22 2002
@@ -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
--- a/arch/i386/mm/fault.c Thu Jul 25 15:02:22 2002
+++ b/arch/i386/mm/fault.c Thu Jul 25 15:02:22 2002
@@ -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
--- a/arch/ppc/kernel/process.c Thu Jul 25 15:02:22 2002
+++ b/arch/ppc/kernel/process.c Thu Jul 25 15:02:22 2002
@@ -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
--- a/include/linux/module.h Thu Jul 25 15:02:22 2002
+++ b/include/linux/module.h Thu Jul 25 15:02:22 2002
@@ -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 */
diff -Nru a/kernel/module.c b/kernel/module.c
--- a/kernel/module.c Thu Jul 25 15:02:22 2002
+++ b/kernel/module.c Thu Jul 25 15:02:22 2002
@@ -238,6 +238,63 @@
struct module *find_module(const char *name);
void free_module(struct module *, int tag_freed);
+/*
+ * Lookup an address in all modules (including the kernel) ksyms
+ * and print any corresponding symbol name. Useful for quick
+ * recognition of faulting addresses during panics without
+ * 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[PRINT_ADDR_LENGTH];
+ struct module *mod;
+ struct module_symbol *sym;
+ int j;
+
+ /* user addresses - just print user and return -- Cort */
+ if ( addr < PAGE_OFFSET ) {
+ if ( s1 )
+ printk("%s", s1);
+ printk("(user)");
+ if ( s2 )
+ printk("%s", s2);
+ return;
+ }
+
+ 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) {
+ best_match = sym->value;
+ /* kernelmodule.name is "" so we
+ * have a special case -- Cort */
+ if ( mod->name[0] == 0 )
+ snprintf(best_match_string,
+ PRINT_ADDR_LENGTH, "%s",
+ sym->name);
+ else
+ snprintf(best_match_string,
+ PRINT_ADDR_LENGTH, "%s:%s",
+ sym->name, mod->name);
+ }
+ }
+ }
+
+ if ( best_match ) {
+ if ( s1 )
+ printk("%s", s1);
+ printk("(%s + 0x%lx)", best_match_string, addr - best_match);
+ if ( s2 )
+ printk("%s", s2);
+ }
+}
/*
* Called at boot time
On 2002-07-25T22:59:10,
Andrea Arcangeli <[email protected]> said:
> 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.
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
On Thu, 2002-07-25 at 18:00, Cort Dougan wrote:
> 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.
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...
On Thu, Jul 25, 2002 at 03:05:25PM -0600, Cort Dougan wrote:
> 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.
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 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 :)
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! :).
> } 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.
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
On Thu, Jul 25, 2002 at 11:12:25PM +0200, Lars Marowsky-Bree wrote:
> On 2002-07-25T22:59:10,
> Andrea Arcangeli <[email protected]> said:
>
> > 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.
>
> 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.
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
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.
On Thu, Jul 25, 2002 at 03:44:25PM -0600, Cort Dougan wrote:
> 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 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
On Thu, 25 Jul 2002, Cort Dougan wrote:
> 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.
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/
On Thu, 25 Jul 2002, Andrea Arcangeli wrote:
> 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.
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/
On 25 Jul 2002, Alan Cox wrote:
> On Thu, 2002-07-25 at 18:00, Cort Dougan wrote:
> > 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.
>
> I would much rather have hex data. It makes all the parsing tools
> connected to the serial port that much easier.
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/
On Thu, Jul 25, 2002 at 04:05:59PM -0600, Cort Dougan wrote:
> 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
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.
> 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.
:) 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).
> } 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.
That's more a ksymoops problem then, the symbol resolution is really
trivial to implement in a generic manner.
Andrea
On Thu, Jul 25, 2002 at 07:41:39PM -0300, Rik van Riel wrote:
> On Thu, 25 Jul 2002, Andrea Arcangeli wrote:
>
> > 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.
>
> If you're willing to teach all our users how to use ksymoops ... ;)
I don't even need to ask them the system.map if I compiled the kernel
myself.
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.
Christoph Hellwig wrote:
> On Thu, Jul 25, 2002 at 11:00:33AM -0600, Cort Dougan wrote:
>
>>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.
>
>
> Wow! very usefull patch. O want it for 2.4 and 2.5, please.
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".
On 2002-07-25T19:41:39,
Rik van Riel <[email protected]> said:
> > 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.
> If you're willing to teach all our users how to use ksymoops ... ;)
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
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
--- ref/arch/i386/kernel/traps.c Fri Jul 26 23:01:38 2002
+++ oops-x/arch/i386/kernel/traps.c Fri Jul 26 23:06:35 2002
@@ -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) (®s->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
--- ref/include/linux/kernel.h Fri Jul 26 23:01:52 2002
+++ oops-x/include/linux/kernel.h Fri Jul 26 23:18:37 2002
@@ -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
--- ref/include/linux/module.h Wed Jul 24 21:52:27 2002
+++ oops-x/include/linux/module.h Fri Jul 26 23:25:12 2002
@@ -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
--- ref/init/main.c Fri Jul 26 23:01:52 2002
+++ oops-x/init/main.c Fri Jul 26 23:18:34 2002
@@ -81,7 +81,6 @@ extern int irda_device_init(void);
#endif
extern char _stext, _etext;
-extern char *linux_banner;
static int init(void *);
diff -urNp ref/kernel/module.c oops-x/kernel/module.c
--- ref/kernel/module.c Fri Jul 26 23:01:29 2002
+++ oops-x/kernel/module.c Fri Jul 26 22:50:22 2002
@@ -1242,6 +1242,57 @@ 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.
+ */
+#define MODULE_OOPS_TRACKING_NR_LONGS 10
+#define MODULE_OOPS_TRACKING_NR_BITS (BITS_PER_LONG * MODULE_OOPS_TRACKING_NR_LONGS)
+static unsigned long module_oops_tracking[MODULE_OOPS_TRACKING_NR_LONGS];
+
+void module_oops_tracking_init(void)
+{
+ memset(module_oops_tracking, 0, sizeof(module_oops_tracking));
+}
+
+void module_oops_tracking_mark(int nr)
+{
+ if (nr < MODULE_OOPS_TRACKING_NR_BITS)
+ set_bit(nr, module_oops_tracking);
+}
+
+static void __module_oops_tracking_print(int nr)
+{
+ struct module *mod;
+
+ for (mod = module_list; mod != &kernel_module; mod = mod->next) {
+ if (!nr--)
+ printk(" [(%s:<%p>:<%p>)]\n",
+ mod->name,
+ (char *) mod + mod->size_of_struct,
+ (char *) mod + mod->size);
+ }
+
+}
+
+void module_oops_tracking_print(void)
+{
+ int nr;
+
+ printk("Module Oops Tracking:\n");
+ nr = find_first_bit(module_oops_tracking, MODULE_OOPS_TRACKING_NR_BITS);
+ for (;;) {
+ if (nr == MODULE_OOPS_TRACKING_NR_BITS)
+ return;
+ __module_oops_tracking_print(nr);
+ nr = find_next_bit(module_oops_tracking, MODULE_OOPS_TRACKING_NR_BITS, nr+1);
+ }
+}
+
#else /* CONFIG_MODULES */
/* Dummy syscalls for people who don't want modules */
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.
On Fri, Jul 26, 2002 at 04:55:35PM -0600, Cort Dougan wrote:
> 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
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.
> but many of us do have those symbols we'd like to see.
> -
> 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/
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.
On Fri, Jul 26, 2002 at 05:31:27PM -0600, Cort Dougan wrote:
> 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?
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
On Sat, 27 Jul 2002 00:37:50 +0200,
Andrea Arcangeli <[email protected]> wrote:
>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.
>+#define MODULE_OOPS_TRACKING_NR_LONGS 10
>+#define MODULE_OOPS_TRACKING_NR_BITS (BITS_PER_LONG * MODULE_OOPS_TRACKING_NR_LONGS)
>+static unsigned long module_oops_tracking[MODULE_OOPS_TRACKING_NR_LONGS];
>+void module_oops_tracking_mark(int nr)
>+{
>+ if (nr < MODULE_OOPS_TRACKING_NR_BITS)
>+ set_bit(nr, module_oops_tracking);
>+}
>+
>+static void __module_oops_tracking_print(int nr)
>+{
>+ struct module *mod;
>+
>+ for (mod = module_list; mod != &kernel_module; mod = mod->next) {
>+ if (!nr--)
>+ printk(" [(%s:<%p>:<%p>)]\n",
>+ mod->name,
>+ (char *) mod + mod->size_of_struct,
>+ (char *) mod + mod->size);
>+ }
>+
>+}
Instead of adding a separate bit map and scanning the module chain to
convert a bit number to a module entry, add a new entry to mod->flags.
#define MOD_OOPS_PRINTED 128
That simplifies the code and reduces the number of times you have to
scan the module list.
Beware if Rusty's idea of discarding init code/data from modules ever
takes off. Then the text will not be contiguous, nor will it be at the
start of the module.
On Sat, Jul 27, 2002 at 10:19:31AM +1000, Keith Owens wrote:
> On Sat, 27 Jul 2002 00:37:50 +0200,
> Andrea Arcangeli <[email protected]> wrote:
> >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.
> >+#define MODULE_OOPS_TRACKING_NR_LONGS 10
> >+#define MODULE_OOPS_TRACKING_NR_BITS (BITS_PER_LONG * MODULE_OOPS_TRACKING_NR_LONGS)
> >+static unsigned long module_oops_tracking[MODULE_OOPS_TRACKING_NR_LONGS];
> >+void module_oops_tracking_mark(int nr)
> >+{
> >+ if (nr < MODULE_OOPS_TRACKING_NR_BITS)
> >+ set_bit(nr, module_oops_tracking);
> >+}
> >+
> >+static void __module_oops_tracking_print(int nr)
> >+{
> >+ struct module *mod;
> >+
> >+ for (mod = module_list; mod != &kernel_module; mod = mod->next) {
> >+ if (!nr--)
> >+ printk(" [(%s:<%p>:<%p>)]\n",
> >+ mod->name,
> >+ (char *) mod + mod->size_of_struct,
> >+ (char *) mod + mod->size);
> >+ }
> >+
> >+}
>
> Instead of adding a separate bit map and scanning the module chain to
> convert a bit number to a module entry, add a new entry to mod->flags.
>
> #define MOD_OOPS_PRINTED 128
>
> That simplifies the code and reduces the number of times you have to
> scan the module list.
ok, the prepare stage will be a bit more complicated but it seems
a worthwhile change, thanks.
>
> Beware if Rusty's idea of discarding init code/data from modules ever
> takes off. Then the text will not be contiguous, nor will it be at the
> start of the module.
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
On Sat, Jul 27, 2002 at 02:31:21AM +0200, Andrea Arcangeli wrote:
> On Sat, Jul 27, 2002 at 10:19:31AM +1000, Keith Owens wrote:
> > On Sat, 27 Jul 2002 00:37:50 +0200,
> > Andrea Arcangeli <[email protected]> wrote:
> > >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.
> > >+#define MODULE_OOPS_TRACKING_NR_LONGS 10
> > >+#define MODULE_OOPS_TRACKING_NR_BITS (BITS_PER_LONG * MODULE_OOPS_TRACKING_NR_LONGS)
> > >+static unsigned long module_oops_tracking[MODULE_OOPS_TRACKING_NR_LONGS];
> > >+void module_oops_tracking_mark(int nr)
> > >+{
> > >+ if (nr < MODULE_OOPS_TRACKING_NR_BITS)
> > >+ set_bit(nr, module_oops_tracking);
> > >+}
> > >+
> > >+static void __module_oops_tracking_print(int nr)
> > >+{
> > >+ struct module *mod;
> > >+
> > >+ for (mod = module_list; mod != &kernel_module; mod = mod->next) {
> > >+ if (!nr--)
> > >+ printk(" [(%s:<%p>:<%p>)]\n",
> > >+ mod->name,
> > >+ (char *) mod + mod->size_of_struct,
> > >+ (char *) mod + mod->size);
> > >+ }
> > >+
> > >+}
> >
> > Instead of adding a separate bit map and scanning the module chain to
> > convert a bit number to a module entry, add a new entry to mod->flags.
> >
> > #define MOD_OOPS_PRINTED 128
> >
> > That simplifies the code and reduces the number of times you have to
> > scan the module list.
>
> ok, the prepare stage will be a bit more complicated but it seems
> a worthwhile change, thanks.
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
--- z/arch/i386/kernel/traps.c Sat Jul 27 03:13:07 2002
+++ 2.4.19rc3aa2/arch/i386/kernel/traps.c Sat Jul 27 03:14:30 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;
}
@@ -201,6 +202,8 @@ void show_registers(struct pt_regs *regs
unsigned long esp;
unsigned short ss;
+ printk(oops_id);
+
esp = (unsigned long) (®s->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
--- z/include/linux/kernel.h Sat Jul 27 03:13:07 2002
+++ 2.4.19rc3aa2/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
--- z/include/linux/module.h Sat Jul 27 03:13:07 2002
+++ 2.4.19rc3aa2/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 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
--- z/init/version.c Tue Jan 22 18:56:00 2002
+++ 2.4.19rc3aa2/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
--- z/kernel/module.c Sat Jul 27 03:13:07 2002
+++ 2.4.19rc3aa2/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 */
Andrea
On Sat, 27 Jul 2002 03:19:06 +0200,
Andrea Arcangeli <[email protected]> wrote:
>here an updated patch that moves the bitflag from the dedicated array to
>the mod->flags per your suggesiton.
>Oops: 0002
>2.4.19-rc3aa2 #3 SMP Sat Jul 27 05:06:19 CEST 2002
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
>+void module_oops_tracking_init(void)
>+{
>+ struct module * mod;
>+
>+ for (mod = module_list; mod != &kernel_module; mod = mod->next)
>+ mod->flags &= MOD_OOPS_PRINT;
mod->flags &= ~MOD_OOPS_PRINT;
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
At 07:41 PM 7/25/2002 -0300, Rik van Riel wrote:
>> 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.
>
>If you're willing to teach all our users how to use ksymoops ... ;)
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
On Sat, Jul 27, 2002 at 11:33:11AM +1000, Keith Owens wrote:
> On Sat, 27 Jul 2002 03:19:06 +0200,
> Andrea Arcangeli <[email protected]> wrote:
> >here an updated patch that moves the bitflag from the dedicated array to
> >the mod->flags per your suggesiton.
> >Oops: 0002
> >2.4.19-rc3aa2 #3 SMP Sat Jul 27 05:06:19 CEST 2002
>
> 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
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
--- z/arch/i386/kernel/traps.c Sat Jul 27 03:42:39 2002
+++ 2.4.19rc3aa2/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
--- z/include/linux/kernel.h Sat Jul 27 03:42:39 2002
+++ 2.4.19rc3aa2/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
--- z/include/linux/module.h Sat Jul 27 03:42:39 2002
+++ 2.4.19rc3aa2/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 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
--- z/init/version.c Sat Jul 27 03:42:39 2002
+++ 2.4.19rc3aa2/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
--- z/kernel/module.c Sat Jul 27 03:42:39 2002
+++ 2.4.19rc3aa2/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 */
Andrea