mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Ingo Molnar <mingo@kernel.org>
To: Peter Zijlstra <peterz@infradead.org>
Cc: linux-kernel@vger.kernel.org, linux-tip-commits@vger.kernel.org,
	Shrikanth Hegde <sshegde@linux.ibm.com>,
	Juri Lelli <juri.lelli@redhat.com>,
	Vincent Guittot <vincent.guittot@linaro.org>,
	Dietmar Eggemann <dietmar.eggemann@arm.com>,
	Steven Rostedt <rostedt@goodmis.org>,
	Ben Segall <bsegall@google.com>, Mel Gorman <mgorman@suse.de>,
	Valentin Schneider <vschneid@redhat.com>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	x86@kernel.org
Subject: [PATCH] bug: Introduce CONFIG_DEBUG_BUGVERBOSE_EXTRA=y to also log warning conditions
Date: Tue, 25 Mar 2025 12:18:39 +0100	[thread overview]
Message-ID: <Z-KRD3ODxT9f8Yjw@gmail.com> (raw)
In-Reply-To: <Z-J5UEFwM3gh6VXR@gmail.com>


* Ingo Molnar <mingo@kernel.org> wrote:

> > >    #define SCHED_WARN_ON(x)      WARN_ONCE(x, #x)
> > > 
> > > Because SCHED_WARN_ON() would output the 'x' condition
> > > as well, while WARN_ONCE() will only show a backtrace.
> > > 
> > > Hopefully these are rare enough to not really matter.
> > > 
> > > If it does, we should probably introduce a new WARN_ON()
> > > variant that outputs the condition in stringified form,
> > > or improve WARN_ON() itself.
> > 
> > So those strings really were useful, trouble is WARN_ONCE() generates
> > utter crap code compared to WARN_ON_ONCE(), but since SCHED_DEBUG that
> > doesn't really matter.
> 
> Why wouldn't it matter? CONFIG_SCHED_DEBUG was turned on for 99.9999% 
> of Linux users, ie. we generated crap code for most of our users.
> 
> And as a side effect of using the standard WARN_ON_ONCE() primitive we 
> now generate better code, at the expense of harder to interpret debug 
> output, right?
> 
> Ie. CONFIG_SCHED_DEBUG has obfuscated crappy code generation under the 
> "it's only debugging code" pretense, right?

So, to argue this via code, we'd like to have something like the patch below?

When enabled it will warn in the following fashion:

  static void super_perfect_kernel_function(void *ptr)
  {
	...
	WARN_ON_ONCE(ptr == 0 && 1);
	...
  }


  ------------[ cut here ]------------
  FAIL: 'ptr == 0 && 1' is true
  WARNING: CPU: 0 PID: 0 at kernel/sched/core.c:8511 sched_init+0x44/0x430
  ...

But the real question is, how do we keep distros from enabling 
CONFIG_DEBUG_BUGVERBOSE_EXTRA=y?

It does bloat the defconfig by about +144k .text and ~64k data, so 
maybe that's deterrence enough.

The BSS shift is due to it not using the clever x86 U2D tricks, right?

Thanks,

	Ingo

=================>
From: Ingo Molnar <mingo@kernel.org>
Date: Tue, 25 Mar 2025 11:35:20 +0100
Subject: [PATCH] bug: Introduce CONFIG_DEBUG_BUGVERBOSE_EXTRA=y to also log warning conditions

      text         data	    bss	     dec	    hex	filename
  29522704	7926322	1389904	38838930	250a292	vmlinux.before
  29667392	8017958	1363024	39048374	253d4b6	vmlinux.after

Totally-Not-Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 include/asm-generic/bug.h |  7 +++++++
 kernel/sched/core.c       |  2 ++
 lib/Kconfig.debug         | 12 ++++++++++++
 3 files changed, 21 insertions(+)

diff --git a/include/asm-generic/bug.h b/include/asm-generic/bug.h
index 387720933973..5475258a99dc 100644
--- a/include/asm-generic/bug.h
+++ b/include/asm-generic/bug.h
@@ -92,6 +92,11 @@ void warn_slowpath_fmt(const char *file, const int line, unsigned taint,
 		       const char *fmt, ...);
 extern __printf(1, 2) void __warn_printk(const char *fmt, ...);
 
+#ifdef CONFIG_DEBUG_BUGVERBOSE_EXTRA
+#define WARN_ON_ONCE(condition)						\
+	DO_ONCE_LITE_IF(condition, WARN, 1, "FAIL: '%s' is true", #condition)
+#endif
+
 #ifndef __WARN_FLAGS
 #define __WARN()		__WARN_printf(TAINT_WARN, NULL)
 #define __WARN_printf(taint, arg...) do {				\
@@ -107,6 +112,7 @@ extern __printf(1, 2) void __warn_printk(const char *fmt, ...);
 		__WARN_FLAGS(BUGFLAG_NO_CUT_HERE | BUGFLAG_TAINT(taint));\
 		instrumentation_end();					\
 	} while (0)
