[PATCH 1/2] genirq: Introduce a new flag IRQ_IS_CHAINED for chained interrupts

[PATCH 1/2] genirq: Introduce a new flag IRQ_IS_CHAINED for chained interrupts

[Mika Westerberg]

In some cases it is useful to know if the interrupt in question has chained
handler installed. For example when a cpu is offlined the architecture code
needs to know if it has any users so that it can fixup affinity
accordingly.

To make this possible we introduce a new flag IRQ_IS_CHAINED that is set by
the core code when chained interrupt handler is installed. We also make it
possible for core and architecture code to check the flag by introducing
function irq_has_chained_handler() for this.

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
 include/linux/irq.h   |  6 +++++-
 kernel/irq/chip.c     | 11 +++++++++++
 kernel/irq/settings.h | 16 ++++++++++++++++
 3 files changed, 32 insertions(+), 1 deletion(-)

diff --git a/include/linux/irq.h b/include/linux/irq.h
index 6bcfe8ac7594..cefdb36eb3e9 100644
--- a/include/linux/irq.h
+++ b/include/linux/irq.h
@@ -72,6 +72,7 @@ enum irqchip_irq_state;
  * IRQ_IS_POLLED		- Always polled by another interrupt. Exclude
  *				  it from the spurious interrupt detection
  *				  mechanism and from core side polling.
+ * IRQ_IS_CHAINED		- Interrupt has chained handler installed
  */
 enum {
 	IRQ_TYPE_NONE		= 0x00000000,
@@ -97,13 +98,14 @@ enum {
 	IRQ_NOTHREAD		= (1 << 16),
 	IRQ_PER_CPU_DEVID	= (1 << 17),
 	IRQ_IS_POLLED		= (1 << 18),
+	IRQ_IS_CHAINED		= (1 << 19),
 };
 
 #define IRQF_MODIFY_MASK	\
 	(IRQ_TYPE_SENSE_MASK | IRQ_NOPROBE | IRQ_NOREQUEST | \
 	 IRQ_NOAUTOEN | IRQ_MOVE_PCNTXT | IRQ_LEVEL | IRQ_NO_BALANCING | \
 	 IRQ_PER_CPU | IRQ_NESTED_THREAD | IRQ_NOTHREAD | IRQ_PER_CPU_DEVID | \
-	 IRQ_IS_POLLED)
+	 IRQ_IS_POLLED | IRQ_IS_CHAINED)
 
 #define IRQ_NO_BALANCING_MASK	(IRQ_PER_CPU | IRQ_NO_BALANCING)
 
@@ -544,6 +546,8 @@ void
 irq_set_chained_handler_and_data(unsigned int irq, irq_flow_handler_t handle,
 				 void *data);
 
+bool irq_has_chained_handler(unsigned int irq);
+
 void irq_modify_status(unsigned int irq, unsigned long clr, unsigned long set);
 
 static inline void irq_set_status_flags(unsigned int irq, unsigned long set)
diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
index e28169dd1c36..826adc1e2c3c 100644
--- a/kernel/irq/chip.c
+++ b/kernel/irq/chip.c
@@ -747,6 +747,7 @@ __irq_do_set_handler(struct irq_desc *desc, irq_flow_handler_t handle,
 			mask_ack_irq(desc);
 		irq_state_set_disabled(desc);
 		desc->depth = 1;
+		irq_settings_clr_chained(desc);
 	}
 	desc->handle_irq = handle;
 	desc->name = name;
@@ -755,6 +756,7 @@ __irq_do_set_handler(struct irq_desc *desc, irq_flow_handler_t handle,
 		irq_settings_set_noprobe(desc);
 		irq_settings_set_norequest(desc);
 		irq_settings_set_nothread(desc);
+		irq_settings_set_chained(desc);
 		irq_startup(desc, true);
 	}
 }
@@ -791,6 +793,15 @@ irq_set_chained_handler_and_data(unsigned int irq, irq_flow_handler_t handle,
 }
 EXPORT_SYMBOL_GPL(irq_set_chained_handler_and_data);
 
+bool irq_has_chained_handler(unsigned int irq)
+{
+	struct irq_desc *desc = irq_to_desc(irq);
+
+	if (!desc)
+		return false;
+	return irq_settings_is_chained(desc);
+}
+
 void
 irq_set_chip_and_handler_name(unsigned int irq, struct irq_chip *chip,
 			      irq_flow_handler_t handle, const char *name)
