* Linus Torvalds <torvalds@linux-foundation.org> wrote:

> How about going a different route instead? Right now we have that 
> CONFIG_DEBUG_BUGVERBOSE thing which adds the file name and line 
> number information. That has been very good.
> 
> But maybe that should be extended to also always take the 
> compile-time '#condition' string?
> 
> So then all warnings would have the warning condition string 
> (assuming you end up enabling DEBUG_BUGVERBOSE, of course, which I 
> think everybody pretty much does). With no extra code.

So something like the patch below?

Testcase:

  @@ -8508,6 +8508,8 @@ void __init sched_init(void)
          unsigned long ptr = 0;
          int i;
 
  +       WARN_ON_ONCE(ptr == 0 && 1);
  +

Before:

  WARNING: CPU: 0 PID: 0 at kernel/sched/core.c:8511 sched_init+0x20/0x410

After:

  WARNING: CPU: 0 PID: 0 at [ptr == 0 && 1] kernel/sched/core.c:8511 sched_init+0x20/0x410
                            ^^^^^^^^^^^^^^^

I concatenated the condition string with the file string, so didn't 
have to extend the 'struct bug_entry' backend, and could be shared with 
the regular WARN() and BUG*() code as well without modifying its 
output.

The .text impact is zero, as hoped for:

       text       data        bss         dec        hex    filename
   29523998    7926322    1389904    38840224    250a7a0    vmlinux.before
   29523998    8024626    1389904    38938528    25227a0    vmlinue.after

So this does have the debugging advantages of SCHED_WARN_ON() and the 
code generation benefits of WARN_ON_ONCE().

Note that the patch has still the maturity of a Labradoodle puppy: it 
won't build on the majority of non-x86 architectures, has only been 
built and booted once, etc. - so it's not signed off on.

Thanks,

	Ingo

===================>
From: Ingo Molnar <mingo@kernel.org>
Date: Tue, 25 Mar 2025 12:18:44 +0100
Subject: [PATCH] bug: Add the condition string to the CONFIG_DEBUG_BUGVERBOSE=y output

       text       data        bss         dec        hex    filename
   29523998    7926322    1389904    38840224    250a7a0    vmlinux.before
   29523998    8024626    1389904    38938528    25227a0    vmlinue.after

Before:

  WARNING: CPU: 0 PID: 0 at kernel/sched/core.c:8511 sched_init+0x20/0x410

After:

  WARNING: CPU: 0 PID: 0 at [ptr == 0 && 1] kernel/sched/core.c:8511 sched_init+0x20/0x410
                            ^^^^^^^^^^^^^^^
Not-Signed-off-by: Ingo Molnar <mingo@kernel.org>
Link: https://lore.kernel.org/r/Z-KRD3ODxT9f8Yjw@gmail.com
---
 arch/x86/include/asm/bug.h | 14 +++++++-------
 include/asm-generic/bug.h  |  7 ++++---
 kernel/sched/core.c        |  2 ++
 3 files changed, 13 insertions(+), 10 deletions(-)

diff --git a/arch/x86/include/asm/bug.h b/arch/x86/include/asm/bug.h
index f0e9acf72547..e966199c8ef7 100644
--- a/arch/x86/include/asm/bug.h
+++ b/arch/x86/include/asm/bug.h
@@ -39,7 +39,7 @@
 
 #ifdef CONFIG_DEBUG_BUGVERBOSE
 
