powerpc/64: irq_work avoid immediate interrupt when raised with hard irqs enabled
Nicholas Piggin
reviewerPATCH 1/1 powerpc/64: irq_work avoid immediate interrupt when raised with hard irqs enabled
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.
--- arch/powerpc/kernel/time.c | 35 +++++++++++++++++++++++++++++++++-- 1 file changed, 33 insertions(+), 2 deletions(-)
arch/powerpc/kernel/time.c +33 -2
@@ -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
Signature · 1 line
2.16.3
Quoted 14 lines · Nicholas Piggin
irq_work_raiseshould 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
Quoted 2 lines
Set the decrementer pending in the
irq_happenedmask 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.
Quoted 66 lines
Signed-off-by Nicholas Piggin <[email protected]>--- arch/powerpc/kernel/time.c | 35 +++++++++++++++++++++++++++++++++-- 1 file changed, 33 insertions(+), 2 deletions(-)arch/powerpc/kernel/time.c +33
-2@@ -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
Nicholas Piggin
reviewerQuoted 15 lines · On Mon, 09 Apr 2018 18:46:29 +1000 Benjamin Herrenschmidt…
Quoted 14 lines · Nicholas Piggin
irq_work_raiseshould 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
Quoted 4 lines
Quoted 2 lines
Set the decrementer pending in the
irq_happenedmask 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 :)
Quoted 2 lines
also
preempt_disable
doesn't look necessary if IRQs are off.
True, just copied from existing code.
Thanks,
Nick