From: Brian Norris <briannorris@chromium.org>
To: Thomas Gleixner <tglx@linutronix.de>
Cc: Tsai Sung-Fu <danielsftsai@google.com>,
Douglas Anderson <dianders@chromium.org>,
linux-kernel@vger.kernel.org,
Brian Norris <briannorris@chromium.org>
Subject: [PATCH v2 1/2] genirq: Retain depth for managed IRQs across CPU hotplug
Date: Wed, 14 May 2025 13:13:16 -0700 [thread overview]
Message-ID: <20250514201353.3481400-2-briannorris@chromium.org> (raw)
In-Reply-To: <20250514201353.3481400-1-briannorris@chromium.org>
Affinity-managed IRQs may be shut down and restarted during CPU
hotunplug/plug, and the IRQ may be left in an unexpected state.
Specifically:
1. IRQ affines to CPU N
2. disable_irq() -> depth is 1
3. CPU N goes offline
4. irq_shutdown() -> depth is set to 1 (again)
5. CPU N goes online
6. irq_startup() -> depth is set to 0 (BUG! client expected IRQ is
still disabled)
7. enable_irq() -> depth underflow / unbalanced enable_irq() WARN
It seems depth only needs preserved for managed IRQs + CPU hotplug, so
per Thomas's recommendation, we make that explicit.
I add kunit tests that cover some of this in a following patch.
Signed-off-by: Brian Norris <briannorris@chromium.org>
Co-developed-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
Thomas provided a better suggestion than my v1, without fully-formed
patch metadata. I've incorporated that as "Co-developed-by". Feel free to
suggest something different.
Changes in v2:
* Adapt Thomas Gleixner's alternative solution, to focus only on CPU
hotplug cases
kernel/irq/chip.c | 22 +++++++++++++++++++++-
kernel/irq/cpuhotplug.c | 2 +-
kernel/irq/internals.h | 1 +
3 files changed, 23 insertions(+), 2 deletions(-)
diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
index 36cf1b09cc84..ab2bf0de3422 100644
--- a/kernel/irq/chip.c
+++ b/kernel/irq/chip.c
@@ -223,6 +223,19 @@ __irq_startup_managed(struct irq_desc *desc, const struct cpumask *aff,
return IRQ_STARTUP_ABORT;
return IRQ_STARTUP_MANAGED;
}
+
+void irq_startup_managed(struct irq_desc *desc)
+{
+ /*
+ * Only start it up when the disable depth is 1, so that a disable,
+ * hotunplug, hotplug sequence does not end up enabling it during
+ * hotplug unconditionally.
+ */
+ desc->depth--;
+ if (!desc->depth)
+ irq_startup(desc, IRQ_RESEND, IRQ_START_COND);
+}
+
#else
static __always_inline int
__irq_startup_managed(struct irq_desc *desc, const struct cpumask *aff,
@@ -290,6 +303,7 @@ int irq_startup(struct irq_desc *desc, bool resend, bool force)
ret = __irq_startup(desc);
break;
case IRQ_STARTUP_ABORT:
+ desc->depth = 1;
irqd_set_managed_shutdown(d);
return 0;
}
@@ -322,7 +336,13 @@ void irq_shutdown(struct irq_desc *desc)
{
if (irqd_is_started(&desc->irq_data)) {
clear_irq_resend(desc);
- desc->depth = 1;
+ /*
+ * Increment disable depth, so that a managed shutdown on
+ * CPU hotunplug preserves the actual disabled state when the
+ * CPU comes back online. See irq_startup_managed().
+ */
+ desc->depth++;
+
if (desc->irq_data.chip->irq_shutdown) {
desc->irq_data.chip->irq_shutdown(&desc->irq_data);
irq_state_set_disabled(desc);
diff --git a/kernel/irq/cpuhotplug.c b/kernel/irq/cpuhotplug.c
index 15a7654eff68..3ed5b1592735 100644
--- a/kernel/irq/cpuhotplug.c
+++ b/kernel/irq/cpuhotplug.c
@@ -219,7 +219,7 @@ static void irq_restore_affinity_of_irq(struct irq_desc *desc, unsigned int cpu)
return;
if (irqd_is_managed_and_shutdown(data))
- irq_startup(desc, IRQ_RESEND, IRQ_START_COND);
+ irq_startup_managed(desc);
/*
* If the interrupt can only be directed to a single target
diff --git a/kernel/irq/internals.h b/kernel/irq/internals.h
index b0290849c395..7111747ecb86 100644
--- a/kernel/irq/internals.h
+++ b/kernel/irq/internals.h
@@ -87,6 +87,7 @@ extern void __enable_irq(struct irq_desc *desc);
extern int irq_activate(struct irq_desc *desc);
extern int irq_activate_and_startup(struct irq_desc *desc, bool resend);
extern int irq_startup(struct irq_desc *desc, bool resend, bool force);
+extern void irq_startup_managed(struct irq_desc *desc);
extern void irq_shutdown(struct irq_desc *desc);
extern void irq_shutdown_and_deactivate(struct irq_desc *desc);
--
2.49.0.1045.g170613ef41-goog
next prev parent reply other threads:[~2025-05-14 20:14 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-05-14 20:13 [PATCH v2 0/2] genirq: Retain depth for managed IRQs across CPU hotplug Brian Norris
2025-05-14 20:13 ` Brian Norris [this message]
2025-05-15 14:51 ` [tip: irq/core] genirq: Retain disable depth for managed interrupts " tip-bot2 for Brian Norris
2025-06-06 12:21 ` [PATCH v2 1/2] genirq: Retain depth for managed IRQs " Aleksandrs Vinarskis
2025-06-09 17:13 ` Brian Norris
2025-06-09 18:19 ` Aleksandrs Vinarskis
2025-06-10 20:07 ` Brian Norris
2025-06-11 6:50 ` Thomas Gleixner
2025-06-11 8:50 ` Thomas Gleixner
2025-06-11 18:51 ` Brian Norris
2025-06-11 6:56 ` Aleksandrs Vinarskis
2025-06-11 19:08 ` Brian Norris
2025-06-12 18:40 ` Brian Norris
2025-06-18 10:17 ` Johan Hovold
2025-06-18 17:10 ` Brian Norris
2025-06-19 8:32 ` Johan Hovold
2025-05-14 20:13 ` [PATCH v2 2/2] genirq: Add kunit tests for depth counts Brian Norris
2025-05-15 14:01 ` kernel test robot
2025-05-15 17:21 ` Brian Norris
2025-05-15 22:24 ` Thomas Gleixner
2025-05-15 14:51 ` [tip: irq/core] genirq: Add kunit tests for disable " tip-bot2 for 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=20250514201353.3481400-2-briannorris@chromium.org \
--to=briannorris@chromium.org \
--cc=danielsftsai@google.com \
--cc=dianders@chromium.org \
--cc=linux-kernel@vger.kernel.org \
--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 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).