Re: [PATCH v7 1/5] bug/kunit: Core support for suppressing warning backtraces

[Albert Esteve]

On Wed, Apr 22, 2026 at 2:21 PM David Gow <david@xxxxxxxxxxxx> wrote:
>
> Thanks very much for keeping this series alive! I'm very much in favour
> of it, and I think the overall design is good. Lots of more detailed
> nitpicks below, though.
>

Hi David,

Thank you for taking the time to review the series!

> Le 20/04/2026 à 8:28 PM, Albert Esteve a écrit :
> > From: Alessandro Carminati <acarmina@xxxxxxxxxx>
> >
> > Some unit tests intentionally trigger warning backtraces by passing bad
> > parameters to kernel API functions. Such unit tests typically check the
> > return value from such calls, not the existence of the warning backtrace.
> >
> > Such intentionally generated warning backtraces are neither desirable
> > nor useful for a number of reasons:
> > - They can result in overlooked real problems.
> > - A warning that suddenly starts to show up in unit tests needs to be
> >    investigated and has to be marked to be ignored, for example by
> >    adjusting filter scripts. Such filters are ad hoc because there is
> >    no real standard format for warnings. On top of that, such filter
> >    scripts would require constant maintenance.
> >
> > Solve the problem by providing a means to identify and suppress specific
> > warning backtraces while executing test code. Support suppressing multiple
> > backtraces while at the same time limiting changes to generic code to the
> > absolute minimum.
>
> It sounds from the description here that suppressing "specific
> backtraces" means that we're matching on the _contents_ of the stack
> trace. This sort-of does that implicitly by checking they're in the same
> kthread, but I think the fact that it's matching based on kthread should
> be explicit in the commit message, like it is in the documentation.
>

You are right. Previous version was based on function name matches. I
mostly fixed all commit messages, but this section remained. I will
rewrite it.

> >
> > Implementation details:
> > Suppression is checked at two points in the warning path:
> > - In warn_slowpath_fmt(), the check runs before any output, fully
> >    suppressing both message and backtrace.
> > - In __report_bug(), the check runs before __warn() is called,
> >    suppressing the backtrace and stack dump. Note that on this path,
> >    the WARN() format message may still appear in the kernel log since
> >    __warn_printk() runs before the trap that enters __report_bug().
>
> Would it make sense to output a 'backtrace suppressed due to running
> test' message in this latter case, so we don't just end up with the
> WARN() format message by itself? (My gut feeling is 'no, it isn't worth
> it', but it's food for thought.)
>

I had the same thought and landed on the same conclusion. It would
also become moot if we add the suppression check to __warn_printk() as
discussed with Peter, since the message would be fully suppressed on
that path too.

> >
> > A helper function, `__kunit_is_suppressed_warning()`, walks an
> > RCU-protected list of active suppressions, matching by current task.
> > The suppression state is tied to the KUnit test lifecycle via
> > kunit_add_action(), ensuring automatic cleanup at test exit.
> >
> > The list of suppressed warnings is protected with RCU to allow
> > concurrent read access without locks.
> >
> > The implementation is deliberately simple and avoids architecture-specific
> > optimizations to preserve portability.
> >
> > Signed-off-by: Guenter Roeck <linux@xxxxxxxxxxxx>
> > Signed-off-by: Alessandro Carminati <acarmina@xxxxxxxxxx>
> > Reviewed-by: Kees Cook <kees@xxxxxxxxxx>
> > Signed-off-by: Albert Esteve <aesteve@xxxxxxxxxx>
> > ---
> >   include/kunit/bug.h  | 56 +++++++++++++++++++++++++++++++++++
> >   include/kunit/test.h |  1 +
> >   kernel/panic.c       |  8 ++++-
> >   lib/bug.c            |  8 +++++
> >   lib/kunit/Kconfig    |  9 ++++++
> >   lib/kunit/Makefile   |  6 ++--
> >   lib/kunit/bug.c      | 84 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> >   7 files changed, 169 insertions(+), 3 deletions(-)
> >
> > diff --git a/include/kunit/bug.h b/include/kunit/bug.h
> > new file mode 100644
> > index 0000000000000..e52c9d21d9fe6
> > --- /dev/null
> > +++ b/include/kunit/bug.h
>
> It's a bit confusing to name this bug.h when we have the (admittedly
> terribly-named) test-bug.h header already. I'm pretty tempted to rename
> the latter to something like 'hooks.h', as that's really what it's for,
> and having a separate bug.h would be an incentive to do so, though, sit
> it's not a big problem.
>
> I do think that it'd be reasonable to include the backtrace suppression
> tuff in the same file, though, if you'd rather. The
> __kunit_is_suppressed_warning() stuff in particular fits the category of
> "code called to change behaviour based on whether or not a test is
> running", which is generally what the hooks are for. (And, if you'd
> rather, there's a bunch of existing hooks and hook infrastructure you
> could use.)

