Hi Peter,
Thank you for your follow-up and for reiterating your point.
On Thu, May 29, 2025 at 11:01 AM Peter Zijlstra peterz@infradead.org wrote:
On Mon, May 26, 2025 at 01:27:51PM +0000, Alessandro Carminati wrote:
#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));\instrumentation_end(); \
if (!KUNIT_IS_SUPPRESSED_WARNING(__func__)) { \instrumentation_begin(); \__warn_printk(arg); \__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)) \
if (unlikely(__ret_warn_on) && !KUNIT_IS_SUPPRESSED_WARNING(__func__)) \ __WARN_FLAGS(BUGFLAG_ONCE | \ BUGFLAG_TAINT(TAINT_WARN)); \ unlikely(__ret_warn_on); \@@ -121,7 +130,7 @@ extern __printf(1, 2) void __warn_printk(const char *fmt, ...); #ifndef WARN_ON #define WARN_ON(condition) ({ \ int __ret_warn_on = !!(condition); \
if (unlikely(__ret_warn_on)) \
if (unlikely(__ret_warn_on) && !KUNIT_IS_SUPPRESSED_WARNING(__func__)) \ __WARN(); \ unlikely(__ret_warn_on); \}) @@ -138,7 +147,7 @@ extern __printf(1, 2) void __warn_printk(const char *fmt, ...);
#define WARN_TAINT(condition, taint, format...) ({ \ int __ret_warn_on = !!(condition); \
if (unlikely(__ret_warn_on)) \
if (unlikely(__ret_warn_on) && !KUNIT_IS_SUPPRESSED_WARNING(__func__)) \ __WARN_printf(taint, format); \ unlikely(__ret_warn_on); \}) @@ -157,8 +166,10 @@ extern __printf(1, 2) void __warn_printk(const char *fmt, ...); #else /* !CONFIG_BUG */ #ifndef HAVE_ARCH_BUG #define BUG() do { \
do {} while (1); \unreachable(); \
if (!KUNIT_IS_SUPPRESSED_WARNING(__func__)) { \do {} while (1); \unreachable(); \} \} while (0) #endif
NAK
This is again doing it wrong -- this will bloat every frigging bug/warn site for no reason.
Like I said before; you need to do this on the report_bug() size of things.
I fully understand your concerns, and I truly appreciate both yours and Josh’s feedback on this matter. Please rest assured that I took your suggestions seriously and carefully evaluated the possibility of consolidating all related logic within the exception handler. After a thorough investigation, however, I encountered several limitations that led me to maintain the check in the macro. I’d like to share the rationale behind this decision: * In the case of WARN() messages, part of the output, the user-specified content, is emitted directly by the macro, prior to reaching the exception handler [1]. Moving the check solely to the exception handler would not prevent this early output. * Unless we change the user-facing interface that allows suppression based on function names, we still need to work with those names at runtime. * This leaves us with two main strategies: converting function names to pointers (e.g., via kallsyms) or continuing to work with names. The former requires name resolution at suppression time and pointer comparison in the handler, but function names are often altered by the compiler due to inlining or other optimizations[2]. Some WARN() sites are even marked __always_inline[3], making it difficult to prevent inlining altogether. * An alternative is to embed function names in the __bug_table. While potentially workable, this increases the size of the table and requires attention to handle position-independent builds (-fPIC/-fPIE), such as using offsets relative to __start_rodata.
However, the central challenge remains: any logic that aims to suppress WARN() output must either move the entire message emission into the exception handler or accept that user-specified parts of the message will still be printed. As a secondary point, there are also less common architectures where it's unclear whether suppressing these warnings is a priority, which might influence how broadly the effort is applied. I hoped to have addressed the concern of having faster runtime, by exposing a counter that could skip the logic. Kess suggested using static branching that would make things even better. Could Kess' suggestion mitigate your concern on this strategy? I’m absolutely open to any further thoughts or suggestions you may have, and I appreciate your continued guidance.
[1]. https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/incl... [2]. https://godbolt.org/z/d8aja1Wfz Compiler here emits inlined function and stand alone function to allow pointer usage. [3]. https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/incl... this is one example, others exist.