Messages in this thread Patch in this message | | | Date | Fri, 15 May 2009 10:52:44 -0700 (PDT) | From | Linus Torvalds <> | Subject | Re: [PATCH] Fix print out of function which called WARN_ON() |
| |
[ Jakub - I added you to the participants list because it really looks like this patch (without 'noinline') triggers a gcc bug.
Or maybe gcc is truly smart enough to turn the "NULL arg pointer" case into a "empty format" call, in which case we actually just solved the original warning case in a _really_ subtle way. Somebody who understands gcc varags should take a look.
Regardless, even if the optimization happens to _work_, it seems bogus. Even if "vprintk()" has been marked to have a printf-like format, that doesn't mean that you can call it unconditionally by making the format empty - it might have side effects.
This is with 'gcc -v' reporting:
gcc version 4.4.0 20090506 (Red Hat 4.4.0-4) (GCC)
and is the current fedora-11 compiler (as of a yum update yesterday or the day before) on x86-64 ]
On Fri, 15 May 2009, Ian Campbell wrote: > > All WARN_ON()'s currently appear to come from warn_slowpath_null e.g.: > WARNING: at kernel/softirq.c:143 warn_slowpath_null+0x1c/0x20() > > This is because since: > > commit 57adc4d2dbf968fdbe516359688094eef4d46581 > Author: Andi Kleen <andi@firstfloor.org> > Date: Wed May 6 16:02:53 2009 -0700 > > Eliminate thousands of warnings with gcc 3.2 build > > the caller of warn_slowpath_fmt really is warn_flowpath_null not the > interesting caller next up the chain. Since __builtin_return_address(X) for X > > 0 is not reliable, pass the real caller as an argument to warn_slowpath_fmt.
I'd actually much rather re-organize it so that there are the two exported functions (with the current interface) that use a common shared static function. Rather than passing in '0' in a macro.
IOW, something like this..
Btw, when doing this, it really looked to me like the "noinline" matters. I did it first to try to make the assembly clearer to read, but without it gcc-4.4 seems to actually generate the wrong code, and does an _unconditional_ call to vprintk().
Very odd. I've seen other bugs triggered by gcc-4.4 (alpine miscompiles), it all makes me very nervous about the compiler in current fedora-11.
Anybody want to double-check this?
Linus
--- kernel/panic.c | 35 ++++++++++++++++++++--------------- 1 files changed, 20 insertions(+), 15 deletions(-)
diff --git a/kernel/panic.c b/kernel/panic.c index 874ecf1..3c8048d 100644 --- a/kernel/panic.c +++ b/kernel/panic.c @@ -340,39 +340,44 @@ void oops_exit(void) } #ifdef WANT_WARN_ON_SLOWPATH -void warn_slowpath_fmt(const char *file, int line, const char *fmt, ...) -{ +struct slowpath_args { + const char *fmt; va_list args; - char function[KSYM_SYMBOL_LEN]; - unsigned long caller = (unsigned long)__builtin_return_address(0); - const char *board; +}; - sprint_symbol(function, caller); +static void noinline warn_slowpath_common(const char *file, int line, void *caller, struct slowpath_args *args) +{ + const char *board; printk(KERN_WARNING "------------[ cut here ]------------\n"); - printk(KERN_WARNING "WARNING: at %s:%d %s()\n", file, - line, function); + printk(KERN_WARNING "WARNING: at %s:%d %pS()\n", file, line, caller); board = dmi_get_system_info(DMI_PRODUCT_NAME); if (board) printk(KERN_WARNING "Hardware name: %s\n", board); - if (*fmt) { - va_start(args, fmt); - vprintk(fmt, args); - va_end(args); - } + if (args) + vprintk(args->fmt, args->args); print_modules(); dump_stack(); print_oops_end_marker(); add_taint(TAINT_WARN); } + +void warn_slowpath_fmt(const char *file, int line, const char *fmt, ...) +{ + struct slowpath_args args; + + args.fmt = fmt; + va_start(args.args, fmt); + warn_slowpath_common(file, line, __builtin_return_address(0), &args); + va_end(args.args); +} EXPORT_SYMBOL(warn_slowpath_fmt); void warn_slowpath_null(const char *file, int line) { - static const char *empty = ""; - warn_slowpath_fmt(file, line, empty); + warn_slowpath_fmt(file, line, __builtin_return_address(0), NULL); } EXPORT_SYMBOL(warn_slowpath_null); #endif
| |