We've found in the past that various bug reports have had minimal
information, just a few printk's rather than a complete oops,
and it's taken several round-trips with the bug reporter before
we've discovered they had some proprietary module loaded.
The patches below adds dumping of the tainted state to some
extra parts of the kernel that report 'bad things' happening.
These patches have been in the Fedora kernel for some time now,
and have proved useful often.
Signed-off-by: Dave Jones <[email protected]>
--- linux-2.6/include/asm-generic/bug.h~ 2007-02-12 16:18:21.000000000 -0500
+++ linux-2.6/include/asm-generic/bug.h 2007-02-12 16:19:57.000000000 -0500
@@ -3,6 +3,10 @@
#include <linux/compiler.h>
+#ifndef __ASSEMBLY__
+extern const char *print_tainted(void);
+#endif
+
#ifdef CONFIG_BUG
#ifdef CONFIG_GENERIC_BUG
@@ -22,7 +26,8 @@ struct bug_entry {
#ifndef HAVE_ARCH_BUG
#define BUG() do { \
- printk("BUG: failure at %s:%d/%s()!\n", __FILE__, __LINE__, __FUNCTION__); \
+ printk(KERN_ERR "BUG: failure at %s:%d/%s()! (%s)\n",
+ __FILE__, __LINE__, __FUNCTION__, print_tainted()); \
panic("BUG!"); \
} while (0)
#endif
@@ -39,8 +39,9 @@ struct bug_entry {
#define WARN_ON(condition) ({ \
int __ret_warn_on = !!(condition); \
if (unlikely(__ret_warn_on)) { \
- printk("WARNING: at %s:%d %s()\n", __FILE__, \
- __LINE__, __FUNCTION__); \
+ printk(KERN_ERR "WARNING: at %s:%d %s() (%s)\n", \
+ __FILE__, __LINE__, __FUNCTION__, \
+ print_tainted()); \
dump_stack(); \
} \
unlikely(__ret_warn_on); \
diff -urNp --exclude-from=/home/davej/.exclude linux-1740/kernel/panic.c linux-2000/kernel/panic.c
--- linux-1740/kernel/panic.c
+++ linux-2000/kernel/panic.c
@@ -151,6 +151,7 @@ const char *print_tainted(void)
snprintf(buf, sizeof(buf), "Not tainted");
return(buf);
}
+EXPORT_SYMBOL(print_tainted);
void add_taint(unsigned flag)
{
--- linux-2.6/mm/page_alloc.c~ 2006-01-07 20:48:33.000000000 -0500
+++ linux-2.6/mm/page_alloc.c 2006-01-07 20:49:24.000000000 -0500
@@ -137,12 +137,12 @@ static inline int bad_range(struct zone
static void bad_page(struct page *page)
{
printk(KERN_EMERG "Bad page state in process '%s'\n"
- KERN_EMERG "page:%p flags:0x%0*lx mapping:%p mapcount:%d count:%d\n"
+ KERN_EMERG "page:%p flags:0x%0*lx mapping:%p mapcount:%d count:%d (%s)\n"
KERN_EMERG "Trying to fix it up, but a reboot is needed\n"
KERN_EMERG "Backtrace:\n",
current->comm, page, (int)(2*sizeof(unsigned long)),
(unsigned long)page->flags, page->mapping,
- page_mapcount(page), page_count(page));
+ page_mapcount(page), page_count(page), print_tainted());
dump_stack();
page->flags &= ~(1 << PG_lru |
1 << PG_private |
--- linux-2.6/mm/slab.c~ 2007-05-27 22:57:44.000000000 -0400
+++ linux-2.6/mm/slab.c 2007-05-27 22:58:08.000000000 -0400
@@ -1816,8 +1816,8 @@ static void check_poison_obj(struct kmem
/* Print header */
if (lines == 0) {
printk(KERN_ERR
- "Slab corruption: %s start=%p, len=%d\n",
- cachep->name, realobj, size);
+ "Slab corruption (%s): %s start=%p, len=%d\n",
+ print_tainted(), cachep->name, realobj, size);
print_objinfo(cachep, objp, 0);
}
/* Hexdump the affected line */
@@ -2924,8 +2924,8 @@ static void check_slabp(struct kmem_cach
if (entries != cachep->num - slabp->inuse) {
bad:
printk(KERN_ERR "slab: Internal list corruption detected in "
- "cache '%s'(%d), slabp %p(%d). Hexdump:\n",
- cachep->name, cachep->num, slabp, slabp->inuse);
+ "cache '%s'(%d), slabp %p(%d). Tainted(%s). Hexdump:\n",
+ cachep->name, cachep->num, slabp, slabp->inuse, print_tainted());
for (i = 0;
i < sizeof(*slabp) + cachep->num * sizeof(kmem_bufctl_t);
i++) {
--- linux-2.6/mm/slub.c~ 2007-07-20 14:15:05.000000000 -0400
+++ linux-2.6/mm/slub.c 2007-07-20 14:17:37.000000000 -0400
@@ -450,7 +450,7 @@ static void slab_bug(struct kmem_cache *
va_end(args);
printk(KERN_ERR "========================================"
"=====================================\n");
- printk(KERN_ERR "BUG %s: %s\n", s->name, buf);
+ printk(KERN_ERR "BUG %s (%s): %s\n", s->name, print_tainted(), buf);
printk(KERN_ERR "----------------------------------------"
"-------------------------------------\n\n");
}
--- linux-2.6/lib/spinlock_debug.c~ 2007-10-22 01:15:04.000000000 -0400
+++ linux-2.6/lib/spinlock_debug.c 2007-10-22 01:17:03.000000000 -0400
@@ -58,9 +58,9 @@ static void spin_bug(spinlock_t *lock, c
if (lock->owner && lock->owner != SPINLOCK_OWNER_INIT)
owner = lock->owner;
- printk(KERN_EMERG "BUG: spinlock %s on CPU#%d, %s/%d\n",
+ printk(KERN_EMERG "BUG: spinlock %s on CPU#%d, %s/%d (%s)\n",
msg, raw_smp_processor_id(),
- current->comm, task_pid_nr(current));
+ current->comm, task_pid_nr(current), print_tainted());
printk(KERN_EMERG " lock: %p, .magic: %08x, .owner: %s/%d, "
".owner_cpu: %d\n",
lock, lock->magic,
@@ -114,9 +114,9 @@ static void __spin_lock_debug(spinlock_t
if (print_once) {
print_once = 0;
printk(KERN_EMERG "BUG: spinlock lockup on CPU#%d, "
- "%s/%d, %p\n",
+ "%s/%d, %p (%s)\n",
raw_smp_processor_id(), current->comm,
- task_pid_nr(current), lock);
+ task_pid_nr(current), lock, print_tainted());
dump_stack();
#ifdef CONFIG_SMP
trigger_all_cpu_backtrace();
@@ -159,9 +159,9 @@ static void rwlock_bug(rwlock_t *lock, c
if (!debug_locks_off())
return;
- printk(KERN_EMERG "BUG: rwlock %s on CPU#%d, %s/%d, %p\n",
+ printk(KERN_EMERG "BUG: rwlock %s on CPU#%d, %s/%d, %p (%s)\n",
msg, raw_smp_processor_id(), current->comm,
- task_pid_nr(current), lock);
+ task_pid_nr(current), lock, print_tainted());
dump_stack();
}
@@ -177,9 +177,9 @@ static void __read_lock_debug(rwlock_t *
if (print_once) {
print_once = 0;
printk(KERN_EMERG "BUG: read-lock lockup on CPU#%d, "
- "%s/%d, %p\n",
+ "%s/%d, %p (%s)\n",
raw_smp_processor_id(), current->comm,
- current->pid, lock);
+ current->pid, lock, print_tainted());
dump_stack();
}
}
@@ -250,9 +250,9 @@ static void __write_lock_debug(rwlock_t
if (print_once) {
print_once = 0;
printk(KERN_EMERG "BUG: write-lock lockup on CPU#%d, "
- "%s/%d, %p\n",
+ "%s/%d, %p (%s)\n",
raw_smp_processor_id(), current->comm,
- current->pid, lock);
+ current->pid, lock, print_tainted());
dump_stack();
}
}
--
http://www.codemonkey.org.uk
Dave Jones <[email protected]> writes:
> #define WARN_ON(condition) ({ \
> int __ret_warn_on = !!(condition); \
> if (unlikely(__ret_warn_on)) { \
> - printk("WARNING: at %s:%d %s()\n", __FILE__, \
> - __LINE__, __FUNCTION__); \
> + printk(KERN_ERR "WARNING: at %s:%d %s() (%s)\n", \
> + __FILE__, __LINE__, __FUNCTION__, \
> + print_tainted()); \
> dump_stack(); \
Have you checked how this affects code size? It might be worth it
now to do a out of line helper.
-Andi
2007/12/13, Andi Kleen <[email protected]>:
> Dave Jones <[email protected]> writes:
>
> > #define WARN_ON(condition) ({ \
> > int __ret_warn_on = !!(condition); \
> > if (unlikely(__ret_warn_on)) { \
> > - printk("WARNING: at %s:%d %s()\n", __FILE__, \
> > - __LINE__, __FUNCTION__); \
> > + printk(KERN_ERR "WARNING: at %s:%d %s() (%s)\n", \
> > + __FILE__, __LINE__, __FUNCTION__, \
> > + print_tainted()); \
> > dump_stack(); \
>
> Have you checked how this affects code size? It might be worth it
> now to do a out of line helper.
>
Have you checked how does spotting a bug is worth the extra size
sometimes (almost all the time)?
On Fri, Dec 14, 2007 at 12:08:46AM +0100, Andi Kleen wrote:
> Dave Jones <[email protected]> writes:
>
> > #define WARN_ON(condition) ({ \
> > int __ret_warn_on = !!(condition); \
> > if (unlikely(__ret_warn_on)) { \
> > - printk("WARNING: at %s:%d %s()\n", __FILE__, \
> > - __LINE__, __FUNCTION__); \
> > + printk(KERN_ERR "WARNING: at %s:%d %s() (%s)\n", \
> > + __FILE__, __LINE__, __FUNCTION__, \
> > + print_tainted()); \
> > dump_stack(); \
>
> Have you checked how this affects code size? It might be worth it
> now to do a out of line helper.
64 bit debug build of the fedora kernel (which should be worse
case for WARN_ONs etc).
text data bss dec hex filename
before 3833875 834267 631136 5299278 50dc4e vmlinux
after 3837054 834267 631136 5302457 50e8b9 vmlinux
Some of that growth is the addition of the missing KERN_ERR,
but yeah, it grows a little bit.
Dave
--
http://www.codemonkey.org.uk
On Thu, Dec 13, 2007 at 05:49:27PM -0500, Dave Jones wrote:
> We've found in the past that various bug reports have had minimal
> information, just a few printk's rather than a complete oops,
> and it's taken several round-trips with the bug reporter before
> we've discovered they had some proprietary module loaded.
>
> The patches below adds dumping of the tainted state to some
> extra parts of the kernel that report 'bad things' happening.
> These patches have been in the Fedora kernel for some time now,
> and have proved useful often.
>
> Signed-off-by: Dave Jones <[email protected]>
>
>
> --- linux-2.6/include/asm-generic/bug.h~ 2007-02-12 16:18:21.000000000 -0500
> +++ linux-2.6/include/asm-generic/bug.h 2007-02-12 16:19:57.000000000 -0500
> @@ -3,6 +3,10 @@
>
> #include <linux/compiler.h>
>
> +#ifndef __ASSEMBLY__
> +extern const char *print_tainted(void);
> +#endif
> +
> #ifdef CONFIG_BUG
>
> #ifdef CONFIG_GENERIC_BUG
> @@ -22,7 +26,8 @@ struct bug_entry {
>
> #ifndef HAVE_ARCH_BUG
> #define BUG() do { \
> - printk("BUG: failure at %s:%d/%s()!\n", __FILE__, __LINE__, __FUNCTION__); \
> + printk(KERN_ERR "BUG: failure at %s:%d/%s()! (%s)\n",
> + __FILE__, __LINE__, __FUNCTION__, print_tainted()); \
> panic("BUG!"); \
> } while (0)
> #endif
>...
Note that this only changes a handful of architectures and most likely
not the ones you are interested in.
cu
Adrian
--
"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed
On Thu, Dec 13, 2007 at 06:52:42PM -0500, Dave Jones wrote:
> > Have you checked how this affects code size? It might be worth it
> > now to do a out of line helper.
>
> 64 bit debug build of the fedora kernel (which should be worse
> case for WARN_ONs etc).
>
> text data bss dec hex filename
> before 3833875 834267 631136 5299278 50dc4e vmlinux
> after 3837054 834267 631136 5302457 50e8b9 vmlinux
>
> Some of that growth is the addition of the missing KERN_ERR,
> but yeah, it grows a little bit.
And the out-of-line variant of WARN_ON ends up at ..
text data bss dec hex filename
3828632 834267 631136 5294035 50c7d3 vmlinux
Dave
--
http://www.codemonkey.org.uk
On Fri, Dec 14, 2007 at 01:03:50AM +0100, Adrian Bunk wrote:
> > #ifndef HAVE_ARCH_BUG
> > #define BUG() do { \
> > - printk("BUG: failure at %s:%d/%s()!\n", __FILE__, __LINE__, __FUNCTION__); \
> > + printk(KERN_ERR "BUG: failure at %s:%d/%s()! (%s)\n",
> > + __FILE__, __LINE__, __FUNCTION__, print_tainted()); \
> > panic("BUG!"); \
> > } while (0)
> > #endif
> >...
>
> Note that this only changes a handful of architectures and most likely
> not the ones you are interested in.
Good catch. I'm sure we had coverage on at least x86 before, so
it seems over the many rediffs this patch has had, some chunks got lost.
I'll resurrect that.
thanks,
Dave
--
http://www.codemonkey.org.uk
On Fri, Dec 14, 2007 at 01:03:50AM +0100, Adrian Bunk wrote:
> > #ifndef HAVE_ARCH_BUG
> > #define BUG() do { \
> > - printk("BUG: failure at %s:%d/%s()!\n", __FILE__, __LINE__, __FUNCTION__); \
> > + printk(KERN_ERR "BUG: failure at %s:%d/%s()! (%s)\n",
> > + __FILE__, __LINE__, __FUNCTION__, print_tainted()); \
> > panic("BUG!"); \
> > } while (0)
> > #endif
> >...
>
> Note that this only changes a handful of architectures and most likely
> not the ones you are interested in.
Hmm, it appears that I was mistaken, and we never did patch x86.
Which leaves me wondering if its worth it or not to patch BUG()
Anyways, here's the latest rev with the out-of-line changes as
suggested by Andi.
init/main.c may not be the best place for the ool variant. suggestions?
Dave
diff --git a/include/asm-generic/bug.h b/include/asm-generic/bug.h
index d56fedb..e35833a 100644
--- a/include/asm-generic/bug.h
+++ b/include/asm-generic/bug.h
@@ -3,6 +3,10 @@
#include <linux/compiler.h>
+#ifndef __ASSEMBLY__
+extern const char *print_tainted(void);
+#endif
+
#ifdef CONFIG_BUG
#ifdef CONFIG_GENERIC_BUG
@@ -22,7 +26,8 @@ struct bug_entry {
#ifndef HAVE_ARCH_BUG
#define BUG() do { \
- printk("BUG: failure at %s:%d/%s()!\n", __FILE__, __LINE__, __FUNCTION__); \
+ printk(KERN_ERR "BUG: failure at %s:%d/%s()! (%s)\n",
+ __FILE__, __LINE__, __FUNCTION__, print_tainted()); \
panic("BUG!"); \
} while (0)
#endif
@@ -32,13 +37,11 @@ struct bug_entry {
#endif
#ifndef HAVE_ARCH_WARN_ON
+void out_of_line_warnon(char *file, unsigned int line, const char *func);
#define WARN_ON(condition) ({ \
int __ret_warn_on = !!(condition); \
- if (unlikely(__ret_warn_on)) { \
- printk("WARNING: at %s:%d %s()\n", __FILE__, \
- __LINE__, __FUNCTION__); \
- dump_stack(); \
- } \
+ if (unlikely(__ret_warn_on)) \
+ out_of_line_warnon(__FILE__, __LINE__, __FUNCTION__); \
unlikely(__ret_warn_on); \
})
#endif
diff --git a/init/main.c b/init/main.c
index 80b04b6..b1fad76 100644
--- a/init/main.c
+++ b/init/main.c
@@ -855,3 +855,11 @@ static int __init kernel_init(void * unused)
init_post();
return 0;
}
+
+void out_of_line_warnon(char *file, unsigned int line, const char *func)
+{
+ printk(KERN_ERR "WARNING: at %s:%d %s() (%s)\n",
+ file, line, func, print_tainted());
+ dump_stack();
+}
+EXPORT_SYMBOL(out_of_line_warnon);
diff --git a/kernel/panic.c b/kernel/panic.c
index 6f6e03e..198fc58 100644
--- a/kernel/panic.c
+++ b/kernel/panic.c
@@ -173,6 +173,7 @@ const char *print_tainted(void)
snprintf(buf, sizeof(buf), "Not tainted");
return(buf);
}
+EXPORT_SYMBOL(print_tainted);
void add_taint(unsigned flag)
{
diff --git a/lib/spinlock_debug.c b/lib/spinlock_debug.c
index 9c4b025..b7a010a 100644
--- a/lib/spinlock_debug.c
+++ b/lib/spinlock_debug.c
@@ -58,9 +58,9 @@ static void spin_bug(spinlock_t *lock, const char *msg)
if (lock->owner && lock->owner != SPINLOCK_OWNER_INIT)
owner = lock->owner;
- printk(KERN_EMERG "BUG: spinlock %s on CPU#%d, %s/%d\n",
+ printk(KERN_EMERG "BUG: spinlock %s on CPU#%d, %s/%d (%s)\n",
msg, raw_smp_processor_id(),
- current->comm, task_pid_nr(current));
+ current->comm, task_pid_nr(current), print_tainted());
printk(KERN_EMERG " lock: %p, .magic: %08x, .owner: %s/%d, "
".owner_cpu: %d\n",
lock, lock->magic,
@@ -114,9 +114,9 @@ static void __spin_lock_debug(spinlock_t *lock)
if (print_once) {
print_once = 0;
printk(KERN_EMERG "BUG: spinlock lockup on CPU#%d, "
- "%s/%d, %p\n",
+ "%s/%d, %p (%s)\n",
raw_smp_processor_id(), current->comm,
- task_pid_nr(current), lock);
+ task_pid_nr(current), lock, print_tainted());
dump_stack();
#ifdef CONFIG_SMP
trigger_all_cpu_backtrace();
@@ -159,9 +159,9 @@ static void rwlock_bug(rwlock_t *lock, const char *msg)
if (!debug_locks_off())
return;
- printk(KERN_EMERG "BUG: rwlock %s on CPU#%d, %s/%d, %p\n",
+ printk(KERN_EMERG "BUG: rwlock %s on CPU#%d, %s/%d, %p (%s)\n",
msg, raw_smp_processor_id(), current->comm,
- task_pid_nr(current), lock);
+ task_pid_nr(current), lock, print_tainted());
dump_stack();
}
@@ -184,9 +184,9 @@ static void __read_lock_debug(rwlock_t *lock)
if (print_once) {
print_once = 0;
printk(KERN_EMERG "BUG: read-lock lockup on CPU#%d, "
- "%s/%d, %p\n",
+ "%s/%d, %p (%s)\n",
raw_smp_processor_id(), current->comm,
- current->pid, lock);
+ current->pid, lock, print_tainted());
dump_stack();
}
}
@@ -259,9 +259,9 @@ static void __write_lock_debug(rwlock_t *lock)
if (print_once) {
print_once = 0;
printk(KERN_EMERG "BUG: write-lock lockup on CPU#%d, "
- "%s/%d, %p\n",
+ "%s/%d, %p (%s)\n",
raw_smp_processor_id(), current->comm,
- current->pid, lock);
+ current->pid, lock, print_tainted());
dump_stack();
}
}
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index b5a58d4..7a0c25d 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -221,12 +221,12 @@ static inline int bad_range(struct zone *zone, struct page *page)
static void bad_page(struct page *page)
{
printk(KERN_EMERG "Bad page state in process '%s'\n"
- KERN_EMERG "page:%p flags:0x%0*lx mapping:%p mapcount:%d count:%d\n"
+ KERN_EMERG "page:%p flags:0x%0*lx mapping:%p mapcount:%d count:%d (%s)\n"
KERN_EMERG "Trying to fix it up, but a reboot is needed\n"
KERN_EMERG "Backtrace:\n",
current->comm, page, (int)(2*sizeof(unsigned long)),
(unsigned long)page->flags, page->mapping,
- page_mapcount(page), page_count(page));
+ page_mapcount(page), page_count(page), print_tainted());
dump_stack();
page->flags &= ~(1 << PG_lru |
1 << PG_private |
diff --git a/mm/slab.c b/mm/slab.c
index 2e338a5..e5627f9 100644
--- a/mm/slab.c
+++ b/mm/slab.c
@@ -1846,8 +1846,8 @@ static void check_poison_obj(struct kmem_cache *cachep, void *objp)
/* Print header */
if (lines == 0) {
printk(KERN_ERR
- "Slab corruption: %s start=%p, len=%d\n",
- cachep->name, realobj, size);
+ "Slab corruption (%s): %s start=%p, len=%d\n",
+ print_tainted(), cachep->name, realobj, size);
print_objinfo(cachep, objp, 0);
}
/* Hexdump the affected line */
@@ -2935,8 +2935,8 @@ static void check_slabp(struct kmem_cache *cachep, struct slab *slabp)
if (entries != cachep->num - slabp->inuse) {
bad:
printk(KERN_ERR "slab: Internal list corruption detected in "
- "cache '%s'(%d), slabp %p(%d). Hexdump:\n",
- cachep->name, cachep->num, slabp, slabp->inuse);
+ "cache '%s'(%d), slabp %p(%d). Tainted(%s). Hexdump:\n",
+ cachep->name, cachep->num, slabp, slabp->inuse, print_tainted());
for (i = 0;
i < sizeof(*slabp) + cachep->num * sizeof(kmem_bufctl_t);
i++) {
diff --git a/mm/slub.c b/mm/slub.c
index 9c1d9f3..e11d58d 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -451,7 +451,7 @@ static void slab_bug(struct kmem_cache *s, char *fmt, ...)
va_end(args);
printk(KERN_ERR "========================================"
"=====================================\n");
- printk(KERN_ERR "BUG %s: %s\n", s->name, buf);
+ printk(KERN_ERR "BUG %s (%s): %s\n", s->name, print_tainted(), buf);
printk(KERN_ERR "----------------------------------------"
"-------------------------------------\n\n");
}
--
http://www.codemonkey.org.uk
Dave Jones wrote:
> On Fri, Dec 14, 2007 at 01:03:50AM +0100, Adrian Bunk wrote:
>
> > > #ifndef HAVE_ARCH_BUG
> > > #define BUG() do { \
> > > - printk("BUG: failure at %s:%d/%s()!\n", __FILE__, __LINE__, __FUNCTION__); \
> > > + printk(KERN_ERR "BUG: failure at %s:%d/%s()! (%s)\n",
> > > + __FILE__, __LINE__, __FUNCTION__, print_tainted()); \
> > > panic("BUG!"); \
> > > } while (0)
> > > #endif
> > >...
> >
> > Note that this only changes a handful of architectures and most likely
> > not the ones you are interested in.
>
> Hmm, it appears that I was mistaken, and we never did patch x86.
> Which leaves me wondering if its worth it or not to patch BUG()
> Anyways, here's the latest rev with the out-of-line changes as
> suggested by Andi.
>
> init/main.c may not be the best place for the ool variant. suggestions?
>
lib/bug.c would be the place for architectures using
CONFIG_GENERIC_BUG. x86 could be converted to use the BUG-trapping
mechanism for WARN_ON like Power does, so it would be inherently out of
line anyway.
J
On Thu, Dec 13, 2007 at 11:25:06PM -0800, Jeremy Fitzhardinge wrote:
> Dave Jones wrote:
> > On Fri, Dec 14, 2007 at 01:03:50AM +0100, Adrian Bunk wrote:
> >
> > > > #ifndef HAVE_ARCH_BUG
> > > > #define BUG() do { \
> > > > - printk("BUG: failure at %s:%d/%s()!\n", __FILE__, __LINE__, __FUNCTION__); \
> > > > + printk(KERN_ERR "BUG: failure at %s:%d/%s()! (%s)\n",
> > > > + __FILE__, __LINE__, __FUNCTION__, print_tainted()); \
> > > > panic("BUG!"); \
> > > > } while (0)
> > > > #endif
> > > >...
> > >
> > > Note that this only changes a handful of architectures and most likely
> > > not the ones you are interested in.
> >
> > Hmm, it appears that I was mistaken, and we never did patch x86.
> > Which leaves me wondering if its worth it or not to patch BUG()
> > Anyways, here's the latest rev with the out-of-line changes as
> > suggested by Andi.
> >
> > init/main.c may not be the best place for the ool variant. suggestions?
> >
>
> lib/bug.c would be the place for architectures using
> CONFIG_GENERIC_BUG. x86 could be converted to use the BUG-trapping
> mechanism for WARN_ON like Power does, so it would be inherently out of
> line anyway.
The BUG()/WARN_ON() parts of my patch are becoming something of a rathole,
which I don't really have time to dig into right now, so I'm going to split
this up I think into mm/ additions, spinlockdebug additions, and bugon/warnon foo.
The three should be independant, and blocking the first two until I get
time to look at the third seems pointless.
Dave
--
http://www.codemonkey.org.uk
On Thu, 2007-12-13 at 17:49 -0500, Dave Jones wrote:
> We've found in the past that various bug reports have had minimal
> information, just a few printk's rather than a complete oops,
> and it's taken several round-trips with the bug reporter before
> we've discovered they had some proprietary module loaded.
>
> The patches below adds dumping of the tainted state to some
> extra parts of the kernel that report 'bad things' happening.
> These patches have been in the Fedora kernel for some time now,
> and have proved useful often.
>
> Signed-off-by: Dave Jones <[email protected]>
In my opinion, this is a very good idea :-)
Jon.
On Thu, Dec 13, 2007 at 08:30:41PM -0500, Dave Jones wrote:
> #ifndef HAVE_ARCH_BUG
> #define BUG() do { \
> - printk("BUG: failure at %s:%d/%s()!\n", __FILE__, __LINE__, __FUNCTION__); \
> + printk(KERN_ERR "BUG: failure at %s:%d/%s()! (%s)\n",
> + __FILE__, __LINE__, __FUNCTION__, print_tainted()); \
> panic("BUG!"); \
> } while (0)
Might as well make this inline too, no?
> #endif
> @@ -32,13 +37,11 @@ struct bug_entry {
> #endif
>
> #ifndef HAVE_ARCH_WARN_ON
> +void out_of_line_warnon(char *file, unsigned int line, const char *func);
How about __warn_on?
> diff --git a/init/main.c b/init/main.c
> index 80b04b6..b1fad76 100644
> --- a/init/main.c
> +++ b/init/main.c
> @@ -855,3 +855,11 @@ static int __init kernel_init(void * unused)
> init_post();
> return 0;
> }
> +
> +void out_of_line_warnon(char *file, unsigned int line, const char *func)
Might as well make *file const too.
> diff --git a/kernel/panic.c b/kernel/panic.c
> index 6f6e03e..198fc58 100644
> --- a/kernel/panic.c
> +++ b/kernel/panic.c
> @@ -173,6 +173,7 @@ const char *print_tainted(void)
> snprintf(buf, sizeof(buf), "Not tainted");
> return(buf);
> }
> +EXPORT_SYMBOL(print_tainted);
Looks like you've got two patches here..
--
Mathematics is the supreme nostalgia of our time.
On Fri, Dec 14, 2007 at 09:38:44AM -0600, Matt Mackall wrote:
> On Thu, Dec 13, 2007 at 08:30:41PM -0500, Dave Jones wrote:
> > #ifndef HAVE_ARCH_BUG
> > #define BUG() do { \
> > - printk("BUG: failure at %s:%d/%s()!\n", __FILE__, __LINE__, __FUNCTION__); \
> > + printk(KERN_ERR "BUG: failure at %s:%d/%s()! (%s)\n",
> > + __FILE__, __LINE__, __FUNCTION__, print_tainted()); \
> > panic("BUG!"); \
> > } while (0)
>
> Might as well make this inline too, no?
Maybe.
> > +void out_of_line_warnon(char *file, unsigned int line, const char *func);
>
> How about __warn_on?
I'm easily swayed :)
> > +void out_of_line_warnon(char *file, unsigned int line, const char *func)
>
> Might as well make *file const too.
ok
> > diff --git a/kernel/panic.c b/kernel/panic.c
> > index 6f6e03e..198fc58 100644
> > --- a/kernel/panic.c
> > +++ b/kernel/panic.c
> > @@ -173,6 +173,7 @@ const char *print_tainted(void)
> > snprintf(buf, sizeof(buf), "Not tainted");
> > return(buf);
> > }
> > +EXPORT_SYMBOL(print_tainted);
>
> Looks like you've got two patches here..
no, up top we added a print_tainted() call to BUG()
BUG() gets used in modules == needs its ancillary functions
also exported.
Dave
--
http://www.codemonkey.org.uk