> 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 ?

More specifically, your backtrace seems to indicate that
__check_irq_repay() was called from arch_local_irq_restore() which
should have done this before calling __check_irq_replay():

        if (unlikely(irq_happened != PACA_IRQ_HARD_DIS))
                __hard_irq_disable();

Now, the only possibility that I can see for an interrupt to come in
and trip the problem you observed would be if for some reason we
had irq_happened set to PACA_IRQ_HARD_DIS while interrupts were
not hard disabled.

Can you try if removing the test (and thus unconditionally calling
__hard_irq_disable()) fixes the problem for you ?

If that is the case, then we need to audit the code to figure out how we
can end up with that bit in irq_happened set and interrupts hard
enabled.

Something like may_hard_irq_enable() shouldn't cause it since it should
only be called while hard disabled but adding a check in there might be
worth it (something like WARN_ON(mfmsr() & MSR_EE)).

Cheers,
Ben.

> 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)
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/


_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Reply via email to