| From 2ecdf4e5e789c1d5fe4c6e89ba8d746f9222cc0e Mon Sep 17 00:00:00 2001 |
| From: Thomas Gleixner <tglx@linutronix.de> |
| Date: Fri, 24 Jul 2020 22:44:41 +0200 |
| Subject: [PATCH] genirq/affinity: Make affinity setting if activated opt-in |
| |
| commit f0c7baca180046824e07fc5f1326e83a8fd150c7 upstream. |
| |
| John reported that on a RK3288 system the perf per CPU interrupts are all |
| affine to CPU0 and provided the analysis: |
| |
| "It looks like what happens is that because the interrupts are not per-CPU |
| in the hardware, armpmu_request_irq() calls irq_force_affinity() while |
| the interrupt is deactivated and then request_irq() with IRQF_PERCPU | |
| IRQF_NOBALANCING. |
| |
| Now when irq_startup() runs with IRQ_STARTUP_NORMAL, it calls |
| irq_setup_affinity() which returns early because IRQF_PERCPU and |
| IRQF_NOBALANCING are set, leaving the interrupt on its original CPU." |
| |
| This was broken by the recent commit which blocked interrupt affinity |
| setting in hardware before activation of the interrupt. While this works in |
| general, it does not work for this particular case. As contrary to the |
| initial analysis not all interrupt chip drivers implement an activate |
| callback, the safe cure is to make the deferred interrupt affinity setting |
| at activation time opt-in. |
| |
| Implement the necessary core logic and make the two irqchip implementations |
| for which this is required opt-in. In hindsight this would have been the |
| right thing to do, but ... |
| |
| Fixes: baedb87d1b53 ("genirq/affinity: Handle affinity setting on inactive interrupts correctly") |
| Reported-by: John Keeping <john@metanate.com> |
| Signed-off-by: Thomas Gleixner <tglx@linutronix.de> |
| Tested-by: Marc Zyngier <maz@kernel.org> |
| Acked-by: Marc Zyngier <maz@kernel.org> |
| Cc: stable@vger.kernel.org |
| Link: https://lkml.kernel.org/r/87blk4tzgm.fsf@nanos.tec.linutronix.de |
| Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com> |
| |
| diff --git a/arch/x86/kernel/apic/vector.c b/arch/x86/kernel/apic/vector.c |
| index c1857c08706e..259d17d08983 100644 |
| --- a/arch/x86/kernel/apic/vector.c |
| +++ b/arch/x86/kernel/apic/vector.c |
| @@ -560,6 +560,10 @@ static int x86_vector_alloc_irqs(struct irq_domain *domain, unsigned int virq, |
| * as that can corrupt the affinity move state. |
| */ |
| irqd_set_handle_enforce_irqctx(irqd); |
| + |
| + /* Don't invoke affinity setter on deactivated interrupts */ |
| + irqd_set_affinity_on_activate(irqd); |
| + |
| /* |
| * Legacy vectors are already assigned when the IOAPIC |
| * takes them over. They stay on the same vector. This is |
| diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c |
| index eeea66daa389..0e8c08ce2cd6 100644 |
| --- a/drivers/irqchip/irq-gic-v3-its.c |
| +++ b/drivers/irqchip/irq-gic-v3-its.c |
| @@ -2581,6 +2581,7 @@ static int its_irq_domain_alloc(struct irq_domain *domain, unsigned int virq, |
| msi_alloc_info_t *info = args; |
| struct its_device *its_dev = info->scratchpad[0].ptr; |
| struct its_node *its = its_dev->its; |
| + struct irq_data *irqd; |
| irq_hw_number_t hwirq; |
| int err; |
| int i; |
| @@ -2600,7 +2601,9 @@ static int its_irq_domain_alloc(struct irq_domain *domain, unsigned int virq, |
| |
| irq_domain_set_hwirq_and_chip(domain, virq + i, |
| hwirq + i, &its_irq_chip, its_dev); |
| - irqd_set_single_target(irq_desc_get_irq_data(irq_to_desc(virq + i))); |
| + irqd = irq_get_irq_data(virq + i); |
| + irqd_set_single_target(irqd); |
| + irqd_set_affinity_on_activate(irqd); |
| pr_debug("ID:%d pID:%d vID:%d\n", |
| (int)(hwirq + i - its_dev->event_map.lpi_base), |
| (int)(hwirq + i), virq + i); |
| diff --git a/include/linux/irq.h b/include/linux/irq.h |
| index b736900394d3..8060a53805a0 100644 |
| --- a/include/linux/irq.h |
| +++ b/include/linux/irq.h |
| @@ -213,6 +213,8 @@ struct irq_data { |
| * required |
| * IRQD_HANDLE_ENFORCE_IRQCTX - Enforce that handle_irq_*() is only invoked |
| * from actual interrupt context. |
| + * IRQD_AFFINITY_ON_ACTIVATE - Affinity is set on activation. Don't call |
| + * irq_chip::irq_set_affinity() when deactivated. |
| */ |
| enum { |
| IRQD_TRIGGER_MASK = 0xf, |
| @@ -237,6 +239,7 @@ enum { |
| IRQD_CAN_RESERVE = (1 << 26), |
| IRQD_MSI_NOMASK_QUIRK = (1 << 27), |
| IRQD_HANDLE_ENFORCE_IRQCTX = (1 << 28), |
| + IRQD_AFFINITY_ON_ACTIVATE = (1 << 29), |
| }; |
| |
| #define __irqd_to_state(d) ACCESS_PRIVATE((d)->common, state_use_accessors) |
| @@ -421,6 +424,16 @@ static inline bool irqd_msi_nomask_quirk(struct irq_data *d) |
| return __irqd_to_state(d) & IRQD_MSI_NOMASK_QUIRK; |
| } |
| |
| +static inline void irqd_set_affinity_on_activate(struct irq_data *d) |
| +{ |
| + __irqd_to_state(d) |= IRQD_AFFINITY_ON_ACTIVATE; |
| +} |
| + |
| +static inline bool irqd_affinity_on_activate(struct irq_data *d) |
| +{ |
| + return __irqd_to_state(d) & IRQD_AFFINITY_ON_ACTIVATE; |
| +} |
| + |
| #undef __irqd_to_state |
| |
| static inline irq_hw_number_t irqd_to_hwirq(struct irq_data *d) |
| diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c |
| index 2f5ce125e9e6..e557a375cd02 100644 |
| --- a/kernel/irq/manage.c |
| +++ b/kernel/irq/manage.c |
| @@ -281,12 +281,16 @@ static bool irq_set_affinity_deactivated(struct irq_data *data, |
| struct irq_desc *desc = irq_data_to_desc(data); |
| |
| /* |
| + * Handle irq chips which can handle affinity only in activated |
| + * state correctly |
| + * |
| * If the interrupt is not yet activated, just store the affinity |
| * mask and do not call the chip driver at all. On activation the |
| * driver has to make sure anyway that the interrupt is in a |
| * useable state so startup works. |
| */ |
| - if (!IS_ENABLED(CONFIG_IRQ_DOMAIN_HIERARCHY) || irqd_is_activated(data)) |
| + if (!IS_ENABLED(CONFIG_IRQ_DOMAIN_HIERARCHY) || |
| + irqd_is_activated(data) || !irqd_affinity_on_activate(data)) |
| return false; |
| |
| cpumask_copy(desc->irq_common_data.affinity, mask); |
| -- |
| 2.27.0 |
| |