[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <Z-MxULQtc--KoKMW@gmail.com>
Date: Tue, 25 Mar 2025 23:42:24 +0100
From: Ingo Molnar <mingo@...nel.org>
To: Linus Torvalds <torvalds@...ux-foundation.org>
Cc: Peter Zijlstra <peterz@...radead.org>, linux-kernel@...r.kernel.org,
linux-tip-commits@...r.kernel.org,
Shrikanth Hegde <sshegde@...ux.ibm.com>,
Juri Lelli <juri.lelli@...hat.com>,
Vincent Guittot <vincent.guittot@...aro.org>,
Dietmar Eggemann <dietmar.eggemann@....com>,
Steven Rostedt <rostedt@...dmis.org>,
Ben Segall <bsegall@...gle.com>, Mel Gorman <mgorman@...e.de>,
Valentin Schneider <vschneid@...hat.com>, x86@...nel.org
Subject: [PATCH] bug: Add the condition string to the
CONFIG_DEBUG_BUGVERBOSE=y output
* Linus Torvalds <torvalds@...ux-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@...nel.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@...nel.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));
Powered by blists - more mailing lists