All of lore.kernel.org
 help / color / mirror / Atom feed
* [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.