I wasn't aware of the hooks infrastructure! I was so focused on the
solution carried over from initial versions that I did not seek
alternatives.

Honestly, that's a much better fit -- I'll integrate it into
test-bug.h for next version.

>
> > @@ -0,0 +1,56 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +/*
> > + * KUnit helpers for backtrace suppression
> > + *
> > + * Copyright (C) 2025 Alessandro Carminati <acarmina@xxxxxxxxxx>
> > + * Copyright (C) 2024 Guenter Roeck <linux@xxxxxxxxxxxx>
> > + */
> > +
> > +#ifndef _KUNIT_BUG_H
> > +#define _KUNIT_BUG_H
> > +
> > +#ifndef __ASSEMBLY__
> > +
> > +#include <linux/kconfig.h>
> > +
> > +struct kunit;
> > +
> > +#ifdef CONFIG_KUNIT_SUPPRESS_BACKTRACE
> > +
> > +#include <linux/types.h>
> > +
> > +struct task_struct;
> > +
> > +struct __suppressed_warning {
> > +     struct list_head node;
> > +     struct task_struct *task;
> > +     int counter;
> > +};
> > +
> > +struct __suppressed_warning *
> > +__kunit_start_suppress_warning(struct kunit *test);
> > +void __kunit_end_suppress_warning(struct kunit *test,
> > +                               struct __suppressed_warning *warning);
> > +int __kunit_suppressed_warning_count(struct __suppressed_warning *warning);
> > +bool __kunit_is_suppressed_warning(void);
> > +
> > +#define KUNIT_START_SUPPRESSED_WARNING(test) \
> > +     struct __suppressed_warning *__kunit_suppress = \
> > +             __kunit_start_suppress_warning(test)
> > +
> > +#define KUNIT_END_SUPPRESSED_WARNING(test) \
> > +     __kunit_end_suppress_warning(test, __kunit_suppress)
> > +
> > +#define KUNIT_SUPPRESSED_WARNING_COUNT() \
> > +     __kunit_suppressed_warning_count(__kunit_suppress)
>
> Using a local variable (__kunit_suppress) here means that all of the
> above macros must live in the same function. This is probably okay for
> most use-cases, but more complicated tests may have to structure things
> carefully. It also prevents there from being multiple START/END pairs in
> the same function, and the KUNIT_SUPPRESSED_WARNING_COUNT() macro from
> appearing before _START().
>
> It also makes it less obvious that this cleans up nicely if the test
> exits uncleanly, as the variable will have gone out-of-scope. (But given
> we're just storing a pointer to heap-allocated memory, and
> kunit_add_action() is used, it should be okay.)
>
> One other option would be to allocate the suppression as a named
> resource, which could then be retrieved from anywhere within the test
> with kunit_find_named_resource(). (Though handling nested suppressions
> gets a little more complicated here.)
>
> Or you could make the struct __suppressed_warning pointer returned
> explicitly user-visible, and let the user pass it around (though that
> seems more work).
>

Good points. The function matching approach used the function name to
allow concurrent suppressions to coexist. This is a limitation of the
current approach that I traded off for simplicity (without properly
considering the more complicated scenarios). I'll add a name parameter
to the macros (or expose the pointer directly) so multiple pairs and
cross-function usage work naturally. I think I lean more towards
making the return value user-visible? But I will decide based on how
the API evolves, especially if I change to the scoped approach
suggested in a different thread.

