* [PATCH] powerpc/64: irq_work avoid immediate interrupt when raised with hard irqs enabled
@ 2018-04-05 14:31 Nicholas Piggin
2018-04-09 8:46 ` Benjamin Herrenschmidt
0 siblings, 1 reply; 3+ messages in thread
From: Nicholas Piggin @ 2018-04-05 14:31 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Nicholas Piggin
irq_work_raise should not schedule the hardware decrementer interrupt
unless it is called from NMI context. Doing so often just results in an
immediate masked decrementer interrupt:
<...>-550 90d... 4us : update_curr_rt <-dequeue_task_rt
<...>-550 90d... 5us : dbs_update_util_handler <-update_curr_rt
<...>-550 90d... 6us : arch_irq_work_raise <-irq_work_queue
<...>-550 90d... 7us : soft_nmi_interrupt <-soft_nmi_common
<...>-550 90d... 7us : printk_nmi_enter <-soft_nmi_interrupt
<...>-550 90d.Z. 8us : rcu_nmi_enter <-soft_nmi_interrupt
<...>-550 90d.Z. 9us : rcu_nmi_exit <-soft_nmi_interrupt
<...>-550 90d... 9us : printk_nmi_exit <-soft_nmi_interrupt
<...>-550 90d... 10us : cpuacct_charge <-update_curr_rt
Set the decrementer pending in the irq_happened mask directly, rather
than having the masked decrementer handler do it.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/kernel/time.c | 35 +++++++++++++++++++++++++++++++++--
1 file changed, 33 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/kernel/time.c b/arch/powerpc/kernel/time.c
index a32823dcd9a4..9d1cc183c974 100644
--- a/arch/powerpc/kernel/time.c
+++ b/arch/powerpc/kernel/time.c
@@ -510,6 +510,35 @@ static inline void clear_irq_work_pending(void)
"i" (offsetof(struct paca_struct, irq_work_pending)));
}
+void arch_irq_work_raise(void)
+{
+ WARN_ON(!irqs_disabled());
+
+ preempt_disable();
+ set_irq_work_pending_flag();
+ /*
+ * Regular iterrupts will check pending irq_happened as they return,
+ * or process context when it next enables interrupts, so the
+ * decrementer can be scheduled there.
+ *
+ * NMI interrupts do not, so setting the decrementer hardware
+ * interrupt to fire ensures the work runs upon RI (if it's to a
+ * MSR[EE]=1 context). We do not want to do this in other contexts
+ * because if interrupts are hard enabled, the decrementer will
+ * fire immediately here and just go to the masked handler to be
+ * recorded in irq_happened.
+ *
+ * BookE does not support this yet, it must audit all NMI
+ * interrupt handlers call nmi_enter().
+ */
+ if (IS_ENABLED(CONFIG_BOOKE) || in_nmi()) {
+ set_dec(1);
+ } else {
+ local_paca->irq_happened |= PACA_IRQ_DEC;
+ }
+ preempt_enable();
+}
+
#else /* 32-bit */
DEFINE_PER_CPU(u8, irq_work_pending);
@@ -518,16 +547,18 @@ DEFINE_PER_CPU(u8, irq_work_pending);
#define test_irq_work_pending() __this_cpu_read(irq_work_pending)
#define clear_irq_work_pending() __this_cpu_write(irq_work_pending, 0)
-#endif /* 32 vs 64 bit */
-
void arch_irq_work_raise(void)
{
+ WARN_ON(!irqs_disabled());
+
preempt_disable();
set_irq_work_pending_flag();
set_dec(1);
preempt_enable();
}
+#endif /* 32 vs 64 bit */
+
#else /* CONFIG_IRQ_WORK */
#define test_irq_work_pending() 0
--
2.16.3
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] powerpc/64: irq_work avoid immediate interrupt when raised with hard irqs enabled
2018-04-05 14:31 [PATCH] powerpc/64: irq_work avoid immediate interrupt when raised with hard irqs enabled Nicholas Piggin
@ 2018-04-09 8:46 ` Benjamin Herrenschmidt
2018-04-09 10:45 ` Nicholas Piggin
0 siblings, 1 reply; 3+ messages in thread
From: Benjamin Herrenschmidt @ 2018-04-09 8:46 UTC (permalink / raw)
To: Nicholas Piggin, linuxppc-dev
On Fri, 2018-04-06 at 00:31 +1000, Nicholas Piggin wrote:
> irq_work_raise should not schedule the hardware decrementer interrupt
> unless it is called from NMI context. Doing so often just results in an
> immediate masked decrementer interrupt:
>
> <...>-550 90d... 4us : update_curr_rt <-dequeue_task_rt
> <...>-550 90d... 5us : dbs_update_util_handler <-update_curr_rt
> <...>-550 90d... 6us : arch_irq_work_raise <-irq_work_queue
> <...>-550 90d... 7us : soft_nmi_interrupt <-soft_nmi_common
> <...>-550 90d... 7us : printk_nmi_enter <-soft_nmi_interrupt
> <...>-550 90d.Z. 8us : rcu_nmi_enter <-soft_nmi_interrupt
> <...>-550 90d.Z. 9us : rcu_nmi_exit <-soft_nmi_interrupt
> <...>-550 90d... 9us : printk_nmi_exit <-soft_nmi_interrupt
> <...>-550 90d... 10us : cpuacct_charge <-update_curr_rt
>
> Set the decrementer pending in the irq_happened mask directly, rather
> than having the masked decrementer handler do it.
Setting the paca field needs hard irqs off... also preempt_disable
doesn't look necessary if IRQs are off.
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> arch/powerpc/kernel/time.c | 35 +++++++++++++++++++++++++++++++++--
> 1 file changed, 33 insertions(+), 2 deletions(-)
>
> diff --git a/arch/powerpc/kernel/time.c b/arch/powerpc/kernel/time.c
> index a32823dcd9a4..9d1cc183c974 100644
> --- a/arch/powerpc/kernel/time.c
> +++ b/arch/powerpc/kernel/time.c
> @@ -510,6 +510,35 @@ static inline void clear_irq_work_pending(void)
> "i" (offsetof(struct paca_struct, irq_work_pending)));
> }
>
> +void arch_irq_work_raise(void)
> +{
> + WARN_ON(!irqs_disabled());
> +
> + preempt_disable();
> + set_irq_work_pending_flag();
> + /*
> + * Regular iterrupts will check pending irq_happened as they return,
> + * or process context when it next enables interrupts, so the
> + * decrementer can be scheduled there.
> + *
> + * NMI interrupts do not, so setting the decrementer hardware
> + * interrupt to fire ensures the work runs upon RI (if it's to a
> + * MSR[EE]=1 context). We do not want to do this in other contexts
> + * because if interrupts are hard enabled, the decrementer will
> + * fire immediately here and just go to the masked handler to be
> + * recorded in irq_happened.
> + *
> + * BookE does not support this yet, it must audit all NMI
> + * interrupt handlers call nmi_enter().
> + */
> + if (IS_ENABLED(CONFIG_BOOKE) || in_nmi()) {
> + set_dec(1);
> + } else {
> + local_paca->irq_happened |= PACA_IRQ_DEC;
> + }
> + preempt_enable();
> +}
> +
> #else /* 32-bit */
>
> DEFINE_PER_CPU(u8, irq_work_pending);
> @@ -518,16 +547,18 @@ DEFINE_PER_CPU(u8, irq_work_pending);
> #define test_irq_work_pending() __this_cpu_read(irq_work_pending)
> #define clear_irq_work_pending() __this_cpu_write(irq_work_pending, 0)
>
> -#endif /* 32 vs 64 bit */
> -
> void arch_irq_work_raise(void)
> {
> + WARN_ON(!irqs_disabled());
> +
> preempt_disable();
> set_irq_work_pending_flag();
> set_dec(1);
> preempt_enable();
> }
>
> +#endif /* 32 vs 64 bit */
> +
> #else /* CONFIG_IRQ_WORK */
>
> #define test_irq_work_pending() 0
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] powerpc/64: irq_work avoid immediate interrupt when raised with hard irqs enabled
2018-04-09 8:46 ` Benjamin Herrenschmidt
@ 2018-04-09 10:45 ` Nicholas Piggin
0 siblings, 0 replies; 3+ messages in thread
From: Nicholas Piggin @ 2018-04-09 10:45 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: linuxppc-dev
On Mon, 09 Apr 2018 18:46:29 +1000
Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:
> On Fri, 2018-04-06 at 00:31 +1000, Nicholas Piggin wrote:
> > irq_work_raise should not schedule the hardware decrementer interrupt
> > unless it is called from NMI context. Doing so often just results in an
> > immediate masked decrementer interrupt:
> >
> > <...>-550 90d... 4us : update_curr_rt <-dequeue_task_rt
> > <...>-550 90d... 5us : dbs_update_util_handler <-update_curr_rt
> > <...>-550 90d... 6us : arch_irq_work_raise <-irq_work_queue
> > <...>-550 90d... 7us : soft_nmi_interrupt <-soft_nmi_common
> > <...>-550 90d... 7us : printk_nmi_enter <-soft_nmi_interrupt
> > <...>-550 90d.Z. 8us : rcu_nmi_enter <-soft_nmi_interrupt
> > <...>-550 90d.Z. 9us : rcu_nmi_exit <-soft_nmi_interrupt
> > <...>-550 90d... 9us : printk_nmi_exit <-soft_nmi_interrupt
> > <...>-550 90d... 10us : cpuacct_charge <-update_curr_rt
> >
> > Set the decrementer pending in the irq_happened mask directly, rather
> > than having the masked decrementer handler do it.
>
> Setting the paca field needs hard irqs off...
Doh! Good catch, I should have noticed that :)
> also preempt_disable
> doesn't look necessary if IRQs are off.
True, just copied from existing code.
Thanks,
Nick
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2018-04-09 10:46 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-05 14:31 [PATCH] powerpc/64: irq_work avoid immediate interrupt when raised with hard irqs enabled Nicholas Piggin
2018-04-09 8:46 ` Benjamin Herrenschmidt
2018-04-09 10:45 ` Nicholas Piggin
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.