diff --git a/kernel/irq/settings.h b/kernel/irq/settings.h
index 3320b84cc60f..b364dc873314 100644
--- a/kernel/irq/settings.h
+++ b/kernel/irq/settings.h
@@ -15,6 +15,7 @@ enum {
 	_IRQ_NESTED_THREAD	= IRQ_NESTED_THREAD,
 	_IRQ_PER_CPU_DEVID	= IRQ_PER_CPU_DEVID,
 	_IRQ_IS_POLLED		= IRQ_IS_POLLED,
+	_IRQ_IS_CHAINED		= IRQ_IS_CHAINED,
 	_IRQF_MODIFY_MASK	= IRQF_MODIFY_MASK,
 };
 
@@ -154,3 +155,18 @@ static inline bool irq_settings_is_polled(struct irq_desc *desc)
 {
 	return desc->status_use_accessors & _IRQ_IS_POLLED;
 }
+
+static inline void irq_settings_set_chained(struct irq_desc *desc)
+{
+	desc->status_use_accessors |= _IRQ_IS_CHAINED;
+}
+
+static inline void irq_settings_clr_chained(struct irq_desc *desc)
+{
+	desc->status_use_accessors &= ~_IRQ_IS_CHAINED;
+}
+
+static inline bool irq_settings_is_chained(struct irq_desc *desc)
+{
+	return desc->status_use_accessors & _IRQ_IS_CHAINED;
+}
-- 
2.5.1

[PATCH 2/2] x86/irq: Take chained interrupts into account in fixup_irqs()

[Mika Westerberg]

When a CPU is offlined all interrupts that have action are migrated to
other still online CPUs. However, if the interrupt has chained handler
installed this is not done. Chained handlers are used by GPIO drivers which
support interrupts, for instance.

When affinity is not corrected properly we end up in situation where some
interrupts are not arriving to the online CPUs anymore. For example on
Intel Braswell system which has SD-card card detection signal connected to
a GPIO the IO-APIC routing entries look like below after CPU1 put offline:

  pin30, enabled , level, low , V(52), IRR(0), S(0), logical , D(03), M(1)
  pin31, enabled , level, low , V(42), IRR(0), S(0), logical , D(03), M(1)
  pin32, enabled , level, low , V(62), IRR(0), S(0), logical , D(03), M(1)
  pin5b, enabled , level, low , V(72), IRR(0), S(0), logical , D(03), M(1)

The problem here is that the destination mask still contains both CPUs even
if CPU1 is already offline. This means that the IO-APIC still routes
interrupts to the other CPU as well.

Fix this by correcting affinity also for interrupts that have chained
handler installed.

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
Discussion about the issue can be read here:

https://lkml.org/lkml/2015/10/1/554

 arch/x86/kernel/irq.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kernel/irq.c b/arch/x86/kernel/irq.c
index f8062aaf5df9..e7cc9f199350 100644
--- a/arch/x86/kernel/irq.c
+++ b/arch/x86/kernel/irq.c
@@ -440,6 +440,7 @@ void fixup_irqs(void)
 		int break_affinity = 0;
 		int set_affinity = 1;
 		const struct cpumask *affinity;
+		bool has_user;
 
 		if (!desc)
 			continue;
@@ -451,7 +452,10 @@ void fixup_irqs(void)
 
 		data = irq_desc_get_irq_data(desc);
 		affinity = irq_data_get_affinity_mask(data);
