Re: powerpc/powernv: process all OPAL event interrupts with kopald

[Michael Ellerman]

On Thu, 2018-05-10 at 17:20:05 UTC, Nicholas Piggin wrote:
> Using irq_work for processing OPAL event interrupts is not necessary.
> irq_work is typically used to schedule work from NMI context, a
> softirq may be more appropriate. However OPAL events are not
> particularly performance or latency critical, so they can all be
> invoked by kopald.
> 
> This patch removes the irq_work queueing, and instead wakes up
> kopald when there is an event to be processed. kopald processes
> interrupts individually, enabling irqs and calling cond_resched
> between each one to minimise latencies.
> 
> Event handlers themselves should still use threaded handlers,
> workqueues, etc. as necessary to avoid high interrupts-off latencies
> within any single interrupt.
> 
> Signed-off-by: Nicholas Piggin 

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/56c0b48b1e443efa5d6f4d60513302

cheers

[PATCH] powerpc/powernv: process all OPAL event interrupts with kopald

[Nicholas Piggin]

Using irq_work for processing OPAL event interrupts is not necessary.
irq_work is typically used to schedule work from NMI context, a
softirq may be more appropriate. However OPAL events are not
particularly performance or latency critical, so they can all be
invoked by kopald.

This patch removes the irq_work queueing, and instead wakes up
kopald when there is an event to be processed. kopald processes
interrupts individually, enabling irqs and calling cond_resched
between each one to minimise latencies.

Event handlers themselves should still use threaded handlers,
workqueues, etc. as necessary to avoid high interrupts-off latencies
within any single interrupt.

Signed-off-by: Nicholas Piggin 
---
 arch/powerpc/platforms/powernv/opal-irqchip.c | 87 +--
 arch/powerpc/platforms/powernv/opal.c | 23 +++--
 arch/powerpc/platforms/powernv/powernv.h  |  3 +-
 3 files changed, 52 insertions(+), 61 deletions(-)

diff --git a/arch/powerpc/platforms/powernv/opal-irqchip.c 
b/arch/powerpc/platforms/powernv/opal-irqchip.c
index 9d1b8c0aaf93..354fa53947be 100644
--- a/arch/powerpc/platforms/powernv/opal-irqchip.c
+++ b/arch/powerpc/platforms/powernv/opal-irqchip.c
@@ -22,7 +22,6 @@
 #include 
 #include 
 #include 
-#include 
 
 #include 
 #include 
@@ -38,37 +37,47 @@ struct opal_event_irqchip {
unsigned long mask;
 };
 static struct opal_event_irqchip opal_event_irqchip;
-
+static u64 last_outstanding_events;
 static unsigned int opal_irq_count;
 static unsigned int *opal_irqs;
 
-static void opal_handle_irq_work(struct irq_work *work);
-static u64 last_outstanding_events;
-static struct irq_work opal_event_irq_work = {
-   .func = opal_handle_irq_work,
-};
-
-void opal_handle_events(uint64_t events)
+void opal_handle_events(void)
 {
-   int virq, hwirq = 0;
-   u64 mask = opal_event_irqchip.mask;
+   __be64 events = 0;
+   u64 e;
+
+   e = READ_ONCE(last_outstanding_events) & opal_event_irqchip.mask;
+again:
+   while (e) {
+   int virq, hwirq;
+
+   hwirq = fls64(e) - 1;
+   e &= ~BIT_ULL(hwirq);
+
+   local_irq_disable();
+   virq = irq_find_mapping(opal_event_irqchip.domain, hwirq);
+   if (virq) {
+   irq_enter();
+   generic_handle_irq(virq);
+   irq_exit();
+   }
+   local_irq_enable();
 
-   if (!in_irq() && (events & mask)) {
-   last_outstanding_events = events;
-   irq_work_queue(_event_irq_work);
-   return;
+   cond_resched();
}
+   last_outstanding_events = 0;
+   if (opal_poll_events() != OPAL_SUCCESS)
+   return;
+   e = be64_to_cpu(events) & opal_event_irqchip.mask;
+   if (e)
+   goto again;
+}
 
-   while (events & mask) {
-   hwirq = fls64(events) - 1;
-   if (BIT_ULL(hwirq) & mask) {
-   virq = irq_find_mapping(opal_event_irqchip.domain,
-   hwirq);
-   if (virq)
-   generic_handle_irq(virq);
-   }
-   events &= ~BIT_ULL(hwirq);
-   }
+bool opal_have_pending_events(void)
+{
+   if (last_outstanding_events & opal_event_irqchip.mask)
+   return true;
+   return false;
 }
 
 static void opal_event_mask(struct irq_data *d)
@@ -78,24 +87,9 @@ static void opal_event_mask(struct irq_data *d)
 
 static void opal_event_unmask(struct irq_data *d)
 {
-   __be64 events;
-
set_bit(d->hwirq, _event_irqchip.mask);
-
-   opal_poll_events();
-   last_outstanding_events = be64_to_cpu(events);
-
-   /*
-* We can't just handle the events now with opal_handle_events().
-* If we did we would deadlock when opal_event_unmask() is called from
-* handle_level_irq() with the irq descriptor lock held, because
-* calling opal_handle_events() would call generic_handle_irq() and
-* then handle_level_irq() which would try to take the descriptor lock
-* again. Instead queue the events for later.
-*/
-   if (last_outstanding_events & opal_event_irqchip.mask)
-   /* Need to retrigger the interrupt */
-   irq_work_queue(_event_irq_work);
+   if (opal_have_pending_events())
+   opal_wake_poller();
 }
 
 static int opal_event_set_type(struct irq_data *d, unsigned int flow_type)
@@ -136,16 +130,13 @@ static irqreturn_t opal_interrupt(int irq, void *data)
__be64 events;
 
opal_handle_interrupt(virq_to_hw(irq), );
-   opal_handle_events(be64_to_cpu(events));
+   last_outstanding_events = be64_to_cpu(events);
+   if (opal_have_pending_events())
+   opal_wake_poller();
 
return IRQ_HANDLED;
 }
 
-static void opal_handle_irq_work(struct irq_work *work)