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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <1444039935-30475-1-git-send-email-mika.westerberg@linux.intel.com>
Date:	Mon,  5 Oct 2015 13:12:15 +0300
From:	Mika Westerberg <mika.westerberg@...ux.intel.com>
To:	Thomas Gleixner <tglx@...utronix.de>
Cc:	Ingo Molnar <mingo@...hat.com>, "H. Peter Anvin" <hpa@...or.com>,
	Jiang Liu <jiang.liu@...ux.intel.com>,
	Mika Westerberg <mika.westerberg@...ux.intel.com>,
	linux-kernel@...r.kernel.org
Subject: [PATCH v2] 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@...ux.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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/