Re: [PATCH v7 2/5] bug/kunit: Reduce runtime impact of warning backtrace suppression

Albert Esteve
2026-04-21T08:41:32Z

On Mon, Apr 20, 2026 at 4:45 PM Peter Zijlstra <peterz@infradead.org> wrote:
>
> On Mon, Apr 20, 2026 at 02:28:04PM +0200, Albert Esteve wrote:
> > From: Alessandro Carminati <acarmina@redhat.com>
> >
> > KUnit support is not consistently present across distributions, some
> > include it in their stock kernels, while others do not.
> > While both KUNIT and KUNIT_SUPPRESS_BACKTRACE can be considered debug
> > features, the fact that some distros ship with KUnit enabled means it's
> > important to minimize the runtime impact of this patch.
> >
> > To that end, this patch adds an atomic counter that tracks the number
> > of active suppressions. __kunit_is_suppressed_warning() checks this
> > counter first and returns immediately when no suppressions are active,
> > avoiding RCU-protected list traversal in the common case.
> >
> > Signed-off-by: Alessandro Carminati <acarmina@redhat.com>
> > Signed-off-by: Albert Esteve <aesteve@redhat.com>
> > ---
> > lib/kunit/bug.c | 7 +++++++
> > 1 file changed, 7 insertions(+)
> >
> > diff --git a/lib/kunit/bug.c b/lib/kunit/bug.c
> > index 356c8a5928828..a7a88f0670d44 100644
> > --- a/lib/kunit/bug.c
> > +++ b/lib/kunit/bug.c
> > @@ -8,6 +8,7 @@
> >
> > #include <kunit/bug.h>
> > #include <kunit/resource.h>
> > +#include <linux/atomic.h>
> > #include <linux/export.h>
> > #include <linux/rculist.h>
> > #include <linux/sched.h>
> > @@ -15,11 +16,13 @@
> > #ifdef CONFIG_KUNIT_SUPPRESS_BACKTRACE
> >
> > static LIST_HEAD(suppressed_warnings);
> > +static atomic_t suppressed_warnings_cnt = ATOMIC_INIT(0);
> >
> > static void __kunit_suppress_warning_remove(struct __suppressed_warning *warning)
> > {
> > list_del_rcu(&warning->node);
> > synchronize_rcu(); /* Wait for readers to finish */
> > + atomic_dec(&suppressed_warnings_cnt);
> > }
> >
> > KUNIT_DEFINE_ACTION_WRAPPER(__kunit_suppress_warning_cleanup,
> > @@ -37,6 +40,7 @@ __kunit_start_suppress_warning(struct kunit *test)
> > return NULL;
> >
> > warning->task = current;
> > + atomic_inc(&suppressed_warnings_cnt);
> > list_add_rcu(&warning->node, &suppressed_warnings);
> >
> > ret = kunit_add_action_or_reset(test,
> > @@ -68,6 +72,9 @@ bool __kunit_is_suppressed_warning(void)
> > {
> > struct __suppressed_warning *warning;
> >
> > + if (!atomic_read(&suppressed_warnings_cnt))
> > + return false;
> > +
> > rcu_read_lock();
> > list_for_each_entry_rcu(warning, &suppressed_warnings, node) {
> > if (warning->task == current) {
> >
>
> So the thing you're skipping is:
>
> rcu_read_lock();
> list_for_each_entry_rcu() {
> }
> rcu_read_unlock();
>
> Which is really cheap. Did you actually have performance numbers for
> this?

No, I do not have performance numbers. I kept the counter and the
separate patch for consistency with the previous version of the
series. But you have a good point, the skipped part is really cheap.

>
> A possibly better option is to add a static_branch() that could elide
> any and all memory access.
>

Previous version had static_branch and I removed it because I
understood from the discussion that the gains would not be significant
as performance gains are irrelevant in warn slowpath. But I think it
would make sense for a disabled feature. I will rework this for the
next version, remove the counter and use static_branch as suggested.