Thread (15 messages) 15 messages, 2 authors, 2012-05-10

Re: [PATCH] powerpc: use local var instead of local_paca->irq_happened directly in __check_irq_replay

From: Wang Sheng-Hui <hidden>
Date: 2012-05-03 05:51:38
Also in: lkml

On 2012年05月03日 12:22, Benjamin Herrenschmidt wrote:
quoted
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.
I have a chance to notice that the value is 0x05, not just 0x01.
So I think this is not the case.
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.
quoted
Cheers,
Ben.
quoted
Signed-off-by: Wang Sheng-Hui <redacted>
---
 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/
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help