linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Rafael J. Wysocki" <rjw@rjwysocki.net>
To: Thomas Gleixner <tglx@linutronix.de>
Cc: Peter Zijlstra <peterz@infradead.org>,
	Linux PM list <linux-pm@vger.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Linux PCI <linux-pci@vger.kernel.org>
Subject: [PATCH 2/6 v2] irq / PM: Make IRQF_NO_SUSPEND work with shared interrupts
Date: Mon, 11 Aug 2014 15:59:26 +0200	[thread overview]
Message-ID: <1748149.0fqmHNfelP@vostro.rjw.lan> (raw)
In-Reply-To: <26580319.OZP7jvJnA9@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 IRQ, the
interrupt handlers of the irqactions with IRQF_NO_SUSPEND unset
will be invoked after suspend_device_irqs(), even though they are
not supposed to be invoked at that time.  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 IRQ, the
interrupt handlers of the irqactions with IRQF_NO_SUSPEND set will
not be invoked after suspend_device_irqs(), although they are
supposed to be invoked at that time.  That may be a problem if
interrupts from the corresponding sources have to be properly
handled during system suspend/resume too, which is the case for
timer interrupts or the ACPI SCI, for example.

Both the situations described above have to be avoided, but the
fix should only impact the handling of interrupts during system
suspend and resume.  That means no changes to the code in
kernel/irq/handle.c and to the code called from there.  Moreover,
kernels build with CONFIG_PM_SLEEP unset should not be affected
at all.

Another thing to consider is what to do if there's an unhandled
interrupt during system suspend after suspend_device_irqs() and
the consensus here seems to be to abort the suspend it that case
(or trigger a wakeup from suspend-to-idle if already there).

Taking the above into account leads to the following approach.

During suspend_device_irqs() all of the shared IRQs that are not to
be suspended (that is, shared IRQs with at least one IRQF_NO_SUSPEND
irqaction) are switched over to a special "suspend mode" by replacing
the original interrupt handlers in their irqactions by a wrapper
handler.

That wrapper handler executes the original interrupt handlers for
the IRQF_NO_SUSPEND irqactions only and tracks their return codes.
It does that by combining the return code of the current irqaction's
handler with the return codes from all of the previous irqactions
in the chain and propagating that value to the next irqaction in the
chain with the help of the (new) prev_ret field in struct irqaction.
For the last irqaction in the chain, the combined return code is thus
equal to the final value of retval in handle_irq_event_percpu().  If
that value is IRQ_NONE, the wrapper handler regards the interrupt as
unhandled, disables the IRQ, marks it as "suspended" and triggers
system wakeup to allow it to recover from that potentially dangerous
situation.

During resume_device_irqs() all IRQs that were previously in the
"suspend mode" described above are switched back to their original
configuration.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 include/linux/interrupt.h |    8 ++++
 include/linux/irqdesc.h   |    3 +
 kernel/irq/internals.h    |   20 ++++++++++
 kernel/irq/manage.c       |   11 ++++-
 kernel/irq/pm.c           |   89 ++++++++++++++++++++++++++++++++++++++++++++--
 5 files changed, 127 insertions(+), 4 deletions(-)

