Hi Peter,
On Tue, Mar 17, 2026 at 12:33 PM Peter Zijlstra <[email protected]> wrote:
>
> On Tue, Mar 17, 2026 at 12:20:26PM +0100, Vlastimil Babka (SUSE) wrote:
> > > For this iteration, the `__report_bug()` centralized approach was
> > > revisited after the discussion in the previous version [1].
> >
> > Discussion with PeterZ, who is not CC'd here? (did it now for my reply).
(Sorry not to put you on CC, I thought auto-cc would pick you up already)
> >
> > > However, again this approach did not work because:
> > > - Some warning output is generated directly in the macros before calling
> > > the centralized functions (e.g., `__warn_printk()` in `__WARN_printf()`)
> > > - Functions in the warning path like `warn_slowpath_fmt()` are marked
> > > `__always_inline`, making it difficult to intercept early enough
> > > - So, by the time `__report_bug()` is called, output has already been
> > > written
> > > to the console, making suppression ineffective
> > >
> > > Current Proposal: Check Directly in the `WARN()` Macros.
> > > This avoids the need for function symbol resolution or ELF section
> > > modification.
> > > Suppression is implemented directly in the `WARN*()` macros.
> >
> > So does that bloat every warn/bug site (as Peter objected to) or not?
> > And is it compatible with x86? I see you modify include/asm-generic/bug.h
> > but x86 has its own version of e.g. __WARN_printf ?
>
> Yeah, they done it all wrong again :-(
>
> This should be pushed inside __report_bug() through __WARN_printf with a
> new BUGFLAG thing.
Thanks for the specific suggestion to use BUGFLAG, but I do not think
this is what we need. These flags seem to be used statically, but the
goal is to enable and disable these warning suppressions dynamically
in the tests. Happy to be corrected if I missed something.
I want to highlight that I did not bluntly ignore your point in the
last version. I thoroughly read the discussions there, and as I
mentioned in the cover letter, I tried to move away from the per-macro
approach. However, while doing that, I encountered the same issues
Alessandro found in the last iteration. I’ll address that below, but I
decided to give this approach one more try and tackle the raised
drawbacks.
The main concern was the increased size of the code emitted by the
WARN*() macros, which was a fair point. I tried to alleviate this with
a static key and branching, and addressed other concerns by using RCU
protection on the list for thread safety. I think it was worth a try,
as all options seemed to have their own tradeoffs. However, if this
option remains unacceptable even after trying to diminish its
drawbacks, I am happy to focus on finding the best solution for the
centralised approach, and keeping the discussion constructive.
So back to my test on this. Alessandro detailed two strategies in the
last version, one of them (storing the function name in `struct
bug_entry`) was already used and discarded in older iterations of this
series. So let's focus on the other strategy, using 'kallsyms`. Let's
assume we still store the function name when registering a new symbol
to suppress. Otherwise, we might need to check address ranges to
ensure bugaddr is within the function's scope, which sounds trickier?
With `kallsyms` we can infer the originating functionid. But this
approach works unreliably with compiler-induced transformations (e.g.,
inlining, cloning, code fragmentation). And we still cannot prevent
all output. Additionally, we would need to prevent prints in
`warn_slowpath_fmt()`. There may be other `printk`s embedded in the
macros, but let's focus on suppressing all warnings as a best effort.
It would already improve the quality of life for testers.
Considering these remaining issues, I managed to create a centralised
proposal. Please find the main changes at the bottom of this message.
But again, even with these, the solution remains unreliable. We can
mitigate this by registering the test name on the suppression list (at
least, I can make the new test in this series pass with that). Not
ideal, but we could mention it in the documentation. Something like
"Suppression is matched by the function where the warning is reported.
If the warning is triggered from a helper (or the compiler inlines it
into the test), that function name may differ. In that case, register
and start suppression for both the test and the helper so the test
passes regardless of inlining."
Would that be a more acceptable solution? Is there a better option I
am not seeing?
BR,
Albert.
diff --git a/kernel/panic.c b/kernel/panic.c
index c78600212b6c1..3cb004a7803f4 100644
--- a/kernel/panic.c
+++ b/kernel/panic.c
@@ -39,6 +39,7 @@
#include <linux/sys_info.h>
#include <trace/events/error_report.h>
#include <asm/sections.h>
+#include <kunit/bug.h>
#define PANIC_TIMER_STEP 100
#define PANIC_BLINK_SPD 18
@@ -1080,9 +1081,14 @@ void __warn(const char *file, int line, void
*caller, unsigned taint,
void warn_slowpath_fmt(const char *file, int line, unsigned taint,
const char *fmt, ...)
{
- bool rcu = warn_rcu_enter();
+ bool rcu;
struct warn_args args;
+ if (__kunit_is_suppressed_warning_at((unsigned
long)__builtin_return_address(0)))
+ return;
+
+ rcu = warn_rcu_enter();
+
pr_warn(CUT_HERE);
if (!fmt) {
diff --git a/lib/bug.c b/lib/bug.c
index 623c467a8b76c..e90b038d9225e 100644
--- a/lib/bug.c
+++ b/lib/bug.c
@@ -48,6 +48,7 @@
#include <linux/rculist.h>
#include <linux/ftrace.h>
#include <linux/context_tracking.h>
+#include <kunit/bug.h>
extern struct bug_entry __start___bug_table[], __stop___bug_table[];
@@ -223,6 +224,9 @@ static enum bug_trap_type __report_bug(struct
bug_entry *bug, unsigned long buga
no_cut = bug->flags & BUGFLAG_NO_CUT_HERE;
has_args = bug->flags & BUGFLAG_ARGS;
+ if (warning && __kunit_is_suppressed_warning_at(bugaddr))
+ return BUG_TRAP_TYPE_WARN;
+
if (warning && once) {
if (done)
return BUG_TRAP_TYPE_WARN;
diff --git a/lib/kunit/bug.c b/lib/kunit/bug.c
index 9c2c4ee013d92..13ffddb044636 100644
--- a/lib/kunit/bug.c
+++ b/lib/kunit/bug.c
@@ -11,6 +11,7 @@
#include <linux/export.h>
#include <linux/instrumentation.h>
#include <linux/jump_label.h>
+#include <linux/kallsyms.h>
#include <linux/rculist.h>
#include <linux/string.h>
@@ -68,4 +69,16 @@ noinstr bool __kunit_is_suppressed_warning(const
char *function)
}
EXPORT_SYMBOL_GPL(__kunit_is_suppressed_warning);
+bool __kunit_is_suppressed_warning_at(unsigned long addr)
+{
+ char buf[KSYM_SYMBOL_LEN];
+
+ if (!static_branch_unlikely(&kunit_suppress_warnings_key))
+ return false;
+ if (sprint_symbol_no_offset(buf, addr) <= 0)
+ return false;
+ return __kunit_check_suppress(buf);
+}
+EXPORT_SYMBOL_GPL(__kunit_is_suppressed_warning_at);
+
#endif /* CONFIG_KUNIT_SUPPRESS_BACKTRACE */
>
> So NAK from me on this -- again!
>
