* [PATCH v4] genirq: Add kunit tests for depth counts
@ 2025-05-22 21:08 Brian Norris
2025-06-13 13:34 ` [tip: irq/core] " tip-bot2 for Brian Norris
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Brian Norris @ 2025-05-22 21:08 UTC (permalink / raw)
To: Thomas Gleixner
Cc: Tsai Sung-Fu, Douglas Anderson, linux-kernel, Brian Norris
There have been a few bugs and/or misunderstandings about the reference
counting, and startup/shutdown behaviors in the IRQ core and related CPU
hotplug code. These 4 test cases try to capture a few interesting cases.
* irq_disable_depth_test: basic request/disable/enable sequence
* irq_free_disabled_test: request/disable/free/re-request sequence -
this catches errors on previous revisions of my work
* irq_cpuhotplug_test: exercises managed-affinity IRQ + CPU hotplug.
This captures a problematic test case that I've fixed.
This test requires CONFIG_SMP and a hotpluggable CPU#1.
* irq_shutdown_depth_test: exercises similar behavior from
irq_cpuhotplug_test, but directly using irq_*() APIs instead of going
through CPU hotplug. This still requires CONFIG_SMP, because
managed-affinity is stubbed out (and not all APIs are even present)
without it.
Note the use of 'imply SMP': ARCH=um doesn't support SMP, and kunit is
often exercised there. Thus, 'imply' will force SMP on where possible
(such as ARCH=x86_64), but leave it off where it's not.
Behavior on various SMP and ARCH configurations:
$ tools/testing/kunit/kunit.py run 'irq_test_cases*' --arch x86_64 --qemu_args '-smp 2'
[...]
[11:12:24] Testing complete. Ran 4 tests: passed: 4
$ tools/testing/kunit/kunit.py run 'irq_test_cases*' --arch x86_64
[...]
[11:13:27] [SKIPPED] irq_cpuhotplug_test
[11:13:27] ================= [PASSED] irq_test_cases ==================
[11:13:27] ============================================================
[11:13:27] Testing complete. Ran 4 tests: passed: 3, skipped: 1
# default: ARCH=um
$ tools/testing/kunit/kunit.py run 'irq_test_cases*'
[11:14:26] [SKIPPED] irq_shutdown_depth_test
[11:14:26] [SKIPPED] irq_cpuhotplug_test
[11:14:26] ================= [PASSED] irq_test_cases ==================
[11:14:26] ============================================================
[11:14:26] Testing complete. Ran 4 tests: passed: 2, skipped: 2
Without commit 788019eb559f ("genirq: Retain disable depth for managed
interrupts across CPU hotplug"), we fail as follows:
[11:18:55] =============== irq_test_cases (4 subtests) ================
[11:18:55] [PASSED] irq_disable_depth_test
[11:18:55] [PASSED] irq_free_disabled_test
[11:18:55] # irq_shutdown_depth_test: EXPECTATION FAILED at kernel/irq/irq_test.c:147
[11:18:55] Expected desc->depth == 1, but
[11:18:55] desc->depth == 0 (0x0)
[11:18:55] ------------[ cut here ]------------
[11:18:55] Unbalanced enable for IRQ 26
[11:18:55] WARNING: CPU: 1 PID: 36 at kernel/irq/manage.c:792 __enable_irq+0x36/0x60
...
[11:18:55] [FAILED] irq_shutdown_depth_test
[11:18:55] #1
[11:18:55] # irq_cpuhotplug_test: EXPECTATION FAILED at kernel/irq/irq_test.c:202
[11:18:55] Expected irqd_is_activated(data) to be false, but is true
[11:18:55] # irq_cpuhotplug_test: EXPECTATION FAILED at kernel/irq/irq_test.c:203
[11:18:55] Expected irqd_is_started(data) to be false, but is true
[11:18:55] # irq_cpuhotplug_test: EXPECTATION FAILED at kernel/irq/irq_test.c:204
[11:18:55] Expected desc->depth == 1, but
[11:18:55] desc->depth == 0 (0x0)
[11:18:55] ------------[ cut here ]------------
[11:18:55] Unbalanced enable for IRQ 27
[11:18:55] WARNING: CPU: 0 PID: 38 at kernel/irq/manage.c:792 __enable_irq+0x36/0x60
...
[11:18:55] [FAILED] irq_cpuhotplug_test
[11:18:55] # module: irq_test
[11:18:55] # irq_test_cases: pass:2 fail:2 skip:0 total:4
[11:18:55] # Totals: pass:2 fail:2 skip:0 total:4
[11:18:55] ================= [FAILED] irq_test_cases ==================
[11:18:55] ============================================================
[11:18:55] Testing complete. Ran 4 tests: passed: 2, failed: 2
Signed-off-by: Brian Norris <briannorris@chromium.org>
---
Changes in v4:
* depend on KUNIT=y: CONFIG_IRQ_KUNIT_TEST=y is incompatible with
CONFIG_KUNIT=m -- same problem as in commit 35c57fc3f8ea ("kunit:
building kunit as a module breaks allmodconfig")
Changes in v3:
* send as standalone patch, since dependent patch was merged
* make IRQ_KUNIT_TEST bool; we depend on a few internal functions that
are not exported to modules. Reported:
https://lore.kernel.org/oe-kbuild-all/202505152136.y04AHovS-lkp@intel.com/
Changes in v2:
* add request_irq()/disable_irq()/free_irq()/request_irq() test
sequence
* clean up more resources in tests
* move tests to patch 2 (i.e., after bugs are fixed and tests pass)
* adapt to irq_startup_managed() (new API)
kernel/irq/Kconfig | 11 ++
kernel/irq/Makefile | 1 +
kernel/irq/irq_test.c | 229 ++++++++++++++++++++++++++++++++++++++++++
3 files changed, 241 insertions(+)
create mode 100644 kernel/irq/irq_test.c
diff --git a/kernel/irq/Kconfig b/kernel/irq/Kconfig
index 3f02a0e45254..1da5e9d9da71 100644
--- a/kernel/irq/Kconfig
+++ b/kernel/irq/Kconfig
@@ -144,6 +144,17 @@ config GENERIC_IRQ_DEBUGFS
config GENERIC_IRQ_KEXEC_CLEAR_VM_FORWARD
bool
+config IRQ_KUNIT_TEST
+ bool "KUnit tests for IRQ management APIs" if !KUNIT_ALL_TESTS
+ depends on KUNIT=y
+ default KUNIT_ALL_TESTS
+ imply SMP
+ help
+ This option enables KUnit tests for the IRQ subsystem API. These are
+ only for development and testing, not for regular kernel use cases.
+
+ If unsure, say N.
+
endmenu
config GENERIC_IRQ_MULTI_HANDLER
diff --git a/kernel/irq/Makefile b/kernel/irq/Makefile
index c0f44c06d69d..6ab3a4055667 100644
--- a/kernel/irq/Makefile
+++ b/kernel/irq/Makefile
@@ -19,3 +19,4 @@ obj-$(CONFIG_GENERIC_IRQ_IPI_MUX) += ipi-mux.o
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
diff --git a/kernel/irq/irq_test.c b/kernel/irq/irq_test.c
new file mode 100644
index 000000000000..5161b56a12f9
--- /dev/null
+++ b/kernel/irq/irq_test.c
@@ -0,0 +1,229 @@
+// SPDX-License-Identifier: LGPL-2.1+
+
+#include <linux/cpu.h>
+#include <linux/cpumask.h>
+#include <linux/interrupt.h>
+#include <linux/irq.h>
+#include <linux/irqdesc.h>
+#include <linux/irqdomain.h>
+#include <linux/nodemask.h>
+#include <kunit/test.h>
+
+#include "internals.h"
+
+static irqreturn_t noop_handler(int irq, void *data)
+{
+ return IRQ_HANDLED;
+}
+
+static void noop(struct irq_data *data) { }
+static unsigned int noop_ret(struct irq_data *data) { return 0; }
+
+static int noop_affinity(struct irq_data *data, const struct cpumask *dest,
+ bool force)
+{
+ irq_data_update_effective_affinity(data, dest);
+
+ return 0;
+}
+
+static struct irq_chip fake_irq_chip = {
+ .name = "fake",
+ .irq_startup = noop_ret,
+ .irq_shutdown = noop,
+ .irq_enable = noop,
+ .irq_disable = noop,
+ .irq_ack = noop,
+ .irq_mask = noop,
+ .irq_unmask = noop,
+ .irq_set_affinity = noop_affinity,
+ .flags = IRQCHIP_SKIP_SET_WAKE,
+};
+
+static void irq_disable_depth_test(struct kunit *test)
+{
+ struct irq_desc *desc;
+ int virq, ret;
+
+ virq = irq_domain_alloc_descs(-1, 1, 0, NUMA_NO_NODE, NULL);
+ KUNIT_ASSERT_GE(test, virq, 0);
+
+ irq_set_chip_and_handler(virq, &dummy_irq_chip, handle_simple_irq);
+
+ desc = irq_to_desc(virq);
+ KUNIT_ASSERT_PTR_NE(test, desc, NULL);
+
+ ret = request_irq(virq, noop_handler, 0, "test_irq", NULL);
+ KUNIT_EXPECT_EQ(test, ret, 0);
+
+ KUNIT_EXPECT_EQ(test, desc->depth, 0);
+
+ disable_irq(virq);
+ KUNIT_EXPECT_EQ(test, desc->depth, 1);
+
+ enable_irq(virq);
+ KUNIT_EXPECT_EQ(test, desc->depth, 0);
+
+ free_irq(virq, NULL);
+}
+
+static void irq_free_disabled_test(struct kunit *test)
+{
+ struct irq_desc *desc;
+ int virq, ret;
+
+ virq = irq_domain_alloc_descs(-1, 1, 0, NUMA_NO_NODE, NULL);
+ KUNIT_ASSERT_GE(test, virq, 0);
+
+ irq_set_chip_and_handler(virq, &dummy_irq_chip, handle_simple_irq);
+
+ desc = irq_to_desc(virq);
+ KUNIT_ASSERT_PTR_NE(test, desc, NULL);
+
+ ret = request_irq(virq, noop_handler, 0, "test_irq", NULL);
+ KUNIT_EXPECT_EQ(test, ret, 0);
+
+ KUNIT_EXPECT_EQ(test, desc->depth, 0);
+
+ disable_irq(virq);
+ KUNIT_EXPECT_EQ(test, desc->depth, 1);
+
+ free_irq(virq, NULL);
+ KUNIT_EXPECT_GE(test, desc->depth, 1);
+
+ ret = request_irq(virq, noop_handler, 0, "test_irq", NULL);
+ KUNIT_EXPECT_EQ(test, ret, 0);
+ KUNIT_EXPECT_EQ(test, desc->depth, 0);
+
+ free_irq(virq, NULL);
+}
+
+static void irq_shutdown_depth_test(struct kunit *test)
+{
+ struct irq_desc *desc;
+ struct irq_data *data;
+ int virq, ret;
+ struct irq_affinity_desc affinity = {
+ .is_managed = 1,
+ .mask = CPU_MASK_ALL,
+ };
+
+ if (!IS_ENABLED(CONFIG_SMP))
+ kunit_skip(test, "requires CONFIG_SMP for managed shutdown");
+
+ virq = irq_domain_alloc_descs(-1, 1, 0, NUMA_NO_NODE, &affinity);
+ KUNIT_ASSERT_GE(test, virq, 0);
+
+ irq_set_chip_and_handler(virq, &dummy_irq_chip, handle_simple_irq);
+
+ desc = irq_to_desc(virq);
+ KUNIT_ASSERT_PTR_NE(test, desc, NULL);
+
+ data = irq_desc_get_irq_data(desc);
+ KUNIT_ASSERT_PTR_NE(test, data, NULL);
+
+ ret = request_irq(virq, noop_handler, 0, "test_irq", NULL);
+ KUNIT_EXPECT_EQ(test, ret, 0);
+
+ KUNIT_EXPECT_TRUE(test, irqd_is_activated(data));
+ KUNIT_EXPECT_TRUE(test, irqd_is_started(data));
+ KUNIT_EXPECT_TRUE(test, irqd_affinity_is_managed(data));
+
+ KUNIT_EXPECT_EQ(test, desc->depth, 0);
+
+ disable_irq(virq);
+ KUNIT_EXPECT_EQ(test, desc->depth, 1);
+
+ irq_shutdown_and_deactivate(desc);
+
+ KUNIT_EXPECT_FALSE(test, irqd_is_activated(data));
+ KUNIT_EXPECT_FALSE(test, irqd_is_started(data));
+
+ KUNIT_EXPECT_EQ(test, irq_activate(desc), 0);
+#ifdef CONFIG_SMP
+ irq_startup_managed(desc);
+#endif
+
+ KUNIT_EXPECT_EQ(test, desc->depth, 1);
+
+ enable_irq(virq);
+ KUNIT_EXPECT_EQ(test, desc->depth, 0);
+
+ free_irq(virq, NULL);
+}
+
+static void irq_cpuhotplug_test(struct kunit *test)
+{
+ struct irq_desc *desc;
+ struct irq_data *data;
+ int virq, ret;
+ struct irq_affinity_desc affinity = {
+ .is_managed = 1,
+ };
+
+ if (!IS_ENABLED(CONFIG_SMP))
+ kunit_skip(test, "requires CONFIG_SMP for CPU hotplug");
+ if (!get_cpu_device(1))
+ kunit_skip(test, "requires more than 1 CPU for CPU hotplug");
+ if (!cpu_is_hotpluggable(1))
+ kunit_skip(test, "CPU 1 must be hotpluggable");
+
+ cpumask_copy(&affinity.mask, cpumask_of(1));
+
+ virq = irq_domain_alloc_descs(-1, 1, 0, NUMA_NO_NODE, &affinity);
+ KUNIT_ASSERT_GE(test, virq, 0);
+
+ irq_set_chip_and_handler(virq, &fake_irq_chip, handle_simple_irq);
+
+ desc = irq_to_desc(virq);
+ KUNIT_ASSERT_PTR_NE(test, desc, NULL);
+
+ data = irq_desc_get_irq_data(desc);
+ KUNIT_ASSERT_PTR_NE(test, data, NULL);
+
+ ret = request_irq(virq, noop_handler, 0, "test_irq", NULL);
+ KUNIT_EXPECT_EQ(test, ret, 0);
+
+ KUNIT_EXPECT_TRUE(test, irqd_is_activated(data));
+ KUNIT_EXPECT_TRUE(test, irqd_is_started(data));
+ KUNIT_EXPECT_TRUE(test, irqd_affinity_is_managed(data));
+
+ KUNIT_EXPECT_EQ(test, desc->depth, 0);
+
+ disable_irq(virq);
+ KUNIT_EXPECT_EQ(test, desc->depth, 1);
+
+ KUNIT_EXPECT_EQ(test, remove_cpu(1), 0);
+ KUNIT_EXPECT_FALSE(test, irqd_is_activated(data));
+ KUNIT_EXPECT_FALSE(test, irqd_is_started(data));
+ KUNIT_EXPECT_GE(test, desc->depth, 1);
+ KUNIT_EXPECT_EQ(test, add_cpu(1), 0);
+
+ KUNIT_EXPECT_FALSE(test, irqd_is_activated(data));
+ KUNIT_EXPECT_FALSE(test, irqd_is_started(data));
+ KUNIT_EXPECT_EQ(test, desc->depth, 1);
+
+ enable_irq(virq);
+ KUNIT_EXPECT_TRUE(test, irqd_is_activated(data));
+ KUNIT_EXPECT_TRUE(test, irqd_is_started(data));
+ KUNIT_EXPECT_EQ(test, desc->depth, 0);
+
+ free_irq(virq, NULL);
+}
+
+static struct kunit_case irq_test_cases[] = {
+ KUNIT_CASE(irq_disable_depth_test),
+ KUNIT_CASE(irq_free_disabled_test),
+ KUNIT_CASE(irq_shutdown_depth_test),
+ KUNIT_CASE(irq_cpuhotplug_test),
+ {}
+};
+
+static struct kunit_suite irq_test_suite = {
+ .name = "irq_test_cases",
+ .test_cases = irq_test_cases,
+};
+
+kunit_test_suite(irq_test_suite);
+MODULE_DESCRIPTION("IRQ unit test suite");
+MODULE_LICENSE("GPL");
--
2.49.0.1151.ga128411c76-goog
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [tip: irq/core] genirq: Add kunit tests for depth counts
2025-05-22 21:08 [PATCH v4] genirq: Add kunit tests for depth counts Brian Norris
@ 2025-06-13 13:34 ` tip-bot2 for Brian Norris
2025-08-05 17:45 ` [PATCH v4] " Guenter Roeck
2025-08-10 19:37 ` [PATCH v4] genirq: Add kunit tests for depth counts Guenter Roeck
2 siblings, 0 replies; 10+ messages in thread
From: tip-bot2 for Brian Norris @ 2025-06-13 13:34 UTC (permalink / raw)
To: linux-tip-commits; +Cc: Brian Norris, Thomas Gleixner, x86, linux-kernel, maz
The following commit has been merged into the irq/core branch of tip:
Commit-ID: 66067c3c8a1ee097e9c30e8bbd643b12ba54a6b0
Gitweb: https://git.kernel.org/tip/66067c3c8a1ee097e9c30e8bbd643b12ba54a6b0
Author: Brian Norris <briannorris@chromium.org>
AuthorDate: Thu, 22 May 2025 14:08:01 -07:00
Committer: Thomas Gleixner <tglx@linutronix.de>
CommitterDate: Fri, 13 Jun 2025 15:24:44 +02:00
genirq: Add kunit tests for depth counts
There have been a few bugs and/or misunderstandings about the reference
counting, and startup/shutdown behaviors in the IRQ core and related CPU
hotplug code. These 4 test cases try to capture a few interesting cases.
* irq_disable_depth_test: basic request/disable/enable sequence
* irq_free_disabled_test: request/disable/free/re-request sequence -
this catches errors on previous revisions of my work
* irq_cpuhotplug_test: exercises managed-affinity IRQ + CPU hotplug.
This captures a problematic test case which was fixed recently.
This test requires CONFIG_SMP and a hotpluggable CPU#1.
* irq_shutdown_depth_test: exercises similar behavior from
irq_cpuhotplug_test, but directly using irq_*() APIs instead of going
through CPU hotplug. This still requires CONFIG_SMP, because
managed-affinity is stubbed out (and not all APIs are even present)
without it.
Note the use of 'imply SMP': ARCH=um doesn't support SMP, and kunit is
often exercised there. Thus, 'imply' will force SMP on where possible
(such as ARCH=x86_64), but leave it off where it's not.
Behavior on various SMP and ARCH configurations:
$ tools/testing/kunit/kunit.py run 'irq_test_cases*' --arch x86_64 --qemu_args '-smp 2'
[...]
[11:12:24] Testing complete. Ran 4 tests: passed: 4
$ tools/testing/kunit/kunit.py run 'irq_test_cases*' --arch x86_64
[...]
[11:13:27] [SKIPPED] irq_cpuhotplug_test
[11:13:27] ================= [PASSED] irq_test_cases ==================
[11:13:27] ============================================================
[11:13:27] Testing complete. Ran 4 tests: passed: 3, skipped: 1
# default: ARCH=um
$ tools/testing/kunit/kunit.py run 'irq_test_cases*'
[11:14:26] [SKIPPED] irq_shutdown_depth_test
[11:14:26] [SKIPPED] irq_cpuhotplug_test
[11:14:26] ================= [PASSED] irq_test_cases ==================
[11:14:26] ============================================================
[11:14:26] Testing complete. Ran 4 tests: passed: 2, skipped: 2
Without commit 788019eb559f ("genirq: Retain disable depth for managed
interrupts across CPU hotplug"), this fails as follows:
[11:18:55] =============== irq_test_cases (4 subtests) ================
[11:18:55] [PASSED] irq_disable_depth_test
[11:18:55] [PASSED] irq_free_disabled_test
[11:18:55] # irq_shutdown_depth_test: EXPECTATION FAILED at kernel/irq/irq_test.c:147
[11:18:55] Expected desc->depth == 1, but
[11:18:55] desc->depth == 0 (0x0)
[11:18:55] ------------[ cut here ]------------
[11:18:55] Unbalanced enable for IRQ 26
[11:18:55] WARNING: CPU: 1 PID: 36 at kernel/irq/manage.c:792 __enable_irq+0x36/0x60
...
[11:18:55] [FAILED] irq_shutdown_depth_test
[11:18:55] #1
[11:18:55] # irq_cpuhotplug_test: EXPECTATION FAILED at kernel/irq/irq_test.c:202
[11:18:55] Expected irqd_is_activated(data) to be false, but is true
[11:18:55] # irq_cpuhotplug_test: EXPECTATION FAILED at kernel/irq/irq_test.c:203
[11:18:55] Expected irqd_is_started(data) to be false, but is true
[11:18:55] # irq_cpuhotplug_test: EXPECTATION FAILED at kernel/irq/irq_test.c:204
[11:18:55] Expected desc->depth == 1, but
[11:18:55] desc->depth == 0 (0x0)
[11:18:55] ------------[ cut here ]------------
[11:18:55] Unbalanced enable for IRQ 27
[11:18:55] WARNING: CPU: 0 PID: 38 at kernel/irq/manage.c:792 __enable_irq+0x36/0x60
...
[11:18:55] [FAILED] irq_cpuhotplug_test
[11:18:55] # module: irq_test
[11:18:55] # irq_test_cases: pass:2 fail:2 skip:0 total:4
[11:18:55] # Totals: pass:2 fail:2 skip:0 total:4
[11:18:55] ================= [FAILED] irq_test_cases ==================
[11:18:55] ============================================================
[11:18:55] Testing complete. Ran 4 tests: passed: 2, failed: 2
Signed-off-by: Brian Norris <briannorris@chromium.org>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Link: https://lore.kernel.org/all/20250522210837.4135244-1-briannorris@chromium.org
---
kernel/irq/Kconfig | 11 ++-
kernel/irq/Makefile | 1 +-
kernel/irq/irq_test.c | 229 +++++++++++++++++++++++++++++++++++++++++-
3 files changed, 241 insertions(+)
create mode 100644 kernel/irq/irq_test.c
diff --git a/kernel/irq/Kconfig b/kernel/irq/Kconfig
index 3f02a0e..1da5e9d 100644
--- a/kernel/irq/Kconfig
+++ b/kernel/irq/Kconfig
@@ -144,6 +144,17 @@ config GENERIC_IRQ_DEBUGFS
config GENERIC_IRQ_KEXEC_CLEAR_VM_FORWARD
bool
+config IRQ_KUNIT_TEST
+ bool "KUnit tests for IRQ management APIs" if !KUNIT_ALL_TESTS
+ depends on KUNIT=y
+ default KUNIT_ALL_TESTS
+ imply SMP
+ help
+ This option enables KUnit tests for the IRQ subsystem API. These are
+ only for development and testing, not for regular kernel use cases.
+
+ If unsure, say N.
+
endmenu
config GENERIC_IRQ_MULTI_HANDLER
diff --git a/kernel/irq/Makefile b/kernel/irq/Makefile
index c0f44c0..6ab3a40 100644
--- a/kernel/irq/Makefile
+++ b/kernel/irq/Makefile
@@ -19,3 +19,4 @@ obj-$(CONFIG_GENERIC_IRQ_IPI_MUX) += ipi-mux.o
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
diff --git a/kernel/irq/irq_test.c b/kernel/irq/irq_test.c
new file mode 100644
index 0000000..5161b56
--- /dev/null
+++ b/kernel/irq/irq_test.c
@@ -0,0 +1,229 @@
+// SPDX-License-Identifier: LGPL-2.1+
+
+#include <linux/cpu.h>
+#include <linux/cpumask.h>
+#include <linux/interrupt.h>
+#include <linux/irq.h>
+#include <linux/irqdesc.h>
+#include <linux/irqdomain.h>
+#include <linux/nodemask.h>
+#include <kunit/test.h>
+
+#include "internals.h"
+
+static irqreturn_t noop_handler(int irq, void *data)
+{
+ return IRQ_HANDLED;
+}
+
+static void noop(struct irq_data *data) { }
+static unsigned int noop_ret(struct irq_data *data) { return 0; }
+
+static int noop_affinity(struct irq_data *data, const struct cpumask *dest,
+ bool force)
+{
+ irq_data_update_effective_affinity(data, dest);
+
+ return 0;
+}
+
+static struct irq_chip fake_irq_chip = {
+ .name = "fake",
+ .irq_startup = noop_ret,
+ .irq_shutdown = noop,
+ .irq_enable = noop,
+ .irq_disable = noop,
+ .irq_ack = noop,
+ .irq_mask = noop,
+ .irq_unmask = noop,
+ .irq_set_affinity = noop_affinity,
+ .flags = IRQCHIP_SKIP_SET_WAKE,
+};
+
+static void irq_disable_depth_test(struct kunit *test)
+{
+ struct irq_desc *desc;
+ int virq, ret;
+
+ virq = irq_domain_alloc_descs(-1, 1, 0, NUMA_NO_NODE, NULL);
+ KUNIT_ASSERT_GE(test, virq, 0);
+
+ irq_set_chip_and_handler(virq, &dummy_irq_chip, handle_simple_irq);
+
+ desc = irq_to_desc(virq);
+ KUNIT_ASSERT_PTR_NE(test, desc, NULL);
+
+ ret = request_irq(virq, noop_handler, 0, "test_irq", NULL);
+ KUNIT_EXPECT_EQ(test, ret, 0);
+
+ KUNIT_EXPECT_EQ(test, desc->depth, 0);
+
+ disable_irq(virq);
+ KUNIT_EXPECT_EQ(test, desc->depth, 1);
+
+ enable_irq(virq);
+ KUNIT_EXPECT_EQ(test, desc->depth, 0);
+
+ free_irq(virq, NULL);
+}
+
+static void irq_free_disabled_test(struct kunit *test)
+{
+ struct irq_desc *desc;
+ int virq, ret;
+
+ virq = irq_domain_alloc_descs(-1, 1, 0, NUMA_NO_NODE, NULL);
+ KUNIT_ASSERT_GE(test, virq, 0);
+
+ irq_set_chip_and_handler(virq, &dummy_irq_chip, handle_simple_irq);
+
+ desc = irq_to_desc(virq);
+ KUNIT_ASSERT_PTR_NE(test, desc, NULL);
+
+ ret = request_irq(virq, noop_handler, 0, "test_irq", NULL);
+ KUNIT_EXPECT_EQ(test, ret, 0);
+
+ KUNIT_EXPECT_EQ(test, desc->depth, 0);
+
+ disable_irq(virq);
+ KUNIT_EXPECT_EQ(test, desc->depth, 1);
+
+ free_irq(virq, NULL);
+ KUNIT_EXPECT_GE(test, desc->depth, 1);
+
+ ret = request_irq(virq, noop_handler, 0, "test_irq", NULL);
+ KUNIT_EXPECT_EQ(test, ret, 0);
+ KUNIT_EXPECT_EQ(test, desc->depth, 0);
+
+ free_irq(virq, NULL);
+}
+
+static void irq_shutdown_depth_test(struct kunit *test)
+{
+ struct irq_desc *desc;
+ struct irq_data *data;
+ int virq, ret;
+ struct irq_affinity_desc affinity = {
+ .is_managed = 1,
+ .mask = CPU_MASK_ALL,
+ };
+
+ if (!IS_ENABLED(CONFIG_SMP))
+ kunit_skip(test, "requires CONFIG_SMP for managed shutdown");
+
+ virq = irq_domain_alloc_descs(-1, 1, 0, NUMA_NO_NODE, &affinity);
+ KUNIT_ASSERT_GE(test, virq, 0);
+
+ irq_set_chip_and_handler(virq, &dummy_irq_chip, handle_simple_irq);
+
+ desc = irq_to_desc(virq);
+ KUNIT_ASSERT_PTR_NE(test, desc, NULL);
+
+ data = irq_desc_get_irq_data(desc);
+ KUNIT_ASSERT_PTR_NE(test, data, NULL);
+
+ ret = request_irq(virq, noop_handler, 0, "test_irq", NULL);
+ KUNIT_EXPECT_EQ(test, ret, 0);
+
+ KUNIT_EXPECT_TRUE(test, irqd_is_activated(data));
+ KUNIT_EXPECT_TRUE(test, irqd_is_started(data));
+ KUNIT_EXPECT_TRUE(test, irqd_affinity_is_managed(data));
+
+ KUNIT_EXPECT_EQ(test, desc->depth, 0);
+
+ disable_irq(virq);
+ KUNIT_EXPECT_EQ(test, desc->depth, 1);
+
+ irq_shutdown_and_deactivate(desc);
+
+ KUNIT_EXPECT_FALSE(test, irqd_is_activated(data));
+ KUNIT_EXPECT_FALSE(test, irqd_is_started(data));
+
+ KUNIT_EXPECT_EQ(test, irq_activate(desc), 0);
+#ifdef CONFIG_SMP
+ irq_startup_managed(desc);
+#endif
+
+ KUNIT_EXPECT_EQ(test, desc->depth, 1);
+
+ enable_irq(virq);
+ KUNIT_EXPECT_EQ(test, desc->depth, 0);
+
+ free_irq(virq, NULL);
+}
+
+static void irq_cpuhotplug_test(struct kunit *test)
+{
+ struct irq_desc *desc;
+ struct irq_data *data;
+ int virq, ret;
+ struct irq_affinity_desc affinity = {
+ .is_managed = 1,
+ };
+
+ if (!IS_ENABLED(CONFIG_SMP))
+ kunit_skip(test, "requires CONFIG_SMP for CPU hotplug");
+ if (!get_cpu_device(1))
+ kunit_skip(test, "requires more than 1 CPU for CPU hotplug");
+ if (!cpu_is_hotpluggable(1))
+ kunit_skip(test, "CPU 1 must be hotpluggable");
+
+ cpumask_copy(&affinity.mask, cpumask_of(1));
+
+ virq = irq_domain_alloc_descs(-1, 1, 0, NUMA_NO_NODE, &affinity);
+ KUNIT_ASSERT_GE(test, virq, 0);
+
+ irq_set_chip_and_handler(virq, &fake_irq_chip, handle_simple_irq);
+
+ desc = irq_to_desc(virq);
+ KUNIT_ASSERT_PTR_NE(test, desc, NULL);
+
+ data = irq_desc_get_irq_data(desc);
+ KUNIT_ASSERT_PTR_NE(test, data, NULL);
+
+ ret = request_irq(virq, noop_handler, 0, "test_irq", NULL);
+ KUNIT_EXPECT_EQ(test, ret, 0);
+
+ KUNIT_EXPECT_TRUE(test, irqd_is_activated(data));
+ KUNIT_EXPECT_TRUE(test, irqd_is_started(data));
+ KUNIT_EXPECT_TRUE(test, irqd_affinity_is_managed(data));
+
+ KUNIT_EXPECT_EQ(test, desc->depth, 0);
+
+ disable_irq(virq);
+ KUNIT_EXPECT_EQ(test, desc->depth, 1);
+
+ KUNIT_EXPECT_EQ(test, remove_cpu(1), 0);
+ KUNIT_EXPECT_FALSE(test, irqd_is_activated(data));
+ KUNIT_EXPECT_FALSE(test, irqd_is_started(data));
+ KUNIT_EXPECT_GE(test, desc->depth, 1);
+ KUNIT_EXPECT_EQ(test, add_cpu(1), 0);
+
+ KUNIT_EXPECT_FALSE(test, irqd_is_activated(data));
+ KUNIT_EXPECT_FALSE(test, irqd_is_started(data));
+ KUNIT_EXPECT_EQ(test, desc->depth, 1);
+
+ enable_irq(virq);
+ KUNIT_EXPECT_TRUE(test, irqd_is_activated(data));
+ KUNIT_EXPECT_TRUE(test, irqd_is_started(data));
+ KUNIT_EXPECT_EQ(test, desc->depth, 0);
+
+ free_irq(virq, NULL);
+}
+
+static struct kunit_case irq_test_cases[] = {
+ KUNIT_CASE(irq_disable_depth_test),
+ KUNIT_CASE(irq_free_disabled_test),
+ KUNIT_CASE(irq_shutdown_depth_test),
+ KUNIT_CASE(irq_cpuhotplug_test),
+ {}
+};
+
+static struct kunit_suite irq_test_suite = {
+ .name = "irq_test_cases",
+ .test_cases = irq_test_cases,
+};
+
+kunit_test_suite(irq_test_suite);
+MODULE_DESCRIPTION("IRQ unit test suite");
+MODULE_LICENSE("GPL");
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v4] genirq: Add kunit tests for depth counts
2025-05-22 21:08 [PATCH v4] genirq: Add kunit tests for depth counts Brian Norris
2025-06-13 13:34 ` [tip: irq/core] " tip-bot2 for Brian Norris
@ 2025-08-05 17:45 ` Guenter Roeck
2025-08-05 18:32 ` [PATCH] genirq/test: Resolve irq lock inversion warnings Brian Norris
2025-08-10 19:37 ` [PATCH v4] genirq: Add kunit tests for depth counts Guenter Roeck
2 siblings, 1 reply; 10+ messages in thread
From: Guenter Roeck @ 2025-08-05 17:45 UTC (permalink / raw)
To: Brian Norris
Cc: Thomas Gleixner, Tsai Sung-Fu, Douglas Anderson, linux-kernel
Hi Brian,
On Thu, May 22, 2025 at 02:08:01PM -0700, Brian Norris wrote:
> There have been a few bugs and/or misunderstandings about the reference
> counting, and startup/shutdown behaviors in the IRQ core and related CPU
> hotplug code. These 4 test cases try to capture a few interesting cases.
>
> * irq_disable_depth_test: basic request/disable/enable sequence
>
> * irq_free_disabled_test: request/disable/free/re-request sequence -
> this catches errors on previous revisions of my work
>
> * irq_cpuhotplug_test: exercises managed-affinity IRQ + CPU hotplug.
> This captures a problematic test case that I've fixed.
> This test requires CONFIG_SMP and a hotpluggable CPU#1.
>
> * irq_shutdown_depth_test: exercises similar behavior from
> irq_cpuhotplug_test, but directly using irq_*() APIs instead of going
> through CPU hotplug. This still requires CONFIG_SMP, because
> managed-affinity is stubbed out (and not all APIs are even present)
> without it.
>
This test triggers warning tracebacks for me.
[ 5.766715] ok 2 irq_free_disabled_test
[ 5.769030]
[ 5.769106] ========================================================
[ 5.769159] WARNING: possible irq lock inversion dependency detected
[ 5.769355] 6.16.0-11743-g6bcdbd62bd56 #1 Tainted: G N
[ 5.769413] --------------------------------------------------------
[ 5.769465] kunit_try_catch/122 just changed the state of lock:
[ 5.769532] ffffffffb81ace18 (irq_resend_lock){+...}-{2:2}, at: clear_irq_resend+0x14/0x70
[ 5.769899] but this lock was taken by another, HARDIRQ-safe lock in the past:
[ 5.769967] (&irq_desc_lock_class){-.-.}-{2:2}
[ 5.769989]
[ 5.769989]
[ 5.769989] and interrupts could create inverse lock ordering between them.
...
[ 5.776956] ret_from_fork_asm+0x1a/0x30
[ 5.776983] </TASK>
[ 5.778916] # irq_shutdown_depth_test: pass:1 fail:0 skip:0 total:1
[ 5.778953] ok 3 irq_shutdown_depth_test
Is this on purpose ?
Thanks,
Guenter
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH] genirq/test: Resolve irq lock inversion warnings
2025-08-05 17:45 ` [PATCH v4] " Guenter Roeck
@ 2025-08-05 18:32 ` Brian Norris
2025-08-05 19:54 ` Guenter Roeck
2025-08-06 8:33 ` [tip: irq/urgent] " tip-bot2 for Brian Norris
0 siblings, 2 replies; 10+ messages in thread
From: Brian Norris @ 2025-08-05 18:32 UTC (permalink / raw)
To: Guenter Roeck
Cc: Thomas Gleixner, Tsai Sung-Fu, Douglas Anderson, linux-kernel
irq_shutdown_and_deactivate() is normally called with the descriptor
lock held, and interrupts disabled. Nested a few levels down, it grabs
the global irq_resend_lock. Lockdep rightfully complains [1].
Grab the descriptor lock, and disable interrupts, to resolve the
complaint.
Tested with:
tools/testing/kunit/kunit.py run 'irq_test_cases*' \
--arch x86_64 --qemu_args '-smp 2' \
--kconfig_add CONFIG_DEBUG_KERNEL=y \
--kconfig_add CONFIG_PROVE_LOCKING=y \
--raw_output=all
[1]
========================================================
WARNING: possible irq lock inversion dependency detected
6.16.0-11743-g6bcdbd62bd56 #2 Tainted: G N
--------------------------------------------------------
kunit_try_catch/40 just changed the state of lock:
ffffffff898b1538 (irq_resend_lock){+...}-{2:2}, at: clear_irq_resend+0x14/0x70
but this lock was taken by another, HARDIRQ-safe lock in the past:
(&irq_desc_lock_class){-.-.}-{2:2}
and interrupts could create inverse lock ordering between them.
other info that might help us debug this:
Possible interrupt unsafe locking scenario:
CPU0 CPU1
---- ----
lock(irq_resend_lock);
local_irq_disable();
lock(&irq_desc_lock_class);
lock(irq_resend_lock);
<Interrupt>
lock(&irq_desc_lock_class);
[...]
... key at: [<ffffffff898b1538>] irq_resend_lock+0x18/0x60
... acquired at:
__lock_acquire+0x82b/0x2620
lock_acquire+0xc7/0x2c0
_raw_spin_lock+0x2b/0x40
clear_irq_resend+0x14/0x70
irq_shutdown_and_deactivate+0x29/0x80
irq_shutdown_depth_test+0x1ce/0x600
kunit_try_run_case+0x90/0x120
kunit_generic_run_threadfn_adapter+0x1c/0x40
kthread+0xf3/0x200
ret_from_fork+0x140/0x1b0
ret_from_fork_asm+0x1a/0x30
[ 5.766715] ok 2 irq_free_disabled_test
[ 5.769030]
[ 5.769106] ========================================================
[ 5.769159] WARNING: possible irq lock inversion dependency detected
[ 5.769355] 6.16.0-11743-g6bcdbd62bd56 #1 Tainted: G N
[ 5.769413] --------------------------------------------------------
[ 5.769465] kunit_try_catch/122 just changed the state of lock:
[ 5.769532] ffffffffb81ace18 (irq_resend_lock){+...}-{2:2}, at: clear_irq_resend+0x14/0x70
[ 5.769899] but this lock was taken by another, HARDIRQ-safe lock in the past:
[ 5.769967] (&irq_desc_lock_class){-.-.}-{2:2}
[ 5.769989]
[ 5.769989]
[ 5.769989] and interrupts could create inverse lock ordering between them.
...
[ 5.776956] ret_from_fork_asm+0x1a/0x30
[ 5.776983] </TASK>
[ 5.778916] # irq_shutdown_depth_test: pass:1 fail:0 skip:0 total:1
[ 5.778953] ok 3 irq_shutdown_depth_test
Fixes: 66067c3c8a1e ("genirq: Add kunit tests for depth counts")
Reported-by: Guenter Roeck <linux@roeck-us.net>
Closes: https://lore.kernel.org/lkml/31a761e4-8f81-40cf-aaf5-d220ba11911c@roeck-us.net/
Signed-off-by: Brian Norris <briannorris@chromium.org>
---
On Tue, Aug 05, 2025 at 10:45:33AM -0700, Guenter Roeck wrote:
> Hi Brian,
>
> On Thu, May 22, 2025 at 02:08:01PM -0700, Brian Norris wrote:
> > * irq_shutdown_depth_test: exercises similar behavior from
> > irq_cpuhotplug_test, but directly using irq_*() APIs instead of going
> > through CPU hotplug. This still requires CONFIG_SMP, because
> > managed-affinity is stubbed out (and not all APIs are even present)
> > without it.
> >
> This test triggers warning tracebacks for me.
[...]
> Is this on purpose ?
No. I think it's an artifact of trying to imitate CPU hotplug, but doing
so insufficiently. I believe the surrounding patch fixes things.
Let me know if it helps. Thanks for the report.
kernel/irq/irq_test.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/kernel/irq/irq_test.c b/kernel/irq/irq_test.c
index 5161b56a12f9..a75abebed7f2 100644
--- a/kernel/irq/irq_test.c
+++ b/kernel/irq/irq_test.c
@@ -1,5 +1,6 @@
// SPDX-License-Identifier: LGPL-2.1+
+#include <linux/cleanup.h>
#include <linux/cpu.h>
#include <linux/cpumask.h>
#include <linux/interrupt.h>
@@ -134,7 +135,8 @@ static void irq_shutdown_depth_test(struct kunit *test)
disable_irq(virq);
KUNIT_EXPECT_EQ(test, desc->depth, 1);
- irq_shutdown_and_deactivate(desc);
+ scoped_guard(raw_spinlock_irqsave, &desc->lock)
+ irq_shutdown_and_deactivate(desc);
KUNIT_EXPECT_FALSE(test, irqd_is_activated(data));
KUNIT_EXPECT_FALSE(test, irqd_is_started(data));
--
2.50.1.565.gc32cd1483b-goog
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] genirq/test: Resolve irq lock inversion warnings
2025-08-05 18:32 ` [PATCH] genirq/test: Resolve irq lock inversion warnings Brian Norris
@ 2025-08-05 19:54 ` Guenter Roeck
2025-08-06 8:33 ` [tip: irq/urgent] " tip-bot2 for Brian Norris
1 sibling, 0 replies; 10+ messages in thread
From: Guenter Roeck @ 2025-08-05 19:54 UTC (permalink / raw)
To: Brian Norris
Cc: Thomas Gleixner, Tsai Sung-Fu, Douglas Anderson, linux-kernel
On 8/5/25 11:32, Brian Norris wrote:
> irq_shutdown_and_deactivate() is normally called with the descriptor
> lock held, and interrupts disabled. Nested a few levels down, it grabs
> the global irq_resend_lock. Lockdep rightfully complains [1].
>
> Grab the descriptor lock, and disable interrupts, to resolve the
> complaint.
>
> Tested with:
>
> tools/testing/kunit/kunit.py run 'irq_test_cases*' \
> --arch x86_64 --qemu_args '-smp 2' \
> --kconfig_add CONFIG_DEBUG_KERNEL=y \
> --kconfig_add CONFIG_PROVE_LOCKING=y \
> --raw_output=all
>
> [1]
> ========================================================
> WARNING: possible irq lock inversion dependency detected
> 6.16.0-11743-g6bcdbd62bd56 #2 Tainted: G N
> --------------------------------------------------------
> kunit_try_catch/40 just changed the state of lock:
> ffffffff898b1538 (irq_resend_lock){+...}-{2:2}, at: clear_irq_resend+0x14/0x70
> but this lock was taken by another, HARDIRQ-safe lock in the past:
> (&irq_desc_lock_class){-.-.}-{2:2}
>
> and interrupts could create inverse lock ordering between them.
>
> other info that might help us debug this:
> Possible interrupt unsafe locking scenario:
>
> CPU0 CPU1
> ---- ----
> lock(irq_resend_lock);
> local_irq_disable();
> lock(&irq_desc_lock_class);
> lock(irq_resend_lock);
> <Interrupt>
> lock(&irq_desc_lock_class);
>
> [...]
>
> ... key at: [<ffffffff898b1538>] irq_resend_lock+0x18/0x60
> ... acquired at:
> __lock_acquire+0x82b/0x2620
> lock_acquire+0xc7/0x2c0
> _raw_spin_lock+0x2b/0x40
> clear_irq_resend+0x14/0x70
> irq_shutdown_and_deactivate+0x29/0x80
> irq_shutdown_depth_test+0x1ce/0x600
> kunit_try_run_case+0x90/0x120
> kunit_generic_run_threadfn_adapter+0x1c/0x40
> kthread+0xf3/0x200
> ret_from_fork+0x140/0x1b0
> ret_from_fork_asm+0x1a/0x30
>
> [ 5.766715] ok 2 irq_free_disabled_test
> [ 5.769030]
> [ 5.769106] ========================================================
> [ 5.769159] WARNING: possible irq lock inversion dependency detected
> [ 5.769355] 6.16.0-11743-g6bcdbd62bd56 #1 Tainted: G N
> [ 5.769413] --------------------------------------------------------
> [ 5.769465] kunit_try_catch/122 just changed the state of lock:
> [ 5.769532] ffffffffb81ace18 (irq_resend_lock){+...}-{2:2}, at: clear_irq_resend+0x14/0x70
> [ 5.769899] but this lock was taken by another, HARDIRQ-safe lock in the past:
> [ 5.769967] (&irq_desc_lock_class){-.-.}-{2:2}
> [ 5.769989]
> [ 5.769989]
> [ 5.769989] and interrupts could create inverse lock ordering between them.
> ...
> [ 5.776956] ret_from_fork_asm+0x1a/0x30
> [ 5.776983] </TASK>
> [ 5.778916] # irq_shutdown_depth_test: pass:1 fail:0 skip:0 total:1
> [ 5.778953] ok 3 irq_shutdown_depth_test
>
> Fixes: 66067c3c8a1e ("genirq: Add kunit tests for depth counts")
> Reported-by: Guenter Roeck <linux@roeck-us.net>
> Closes: https://lore.kernel.org/lkml/31a761e4-8f81-40cf-aaf5-d220ba11911c@roeck-us.net/
> Signed-off-by: Brian Norris <briannorris@chromium.org>
Tested-by: Guenter Roeck <linux@roeck-us.net>
Thanks for the quick turnaround!
Guenter
^ permalink raw reply [flat|nested] 10+ messages in thread
* [tip: irq/urgent] genirq/test: Resolve irq lock inversion warnings
2025-08-05 18:32 ` [PATCH] genirq/test: Resolve irq lock inversion warnings Brian Norris
2025-08-05 19:54 ` Guenter Roeck
@ 2025-08-06 8:33 ` tip-bot2 for Brian Norris
1 sibling, 0 replies; 10+ messages in thread
From: tip-bot2 for Brian Norris @ 2025-08-06 8:33 UTC (permalink / raw)
To: linux-tip-commits
Cc: Guenter Roeck, Brian Norris, Thomas Gleixner, x86, linux-kernel,
maz
The following commit has been merged into the irq/urgent branch of tip:
Commit-ID: 5b65258229117995eb6c4bd74995e15fb5f2cfe3
Gitweb: https://git.kernel.org/tip/5b65258229117995eb6c4bd74995e15fb5f2cfe3
Author: Brian Norris <briannorris@chromium.org>
AuthorDate: Tue, 05 Aug 2025 11:32:20 -07:00
Committer: Thomas Gleixner <tglx@linutronix.de>
CommitterDate: Wed, 06 Aug 2025 10:29:48 +02:00
genirq/test: Resolve irq lock inversion warnings
irq_shutdown_and_deactivate() is normally called with the descriptor lock
held, and interrupts disabled. Nested a few levels down, it grabs the
global irq_resend_lock. Lockdep rightfully complains when interrupts are
not disabled:
CPU0 CPU1
---- ----
lock(irq_resend_lock);
local_irq_disable();
lock(&irq_desc_lock_class);
lock(irq_resend_lock);
<Interrupt>
lock(&irq_desc_lock_class);
...
_raw_spin_lock+0x2b/0x40
clear_irq_resend+0x14/0x70
irq_shutdown_and_deactivate+0x29/0x80
irq_shutdown_depth_test+0x1ce/0x600
kunit_try_run_case+0x90/0x120
Grab the descriptor lock and disable interrupts, to resolve the
problem.
Fixes: 66067c3c8a1e ("genirq: Add kunit tests for depth counts")
Reported-by: Guenter Roeck <linux@roeck-us.net>
Signed-off-by: Brian Norris <briannorris@chromium.org>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Tested-by: Guenter Roeck <linux@roeck-us.net>
Link: https://lore.kernel.org/all/aJJONEIoIiTSDMqc@google.com
Closes: https://lore.kernel.org/lkml/31a761e4-8f81-40cf-aaf5-d220ba11911c@roeck-us.net/
---
kernel/irq/irq_test.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/kernel/irq/irq_test.c b/kernel/irq/irq_test.c
index 5161b56..a75abeb 100644
--- a/kernel/irq/irq_test.c
+++ b/kernel/irq/irq_test.c
@@ -1,5 +1,6 @@
// SPDX-License-Identifier: LGPL-2.1+
+#include <linux/cleanup.h>
#include <linux/cpu.h>
#include <linux/cpumask.h>
#include <linux/interrupt.h>
@@ -134,7 +135,8 @@ static void irq_shutdown_depth_test(struct kunit *test)
disable_irq(virq);
KUNIT_EXPECT_EQ(test, desc->depth, 1);
- irq_shutdown_and_deactivate(desc);
+ scoped_guard(raw_spinlock_irqsave, &desc->lock)
+ irq_shutdown_and_deactivate(desc);
KUNIT_EXPECT_FALSE(test, irqd_is_activated(data));
KUNIT_EXPECT_FALSE(test, irqd_is_started(data));
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v4] genirq: Add kunit tests for depth counts
2025-05-22 21:08 [PATCH v4] genirq: Add kunit tests for depth counts Brian Norris
2025-06-13 13:34 ` [tip: irq/core] " tip-bot2 for Brian Norris
2025-08-05 17:45 ` [PATCH v4] " Guenter Roeck
@ 2025-08-10 19:37 ` Guenter Roeck
2025-08-15 17:58 ` Brian Norris
2 siblings, 1 reply; 10+ messages in thread
From: Guenter Roeck @ 2025-08-10 19:37 UTC (permalink / raw)
To: Brian Norris
Cc: Thomas Gleixner, Tsai Sung-Fu, Douglas Anderson, linux-kernel
Hi Brian,
On Thu, May 22, 2025 at 02:08:01PM -0700, Brian Norris wrote:
> There have been a few bugs and/or misunderstandings about the reference
> counting, and startup/shutdown behaviors in the IRQ core and related CPU
> hotplug code. These 4 test cases try to capture a few interesting cases.
>
> * irq_disable_depth_test: basic request/disable/enable sequence
>
> * irq_free_disabled_test: request/disable/free/re-request sequence -
> this catches errors on previous revisions of my work
>
> * irq_cpuhotplug_test: exercises managed-affinity IRQ + CPU hotplug.
> This captures a problematic test case that I've fixed.
> This test requires CONFIG_SMP and a hotpluggable CPU#1.
>
> * irq_shutdown_depth_test: exercises similar behavior from
> irq_cpuhotplug_test, but directly using irq_*() APIs instead of going
> through CPU hotplug. This still requires CONFIG_SMP, because
> managed-affinity is stubbed out (and not all APIs are even present)
> without it.
>
> Note the use of 'imply SMP': ARCH=um doesn't support SMP, and kunit is
> often exercised there. Thus, 'imply' will force SMP on where possible
> (such as ARCH=x86_64), but leave it off where it's not.
>
...
>
> +config IRQ_KUNIT_TEST
> + bool "KUnit tests for IRQ management APIs" if !KUNIT_ALL_TESTS
> + depends on KUNIT=y
> + default KUNIT_ALL_TESTS
> + imply SMP
> + help
> + This option enables KUnit tests for the IRQ subsystem API. These are
> + only for development and testing, not for regular kernel use cases.
> +
> + If unsure, say N.
> +
The new code calls irq_domain_alloc_descs(), making it dependent
on IRQ_DOMAIN. However, specifying that dependency directly is not
possible:
config IRQ_KUNIT_TEST
bool "KUnit tests for IRQ management APIs" if !KUNIT_ALL_TESTS
depends on KUNIT=y
+ depends on IRQ_DOMAIN
default KUNIT_ALL_TESTS
imply SMP
help
results in:
*** Default configuration is based on 'defconfig'
error: recursive dependency detected!
symbol SMP is implied by IRQ_KUNIT_TEST
symbol IRQ_KUNIT_TEST depends on IRQ_DOMAIN
symbol IRQ_DOMAIN is selected by IRQ_DOMAIN_HIERARCHY
symbol IRQ_DOMAIN_HIERARCHY is selected by GENERIC_IRQ_IPI
symbol GENERIC_IRQ_IPI depends on SMP
This is seen with alpha configurations such as alpha:defconfig after
adding the IRQ_DOMAIN dependency.
I have no idea how to resolve this. For now I disabled IRQ_KUNIT_TEST
for my alpha test builds.
Guenter
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4] genirq: Add kunit tests for depth counts
2025-08-10 19:37 ` [PATCH v4] genirq: Add kunit tests for depth counts Guenter Roeck
@ 2025-08-15 17:58 ` Brian Norris
2025-08-16 0:24 ` Guenter Roeck
0 siblings, 1 reply; 10+ messages in thread
From: Brian Norris @ 2025-08-15 17:58 UTC (permalink / raw)
To: Guenter Roeck
Cc: Thomas Gleixner, Tsai Sung-Fu, Douglas Anderson, linux-kernel
On Sun, Aug 10, 2025 at 12:37:31PM -0700, Guenter Roeck wrote:
> The new code calls irq_domain_alloc_descs(), making it dependent
> on IRQ_DOMAIN. However, specifying that dependency directly is not
> possible:
>
> config IRQ_KUNIT_TEST
> bool "KUnit tests for IRQ management APIs" if !KUNIT_ALL_TESTS
> depends on KUNIT=y
> + depends on IRQ_DOMAIN
> default KUNIT_ALL_TESTS
> imply SMP
> help
>
> results in:
>
> *** Default configuration is based on 'defconfig'
> error: recursive dependency detected!
> symbol SMP is implied by IRQ_KUNIT_TEST
> symbol IRQ_KUNIT_TEST depends on IRQ_DOMAIN
> symbol IRQ_DOMAIN is selected by IRQ_DOMAIN_HIERARCHY
> symbol IRQ_DOMAIN_HIERARCHY is selected by GENERIC_IRQ_IPI
> symbol GENERIC_IRQ_IPI depends on SMP
>
> This is seen with alpha configurations such as alpha:defconfig after
> adding the IRQ_DOMAIN dependency.
>
> I have no idea how to resolve this. For now I disabled IRQ_KUNIT_TEST
> for my alpha test builds.
How about 'select'? That's the usual way IRQ_DOMAIN is managed anyway.
It builds for me, but my distro doesn't provide an Alpha QEMU, so I
can't test it.
--- a/kernel/irq/Kconfig
+++ b/kernel/irq/Kconfig
@@ -148,6 +148,7 @@ config IRQ_KUNIT_TEST
bool "KUnit tests for IRQ management APIs" if !KUNIT_ALL_TESTS
depends on KUNIT=y
default KUNIT_ALL_TESTS
+ select IRQ_DOMAIN
imply SMP
help
This option enables KUnit tests for the IRQ subsystem API. These are
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4] genirq: Add kunit tests for depth counts
2025-08-15 17:58 ` Brian Norris
@ 2025-08-16 0:24 ` Guenter Roeck
2025-08-16 2:23 ` Brian Norris
0 siblings, 1 reply; 10+ messages in thread
From: Guenter Roeck @ 2025-08-16 0:24 UTC (permalink / raw)
To: Brian Norris
Cc: Thomas Gleixner, Tsai Sung-Fu, Douglas Anderson, linux-kernel
On 8/15/25 10:58, Brian Norris wrote:
> On Sun, Aug 10, 2025 at 12:37:31PM -0700, Guenter Roeck wrote:
>> The new code calls irq_domain_alloc_descs(), making it dependent
>> on IRQ_DOMAIN. However, specifying that dependency directly is not
>> possible:
>>
>> config IRQ_KUNIT_TEST
>> bool "KUnit tests for IRQ management APIs" if !KUNIT_ALL_TESTS
>> depends on KUNIT=y
>> + depends on IRQ_DOMAIN
>> default KUNIT_ALL_TESTS
>> imply SMP
>> help
>>
>> results in:
>>
>> *** Default configuration is based on 'defconfig'
>> error: recursive dependency detected!
>> symbol SMP is implied by IRQ_KUNIT_TEST
>> symbol IRQ_KUNIT_TEST depends on IRQ_DOMAIN
>> symbol IRQ_DOMAIN is selected by IRQ_DOMAIN_HIERARCHY
>> symbol IRQ_DOMAIN_HIERARCHY is selected by GENERIC_IRQ_IPI
>> symbol GENERIC_IRQ_IPI depends on SMP
>>
>> This is seen with alpha configurations such as alpha:defconfig after
>> adding the IRQ_DOMAIN dependency.
>>
>> I have no idea how to resolve this. For now I disabled IRQ_KUNIT_TEST
>> for my alpha test builds.
>
> How about 'select'? That's the usual way IRQ_DOMAIN is managed anyway.
>
> It builds for me, but my distro doesn't provide an Alpha QEMU, so I
> can't test it.
>
I can try, but the irq test code fails for me all over the place, so I am
not sure if it is worth it.
From my current upstream test results (6.17-rc1):
Build results:
total: 162 pass: 162 fail: 0
Qemu test results:
total: 637 pass: 637 fail: 0
Unit test results:
pass: 640017 fail: 649
The failures are all from the irq test code. I didn't have time to analyze it.
You can find details at https://kerneltests.org/builders in the "master" column
if you have time.
Note that "imply SMP" does not make SMP mandatory. I can still disable it after
enabling IRQ_KUNIT_TEST on x86. Frankly I don't really understand what "imply"
is supposed to be useful for.
Guenter
> --- a/kernel/irq/Kconfig
> +++ b/kernel/irq/Kconfig
> @@ -148,6 +148,7 @@ config IRQ_KUNIT_TEST
> bool "KUnit tests for IRQ management APIs" if !KUNIT_ALL_TESTS
> depends on KUNIT=y
> default KUNIT_ALL_TESTS
> + select IRQ_DOMAIN
> imply SMP
> help
> This option enables KUnit tests for the IRQ subsystem API. These are
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4] genirq: Add kunit tests for depth counts
2025-08-16 0:24 ` Guenter Roeck
@ 2025-08-16 2:23 ` Brian Norris
0 siblings, 0 replies; 10+ messages in thread
From: Brian Norris @ 2025-08-16 2:23 UTC (permalink / raw)
To: Guenter Roeck
Cc: Thomas Gleixner, Tsai Sung-Fu, Douglas Anderson, linux-kernel
On Fri, Aug 15, 2025 at 05:24:24PM -0700, Guenter Roeck wrote:
> I can try, but the irq test code fails for me all over the place, so I am
> not sure if it is worth it.
>
> From my current upstream test results (6.17-rc1):
>
> Build results:
> total: 162 pass: 162 fail: 0
> Qemu test results:
> total: 637 pass: 637 fail: 0
> Unit test results:
> pass: 640017 fail: 649
>
> The failures are all from the irq test code. I didn't have time to analyze it.
> You can find details at https://kerneltests.org/builders in the "master" column
> if you have time.
I can't replicate all of those easily, since I don't have tooling ready
for some of the more esoteric architectures. But many of those look like
they boil down to a single oversight: that some architectures default to
IRQ_NOREQUEST, and so I need to throw this onto the fake IRQs I set up:
irq_settings_clr_norequest(desc);
With that, I can pass on ARCH=arm:
tools/testing/kunit/kunit.py run 'irq_test_cases*' --arch arm \
--qemu_args '-smp 2' --cross_compile arm-linux-gnueabi-
I'm less sure about the ARCH=parisc{,64} ones, but I think that boils
down to missing CONFIG_SPARSE_IRQ. I think I can skip tests in the
!SPARSE_IRQ case.
> Note that "imply SMP" does not make SMP mandatory. I can still disable it after
> enabling IRQ_KUNIT_TEST on x86.
Right, that's intentional. There are a few CONFIG_SMP-conditional bits
in the tests, since many users likely run on ARCH=um, which does not
have an SMP build.
> Frankly I don't really understand what "imply"
> is supposed to be useful for.
Documentation/kbuild/kconfig-language.rst calls it a "weak reverse
dependency" (i.e., a weak "select") and:
This is useful e.g. with multiple drivers that want to indicate their
ability to hook into a secondary subsystem while allowing the user to
configure that subsystem out without also having to unset these drivers.
Basically, I want SMP when it's available, but I don't want to force it
when not. And it means on KUNIT_ALL_TESTS builds that otherwise didn't
make an opinionated choice for CONFIG_SMP, we get CONFIG_SMP=y. The
latter point is relevant to tools/testing/kunit/kunit.py.
I have a patch series to clean up a little while fixing the errors
you've pointed me at. I'll probably send it out next week.
Thanks,
Brian
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2025-08-16 2:23 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-05-22 21:08 [PATCH v4] genirq: Add kunit tests for depth counts Brian Norris
2025-06-13 13:34 ` [tip: irq/core] " tip-bot2 for Brian Norris
2025-08-05 17:45 ` [PATCH v4] " Guenter Roeck
2025-08-05 18:32 ` [PATCH] genirq/test: Resolve irq lock inversion warnings Brian Norris
2025-08-05 19:54 ` Guenter Roeck
2025-08-06 8:33 ` [tip: irq/urgent] " tip-bot2 for Brian Norris
2025-08-10 19:37 ` [PATCH v4] genirq: Add kunit tests for depth counts Guenter Roeck
2025-08-15 17:58 ` Brian Norris
2025-08-16 0:24 ` Guenter Roeck
2025-08-16 2:23 ` Brian Norris
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).