Re: [PATCH] Fix print out of function which called WARN_ON()

[Linus Torvalds <torvalds@linux-foundation.org>]

On Fri, 15 May 2009, Andi Kleen wrote:

> Linus Torvalds wrote:
> > [ Jakub - I added you to the participants list because it really looks
> > like this patch (without 'noinline') triggers a gcc bug.
> 
> This whole thing seems to be much more problematic than I originally thought
> :-/

No, sorry, it was my mistake. The code generated was indeed wrong, but it 
was because the source code was wrong (me calling the _fmt() function 
instead of the _common() function).

What's wrong with me that I can spot subtle errors in assembly language, 
but not the obvious ones in the source code?

Anyway, the proper patch is appended, and Jakub, you can ignore these 
things. gcc is fine.

This patch not only avoids the warnings and gets the right caller 
information, it cleans up the code too:

 - it uses '%pS' instead of of sprint_symbol

 - it avoids stupidly wasting stack space for varargs information in the 
   warn_slowpath_null case.

 - the code actually looks more readable too.

I'll commit it.

		Linus

---
 kernel/panic.c |   35 ++++++++++++++++++++---------------
 1 files changed, 20 insertions(+), 15 deletions(-)

diff --git a/kernel/panic.c b/kernel/panic.c
index 874ecf1..984b3ec 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 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_common(file, line, __builtin_return_address(0), NULL);
 }
 EXPORT_SYMBOL(warn_slowpath_null);
 #endif