From: "Rafael J. Wysocki" <rjw@rjwysocki.net>
To: Thomas Gleixner <tglx@linutronix.de>
Cc: Peter Zijlstra <peterz@infradead.org>,
linux-kernel@vger.kernel.org,
Linux PM list <linux-pm@vger.kernel.org>,
Dmitry Torokhov <dtor@google.com>
Subject: [PATCH 2/5] irq / PM: Fix IRQF_NO_SUSPEND problem with shared interrupts
Date: Tue, 05 Aug 2014 17:25:42 +0200 [thread overview]
Message-ID: <1871482.OQTHsF7T50@vostro.rjw.lan> (raw)
In-Reply-To: <3321219.itH23ZEDt4@vostro.rjw.lan>
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Since __disable_irq() only checks IRQF_NO_SUSPEND for the first
irqaction in a given irq_desc, that value of that bit for the
first irqaction affects all of the other irqactions sharing the
interrupt with it. This is problematic in two cases.
First, if IRQF_NO_SUSPEND is set in the first irqaction and unset
in at least one of the other irqactions sharing the same interrupt,
the interrupt handlers in the other irqactions with IRQF_NO_SUSPEND
unset will be invoked after suspend_device_irqs() has returned even
though they are not supposed to. That shouldn't be a problem if
those interrupt handlers are implemented correctly and the
corresponding devices are properly suspended, but it may lead to
functional issues otherwise.
Second, if IRQF_NO_SUSPEND is unset in the first irqaction and set
in at least one of the other irqactions sharing the same interrupt,
the interrupt handlers in the other irqactions with IRQF_NO_SUSPEND
set will not be invoked after suspend_device_irqs() has returned, but
they are supposed to be invoked then. That may be a problem if the
corresponding devices are wakeup devices or if their interrupts have
to be properly handled during system suspend/resume too for other
reasons, which is the case for timer interrupts or the ACPI SCI for
example.
To address this, introduce a wrapper interrupt handler for shared
interrupts, irq_shared_wrapper_handler(), and make __setup_irq()
use it for shared interrupts by default. That wrapper handler
simply invokes the original handler supplied while requesting the
IRQ if the (new) irqs_suspended global variable is unset. Otherwise
it returns IRQ_NONE. The wrapper handler is used for all irqactions
sharing IRQs except for irqactions with IRQF_NO_SUSPEND set that
retain their original handlers.
The irqs_suspended variable is set by suspend_device_irqs() and
unset by resume_device_irqs(). It is checked by note_interrupt()
(now called unconditionally) and if set, any unhandled interrupt
will cause the system to wake up (or system suspend in progress
to be aborted).
The additional (new) skip_suspend field in struct irq_desc is
used to track if any irqactions in the given desc have
IRQF_NO_SUSPEND set. It is used by suspend_device_irqs() to
decide which IRQs to disable during system suspends. This way,
all IRQs with at least one irqaction having IRQF_NO_SUSPEND set
(skip_suspend greater than 0) will stay enabled after
suspend_device_irqs(), but if any irqactions belonging to them
have IRQF_NO_SUSPEND unset, they will use the wrapper handler
that will return IRQ_NONE for them (causing system suspend to
be aborted or system wakeup from suspend-to-idle to happen).
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
include/linux/interrupt.h | 4 +++
include/linux/irqdesc.h | 1
kernel/irq/handle.c | 3 --
kernel/irq/internals.h | 3 ++
kernel/irq/manage.c | 52 ++++++++++++++++++++++++++++++++++++++++++++--
kernel/irq/pm.c | 10 +++++---
kernel/irq/spurious.c | 13 +++++++++++
7 files changed, 78 insertions(+), 8 deletions(-)
Index: linux-pm/kernel/irq/manage.c
===================================================================
--- linux-pm.orig/kernel/irq/manage.c
+++ linux-pm/kernel/irq/manage.c
@@ -385,7 +385,7 @@ setup_affinity(unsigned int irq, struct
void __disable_irq(struct irq_desc *desc, unsigned int irq, bool suspend)
{
if (suspend) {
- if (!desc->action || (desc->action->flags & IRQF_NO_SUSPEND))
+ if (!desc->action || desc->skip_suspend)
return;
desc->istate |= IRQS_SUSPENDED;
}
@@ -658,6 +658,43 @@ int irq_set_parent(int irq, int parent_i
}
#endif
+bool irqs_suspended __read_mostly;
+
+void set_irqs_suspended(bool on)
+{
+ struct irq_desc *desc;
+ int irq;
+
+ irqs_suspended = on;
+ for_each_irq_desc(irq, desc)
+ synchronize_irq(irq);
+}
+
+/*
+ * Wrapper handler for shared interrupts.
+ */
+static irqreturn_t irq_shared_wrapper_handler(int irq, void *dev_id)
+{
+ struct irqaction *action = dev_id;
+
+ if (unlikely(irqs_suspended))
+ return IRQ_NONE;
+
+ return action->s_handler(irq, action->s_dev_id);
+}
+
+static void irq_shared_wrap_handler(struct irqaction *action)
+{
+ if (action->handler == irq_shared_wrapper_handler ||
+ (action->flags & IRQF_NO_SUSPEND))
+ return;
+
+ action->s_handler = action->handler;
+ action->handler = irq_shared_wrapper_handler;
+ action->s_dev_id = action->dev_id;
+ action->dev_id = action;
+}
+
/*
* Default primary interrupt handler for threaded interrupts. Is
* assigned as primary handler when request_threaded_irq is called
@@ -1088,6 +1125,7 @@ __setup_irq(unsigned int irq, struct irq
/* add new interrupt at end of irq queue */
do {
+ irq_shared_wrap_handler(old);
/*
* Or all existing action->thread_mask bits,
* so we can find the next zero bit for this
@@ -1097,6 +1135,7 @@ __setup_irq(unsigned int irq, struct irq
old_ptr = &old->next;
old = *old_ptr;
} while (old);
+ irq_shared_wrap_handler(new);
shared = 1;
}
@@ -1222,6 +1261,9 @@ __setup_irq(unsigned int irq, struct irq
desc->irq_count = 0;
desc->irqs_unhandled = 0;
+ if (new->flags & IRQF_NO_SUSPEND)
+ desc->skip_suspend++;
+
/*
* Check whether we disabled the irq via the spurious handler
* before. Reenable it and give it another chance.
@@ -1328,13 +1370,19 @@ static struct irqaction *__free_irq(unsi
return NULL;
}
- if (action->dev_id == dev_id)
+ if (action->dev_id == dev_id || action->s_dev_id == dev_id)
break;
action_ptr = &action->next;
}
/* Found it - now remove it from the list of entries: */
*action_ptr = action->next;
+ if (action->flags & IRQF_NO_SUSPEND) {
+ desc->skip_suspend--;
+ } else if (action->handler == irq_shared_wrapper_handler) {
+ action->handler = action->s_handler;
+ action->dev_id = action->s_dev_id;
+ }
/* If this was the last handler, shut down the IRQ line: */
if (!desc->action) {
Index: linux-pm/include/linux/interrupt.h
===================================================================
--- linux-pm.orig/include/linux/interrupt.h
+++ linux-pm/include/linux/interrupt.h
@@ -90,8 +90,10 @@ typedef irqreturn_t (*irq_handler_t)(int
/**
* struct irqaction - per interrupt action descriptor
* @handler: interrupt handler function
+ * @s_handler: interrupt handler function for shared interrupts
* @name: name of the device
* @dev_id: cookie to identify the device
+ * @s_dev_id: cookie to identify the device for shared interrupts
* @percpu_dev_id: cookie to identify the device
* @next: pointer to the next irqaction for shared interrupts
* @irq: interrupt number
@@ -105,6 +107,8 @@ typedef irqreturn_t (*irq_handler_t)(int
struct irqaction {
irq_handler_t handler;
void *dev_id;
+ irq_handler_t s_handler;
+ void *s_dev_id;
void __percpu *percpu_dev_id;
struct irqaction *next;
irq_handler_t thread_fn;
Index: linux-pm/kernel/irq/handle.c
===================================================================
--- linux-pm.orig/kernel/irq/handle.c
+++ linux-pm/kernel/irq/handle.c
@@ -175,8 +175,7 @@ handle_irq_event_percpu(struct irq_desc
add_interrupt_randomness(irq, flags);
- if (!noirqdebug)
- note_interrupt(irq, desc, retval);
+ note_interrupt(irq, desc, retval);
return retval;
}
Index: linux-pm/kernel/irq/spurious.c
===================================================================
--- linux-pm.orig/kernel/irq/spurious.c
+++ linux-pm/kernel/irq/spurious.c
@@ -13,6 +13,7 @@
#include <linux/interrupt.h>
#include <linux/moduleparam.h>
#include <linux/timer.h>
+#include <linux/suspend.h>
#include "internals.h"
@@ -275,6 +276,18 @@ try_misrouted_irq(unsigned int irq, stru
void note_interrupt(unsigned int irq, struct irq_desc *desc,
irqreturn_t action_ret)
{
+ if (unlikely(irqs_suspended && action_ret == IRQ_NONE)) {
+ pr_err("IRQ %d: Unhandled while suspended\n", irq);
+ desc->istate |= IRQS_SUSPENDED;
+ desc->depth++;
+ irq_disable(desc);
+ pm_system_wakeup();
+ return;
+ }
+
+ if (noirqdebug)
+ return;
+
if (desc->istate & IRQS_POLL_INPROGRESS ||
irq_settings_is_polled(desc))
return;
Index: linux-pm/kernel/irq/pm.c
===================================================================
--- linux-pm.orig/kernel/irq/pm.c
+++ linux-pm/kernel/irq/pm.c
@@ -10,6 +10,7 @@
#include <linux/module.h>
#include <linux/interrupt.h>
#include <linux/syscore_ops.h>
+#include <linux/suspend.h>
#include "internals.h"
@@ -33,10 +34,7 @@ void suspend_device_irqs(void)
__disable_irq(desc, irq, true);
raw_spin_unlock_irqrestore(&desc->lock, flags);
}
-
- for_each_irq_desc(irq, desc)
- if (desc->istate & IRQS_SUSPENDED)
- synchronize_irq(irq);
+ set_irqs_suspended(true);
}
EXPORT_SYMBOL_GPL(suspend_device_irqs);
@@ -90,6 +88,7 @@ device_initcall(irq_pm_init_ops);
*/
void resume_device_irqs(void)
{
+ set_irqs_suspended(false);
resume_irqs(false);
}
EXPORT_SYMBOL_GPL(resume_device_irqs);
@@ -102,6 +101,9 @@ int check_wakeup_irqs(void)
struct irq_desc *desc;
int irq;
+ if (pm_wakeup_pending())
+ return -EBUSY;
+
for_each_irq_desc(irq, desc) {
/*
* Only interrupts which are marked as wakeup source
Index: linux-pm/kernel/irq/internals.h
===================================================================
--- linux-pm.orig/kernel/irq/internals.h
+++ linux-pm/kernel/irq/internals.h
@@ -17,6 +17,9 @@
#define istate core_internal_state__do_not_mess_with_it
extern bool noirqdebug;
+extern bool irqs_suspended;
+
+extern void set_irqs_suspended(bool on);
/*
* Bits used by threaded handlers:
Index: linux-pm/include/linux/irqdesc.h
===================================================================
--- linux-pm.orig/include/linux/irqdesc.h
+++ linux-pm/include/linux/irqdesc.h
@@ -51,6 +51,7 @@ struct irq_desc {
unsigned int core_internal_state__do_not_mess_with_it;
unsigned int depth; /* nested irq disables */
unsigned int wake_depth; /* nested wake enables */
+ unsigned int skip_suspend;
unsigned int irq_count; /* For detecting broken IRQs */
unsigned long last_unhandled; /* Aging timer for unhandled count */
unsigned int irqs_unhandled;
next prev parent reply other threads:[~2014-08-05 15:11 UTC|newest]
Thread overview: 75+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-07-24 21:26 [RFC][PATCH] irq: Rework IRQF_NO_SUSPENDED Peter Zijlstra
2014-07-24 22:02 ` Rafael J. Wysocki
2014-07-24 23:10 ` Rafael J. Wysocki
2014-07-25 5:58 ` Peter Zijlstra
2014-07-29 19:20 ` Brian Norris
2014-07-29 19:28 ` Peter Zijlstra
2014-07-29 20:41 ` Brian Norris
2014-07-25 9:27 ` Thomas Gleixner
2014-07-25 12:49 ` Rafael J. Wysocki
2014-07-25 13:55 ` Thomas Gleixner
2014-07-25 9:40 ` Thomas Gleixner
2014-07-25 12:40 ` Peter Zijlstra
2014-07-25 13:25 ` Peter Zijlstra
2014-07-25 17:03 ` Rafael J. Wysocki
2014-07-25 16:58 ` Peter Zijlstra
2014-07-25 21:00 ` Thomas Gleixner
2014-07-25 22:25 ` Rafael J. Wysocki
2014-07-25 23:07 ` Rafael J. Wysocki
2014-07-26 11:49 ` Rafael J. Wysocki
2014-07-26 11:53 ` Rafael J. Wysocki
2014-07-28 6:49 ` Peter Zijlstra
2014-07-28 12:33 ` Thomas Gleixner
2014-07-28 13:04 ` Peter Zijlstra
2014-07-28 21:53 ` Rafael J. Wysocki
2014-07-28 23:01 ` Rafael J. Wysocki
2014-07-29 12:46 ` Thomas Gleixner
2014-07-29 13:33 ` Rafael J. Wysocki
2014-07-30 21:46 ` [PATCH 0/3] irq / PM: wakeup interrupt interface for drivers (was: Re: [RFC][PATCH] irq: Rework IRQF_NO_SUSPENDED) Rafael J. Wysocki
2014-07-30 21:51 ` [PATCH 1/3] irq / PM: New driver interface for wakeup interrupts Rafael J. Wysocki
2014-07-30 22:56 ` Thomas Gleixner
2014-07-31 0:12 ` Thomas Gleixner
2014-07-31 2:14 ` Rafael J. Wysocki
2014-07-31 10:44 ` Thomas Gleixner
2014-07-31 18:36 ` Rafael J. Wysocki
2014-07-31 20:12 ` Alan Stern
2014-07-31 20:12 ` Alan Stern
2014-07-31 21:04 ` Rafael J. Wysocki
2014-07-31 23:41 ` Thomas Gleixner
2014-08-01 0:51 ` Rafael J. Wysocki
2014-08-01 14:41 ` Alan Stern
2014-08-01 14:41 ` Alan Stern
2014-07-31 22:16 ` Thomas Gleixner
2014-08-01 0:08 ` Rafael J. Wysocki
2014-08-01 1:24 ` Rafael J. Wysocki
2014-08-01 9:40 ` [PATCH 1/3] irq / PM: New driver interface for wakeup interruptsn Thomas Gleixner
2014-08-01 13:45 ` Rafael J. Wysocki
2014-08-01 13:43 ` Thomas Gleixner
2014-08-01 14:29 ` Rafael J. Wysocki
2014-08-02 1:31 ` Rafael J. Wysocki
2014-08-03 13:42 ` Rafael J. Wysocki
2014-08-04 3:38 ` Rafael J. Wysocki
2014-08-05 15:22 ` [PATCH 0/5] irq / PM: Shared IRQs vs IRQF_NO_SUSPEND and suspend-to-idle wakeup Rafael J. Wysocki
2014-08-05 15:24 ` [PATCH 1/5] PM / sleep: Mechanism for aborting system suspends unconditionally Rafael J. Wysocki
2014-08-05 23:29 ` [Update][PATCH " Rafael J. Wysocki
2014-08-05 15:25 ` Rafael J. Wysocki [this message]
2014-08-05 15:26 ` [PATCH 3/5] irq / PM: Make wakeup interrupts wake up from suspend-to-idle Rafael J. Wysocki
2014-08-08 1:58 ` [Update][PATCH " Rafael J. Wysocki
2014-08-09 0:28 ` Rafael J. Wysocki
2014-08-05 15:27 ` [PATCH 4/5] x86 / PM: Set IRQCHIP_SKIP_SET_WAKE for IOAPIC IRQ chip objects Rafael J. Wysocki
2014-08-05 15:28 ` [PATCH 5/5] PCI / PM: Make PCIe PME interrupts wake up from suspend-to-idle Rafael J. Wysocki
2014-08-05 16:12 ` [PATCH 0/5] irq / PM: Shared IRQs vs IRQF_NO_SUSPEND and suspend-to-idle wakeup Peter Zijlstra
2014-08-08 2:09 ` Rafael J. Wysocki
2014-07-31 22:54 ` [PATCH 1/3] irq / PM: New driver interface for wakeup interrupts Thomas Gleixner
2014-07-30 21:51 ` [PATCH 2/3] PCI / PM: Make PCIe PME interrupts wake up from "freeze" sleep state Rafael J. Wysocki
2014-07-30 21:52 ` [PATCH 3/3] gpio-keys / PM: use enable/disable_device_irq_wake() Rafael J. Wysocki
2014-07-28 21:27 ` [RFC][PATCH] irq: Rework IRQF_NO_SUSPENDED Rafael J. Wysocki
2014-07-27 15:53 ` Rafael J. Wysocki
2014-07-27 22:00 ` [PATCH, v2] Rafael J. Wysocki
2014-07-28 12:11 ` Thomas Gleixner
2014-07-28 21:17 ` [PATCH, v3] irq / PM: Fix IRQF_NO_SUSPEND problem with shared interrupts (was: Re: [PATCH, v2]) Rafael J. Wysocki
2014-07-29 7:28 ` [PATCH, v4] irq / PM: Fix IRQF_NO_SUSPEND problem with shared interrupts Rafael J. Wysocki
2014-07-29 13:46 ` [PATCH, v5] " Rafael J. Wysocki
2014-07-30 0:54 ` [PATCH, v6] " Rafael J. Wysocki
2014-07-25 12:47 ` [RFC][PATCH] irq: Rework IRQF_NO_SUSPENDED Rafael J. Wysocki
2014-07-25 13:22 ` Peter Zijlstra
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=1871482.OQTHsF7T50@vostro.rjw.lan \
--to=rjw@rjwysocki.net \
--cc=dtor@google.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pm@vger.kernel.org \
--cc=peterz@infradead.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 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.