[RFC PATCH] genirq: introduce handle_fasteoi_edge_irq flow handler

* [RFC PATCH] genirq: introduce handle_fasteoi_edge_irq flow handler
@ 2023-03-10 10:14 Yipeng Zou
  2023-04-14 11:25 ` Marc Zyngier
  0 siblings, 1 reply; 3+ messages in thread
From: Yipeng Zou @ 2023-03-10 10:14 UTC (permalink / raw)
  To: tglx, maz, samuel, oleksandr_tyshchenko, andy.shevchenko, apatel,
	lvjianmin, jason, linux-kernel
  Cc: chris.zjh, liaochang1

Recently, We have a LPI migration issue on the ARM SMP platform.

For example, NIC device generates MSI and sends LPI to CPU0 via ITS,
meanwhile irqbalance running on CPU1 set irq affinity of NIC to CPU1,
the next interrupt will be sent to CPU2, due to the state of irq is
still in progress, kernel does not end up performing irq handler on
CPU2, which results in some userland service timeouts, the sequence
of events is shown as follows:

NIC                     CPU0                    CPU1

Generate IRQ#1          READ_IAR
                        Lock irq_desc
                        Set IRQD_IN_PROGRESS
                        Unlock irq_desc
                                                Lock irq_desc
                                                Change LPI Affinity
                                                Unlock irq_desc
                        Call irq_handler
Generate IRQ#2
                                                READ_IAR
                                                Lock irq_desc
                                                Check IRQD_IN_PROGRESS
                                                Unlock irq_desc
                                                Return from interrupt#2
                        Lock irq_desc
                        Clear IRQD_IN_PROGRESS
                        Unlock irq_desc
                        return from interrupt#1

For this scenario, The IRQ#2 will be lost. This does cause some exceptions.

For further information, see [1].

This patch introduced a new flow handler which combines fasteoi and edge
type as a workaround.
An additional loop will be executed if the IRQS_PENDING has been setup.

[1]: https://lore.kernel.org/all/b0f2623b-ec70-d57e-b744-26c62b1ce523@huawei.com/

Fixes: cc2d3216f53c ("irqchip: GICv3: ITS command queue")
Signed-off-by: Yipeng Zou <zouyipeng@huawei.com>
---
 drivers/irqchip/irq-gic-v3.c |  2 +-
 include/linux/irq.h          |  1 +
 kernel/irq/chip.c            | 62 ++++++++++++++++++++++++++++++++++++
 3 files changed, 64 insertions(+), 1 deletion(-)

diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
index fd134e1f481a..625ec64f0218 100644
--- a/drivers/irqchip/irq-gic-v3.c
+++ b/drivers/irqchip/irq-gic-v3.c
@@ -1456,7 +1456,7 @@ static int gic_irq_domain_map(struct irq_domain *d, unsigned int irq,
 		if (!gic_dist_supports_lpis())
 			return -EPERM;
 		irq_domain_set_info(d, irq, hw, chip, d->host_data,
-				    handle_fasteoi_irq, NULL, NULL);
+				    handle_fasteoi_edge_irq, NULL, NULL);
 		break;
 
 	default:
diff --git a/include/linux/irq.h b/include/linux/irq.h
index b1b28affb32a..adca5340edeb 100644
--- a/include/linux/irq.h
+++ b/include/linux/irq.h
@@ -656,6 +656,7 @@ static inline int irq_set_parent(int irq, int parent_irq)
  */
 extern void handle_level_irq(struct irq_desc *desc);
 extern void handle_fasteoi_irq(struct irq_desc *desc);
+extern void handle_fasteoi_edge_irq(struct irq_desc *desc);
 extern void handle_edge_irq(struct irq_desc *desc);
 extern void handle_edge_eoi_irq(struct irq_desc *desc);
 extern void handle_simple_irq(struct irq_desc *desc);
diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
index 49e7bc871fec..bd3d346b468d 100644
--- a/kernel/irq/chip.c
+++ b/kernel/irq/chip.c
@@ -677,6 +677,68 @@ static void cond_unmask_eoi_irq(struct irq_desc *desc, struct irq_chip *chip)
 	}
 }
 
+/**
+ *	handle_fasteoi_edge_irq - irq handler for transparent controllers
+ *	edge type IRQ.
+ *	@desc:	the interrupt description structure for this irq
+ */
+void handle_fasteoi_edge_irq(struct irq_desc *desc)
+{
+	struct irq_chip *chip = desc->irq_data.chip;
+
+	raw_spin_lock(&desc->lock);
+
+	if (!irq_may_run(desc)) {
+		desc->istate |= IRQS_PENDING;
+		mask_irq(desc);
+		goto out;
+	}
+
+	desc->istate &= ~(IRQS_REPLAY | IRQS_WAITING);
+
+	/*
+	 * If its disabled or no action available
+	 * then mask it and get out of here:
+	 */
+	if (unlikely(!desc->action || irqd_irq_disabled(&desc->irq_data))) {
+		desc->istate |= IRQS_PENDING;
+		mask_irq(desc);
+		goto out;
+	}
+
+	kstat_incr_irqs_this_cpu(desc);
+
+	if (desc->istate & IRQS_ONESHOT)
+		mask_irq(desc);
+
+	do {
+		/*
+		 * When another irq arrived while we were handling
+		 * one, we could have masked the irq.
+		 * Reenable it, if it was not disabled in meantime.
+		 */
+		if (unlikely(desc->istate & IRQS_PENDING)) {
+			if (!irqd_irq_disabled(&desc->irq_data) &&
+			    irqd_irq_masked(&desc->irq_data))
+				unmask_irq(desc);
+		}
+
+		handle_irq_event(desc);
+
+	} while ((desc->istate & IRQS_PENDING) &&
+		 !irqd_irq_disabled(&desc->irq_data));
+
+	cond_unmask_eoi_irq(desc, chip);
+
+	raw_spin_unlock(&desc->lock);
+	return;
+out:
+	if (!(chip->flags & IRQCHIP_EOI_IF_HANDLED))
+		chip->irq_eoi(&desc->irq_data);
+	raw_spin_unlock(&desc->lock);
+}
+EXPORT_SYMBOL_GPL(handle_fasteoi_edge_irq);
+
 /**
  *	handle_fasteoi_irq - irq handler for transparent controllers
  *	@desc:	the interrupt description structure for this irq
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread