Skip to content

Commit f0c7bac

Browse files
committed
genirq/affinity: Make affinity setting if activated opt-in
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: baedb87 ("genirq/affinity: Handle affinity setting on inactive interrupts correctly") Reported-by: John Keeping <[email protected]> Signed-off-by: Thomas Gleixner <[email protected]> Tested-by: Marc Zyngier <[email protected]> Acked-by: Marc Zyngier <[email protected]> Cc: [email protected] Link: https://lkml.kernel.org/r/[email protected]
1 parent ec01608 commit f0c7bac

File tree

4 files changed

+26
-2
lines changed

4 files changed

+26
-2
lines changed

arch/x86/kernel/apic/vector.c

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -560,6 +560,10 @@ static int x86_vector_alloc_irqs(struct irq_domain *domain, unsigned int virq,
560560
* as that can corrupt the affinity move state.
561561
*/
562562
irqd_set_handle_enforce_irqctx(irqd);
563+
564+
/* Don't invoke affinity setter on deactivated interrupts */
565+
irqd_set_affinity_on_activate(irqd);
566+
563567
/*
564568
* Legacy vectors are already assigned when the IOAPIC
565569
* takes them over. They stay on the same vector. This is

drivers/irqchip/irq-gic-v3-its.c

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3523,6 +3523,7 @@ static int its_irq_domain_alloc(struct irq_domain *domain, unsigned int virq,
35233523
msi_alloc_info_t *info = args;
35243524
struct its_device *its_dev = info->scratchpad[0].ptr;
35253525
struct its_node *its = its_dev->its;
3526+
struct irq_data *irqd;
35263527
irq_hw_number_t hwirq;
35273528
int err;
35283529
int i;
@@ -3542,7 +3543,9 @@ static int its_irq_domain_alloc(struct irq_domain *domain, unsigned int virq,
35423543

35433544
irq_domain_set_hwirq_and_chip(domain, virq + i,
35443545
hwirq + i, &its_irq_chip, its_dev);
3545-
irqd_set_single_target(irq_desc_get_irq_data(irq_to_desc(virq + i)));
3546+
irqd = irq_get_irq_data(virq + i);
3547+
irqd_set_single_target(irqd);
3548+
irqd_set_affinity_on_activate(irqd);
35463549
pr_debug("ID:%d pID:%d vID:%d\n",
35473550
(int)(hwirq + i - its_dev->event_map.lpi_base),
35483551
(int)(hwirq + i), virq + i);

include/linux/irq.h

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -213,6 +213,8 @@ struct irq_data {
213213
* required
214214
* IRQD_HANDLE_ENFORCE_IRQCTX - Enforce that handle_irq_*() is only invoked
215215
* from actual interrupt context.
216+
* IRQD_AFFINITY_ON_ACTIVATE - Affinity is set on activation. Don't call
217+
* irq_chip::irq_set_affinity() when deactivated.
216218
*/
217219
enum {
218220
IRQD_TRIGGER_MASK = 0xf,
@@ -237,6 +239,7 @@ enum {
237239
IRQD_CAN_RESERVE = (1 << 26),
238240
IRQD_MSI_NOMASK_QUIRK = (1 << 27),
239241
IRQD_HANDLE_ENFORCE_IRQCTX = (1 << 28),
242+
IRQD_AFFINITY_ON_ACTIVATE = (1 << 29),
240243
};
241244

242245
#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)
421424
return __irqd_to_state(d) & IRQD_MSI_NOMASK_QUIRK;
422425
}
423426

427+
static inline void irqd_set_affinity_on_activate(struct irq_data *d)
428+
{
429+
__irqd_to_state(d) |= IRQD_AFFINITY_ON_ACTIVATE;
430+
}
431+
432+
static inline bool irqd_affinity_on_activate(struct irq_data *d)
433+
{
434+
return __irqd_to_state(d) & IRQD_AFFINITY_ON_ACTIVATE;
435+
}
436+
424437
#undef __irqd_to_state
425438

426439
static inline irq_hw_number_t irqd_to_hwirq(struct irq_data *d)

kernel/irq/manage.c

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -320,12 +320,16 @@ static bool irq_set_affinity_deactivated(struct irq_data *data,
320320
struct irq_desc *desc = irq_data_to_desc(data);
321321

322322
/*
323+
* Handle irq chips which can handle affinity only in activated
324+
* state correctly
325+
*
323326
* If the interrupt is not yet activated, just store the affinity
324327
* mask and do not call the chip driver at all. On activation the
325328
* driver has to make sure anyway that the interrupt is in a
326329
* useable state so startup works.
327330
*/
328-
if (!IS_ENABLED(CONFIG_IRQ_DOMAIN_HIERARCHY) || irqd_is_activated(data))
331+
if (!IS_ENABLED(CONFIG_IRQ_DOMAIN_HIERARCHY) ||
332+
irqd_is_activated(data) || !irqd_affinity_on_activate(data))
329333
return false;
330334

331335
cpumask_copy(desc->irq_common_data.affinity, mask);

0 commit comments

Comments
 (0)