Re: [RFC][PATCH] bug: Add _ONCE logic to report_bug()

[Peter Zijlstra]

On Sat, Feb 25, 2017 at 10:18:23AM +0100, Ingo Molnar wrote:

> > Sadly this only works for WARN_ON_ONCE(), since the others have
> > printk() statements prior to triggering the trap.
> 
> Which one is problematic to convert, WARN_ONCE()?

Yes, WARN_ONCE(), all the ones that have printf fmt crud in.

If we want report_bug() to do the _ONCE thing, we also need that to do
the printk().

I tried the below hackery (beware eye and brain damage ahead) which
actually compiles, but generates horrific junk -- far larger than
without.

The __builtin_va_*() crud was really not meant for things like this.

---
--- a/arch/x86/include/asm/bug.h
+++ b/arch/x86/include/asm/bug.h
@@ -66,6 +66,11 @@ do {								\
 
 #define __WARN_FLAGS(flags)	_BUG_FLAGS(ASM_UD0, BUGFLAG_WARNING|(flags))
 
+#define __WARN_ARGS_FLAGS(args, flags) do {		\
+	asm volatile ("" : : "a" (args));		\
+	_BUG_FLAGS(ASM_UD0, BUGFLAG_WARNING|(flags));	\
+} while (0)
+
 #include <asm-generic/bug.h>
 
 #endif /* _ASM_X86_BUG_H */
--- a/include/asm-generic/bug.h
+++ b/include/asm-generic/bug.h
@@ -7,6 +7,7 @@
 #define BUGFLAG_WARNING		(1 << 0)
 #define BUGFLAG_ONCE		(1 << 1)
 #define BUGFLAG_DONE		(1 << 2)
+#define BUGFLAG_ARGS		(1 << 3)
 #define BUGFLAG_TAINT(taint)	(BUGFLAG_WARNING | ((taint) << 8))
 #define BUG_GET_TAINT(bug)	((bug)->flags >> 8)
 #endif
@@ -67,8 +68,28 @@ struct bug_entry {
 		__WARN_ONCE_TAINT(TAINT_WARN);			\
 	unlikely(__ret_warn_on);				\
 })
+
+#ifdef __WARN_ARGS_FLAGS
+static inline void __va_hack(va_list *ap, ...)
+{
+	va_start(*ap, ap);
+}
+
+#define __WARN_printf(arg...) do {					\
+	va_list __ap;							\
+	__va_hack(&__ap, arg);						\
+	__WARN_ARGS_FLAGS(&__ap, BUGFLAG_ARGS|BUGFLAG_TAINT(TAINT_WARN));	\
+} while (0)
+
+#define __WARN_printf_taint(taint, arg...) do {				\
+	va_list __ap;							\
+	__va_hack(&__ap, arg);						\
+	__WARN_ARGS_FLAGS(&__ap, BUGFLAG_ARGS|BUGFLAG_TAINT(taint));	\
+} while (0)
 #endif
 
+#endif /* __WARN_FLAGS */
+
 /*
  * WARN(), WARN_ON(), WARN_ON_ONCE, and so on can be used to report
  * significant issues that need prompt attention if they should ever
@@ -90,10 +111,12 @@ extern void warn_slowpath_null(const cha
 	warn_slowpath_fmt_taint(__FILE__, __LINE__, taint, arg)
 #else
 #define __WARN()		__WARN_TAINT(TAINT_WARN)
+#ifndef __WARN_ARGS_FLAGS
 #define __WARN_printf(arg...)	do { printk(arg); __WARN(); } while (0)
 #define __WARN_printf_taint(taint, arg...)				\
 	do { printk(arg); __WARN_TAINT(taint); } while (0)
 #endif
+#endif
 
 /* used internally by panic.c */
 struct warn_args;
--- a/lib/bug.c
+++ b/lib/bug.c
@@ -143,7 +143,8 @@ enum bug_trap_type report_bug(unsigned l
 {
 	struct bug_entry *bug;
 	const char *file;
-	unsigned line, warning, once, done;
+	unsigned line, warning, once, done, args;
+	va_list *ap;
 
 	if (!is_valid_bugaddr(bugaddr))
 		return BUG_TRAP_TYPE_NONE;
@@ -166,6 +167,7 @@ enum bug_trap_type report_bug(unsigned l
 		warning = (bug->flags & BUGFLAG_WARNING) != 0;
 		once = (bug->flags & BUGFLAG_ONCE) != 0;
 		done = (bug->flags & BUGFLAG_DONE) != 0;
+		args = (bug->flags & BUGFLAG_ARGS) != 0;
 
 		if (warning && once) {
 			if (done)
@@ -176,9 +178,16 @@ enum bug_trap_type report_bug(unsigned l
 			 */
 			bug->flags |= BUGFLAG_DONE;
 		}
+
+		if (args)
+			ap = (va_list *)regs->ax;
 	}
 
 	if (warning) {
+		if (args) {
+			const char *fmt = va_arg(*ap, const char *);
+			vprintk(fmt, *ap);
+		}
 		/* this is a WARN_ON rather than BUG/BUG_ON */
 		__warn(file, line, (void *)bugaddr, BUG_GET_TAINT(bug), regs,
 		       NULL);