Index: linux-pm/include/linux/interrupt.h
===================================================================
--- linux-pm.orig/include/linux/interrupt.h
+++ linux-pm/include/linux/interrupt.h
@@ -101,6 +101,9 @@ typedef irqreturn_t (*irq_handler_t)(int
  * @thread_flags:	flags related to @thread
  * @thread_mask:	bitmask for keeping track of @thread activity
  * @dir:	pointer to the proc/irq/NN/name entry
+ * @s_handler:	original interrupt handler for suspend mode interrupts
+ * @s_dev_id:	original device identification cookie for suspend mode
+ * @prev_ret:	suspend mode return code from the previous action
  */
 struct irqaction {
 	irq_handler_t		handler;
@@ -115,6 +118,11 @@ struct irqaction {
 	unsigned long		thread_mask;
 	const char		*name;
 	struct proc_dir_entry	*dir;
+#ifdef CONFIG_PM_SLEEP
+	irq_handler_t		s_handler;
+	void			*s_dev_id;
+	irqreturn_t		prev_ret;
+#endif
 } ____cacheline_internodealigned_in_smp;
 
 extern irqreturn_t no_action(int cpl, void *dev_id);
Index: linux-pm/include/linux/irqdesc.h
===================================================================
--- linux-pm.orig/include/linux/irqdesc.h
+++ linux-pm/include/linux/irqdesc.h
@@ -71,6 +71,9 @@ struct irq_desc {
 #ifdef CONFIG_PROC_FS
 	struct proc_dir_entry	*dir;
 #endif
+#ifdef CONFIG_PM_SLEEP
+	unsigned int		skip_suspend_depth;
+#endif
 	int			parent_irq;
 	struct module		*owner;
 	const char		*name;
Index: linux-pm/kernel/irq/manage.c
===================================================================
--- linux-pm.orig/kernel/irq/manage.c
+++ linux-pm/kernel/irq/manage.c
@@ -385,7 +385,9 @@ 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)
+			return;
+		if (irq_pm_suspend_mode(desc))
 			return;
 		desc->istate |= IRQS_SUSPENDED;
 	}
@@ -445,6 +447,7 @@ EXPORT_SYMBOL(disable_irq);
 void __enable_irq(struct irq_desc *desc, unsigned int irq, bool resume)
 {
 	if (resume) {
+		irq_pm_normal_mode(desc);
 		if (!(desc->istate & IRQS_SUSPENDED)) {
 			if (!desc->action)
 				return;
@@ -1222,6 +1225,8 @@ __setup_irq(unsigned int irq, struct irq
 	desc->irq_count = 0;
 	desc->irqs_unhandled = 0;
 
+	irq_pm_setup(desc, new);
+
 	/*
 	 * Check whether we disabled the irq via the spurious handler
 	 * before. Reenable it and give it another chance.
@@ -1328,7 +1333,7 @@ static struct irqaction *__free_irq(unsi
 			return NULL;
 		}
 
-		if (action->dev_id == dev_id)
+		if (action->dev_id == dev_id || irq_pm_saved_id(action, dev_id))
 			break;
 		action_ptr = &action->next;
 	}
@@ -1336,6 +1341,8 @@ static struct irqaction *__free_irq(unsi
 	/* Found it - now remove it from the list of entries: */
 	*action_ptr = action->next;
 
+	irq_pm_cleanup(desc, action);
+
 	/* If this was the last handler, shut down the IRQ line: */
 	if (!desc->action) {
 		irq_shutdown(desc);
Index: linux-pm/kernel/irq/pm.c
===================================================================
--- linux-pm.orig/kernel/irq/pm.c
+++ linux-pm/kernel/irq/pm.c
@@ -9,10 +9,96 @@
 #include <linux/irq.h>
 #include <linux/module.h>
 #include <linux/interrupt.h>
+#include <linux/suspend.h>
 #include <linux/syscore_ops.h>
 
 #include "internals.h"
 
+static void irq_pm_restore_handler(struct irqaction *action)
+{
+	if (action->s_handler) {
+		action->handler = action->s_handler;
+		action->s_handler = NULL;
+		action->dev_id = action->s_dev_id;
+		action->s_dev_id = NULL;
+	}
+}
+
+static void irq_pm_disable_and_wakeup(int irq)
+{
+	struct irq_desc *desc = irq_to_desc(irq);
+
+	desc->istate |= IRQS_SUSPENDED;
+	desc->depth++;
+	irq_disable(desc);
+	pm_system_wakeup();
+}
+
+static irqreturn_t irq_suspend_mode_handler(int irq, void *dev_id)
+{
+	struct irqaction *action = dev_id;
+	struct irqaction *next = action->next;
+	irqreturn_t ret = (action->flags & IRQF_NO_SUSPEND) ?
+		action->s_handler(irq, action->s_dev_id) : IRQ_NONE;
+
+	if (next) {
+		/* Propagate the return code. */
+		next->prev_ret = ret | action->prev_ret;
+	} else if ((ret | action->prev_ret) == IRQ_NONE) {
+		/*
+		 * This is the last action is this chain, and all of the
+		 * handlers returned IRQ_NONE.  This means an unhandled
+		 * interrupt during system suspend, so disable the IRQ and
+		 * trigger wakeup.
+		 */
+		pr_err("IRQ %d: Unhandled while suspended\n", irq);
+		irq_pm_disable_and_wakeup(irq);
+	}
+	return ret;
+}
+
+bool irq_pm_suspend_mode(struct irq_desc *desc)
+{
+	struct irqaction *action;
+
+	if (!desc->skip_suspend_depth)
+		return false;
+
+	/* No need to replace the handler if the IRQ is not shared. */
+	if (!desc->action->next)
+		return true;
+
+	for (action = desc->action; action; action = action->next) {
+		action->s_handler = action->handler;
+		action->handler = irq_suspend_mode_handler;
+		action->s_dev_id = action->dev_id;
+		action->dev_id = action;
+		action->prev_ret = IRQ_NONE;
+	}
+	return true;
+}
+
+void irq_pm_normal_mode(struct irq_desc *desc)
+{
+	struct irqaction *action;
+
+	for (action = desc->action; action; action = action->next)
+		irq_pm_restore_handler(action);
+}
+
+void irq_pm_setup(struct irq_desc *desc, struct irqaction *action)
+{
+	if (action->flags & IRQF_NO_SUSPEND)
+		desc->skip_suspend_depth++;
+}
+
+void irq_pm_cleanup(struct irq_desc *desc, struct irqaction *action)
+{
+	irq_pm_restore_handler(action);
+	if (action->flags & IRQF_NO_SUSPEND)
+		desc->skip_suspend_depth--;
+}
+
 /**
  * suspend_device_irqs - disable all currently enabled interrupt lines
  *
@@ -35,8 +121,7 @@ void suspend_device_irqs(void)
 	}
 
 	for_each_irq_desc(irq, desc)
-		if (desc->istate & IRQS_SUSPENDED)
-			synchronize_irq(irq);
+		synchronize_irq(irq);
 }
 EXPORT_SYMBOL_GPL(suspend_device_irqs);
 
Index: linux-pm/kernel/irq/internals.h
===================================================================
--- linux-pm.orig/kernel/irq/internals.h
+++ linux-pm/kernel/irq/internals.h
@@ -194,3 +194,23 @@ static inline void kstat_incr_irqs_this_
 	__this_cpu_inc(*desc->kstat_irqs);
 	__this_cpu_inc(kstat.irqs_sum);
 }
+
+#ifdef CONFIG_PM_SLEEP
+static inline bool irq_pm_saved_id(struct irqaction *action, void *dev_id)
+{
+	return action->s_dev_id == dev_id;
+}
+extern void irq_pm_setup(struct irq_desc *desc, struct irqaction *action);
+extern void irq_pm_cleanup(struct irq_desc *desc, struct irqaction *action);
+extern bool irq_pm_suspend_mode(struct irq_desc *desc);
+extern void irq_pm_normal_mode(struct irq_desc *desc);
+#else
+static inline bool irq_pm_saved_id(struct irqaction *action, void *dev_id)
+{
+	return false;
+}
+static inline void irq_pm_setup(struct irq_desc *desc, struct irqaction *action) {}
+static inline void irq_pm_cleanup(struct irq_desc *desc, struct irqaction *action) {}
+static inline bool irq_pm_suspend_mode(struct irq_desc *desc) { return false; }
+static inline void irq_pm_normal_mode(struct irq_desc *desc) {}
+#endif


  parent reply	other threads:[~2014-08-11 13:44 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-08-11 13:56 [PATCH 0/6 v2] irq / PM: Shared IRQs vs IRQF_NO_SUSPEND and suspend-to-idle wakeup interrupts Rafael J. Wysocki
2014-08-11 13:58 ` [PATCH 1/6 v2] PM / sleep: Mechanism for aborting system suspends unconditionally Rafael J. Wysocki
2014-08-11 13:59 ` Rafael J. Wysocki [this message]
2014-08-11 14:00 ` [PATCH 3/6 v2] irq / PM: Make wakeup interrupts work with suspend-to-idle Rafael J. Wysocki
2014-08-11 14:01 ` [PATCH 4/6 v2] x86 / PM: Set IRQCHIP_SKIP_SET_WAKE for IOAPIC IRQ chip objects Rafael J. Wysocki
2014-08-11 14:02 ` [PATCH 5/6 v2] PCI / PM: Make PCIe PME interrupts wake up from suspend-to-idle Rafael J. Wysocki
2014-08-11 14:03 ` [PATCH 6/6 v2] irq / PM: Document rules related to system suspend and interrupts Rafael J. Wysocki
2014-08-26 23:46 ` [PATCH 0/5 v3] irq / PM: Suspend-to-idle wakeup interrupts Rafael J. Wysocki
2014-08-26 23:47   ` [PATCH 1/5 v3] PM / sleep: Mechanism for aborting system suspends unconditionally Rafael J. Wysocki
2014-08-26 23:49   ` [PATCH 2/5 v3] irq / PM: Make wakeup interrupts work with suspend-to-idle Rafael J. Wysocki
2014-08-27 20:32     ` Thomas Gleixner
2014-08-27 22:51       ` Rafael J. Wysocki
2014-08-28  9:23         ` Thomas Gleixner
2014-08-29  1:51           ` Rafael J. Wysocki
2014-08-26 23:50   ` [PATCH 3/5 v3] x86 / PM: Set IRQCHIP_SKIP_SET_WAKE for IOAPIC IRQ chip objects Rafael J. Wysocki
2014-08-26 23:51   ` [PATCH 4/5 v3] PCI / PM: Make PCIe PME interrupts wake up from suspend-to-idle Rafael J. Wysocki
2014-08-26 23:52   ` [PATCH 5/5 v3] irq / PM: Document rules related to system suspend and interrupts Rafael J. Wysocki
2014-08-28 22:44   ` [PATCH 0/5 v3] irq / PM: Suspend-to-idle wakeup interrupts Thomas Gleixner
2014-08-29  0:54     ` Rafael J. Wysocki
2014-08-29  1:09       ` Thomas Gleixner
2014-09-01 14:18   ` [PATCH 00/13] genirq / PM: Wakeup interrupts handling rework (related to suspend-to-idle) Rafael J. Wysocki
2014-09-01 14:19     ` [PATCH 01/13] PM / sleep: Mechanism for aborting system suspends unconditionally Rafael J. Wysocki
2014-09-01 14:20     ` [PATCH 02/13] genirq: Move suspend/resume logic into irq/pm code Rafael J. Wysocki
2014-09-01 14:21     ` [PATCH 03/13] genirq: Add sanity checks for PM options on shared interrupt lines Rafael J. Wysocki
2014-09-01 14:22     ` [PATCH 04/13] genirq: Make use of pm misfeature accounting Rafael J. Wysocki
2014-09-01 14:22     ` [PATCH 05/13] genirq: Move MASK_ON_SUSPEND handling into suspend_device_irqs() Rafael J. Wysocki
2014-09-01 14:23     ` [PATCH 06/13] genirq: Avoid double loop on suspend Rafael J. Wysocki
2014-09-01 14:24     ` [PATCH 07/13] genirq: Distangle edge handler entry Rafael J. Wysocki
2014-09-01 14:24     ` [PATCH 08/13] genirq: Create helper for flow handler entry check Rafael J. Wysocki
2014-09-01 14:26     ` [PATCH 09/13] genirq: Mark wakeup sources as armed on suspend Rafael J. Wysocki
2014-09-01 14:27     ` [PATCH 10/13] genirq: Simplify wakeup mechanism Rafael J. Wysocki
2014-09-01 14:28     ` [PATCH 11/13] x86 / PM: Set IRQCHIP_SKIP_SET_WAKE for IOAPIC IRQ chip objects Rafael J. Wysocki
2014-09-01 14:28     ` [PATCH 12/13] PCI / PM: Make PCIe PME interrupts wake up from suspend-to-idle Rafael J. Wysocki
2014-09-01 14:29     ` [PATCH 13/13] PM / genirq: Document rules related to system suspend and interrupts Rafael J. Wysocki

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=1748149.0fqmHNfelP@vostro.rjw.lan \
    --to=rjw@rjwysocki.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=peterz@infradead.org \
    --cc=tglx@linutronix.de \
    --subject='Re: [PATCH 2/6 v2] irq / PM: Make IRQF_NO_SUSPEND work with shared interrupts' \
    /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

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).