All of lore.kernel.org
 help / color / mirror / Atom feed
From: Brian Norris <briannorris@chromium.org>
To: Guenter Roeck <linux@roeck-us.net>
Cc: Thomas Gleixner <tglx@linutronix.de>,
	Tsai Sung-Fu <danielsftsai@google.com>,
	Douglas Anderson <dianders@chromium.org>,
	linux-kernel@vger.kernel.org
Subject: [PATCH] genirq/test: Resolve irq lock inversion warnings
Date: Tue, 5 Aug 2025 11:32:20 -0700	[thread overview]
Message-ID: <aJJONEIoIiTSDMqc@google.com> (raw)
In-Reply-To: <31a761e4-8f81-40cf-aaf5-d220ba11911c@roeck-us.net>

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


  reply	other threads:[~2025-08-05 18:32 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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   ` Brian Norris [this message]
2025-08-05 19:54     ` [PATCH] genirq/test: Resolve irq lock inversion warnings 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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aJJONEIoIiTSDMqc@google.com \
    --to=briannorris@chromium.org \
    --cc=danielsftsai@google.com \
    --cc=dianders@chromium.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@roeck-us.net \
    --cc=tglx@linutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.