Re: [PATCH] irq: Make refcount_interrupt kunit test selectable

[Radu Rendec <radu@rendec.net>]

On Sun, 2026-08-23 at 14:08 -0700, Boqun Feng wrote:
> On Sun, Aug 23, 2026 at 04:46:23PM -0400, Radu Rendec wrote:
> > On Sun, 2026-08-23 at 17:17 +0000, Kuan-Wei Chiu wrote:
> > > Currently, refcount_interrupt_test is built unconditionally when
> > > CONFIG_KUNIT is enabled, causing it to run unexpectedly during boot.
> > > 
> > > Fix this by introducing CONFIG_REFCOUNT_INTERRUPT_KUNIT_TEST so the
> > > test can be configured independently, following standard kunit
> > > practices.
> > > 
> > > Fixes: 07a88e2bcd5b ("irq: Add KUnit test for refcounted interrupt enable/disable")
> > > Signed-off-by: Kuan-Wei Chiu <visitorckw@gmail.com>
> > > ---
> > >  kernel/irq/Kconfig  | 12 ++++++++++++
> > >  kernel/irq/Makefile |  2 +-
> > >  2 files changed, 13 insertions(+), 1 deletion(-)
> > 
> > Can someone please explain me why this needs to be a separate unit test
> > in the first place and can't be integrated into the (previously)
> > existing unit test gated by IRQ_KUNIT_TEST?
> > 
> 
> If you look at the irq_test.c, you will find that it primarily tests the
> {request,disable,enable}_irq() API (i.e. allocating IRQ vectors and
> enabling and disabling them), which is not the same scope as
> local_interrupt_{disable,enable}() (i.e. operating the CPU irq disabling
> status), so it makes sense to have a separate test case.

Thanks! If we want to keep the unit tests separated/grouped by scope,
then yes, it makes sense for sure. What I don't like about this is:
 * The naming of the two config options is inconsistent: IRQ_KUNIT_TEST
   and REFCOUNT_INTERRUPT_KUNIT_TEST.
 * The naming and description of the older test become misleading. The
   way they are formulated, I would expect them to test "everything
   related to IRQ" but now we have a separate option (and kunit test)
   dedicated to the refcount stuff.

If they must be separated/grouped by scope, I would find it much nicer
and less confusing if they were called something like:
IRQ_MANAGEMENT_KUNIT_TEST and
IRQ_REFCOUNT_KUNIT_TEST,
and also the corresponding .c files were somewhat consistent with the
Kconfig option names. The description of the older test would have to
be updated to make it clear that it's not just "everything IRQ".

In that scenario, new kunit test(s) related to different aspects of the
IRQ subsystem would get their own Kconfig option and file, and would
hopefully follow the same naming pattern.

To be clear, I'm not arguing that keeping the tests separate is wrong.
I'm arguing that if we do that, we should also make some adjustments to
keep everything clean and tidy.

> But yes, I agree Kuan-Wei's fix is needed.

Yes, I agree, those unit tests must not be gated by KUNIT alone, so
something must be done to fix it.

> > Ironically, both the issue that this patch is trying to fix and my
> > question were raised by sashiko here:
> > https://lore.kernel.org/all/20260605055315.3E96E1F00893@smtp.kernel.org/
> > 
> > > diff --git a/kernel/irq/Kconfig b/kernel/irq/Kconfig
> > > index 05cba4e16dad..6923f37eaab4 100644
> > > --- a/kernel/irq/Kconfig
> > > +++ b/kernel/irq/Kconfig
> > > @@ -150,6 +150,18 @@ config IRQ_KUNIT_TEST
> > >  
> > >     If unsure, say N.
> > >  
> > > +config REFCOUNT_INTERRUPT_KUNIT_TEST
> > > + tristate "Test refcounted interrupt enable/disable" if !KUNIT_ALL_TESTS
> > > + depends on KUNIT
> > > + default KUNIT_ALL_TESTS
> > > + help
> > > +   This builds the kunit tests for the refcounted interrupt
> > > +   infrastructure. It verifies the correctness of single, nested,
> > > +   and multiple interrupt enable/disable state changes and ensures
> > > +   that the underlying reference counting mechanisms work as expected.
> > > +
> > > +   If unsure, say N.
> > > +
> > >  endmenu
> > >  
> > >  config GENERIC_IRQ_MULTI_HANDLER
> > > diff --git a/kernel/irq/Makefile b/kernel/irq/Makefile
> > > index 44c4d6fc502a..0e5df962a149 100644
> > > --- a/kernel/irq/Makefile
> > > +++ b/kernel/irq/Makefile
> > > @@ -16,4 +16,4 @@ obj-$(CONFIG_SMP) += affinity.o
> > >  obj-$(CONFIG_GENERIC_IRQ_DEBUGFS) += debugfs.o
> > >  obj-$(CONFIG_GENERIC_IRQ_MATRIX_ALLOCATOR) += matrix.o
> > >  obj-$(CONFIG_IRQ_KUNIT_TEST) += irq_test.o
> > > -obj-$(CONFIG_KUNIT) += refcount_interrupt_test.o
> > > +obj-$(CONFIG_REFCOUNT_INTERRUPT_KUNIT_TEST) += refcount_interrupt_test.o

-- 
Best regards,
Radu