On Thu, 2012-05-03 at 09:53 +0800, Wang Sheng-Hui wrote:
> local_paca->irq_happened may be changed asychronously.
>
> In my test env (IBM Power 9117-MMA), I installed the RHEL6.2 with the shipped
> oprofile. Then I run into kernel v3.4-rc4, setup/start oprofile and start the
> LTP test suite.
>
> In a short while, the system would crash. Seems that oprofile may change
> the irq_happened.
.../...
> Use local var instead of local_paca->irq_happened directly in this function
> here.
>
> Please check this patch. Any comments are welcome.
It should not as __check_irq_replay() should always be called
with interrupts hard disabled... Do you see any code path
where that is not the case ?
Cheers,
Ben.
> Signed-off-by: Wang Sheng-Hui <shh...@gmail.com>
> ---
> arch/powerpc/kernel/irq.c | 46 +++++++++++++++++++++++++++++---------------
> 1 files changed, 30 insertions(+), 16 deletions(-)
>
> diff --git a/arch/powerpc/kernel/irq.c b/arch/powerpc/kernel/irq.c
> index 5ec1b23..3d48b23 100644
> --- a/arch/powerpc/kernel/irq.c
> +++ b/arch/powerpc/kernel/irq.c
> @@ -137,15 +137,17 @@ static inline notrace int
> decrementer_check_overflow(void)
> */
> notrace unsigned int __check_irq_replay(void)
> {
> + unsigned int ret_val;
> /*
> * We use local_paca rather than get_paca() to avoid all
> * the debug_smp_processor_id() business in this low level
> * function
> */
> - unsigned char happened = local_paca->irq_happened;
> + unsigned char happened, irq_happened;
> + happened = irq_happened = local_paca->irq_happened;
>
> /* Clear bit 0 which we wouldn't clear otherwise */
> - local_paca->irq_happened &= ~PACA_IRQ_HARD_DIS;
> + irq_happened &= ~PACA_IRQ_HARD_DIS;
>
> /*
> * Force the delivery of pending soft-disabled interrupts on PS3.
> @@ -161,33 +163,45 @@ notrace unsigned int __check_irq_replay(void)
> * decrementer itself rather than the paca irq_happened field
> * in case we also had a rollover while hard disabled
> */
> - local_paca->irq_happened &= ~PACA_IRQ_DEC;
> - if (decrementer_check_overflow())
> - return 0x900;
> + irq_happened &= ~PACA_IRQ_DEC;
> + if (decrementer_check_overflow()) {
> + ret_val = 0x900;
> + goto replay;
> + }
>
> /* Finally check if an external interrupt happened */
> - local_paca->irq_happened &= ~PACA_IRQ_EE;
> - if (happened & PACA_IRQ_EE)
> - return 0x500;
> + irq_happened &= ~PACA_IRQ_EE;
> + if (happened & PACA_IRQ_EE) {
> + ret_val = 0x500;
> + goto replay;
> + }
>
> #ifdef CONFIG_PPC_BOOK3E
> /* Finally check if an EPR external interrupt happened
> * this bit is typically set if we need to handle another
> * "edge" interrupt from within the MPIC "EPR" handler
> */
> - local_paca->irq_happened &= ~PACA_IRQ_EE_EDGE;
> - if (happened & PACA_IRQ_EE_EDGE)
> - return 0x500;
> + irq_happened &= ~PACA_IRQ_EE_EDGE;
> + if (happened & PACA_IRQ_EE_EDGE) {
> + ret_val = 0x500;
> + goto replay;
> + }
>
> - local_paca->irq_happened &= ~PACA_IRQ_DBELL;
> - if (happened & PACA_IRQ_DBELL)
> - return 0x280;
> + irq_happened &= ~PACA_IRQ_DBELL;
> + if (happened & PACA_IRQ_DBELL) {
> + ret_val = 0x280;
> + goto replay;
> + }
> #endif /* CONFIG_PPC_BOOK3E */
>
> /* There should be nothing left ! */
> - BUG_ON(local_paca->irq_happened != 0);
> + BUG_ON(irq_happened != 0);
> + ret_val = 0;
>
> - return 0;
> +replay:
> + local_paca->irq_happened = irq_happened;
> +
> + return ret_val;
> }
>
> notrace void arch_local_irq_restore(unsigned long en)
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