>
> > +
> > +#else /* CONFIG_KUNIT_SUPPRESS_BACKTRACE */
> > +
> > +#define KUNIT_START_SUPPRESSED_WARNING(test)
> > +#define KUNIT_END_SUPPRESSED_WARNING(test)
> > +#define KUNIT_SUPPRESSED_WARNING_COUNT() 0
> > +static inline bool __kunit_is_suppressed_warning(void) { return false; }
> > +
> > +#endif /* CONFIG_KUNIT_SUPPRESS_BACKTRACE */
> > +#endif /* __ASSEMBLY__ */
> > +#endif /* _KUNIT_BUG_H */
> > diff --git a/include/kunit/test.h b/include/kunit/test.h
> > index 9cd1594ab697d..4ec07b3fa0204 100644
> > --- a/include/kunit/test.h
> > +++ b/include/kunit/test.h
> > @@ -10,6 +10,7 @@
> >   #define _KUNIT_TEST_H
> >
> >   #include <kunit/assert.h>
> > +#include <kunit/bug.h>
> >   #include <kunit/try-catch.h>
> >
> >   #include <linux/args.h>
> > diff --git a/kernel/panic.c b/kernel/panic.c
> > index c78600212b6c1..d7a7a679f56c4 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())
> > +             return;
> > +
> > +     rcu = warn_rcu_enter();
> > +
> >       pr_warn(CUT_HERE);
> >
> >       if (!fmt) {
> > diff --git a/lib/bug.c b/lib/bug.c
> > index 623c467a8b76c..606205c8c302f 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,13 @@ 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;
> >
> > +     /*
> > +      * Before the once logic so suppressed warnings do not consume
> > +      * the single-fire budget of WARN_ON_ONCE().
> > +      */
> > +     if (warning && __kunit_is_suppressed_warning())
> > +             return BUG_TRAP_TYPE_WARN;
> > +
>
> While any competant optimiser should get rid of this entirely, it might
> be clearer to anyone reading it that this disappears if we just put it
> behind an #ifdef?

Makes sense. Will do.

>
> >       if (warning && once) {
> >               if (done)
> >                       return BUG_TRAP_TYPE_WARN;
> > diff --git a/lib/kunit/Kconfig b/lib/kunit/Kconfig
> > index 498cc51e493dc..57527418fcf09 100644
> > --- a/lib/kunit/Kconfig
> > +++ b/lib/kunit/Kconfig
> > @@ -15,6 +15,15 @@ menuconfig KUNIT
> >
> >   if KUNIT
> >
> > +config KUNIT_SUPPRESS_BACKTRACE
> > +     bool "KUnit - Enable backtrace suppression"
> > +     default y
> > +     help
> > +       Enable backtrace suppression for KUnit. If enabled, backtraces
> > +       generated intentionally by KUnit tests are suppressed. Disable
> > +       to reduce kernel image size if image size is more important than
> > +       suppression of backtraces generated by KUnit tests.
> > +
> >   config KUNIT_DEBUGFS
> >       bool "KUnit - Enable /sys/kernel/debug/kunit debugfs representation" if !KUNIT_ALL_TESTS
> >       default KUNIT_ALL_TESTS
> > diff --git a/lib/kunit/Makefile b/lib/kunit/Makefile
> > index 656f1fa35abcc..fe177ff3ebdef 100644
> > --- a/lib/kunit/Makefile
> > +++ b/lib/kunit/Makefile
> > @@ -16,8 +16,10 @@ ifeq ($(CONFIG_KUNIT_DEBUGFS),y)
> >   kunit-objs +=                               debugfs.o
> >   endif
> >
> > -# KUnit 'hooks' are built-in even when KUnit is built as a module.
> > -obj-$(if $(CONFIG_KUNIT),y) +=               hooks.o
> > +# KUnit 'hooks' and bug handling are built-in even when KUnit is built
> > +# as a module.
> > +obj-$(if $(CONFIG_KUNIT),y) +=               hooks.o \
> > +                                     bug.o
>
> Is there any reason we couldn't implement this on top of the hooks
> mechanism? Then we could include the bug suppression code in the
> kunit.ko module (albeit, with fewer possibilities for the compiler to
> optimise things, as they'd have to go through an indirect pointer).
>

No reason. I'll integrate this feature into the hooks infrastructure.
Thanks for the suggestion!

>
> >
> >   obj-$(CONFIG_KUNIT_TEST) +=         kunit-test.o
> >   obj-$(CONFIG_KUNIT_TEST) +=         platform-test.o
> > diff --git a/lib/kunit/bug.c b/lib/kunit/bug.c
> > new file mode 100644
> > index 0000000000000..356c8a5928828
> > --- /dev/null
> > +++ b/lib/kunit/bug.c
> > @@ -0,0 +1,84 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * KUnit helpers for backtrace suppression
> > + *
> > + * Copyright (C) 2025 Alessandro Carminati <acarmina@xxxxxxxxxx>
> > + * Copyright (C) 2024 Guenter Roeck <linux@xxxxxxxxxxxx>
> > + */
> > +
> > +#include <kunit/bug.h>
> > +#include <kunit/resource.h>
> > +#include <linux/export.h>
> > +#include <linux/rculist.h>
> > +#include <linux/sched.h>
> > +
> > +#ifdef CONFIG_KUNIT_SUPPRESS_BACKTRACE
> > +
> > +static LIST_HEAD(suppressed_warnings);
> > +
> > +static void __kunit_suppress_warning_remove(struct __suppressed_warning *warning)
> > +{
> > +     list_del_rcu(&warning->node);
> > +     synchronize_rcu(); /* Wait for readers to finish */
> > +}
> > +
> > +KUNIT_DEFINE_ACTION_WRAPPER(__kunit_suppress_warning_cleanup,
> > +                         __kunit_suppress_warning_remove,
> > +                         struct __suppressed_warning *);
> > +
> > +struct __suppressed_warning *
> > +__kunit_start_suppress_warning(struct kunit *test)
> > +{
> > +     struct __suppressed_warning *warning;
> > +     int ret;
> > +
> > +     warning = kunit_kzalloc(test, sizeof(*warning), GFP_KERNEL);
> > +     if (!warning)
> > +             return NULL;
> > +
> > +     warning->task = current;
> > +     list_add_rcu(&warning->node, &suppressed_warnings);
> > +
> > +     ret = kunit_add_action_or_reset(test,
> > +                                     __kunit_suppress_warning_cleanup,
> > +                                     warning);
> > +     if (ret)
> > +             return NULL;
> > +
> > +     return warning;
> > +}
> > +EXPORT_SYMBOL_GPL(__kunit_start_suppress_warning);
> > +
> > +void __kunit_end_suppress_warning(struct kunit *test,
> > +                               struct __suppressed_warning *warning)
> > +{
> > +     if (!warning)
> > +             return;
> > +     kunit_release_action(test, __kunit_suppress_warning_cleanup, warning);
> > +}
> > +EXPORT_SYMBOL_GPL(__kunit_end_suppress_warning);
> > +
> > +int __kunit_suppressed_warning_count(struct __suppressed_warning *warning)
> > +{
> > +     return warning ? warning->counter : 0;
> > +}
> > +EXPORT_SYMBOL_GPL(__kunit_suppressed_warning_count);
> > +
> > +bool __kunit_is_suppressed_warning(void)
> > +{
> > +     struct __suppressed_warning *warning;
> > +
> > +     rcu_read_lock();
> > +     list_for_each_entry_rcu(warning, &suppressed_warnings, node) {
> > +             if (warning->task == current) {
> > +                     warning->counter++;
> > +                     rcu_read_unlock();
> > +                     return true;
> > +             }
> > +     }
> > +     rcu_read_unlock();
> > +
> > +     return false;
> > +}
>
> Note to self: this is _not_ exported, as bug.c is being built-in
> regardless of whether or not KUnit is a module. If we used the hook
> system, it could live in kunit.ko, and would be manually exported by
> kunit_install_hooks()
>
> > +
> > +#endif /* CONFIG_KUNIT_SUPPRESS_BACKTRACE */
> >
>
>
> Thanks again,
> -- David
>