+#ifndef WARN_ON_ONCE
 #define WARN_ON_ONCE(condition) ({				\
 	int __ret_warn_on = !!(condition);			\
 	if (unlikely(__ret_warn_on))				\
@@ -115,6 +121,7 @@ extern __printf(1, 2) void __warn_printk(const char *fmt, ...);
 	unlikely(__ret_warn_on);				\
 })
 #endif
+#endif
 
 /* used internally by panic.c */
 
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));
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index b1b92a9a8f24..88f215f712f8 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -206,6 +206,18 @@ config DEBUG_BUGVERBOSE
 	  of the BUG call as well as the EIP and oops trace.  This aids
 	  debugging but costs about 70-100K of memory.
 
+config DEBUG_BUGVERBOSE_EXTRA
+	bool "Extra verbose WARN_ON() reporting" if DEBUG_BUGVERBOSE
+	default n
+	help
+	  Say Y here to make WARN_ON() warnings extra verbose, printing
+	  the condition they warn about.
+
+	  This aids debugging but uses up some memory and causes some
+	  runtime overhead due to worse code generation.
+
+	  If unsure, say N.
+
 endmenu # "printk and dmesg options"
 
 config DEBUG_KERNEL


  reply	other threads:[~2025-03-25 11:18 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-03-17 10:42 [PATCH 0/5] sched: Make CONFIG_SCHED_DEBUG features unconditional Ingo Molnar
2025-03-17 10:42 ` [PATCH 1/5] sched/debug: Change SCHED_WARN_ON() to WARN_ON_ONCE() Ingo Molnar
2025-03-20  9:00   ` [tip: sched/core] " tip-bot2 for Ingo Molnar
2025-03-24 11:59     ` Peter Zijlstra
2025-03-25  9:37       ` Ingo Molnar
2025-03-25 11:18         ` Ingo Molnar [this message]
2025-03-25 12:36           ` [PATCH] bug: Introduce CONFIG_DEBUG_BUGVERBOSE_EXTRA=y to also log warning conditions Peter Zijlstra
2025-03-25 17:48             ` Linus Torvalds
2025-03-25 18:46               ` Peter Zijlstra
2025-03-25 22:42               ` [PATCH] bug: Add the condition string to the CONFIG_DEBUG_BUGVERBOSE=y output Ingo Molnar
2025-03-25 23:12                 ` Linus Torvalds
2025-03-26  7:42                   ` Ingo Molnar
2025-03-17 10:42 ` [PATCH 2/5] sched/debug: Make 'const_debug' tunables unconditional __read_mostly Ingo Molnar
2025-03-20  9:00   ` [tip: sched/core] " tip-bot2 for Ingo Molnar
2025-03-17 10:42 ` [PATCH 3/5] sched/debug: Make CONFIG_SCHED_DEBUG functionality unconditional Ingo Molnar
2025-03-20  9:00   ` [tip: sched/core] " tip-bot2 for Ingo Molnar
2025-03-17 10:42 ` [PATCH 4/5] sched/debug, Documentation: Remove (most) CONFIG_SCHED_DEBUG references from documentation Ingo Molnar
2025-03-20  9:00   ` [tip: sched/core] " tip-bot2 for Ingo Molnar
2025-03-17 10:42 ` [PATCH 5/5] sched/debug: Remove CONFIG_SCHED_DEBUG Ingo Molnar
2025-03-20  8:59   ` [tip: sched/core] " tip-bot2 for Ingo Molnar
2025-03-24 11:57     ` Peter Zijlstra
2025-03-17 21:39 ` [PATCH 0/5] sched: Make CONFIG_SCHED_DEBUG features unconditional Linus Torvalds
2025-03-17 22:24   ` Ingo Molnar
2025-03-17 22:42     ` Ingo Molnar
2025-03-19  8:49 ` Valentin Schneider
2025-03-19 21:09   ` Ingo Molnar
2025-03-19 12:48 ` Shrikanth Hegde
2025-03-19 21:14   ` Ingo Molnar
2025-03-20  4:41     ` Shrikanth Hegde
2025-03-20  9:00     ` [tip: sched/core] sched/debug: Remove CONFIG_SCHED_DEBUG from self-test config files tip-bot2 for Ingo Molnar

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Z-KRD3ODxT9f8Yjw@gmail.com \
    --to=mingo@kernel.org \
    --cc=bsegall@google.com \
    --cc=dietmar.eggemann@arm.com \
    --cc=juri.lelli@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-tip-commits@vger.kernel.org \
    --cc=mgorman@suse.de \
    --cc=peterz@infradead.org \
    --cc=rostedt@goodmis.org \
    --cc=sshegde@linux.ibm.com \
    --cc=torvalds@linux-foundation.org \
    --cc=vincent.guittot@linaro.org \
    --cc=vschneid@redhat.com \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®