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
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.
--- include/linux/irq.h | 6 +++++- kernel/irq/chip.c | 11 +++++++++++ kernel/irq/settings.h | 16 ++++++++++++++++ 3 files changed, 32 insertions(+), 1 deletion(-)
include/linux/irq.h +5 -1
@@ -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)
kernel/irq/chip.c +11 -0
@@ -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)
kernel/irq/settings.h +16 -0
@@ -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; +}
Signature · 1 line
2.5.1
PATCH 2/2 x86/irq: Take chained interrupts into account in fixup_irqs()
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.
---
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(-)
arch/x86/kernel/irq.c +5 -1
@@ -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 ;
Signature · 1 line
2.5.1
Quoted 9 lines · 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_CHAINEDthat 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 functionirq_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 1/1 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.
---
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(-)
kernel/irq/chip.c +17 -0
@@ -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); } }
kernel/irq/internals.h +2 -0
@@ -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
kernel/irq/proc.c +1 -1
@@ -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);
Signature · 1 line
2.5.1
tip-bot for Mika Westerberg
merge bot[tip:irq/core] genirq: Allow migration of chained interrupts by installing default action
genirq: Allow migration of chained interrupts by installing default action
Commit-ID: e509bd7da149dc34916037484cd7545b2d48a2b0 Gitweb: http://git.kernel.org/tip/e509bd7da149dc34916037484cd7545b2d48a2b0 Author: Mika Westerberg <[email protected]> AuthorDate: Mon, 5 Oct 2015 13:12:15 +0300 Committer: Thomas Gleixner <[email protected]> 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.
--- kernel/irq/chip.c | 17 +++++++++++++++++ kernel/irq/internals.h | 2 ++ kernel/irq/proc.c | 2 +- 3 files changed, 20 insertions(+), 1 deletion(-)
kernel/irq/chip.c +17 -0
@@ -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); } }
kernel/irq/internals.h +2 -0
@@ -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
kernel/irq/proc.c +1 -1
@@ -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);