powerpc/perf_event: Fix oops due to perf_event_do_pending call
PATCH 1/1 powerpc/perf_event: Fix oops due to perf_event_do_pending call
Anton Blanchard found that large POWER systems would occasionally crash in the exception exit path when profiling with perf_events. The symptom was that an interrupt would occur late in the exit path when the MSR[RI] (recoverable interrupt) bit was clear. Interrupts should be hard-disabled at this point but they were enabled. Because the interrupt was not recoverable the system panicked.
The reason is that the exception exit path was calling perf_event_do_pending after hard-disabling interrupts, and perf_event_do_pending will re-enable interrupts.
The simplest and cleanest fix for this is to use the same mechanism that 32-bit powerpc does, namely to cause a self-IPI by setting the decrementer to 1. This means we can remove the tests in the exception exit path and raw_local_irq_restore.
This also makes sure that the call to perf_event_do_pending from timer_interrupt() happens within irq_enter/irq_exit. (Note that calling perf_event_do_pending from timer_interrupt does not mean that there is a possible 1/HZ latency; setting the decrementer to 1 ensures that the timer interrupt will happen immediately, i.e. within one timebase tick, which is a few nanoseconds or 10s of nanoseconds.)
---
Ben, please put this in your tree of fixes to go to Linus for 2.6.34, since it fixes a potential panic.
arch/powerpc/include/asm/hw_irq.h | 38 ------------------------ arch/powerpc/kernel/asm-offsets.c | 1 arch/powerpc/kernel/entry_64.S | 9 ----- arch/powerpc/kernel/irq.c | 6 --- arch/powerpc/kernel/time.c | 60 ++++++++++++++++++++++++++++++-------- 5 files changed, 48 insertions(+), 66 deletions(-)
arch/powerpc/include/asm/hw_irq.h +0 -38
@@ -130,43 +130,5 @@ static inline int irqs_disabled_flags(unsigned long flags)*/ struct irq_chip ; -#ifdef CONFIG_PERF_EVENTS - -#ifdef CONFIG_PPC64 -static inline unsigned long test_perf_event_pending (void ) -{ -unsigned long x; - - asmvolatile ("lbz %0,%1(13)" - :"=r" (x) - :"i" (offsetof(struct paca_struct , perf_event_pending))); -return x; -} - -static inline void set_perf_event_pending (void ) -{ - asmvolatile ("stb %0,%1(13)" : : -"r" (1 ), -"i" (offsetof(struct paca_struct , perf_event_pending))); -} - -static inline void clear_perf_event_pending (void ) -{ - asmvolatile ("stb %0,%1(13)" : : -"r" (0 ), -"i" (offsetof(struct paca_struct , perf_event_pending))); -} -#endif /* CONFIG_PPC64 */ - -#else /* CONFIG_PERF_EVENTS */ - -static inline unsigned long test_perf_event_pending (void ) -{ -return 0 ; -} - -static inline void clear_perf_event_pending (void ) {} -#endif /* CONFIG_PERF_EVENTS */ -#endif /* __KERNEL__ */ #endif /* _ASM_POWERPC_HW_IRQ_H */
arch/powerpc/kernel/asm-offsets.c +0 -1
@@ -133,7 +133,6 @@ int main(void)DEFINE (PACAKMSR , offsetof(struct paca_struct , kernel_msr));DEFINE (PACASOFTIRQEN , offsetof(struct paca_struct , soft_enabled));DEFINE (PACAHARDIRQEN , offsetof(struct paca_struct , hard_enabled)); -DEFINE (PACAPERFPEND , offsetof(struct paca_struct , perf_event_pending)); DEFINE(PACACONTEXTID, offsetof(struct paca_struct, context.id));#ifdef CONFIG_PPC_MM_SLICES DEFINE(PACALOWSLICESPSIZE, offsetof(struct paca_struct,
arch/powerpc/kernel/entry_64.S +0 -9
@@ -556,15 +556,6 @@ ALT_FW_FTR_SECTION_END_IFCLR(FW_FEATURE_ISERIES) 2: TRACE_AND_RESTORE_IRQ(r5); -#ifdef CONFIG_PERF_EVENTS - /* check paca->perf_event_pending if we're enabling ints */ - lbz r3,PACAPERFPEND(r13) - and. r3,r3,r5 - beq 27f - bl .perf_event_do_pending -27: -#endif /* CONFIG_PERF_EVENTS */ - /* extract EE bit and use it to restore paca->hard_enabled */ ld r3,_MSR(r1) rldicl r4,r3,49,63 /* r0 = (r3 >> 15) & 1 */
arch/powerpc/kernel/irq.c +0 -6
@@ -53,7 +53,6 @@#include <linux/bootmem.h> #include <linux/pci.h> #include <linux/debugfs.h> -#include <linux/perf_event.h> #include <asm/uaccess.h> #include <asm/system.h>
@@ -145,11 +144,6 @@ notrace void raw_local_irq_restore(unsigned long en) }#endif /* CONFIG_PPC_STD_MMU_64 */ -if (test_perf_event_pending ()) { -clear_perf_event_pending (); -perf_event_do_pending (); - } -/* * if (get_paca()->hard_enabled) return; * But again we need to take care that gcc gets hard_enabled directly
arch/powerpc/kernel/time.c +48 -12
89 lines not on this page · raw message
Quoted 2 lines · Paul Mackerras
Ben, please put this in your tree of fixes to go to Linus for 2.6.34, since it fixes a potential panic.
Should it go to -stable as well ? How far back ?
Cheers,
Ben.
Quoted 7 lines · Paul Mackerras
Anton Blanchard found that large POWER systems would occasionally crash in the exception exit path when profiling with
perf_events. The symptom was that an interrupt would occur late in the exit path when the MSR[RI] (recoverable interrupt) bit was clear. Interrupts should be hard-disabled at this point but they were enabled. Because the interrupt was not recoverable the system panicked.
...
Quoted 6 lines
arch/powerpc/kernel/time.c +0
-0@@ -532,25 +532,60 @@ void __init iSeries_time_init_early(void) }
arch/powerpc/kernel/time.c void __init iSeries_time_init_early(void) · line 539
Quoted 10 lines
arch/powerpc/kernel/time.c +4
-3#endif /* CONFIG_PPC_ISERIES */ -#if defined(CONFIG_PERF_EVENTS ) && defined(CONFIG_PPC32 ) -DEFINE_PER_CPU (u8, perf_event_pending); +#ifdef CONFIG_PERF_EVENTS -void set_perf_event_pending (void ) +/* +* 64-bit uses a byte in the PACA, 32-bit uses a per-cpu variable... +*/
Any reason not to switch to per-cpu for both, now that you don't need to access it from asm?
cheers