From: Thomas Gleixner <tglx@linutronix.de>
To: LKML <linux-kernel@vger.kernel.org>
Cc: x86@kernel.org, Robert Hodaszi <Robert.Hodaszi@digi.com>,
Vadim Pasternak <vadimp@mellanox.com>,
Ido Schimmel <idosch@mellanox.com>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
linux-serial@vger.kernel.org, Marc Zyngier <marc.zyngier@arm.com>
Subject: [patch 2/5] genirq: Add optional hardware synchronization for shutdown
Date: Tue, 25 Jun 2019 13:13:55 +0200 [thread overview]
Message-ID: <20190625112405.666964552@linutronix.de> (raw)
In-Reply-To: <20190625111353.863718167@linutronix.de>
free_irq() ensures that no hardware interrupt handler is executing on a
different CPU before actually releasing resources and deactivating the
interrupt completely in a domain hierarchy.
But that does not catch the case where the interrupt is on flight at the
hardware level but not yet serviced by the target CPU. That creates an
interesing race condition:
CPU 0 CPU 1 IRQ CHIP
interrupt is raised
sent to CPU1
Unable to handle
immediately
(interrupts off,
deep idle delay)
mask()
...
free()
shutdown()
synchronize_irq()
release_resources()
do_IRQ()
-> resources are not available
That might be harmless and just trigger a spurious interrupt warning, but
some interrupt chips might get into a wedged state.
Provide infrastructure for interrupt chips to provide an optional
irq_inflight() callback and use it for the synchronization in free_irq().
synchronize_[hard]irq() are not using this mechanism as it might actually
deadlock unter certain conditions.
Fixes: 464d12309e1b ("x86/vector: Switch IOAPIC to global reservation mode")
Reported-by: Robert Hodaszi <Robert.Hodaszi@digi.com>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
include/linux/irq.h | 2 ++
kernel/irq/manage.c | 29 ++++++++++++++++++++++++-----
2 files changed, 26 insertions(+), 5 deletions(-)
--- a/include/linux/irq.h
+++ b/include/linux/irq.h
@@ -418,6 +418,7 @@ static inline irq_hw_number_t irqd_to_hw
* required. This is used for CPU hotplug where the
* target CPU is not yet set in the cpu_online_mask.
* @irq_retrigger: resend an IRQ to the CPU
+ * @irq_inflight: chip level detection of interrupts in flight (optional)
* @irq_set_type: set the flow type (IRQ_TYPE_LEVEL/etc.) of an IRQ
* @irq_set_wake: enable/disable power-management wake-on of an IRQ
* @irq_bus_lock: function to lock access to slow bus (i2c) chips
@@ -462,6 +463,7 @@ struct irq_chip {
int (*irq_set_affinity)(struct irq_data *data, const struct cpumask *dest, bool force);
int (*irq_retrigger)(struct irq_data *data);
+ int (*irq_inflight)(struct irq_data *data);
int (*irq_set_type)(struct irq_data *data, unsigned int flow_type);
int (*irq_set_wake)(struct irq_data *data, unsigned int on);
--- a/kernel/irq/manage.c
+++ b/kernel/irq/manage.c
@@ -35,8 +35,10 @@ static int __init setup_forced_irqthread
early_param("threadirqs", setup_forced_irqthreads);
#endif
-static void __synchronize_hardirq(struct irq_desc *desc)
+static void __synchronize_hardirq(struct irq_desc *desc, bool sync_chip)
{
+ struct irq_data *irqd = irq_desc_get_irq_data(desc);
+ struct irq_chip *chip = irq_data_get_irq_chip(irqd);
bool inprogress;
do {
@@ -52,6 +54,13 @@ static void __synchronize_hardirq(struct
/* Ok, that indicated we're done: double-check carefully. */
raw_spin_lock_irqsave(&desc->lock, flags);
inprogress = irqd_irq_inprogress(&desc->irq_data);
+
+ /*
+ * If requested and supported, check at the chip whether it
+ * is in flight at the hardware level:
+ */
+ if (!inprogress && sync_chip && chip && chip->irq_inflight)
+ inprogress = chip->irq_inflight(irqd);
raw_spin_unlock_irqrestore(&desc->lock, flags);
/* Oops, that failed? */
@@ -74,13 +83,16 @@ static void __synchronize_hardirq(struct
* Returns: false if a threaded handler is active.
*
* This function may be called - with care - from IRQ context.
+ *
+ * It does not check whether there is an interrupt on flight at the
+ * hardware level, but not serviced yet, as this might deadlock.
*/
bool synchronize_hardirq(unsigned int irq)
{
struct irq_desc *desc = irq_to_desc(irq);
if (desc) {
- __synchronize_hardirq(desc);
+ __synchronize_hardirq(desc, false);
return !atomic_read(&desc->threads_active);
}
@@ -97,13 +109,16 @@ EXPORT_SYMBOL(synchronize_hardirq);
* holding a resource the IRQ handler may need you will deadlock.
*
* This function may be called - with care - from IRQ context.
+ *
+ * It does not check whether there is an interrupt on flight at the
+ * hardware level, but not serviced yet, as this might deadlock.
*/
void synchronize_irq(unsigned int irq)
{
struct irq_desc *desc = irq_to_desc(irq);
if (desc) {
- __synchronize_hardirq(desc);
+ __synchronize_hardirq(desc, false);
/*
* We made sure that no hardirq handler is
* running. Now verify that no threaded handlers are
@@ -1729,8 +1744,12 @@ static struct irqaction *__free_irq(stru
unregister_handler_proc(irq, action);
- /* Make sure it's not being used on another CPU: */
- synchronize_hardirq(irq);
+ /*
+ * Make sure it's not being used on another CPU and if the chip
+ * supports it also make sure that there is no (not yet serviced)
+ * interrupt on flight at the hardware level.
+ */
+ __synchronize_hardirq(desc, true);
#ifdef CONFIG_DEBUG_SHIRQ
/*
next prev parent reply other threads:[~2019-06-25 11:24 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-06-25 11:13 [patch 0/5] x86/irq: Cure various interrupt issues Thomas Gleixner
2019-06-25 11:13 ` [patch 1/5] genirq: Delay deactivation in free_irq() Thomas Gleixner
2019-06-28 7:42 ` Marc Zyngier
2019-06-28 9:28 ` Thomas Gleixner
2019-06-25 11:13 ` Thomas Gleixner [this message]
2019-06-26 13:03 ` [patch 2/5] genirq: Add optional hardware synchronization for shutdown Thomas Gleixner
2019-06-28 7:59 ` Marc Zyngier
2019-06-28 9:41 ` Thomas Gleixner
2019-06-25 11:13 ` [patch 3/5] x86/ioapic: Implement irq_inflight() callback Thomas Gleixner
2019-06-25 11:13 ` [patch 4/5] x86/irq: Handle spurious interrupt after shutdown gracefully Thomas Gleixner
2019-06-25 11:13 ` [patch 5/5] x86/irq: Seperate unused system vectors from spurious entry again Thomas Gleixner
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=20190625112405.666964552@linutronix.de \
--to=tglx@linutronix.de \
--cc=Robert.Hodaszi@digi.com \
--cc=gregkh@linuxfoundation.org \
--cc=idosch@mellanox.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-serial@vger.kernel.org \
--cc=marc.zyngier@arm.com \
--cc=vadimp@mellanox.com \
--cc=x86@kernel.org \
/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
all inboxes | Powered by JetHome®