-#define _BUG_FLAGS(ins, flags, extra)					\
+#define _BUG_FLAGS(cond_str, ins, flags, extra)				\
 do {									\
 	asm_inline volatile("1:\t" ins "\n"				\
 		     ".pushsection __bug_table,\"aw\"\n"		\
@@ -50,14 +50,14 @@ do {									\
 		     "\t.org 2b+%c3\n"					\
 		     ".popsection\n"					\
 		     extra						\
-		     : : "i" (__FILE__), "i" (__LINE__),		\
+		     : : "i" (cond_str __FILE__), "i" (__LINE__),		\
 			 "i" (flags),					\
 			 "i" (sizeof(struct bug_entry)));		\
 } while (0)
 
 #else /* !CONFIG_DEBUG_BUGVERBOSE */
 
-#define _BUG_FLAGS(ins, flags, extra)					\
+#define _BUG_FLAGS(cond_str, ins, flags, extra)				\
 do {									\
 	asm_inline volatile("1:\t" ins "\n"				\
 		     ".pushsection __bug_table,\"aw\"\n"		\
@@ -74,7 +74,7 @@ do {									\
 
 #else
 
-#define _BUG_FLAGS(ins, flags, extra)  asm volatile(ins)
+#define _BUG_FLAGS(cond_str, ins, flags, extra)  asm volatile(ins)
 
 #endif /* CONFIG_GENERIC_BUG */
 
@@ -82,7 +82,7 @@ do {									\
 #define BUG()							\
 do {								\
 	instrumentation_begin();				\
-	_BUG_FLAGS(ASM_UD2, 0, "");				\
+	_BUG_FLAGS("", ASM_UD2, 0, "");				\
 	__builtin_unreachable();				\
 } while (0)
 
@@ -92,11 +92,11 @@ do {								\
  * were to trigger, we'd rather wreck the machine in an attempt to get the
  * message out than not know about it.
  */
-#define __WARN_FLAGS(flags)					\
+#define __WARN_FLAGS(cond_str, flags)				\
 do {								\
 	__auto_type __flags = BUGFLAG_WARNING|(flags);		\
 	instrumentation_begin();				\
-	_BUG_FLAGS(ASM_UD2, __flags, ANNOTATE_REACHABLE(1b));	\
+	_BUG_FLAGS(cond_str, ASM_UD2, __flags, ANNOTATE_REACHABLE(1b)); \
 	instrumentation_end();					\
 } while (0)
 
diff --git a/include/asm-generic/bug.h b/include/asm-generic/bug.h
index 387720933973..c8e7126bc26e 100644
--- a/include/asm-generic/bug.h
+++ b/include/asm-generic/bug.h
@@ -100,17 +100,18 @@ extern __printf(1, 2) void __warn_printk(const char *fmt, ...);
 		instrumentation_end();					\
 	} while (0)
 #else
-#define __WARN()		__WARN_FLAGS(BUGFLAG_TAINT(TAINT_WARN))
+#define __WARN()		__WARN_FLAGS("", BUGFLAG_TAINT(TAINT_WARN))
 #define __WARN_printf(taint, arg...) do {				\
 		instrumentation_begin();				\
 		__warn_printk(arg);					\
-		__WARN_FLAGS(BUGFLAG_NO_CUT_HERE | BUGFLAG_TAINT(taint));\
+		__WARN_FLAGS("", BUGFLAG_NO_CUT_HERE | BUGFLAG_TAINT(taint));\
 		instrumentation_end();					\
 	} while (0)
 #define WARN_ON_ONCE(condition) ({				\
 	int __ret_warn_on = !!(condition);			\
 	if (unlikely(__ret_warn_on))				\
-		__WARN_FLAGS(BUGFLAG_ONCE |			\
+		__WARN_FLAGS("["#condition"] ",			\
+			     BUGFLAG_ONCE |			\
 			     BUGFLAG_TAINT(TAINT_WARN));	\
 	unlikely(__ret_warn_on);				\
 })
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 87540217fc09..71bf94bf68f8 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -8508,6 +8508,8 @@ void __init sched_init(void)
 	unsigned long ptr = 0;
 	int i;
 
+	WARN_ON_ONCE(ptr == 0 && 1);
+
 	/* Make sure the linker didn't screw up */
 #ifdef CONFIG_SMP
 	BUG_ON(!sched_class_above(&stop_sched_class, &dl_sched_class));

On Tue, 25 Mar 2025 at 15:42, Ingo Molnar <mingo@kernel.org> wrote:
>
> So something like the patch below?
> [...]
> After:
>
>   WARNING: CPU: 0 PID: 0 at [ptr == 0 && 1] kernel/sched/core.c:8511 sched_init+0x20/0x410
>                             ^^^^^^^^^^^^^^^

Hmm. Is that the prettiest output ever? No. But it does seem workable,
and the patch is simple.

And I think the added condition string is useful, in that I often end
up looking up warnings that other people report and where the line
numbers have changed enough that it's not immediately obvious exactly
which warning it is. Not only does it disambiguate which warning it
is, it would probably often would obviate having to look it up
entirely because the warning message is now more useful.

So I think I like it. Let's see how it works in practice.

(I actually think the "CPU: 0 PID: 0" is likely the least useful part
of that warning string, and maybe *that* should be moved away and make
things a bit more legible, but I think that discussion might as well
be part of that "Let's see how it works")

            Linus

* Linus Torvalds <torvalds@linux-foundation.org> wrote:

> On Tue, 25 Mar 2025 at 15:42, Ingo Molnar <mingo@kernel.org> wrote:
> >
> > So something like the patch below?
> > [...]
> > After:
> >
> >   WARNING: CPU: 0 PID: 0 at [ptr == 0 && 1] kernel/sched/core.c:8511 sched_init+0x20/0x410
> >                             ^^^^^^^^^^^^^^^
> 
> Hmm. Is that the prettiest output ever? No. But it does seem workable,
> and the patch is simple.
> 
> And I think the added condition string is useful, in that I often end
> up looking up warnings that other people report and where the line
> numbers have changed enough that it's not immediately obvious exactly
> which warning it is. Not only does it disambiguate which warning it
> is, it would probably often would obviate having to look it up
> entirely because the warning message is now more useful.

Yeah, that exactly was the original motivation for SCHED_WARN_ON(): 
core kernel code often gets backported on and changed by distributions, 
so line numbers are fuzzy and with large functions it's sometimes 
unclear exactly where the warning originated from.

> So I think I like it. Let's see how it works in practice.
> 
> (I actually think the "CPU: 0 PID: 0" is likely the least useful part 
> of that warning string, and maybe *that* should be moved away and 
> make things a bit more legible, but I think that discussion might as 
> well be part of that "Let's see how it works")

Okay!

The CPU and PID part is particularly useless, given that it's repeated 
in the splat a few lines later:

  ------------[ cut here ]------------^M
  WARNING: CPU: 0 PID: 0 at [ptr == 0 && 1] kernel/sched/core.c:8511 sched_init+0x20/0x410
  Modules linked in:
  CPU: 0 UID: 0 PID: 0 Comm: swapper Not tainted 6.14.0-01616-g94d7af2844aa #4 PREEMPT(undef)
  Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.3-debian-1.16.3-2 04/01/2014
  RIP: 0010:sched_init+0x20/0x410

So I'll just remove it, which will turn this into:

  WARNING: [ptr == 0 && 1] kernel/sched/core.c:8511 sched_init+0x20/0x410

Which is actually pretty nicely formatted IMHO and orders the 
information by expected entropy: most constant, most valuable 
information comes first.

BTW., there's also another option we still have open: by using a unique 
character separator that isn't 0 we could split up the single string 
into cond_str and FILE_str parts, and leave formatting to 
architectures. But I don't think it's needed if we get rid of the "CPU: 
PID:" noise though.

	Ingo