-		if (!irq_has_action(irq) || irqd_is_per_cpu(data) ||
+
+		has_user = irq_has_action(irq) || irq_has_chained_handler(irq);
+
+		if (!has_user || irqd_is_per_cpu(data) ||
 		    cpumask_subset(affinity, cpu_online_mask)) {
 			raw_spin_unlock(&desc->lock);
 			continue;
-- 
2.5.1

Re: [PATCH 1/2] genirq: Introduce a new flag IRQ_IS_CHAINED for chained interrupts

[Thomas Gleixner]

On Fri, 2 Oct 2015, Mika Westerberg wrote:

> In some cases it is useful to know if the interrupt in question has chained
> handler installed. For example when a cpu is offlined the architecture code
> needs to know if it has any users so that it can fixup affinity
> accordingly.
> 
> To make this possible we introduce a new flag IRQ_IS_CHAINED that is set by
> the core code when chained interrupt handler is installed. We also make it
> possible for core and architecture code to check the flag by introducing
> function irq_has_chained_handler() for this.

While looking at that patch it occured to me, that we can solve this
in a different way, which does not require any changes to the
migration code.

When we install the chained handler, we set the action of that irq
descriptor to a statically allocated chained_action which provides a
handler which emits a fat warning. chained handlers should never end
up calling an action and if they do, it's clearly a bug.

The availability of an action makes the migration code just pick it
and move it along. And that fixes all architectures in one go without
touching them.

Sorry, that I didn't think about this right away.

Thanks,

	tglx

[PATCH v2] genirq: Allow migration of chained interrupts by installing default action

[Mika Westerberg]

When a CPU is offlined all interrupts that have an action are migrated to
other still online CPUs. However, if the interrupt has chained handler
installed this is not done. Chained handlers are used by GPIO drivers which
support interrupts, for instance.

When the affinity is not corrected properly we end up in situation where
most interrupts are not arriving to the online CPUs anymore. For example on
Intel Braswell system which has SD-card card detection signal connected to
a GPIO the IO-APIC routing entries look like below after CPU1 is offlined:

  pin30, enabled , level, low , V(52), IRR(0), S(0), logical , D(03), M(1)
  pin31, enabled , level, low , V(42), IRR(0), S(0), logical , D(03), M(1)
  pin32, enabled , level, low , V(62), IRR(0), S(0), logical , D(03), M(1)
  pin5b, enabled , level, low , V(72), IRR(0), S(0), logical , D(03), M(1)

The problem here is that the destination mask still contains both CPUs even
if CPU1 is already offline. This means that the IO-APIC still routes
interrupts to the other CPU as well.

We solve the problem by providing a default action for chained interrupts.
This action allows the migration code to correct affinity (as it finds
desc->action != NULL).

Also make the default action handler to emit a warning if for some reason a
chained handler ends up calling it.

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
I needed to declare chained_action in internals.h because otherwise chained
IRQs get listed in /proc/interrupts.

 kernel/irq/chip.c      | 17 +++++++++++++++++
 kernel/irq/internals.h |  2 ++
 kernel/irq/proc.c      |  2 +-
 3 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
index e28169dd1c36..b875934768d0 100644
--- a/kernel/irq/chip.c
+++ b/kernel/irq/chip.c
@@ -21,6 +21,20 @@
 
 #include "internals.h"
 
+static irqreturn_t bad_chained_irq(int irq, void *dev_id)
+{
+	WARN_ONCE(1, "Chained irq %d should not call an action\n", irq);
+	return IRQ_NONE;
+}
+
+/*
+ * Chained handlers should never call action on their IRQ. This default
+ * action will emit warning if such thing happens.
+ */
+struct irqaction chained_action = {
+	.handler = bad_chained_irq,
+};
+
 /**
  *	irq_set_chip - set the irq chip for an irq
  *	@irq:	irq number
@@ -746,6 +760,8 @@ __irq_do_set_handler(struct irq_desc *desc, irq_flow_handler_t handle,
 		if (desc->irq_data.chip != &no_irq_chip)
 			mask_ack_irq(desc);
 		irq_state_set_disabled(desc);
+		if (is_chained)
+			desc->action = NULL;
 		desc->depth = 1;
 	}
 	desc->handle_irq = handle;
@@ -755,6 +771,7 @@ __irq_do_set_handler(struct irq_desc *desc, irq_flow_handler_t handle,
 		irq_settings_set_noprobe(desc);
 		irq_settings_set_norequest(desc);
 		irq_settings_set_nothread(desc);
+		desc->action = &chained_action;
 		irq_startup(desc, true);
 	}
 }
diff --git a/kernel/irq/internals.h b/kernel/irq/internals.h
index 5ef0c2dbe930..a44692c8b9f6 100644
--- a/kernel/irq/internals.h
+++ b/kernel/irq/internals.h
@@ -18,6 +18,8 @@
 
 extern bool noirqdebug;
 
+extern struct irqaction chained_action;
+
 /*
  * Bits used by threaded handlers:
  * IRQTF_RUNTHREAD - signals that the interrupt handler thread should run
diff --git a/kernel/irq/proc.c b/kernel/irq/proc.c
index e3a8c9577ba6..7d6090519630 100644
--- a/kernel/irq/proc.c
+++ b/kernel/irq/proc.c
@@ -460,7 +460,7 @@ int show_interrupts(struct seq_file *p, void *v)
 	for_each_online_cpu(j)
 		any_count |= kstat_irqs_cpu(i, j);
 	action = desc->action;
-	if (!action && !any_count)
+	if ((!action || action == &chained_action) && !any_count)
 		goto out;
 
 	seq_printf(p, "%*d: ", prec, i);
-- 
2.5.1

[tip:irq/core] genirq: Allow migration of chained interrupts by installing default action

[tip-bot for Mika Westerberg]

Commit-ID:  e509bd7da149dc34916037484cd7545b2d48a2b0
Gitweb:     http://git.kernel.org/tip/e509bd7da149dc34916037484cd7545b2d48a2b0
Author:     Mika Westerberg <mika.westerberg@linux.intel.com>
AuthorDate: Mon, 5 Oct 2015 13:12:15 +0300
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Fri, 9 Oct 2015 22:47:27 +0200

genirq: Allow migration of chained interrupts by installing default action

When a CPU is offlined all interrupts that have an action are migrated to
other still online CPUs. However, if the interrupt has chained handler
installed this is not done. Chained handlers are used by GPIO drivers which
support interrupts, for instance.

When the affinity is not corrected properly we end up in situation where
most interrupts are not arriving to the online CPUs anymore. For example on
Intel Braswell system which has SD-card card detection signal connected to
a GPIO the IO-APIC routing entries look like below after CPU1 is offlined:

  pin30, enabled , level, low , V(52), IRR(0), S(0), logical , D(03), M(1)
  pin31, enabled , level, low , V(42), IRR(0), S(0), logical , D(03), M(1)
  pin32, enabled , level, low , V(62), IRR(0), S(0), logical , D(03), M(1)
  pin5b, enabled , level, low , V(72), IRR(0), S(0), logical , D(03), M(1)

The problem here is that the destination mask still contains both CPUs even
if CPU1 is already offline. This means that the IO-APIC still routes
interrupts to the other CPU as well.

We solve the problem by providing a default action for chained interrupts.
This action allows the migration code to correct affinity (as it finds
desc->action != NULL).

Also make the default action handler to emit a warning if for some reason a
chained handler ends up calling it.

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Cc: Jiang Liu <jiang.liu@linux.intel.com>
Link: http://lkml.kernel.org/r/1444039935-30475-1-git-send-email-mika.westerberg@linux.intel.com
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 kernel/irq/chip.c      | 17 +++++++++++++++++
 kernel/irq/internals.h |  2 ++
 kernel/irq/proc.c      |  2 +-
 3 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
index 46f1fb5..4aa00d3 100644
--- a/kernel/irq/chip.c
+++ b/kernel/irq/chip.c
@@ -21,6 +21,20 @@
 
 #include "internals.h"
 
+static irqreturn_t bad_chained_irq(int irq, void *dev_id)
+{
+	WARN_ONCE(1, "Chained irq %d should not call an action\n", irq);
+	return IRQ_NONE;
+}
+
+/*
+ * Chained handlers should never call action on their IRQ. This default
+ * action will emit warning if such thing happens.
+ */
+struct irqaction chained_action = {
+	.handler = bad_chained_irq,
+};
+
 /**
  *	irq_set_chip - set the irq chip for an irq
  *	@irq:	irq number
@@ -746,6 +760,8 @@ __irq_do_set_handler(struct irq_desc *desc, irq_flow_handler_t handle,
 		if (desc->irq_data.chip != &no_irq_chip)
 			mask_ack_irq(desc);
 		irq_state_set_disabled(desc);
+		if (is_chained)
+			desc->action = NULL;
 		desc->depth = 1;
 	}
 	desc->handle_irq = handle;
@@ -755,6 +771,7 @@ __irq_do_set_handler(struct irq_desc *desc, irq_flow_handler_t handle,
 		irq_settings_set_noprobe(desc);
 		irq_settings_set_norequest(desc);
 		irq_settings_set_nothread(desc);
+		desc->action = &chained_action;
 		irq_startup(desc, true);
 	}
 }
diff --git a/kernel/irq/internals.h b/kernel/irq/internals.h
index cd60bb4..05c2188 100644
--- a/kernel/irq/internals.h
+++ b/kernel/irq/internals.h
@@ -18,6 +18,8 @@
 
 extern bool noirqdebug;
 
+extern struct irqaction chained_action;
+
 /*
  * Bits used by threaded handlers:
  * IRQTF_RUNTHREAD - signals that the interrupt handler thread should run
diff --git a/kernel/irq/proc.c b/kernel/irq/proc.c
index e3a8c95..7d60905 100644
--- a/kernel/irq/proc.c
+++ b/kernel/irq/proc.c
@@ -460,7 +460,7 @@ int show_interrupts(struct seq_file *p, void *v)
 	for_each_online_cpu(j)
 		any_count |= kstat_irqs_cpu(i, j);
 	action = desc->action;
-	if (!action && !any_count)
+	if ((!action || action == &chained_action) && !any_count)
 		goto out;
 
 	seq_printf(p, "%*d: ", prec, i);