lkml.org 
[lkml]   [2012]   [May]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH] powerpc: use local var instead of local_paca->irq_happened directly in __check_irq_replay
    On 2012年05月03日 10:15, Benjamin Herrenschmidt wrote:
    > 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 ?

    This is the only case.
    I have run LTP test suite on my system without oprofile over 24 hours
    with 3.4-rc4 kernel.
    Then I started oprofile, and the system crashed quickly.

    I wonder if oprofile does some special changes with the running.
    But I'm not familiar with the internal of oprofile.

    I tried to change BUG_ON to WARN_ON, and got lots of warnning messages
    in dmesg. So I changed it to local var here.


    >
    > Cheers,
    > Ben.
    >
    >> Signed-off-by: Wang Sheng-Hui <shhuiw@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 majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2012-05-03 04:41    [W:0.032 / U:120.928 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site