All of lore.kernel.org
 help / color / mirror / Atom feed
From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
To: Wang Sheng-Hui <shhuiw@gmail.com>
Cc: Stephen Rothwell <sfr@canb.auug.org.au>,
	linux-kernel@vger.kernel.org, Milton Miller <miltonm@bga.com>,
	Anton Blanchard <anton@samba.org>,
	linuxppc-dev@lists.ozlabs.org
Subject: Re: [PATCH] powerpc: use local var instead of local_paca->irq_happened directly in __check_irq_replay
Date: Tue, 08 May 2012 13:46:36 +1000	[thread overview]
Message-ID: <1336448796.2540.187.camel@pasglop> (raw)
In-Reply-To: <1336090207.18712.9.camel@pasglop>

Hi Wang !

Does this patch fixes it for you ?

>From 249f8649bf95a4c3e6637284754a165c1d83c394 Mon Sep 17 00:00:00 2001
From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Date: Tue, 8 May 2012 13:31:59 +1000
Subject: [PATCH 2/3] powerpc/irq: Fix bug with new lazy IRQ handling code

We had a case where we could turn on hard interrupts while
leaving the PACA_IRQ_HARD_DIS bit set in the PACA. This can
in turn cause a BUG_ON() to hit in __check_irq_replay() due
to interrupt state getting out of sync.

The assembly code was also way too convoluted. Instead, we
now leave it to the C code to do the right thing which ends
up being smaller and more readable.

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
 arch/powerpc/kernel/entry_64.S |   18 ------------------
 arch/powerpc/kernel/irq.c      |    8 +++++++-
 2 files changed, 7 insertions(+), 19 deletions(-)

diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S
index fd46046..29f1357 100644
--- a/arch/powerpc/kernel/entry_64.S
+++ b/arch/powerpc/kernel/entry_64.S
@@ -763,16 +763,6 @@ do_work:
 	SOFT_DISABLE_INTS(r3,r4)
 1:	bl	.preempt_schedule_irq
 
-	/* Hard-disable interrupts again (and update PACA) */
-#ifdef CONFIG_PPC_BOOK3E
-	wrteei	0
-#else
-	ld	r10,PACAKMSR(r13) /* Get kernel MSR without EE */
-	mtmsrd	r10,1
-#endif /* CONFIG_PPC_BOOK3E */
-	li	r0,PACA_IRQ_HARD_DIS
-	stb	r0,PACAIRQHAPPENED(r13)
-
 	/* Re-test flags and eventually loop */
 	clrrdi	r9,r1,THREAD_SHIFT
 	ld	r4,TI_FLAGS(r9)
@@ -783,14 +773,6 @@ do_work:
 user_work:
 #endif /* CONFIG_PREEMPT */
 
-	/* Enable interrupts */
-#ifdef CONFIG_PPC_BOOK3E
-	wrteei	1
-#else
-	ori	r10,r10,MSR_EE
-	mtmsrd	r10,1
-#endif /* CONFIG_PPC_BOOK3E */
-
 	andi.	r0,r4,_TIF_NEED_RESCHED
 	beq	1f
 	bl	.restore_interrupts
diff --git a/arch/powerpc/kernel/irq.c b/arch/powerpc/kernel/irq.c
index 5ec1b23..3717fb5 100644
--- a/arch/powerpc/kernel/irq.c
+++ b/arch/powerpc/kernel/irq.c
@@ -260,11 +260,17 @@ EXPORT_SYMBOL(arch_local_irq_restore);
  * if they are currently disabled. This is typically called before
  * schedule() or do_signal() when returning to userspace. We do it
  * in C to avoid the burden of dealing with lockdep etc...
+ *
+ * NOTE: This is called with interrupts hard disabled but not marked
+ * as such in paca->irq_happened, so we need to resync this.
  */
 void restore_interrupts(void)
 {
-	if (irqs_disabled())
+	if (irqs_disabled()) {
+		local_paca->irq_happened |= PACA_IRQ_HARD_DIS;
 		local_irq_enable();
+	} else
+		__hard_irq_enable();
 }
 
 #endif /* CONFIG_PPC64 */
-- 
1.7.9.5

WARNING: multiple messages have this Message-ID (diff)
From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
To: Wang Sheng-Hui <shhuiw@gmail.com>
Cc: Milton Miller <miltonm@bga.com>,
	Grant Likely <grant.likely@secretlab.ca>,
	Stephen Rothwell <sfr@canb.auug.org.au>,
	Anton Blanchard <anton@samba.org>,
	linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] powerpc: use local var instead of local_paca->irq_happened directly in __check_irq_replay
Date: Tue, 08 May 2012 13:46:36 +1000	[thread overview]
Message-ID: <1336448796.2540.187.camel@pasglop> (raw)
In-Reply-To: <1336090207.18712.9.camel@pasglop>

Hi Wang !

Does this patch fixes it for you ?

>From 249f8649bf95a4c3e6637284754a165c1d83c394 Mon Sep 17 00:00:00 2001
From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Date: Tue, 8 May 2012 13:31:59 +1000
Subject: [PATCH 2/3] powerpc/irq: Fix bug with new lazy IRQ handling code

We had a case where we could turn on hard interrupts while
leaving the PACA_IRQ_HARD_DIS bit set in the PACA. This can
in turn cause a BUG_ON() to hit in __check_irq_replay() due
to interrupt state getting out of sync.

The assembly code was also way too convoluted. Instead, we
now leave it to the C code to do the right thing which ends
up being smaller and more readable.

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
 arch/powerpc/kernel/entry_64.S |   18 ------------------
 arch/powerpc/kernel/irq.c      |    8 +++++++-
 2 files changed, 7 insertions(+), 19 deletions(-)

diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S
index fd46046..29f1357 100644
--- a/arch/powerpc/kernel/entry_64.S
+++ b/arch/powerpc/kernel/entry_64.S
@@ -763,16 +763,6 @@ do_work:
 	SOFT_DISABLE_INTS(r3,r4)
 1:	bl	.preempt_schedule_irq
 
-	/* Hard-disable interrupts again (and update PACA) */
-#ifdef CONFIG_PPC_BOOK3E
-	wrteei	0
-#else
-	ld	r10,PACAKMSR(r13) /* Get kernel MSR without EE */
-	mtmsrd	r10,1
-#endif /* CONFIG_PPC_BOOK3E */
-	li	r0,PACA_IRQ_HARD_DIS
-	stb	r0,PACAIRQHAPPENED(r13)
-
 	/* Re-test flags and eventually loop */
 	clrrdi	r9,r1,THREAD_SHIFT
 	ld	r4,TI_FLAGS(r9)
@@ -783,14 +773,6 @@ do_work:
 user_work:
 #endif /* CONFIG_PREEMPT */
 
-	/* Enable interrupts */
-#ifdef CONFIG_PPC_BOOK3E
-	wrteei	1
-#else
-	ori	r10,r10,MSR_EE
-	mtmsrd	r10,1
-#endif /* CONFIG_PPC_BOOK3E */
-
 	andi.	r0,r4,_TIF_NEED_RESCHED
 	beq	1f
 	bl	.restore_interrupts
diff --git a/arch/powerpc/kernel/irq.c b/arch/powerpc/kernel/irq.c
index 5ec1b23..3717fb5 100644
--- a/arch/powerpc/kernel/irq.c
+++ b/arch/powerpc/kernel/irq.c
@@ -260,11 +260,17 @@ EXPORT_SYMBOL(arch_local_irq_restore);
  * if they are currently disabled. This is typically called before
  * schedule() or do_signal() when returning to userspace. We do it
  * in C to avoid the burden of dealing with lockdep etc...
+ *
+ * NOTE: This is called with interrupts hard disabled but not marked
+ * as such in paca->irq_happened, so we need to resync this.
  */
 void restore_interrupts(void)
 {
-	if (irqs_disabled())
+	if (irqs_disabled()) {
+		local_paca->irq_happened |= PACA_IRQ_HARD_DIS;
 		local_irq_enable();
+	} else
+		__hard_irq_enable();
 }
 
 #endif /* CONFIG_PPC64 */
-- 
1.7.9.5




  reply	other threads:[~2012-05-08  3:46 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-05-03  1:53 [PATCH] powerpc: use local var instead of local_paca->irq_happened directly in __check_irq_replay Wang Sheng-Hui
2012-05-03  2:15 ` Benjamin Herrenschmidt
2012-05-03  2:15   ` Benjamin Herrenschmidt
2012-05-03  2:27   ` Wang Sheng-Hui
2012-05-03  2:27     ` Wang Sheng-Hui
2012-05-03  2:32   ` Wang Sheng-Hui
2012-05-03  2:32     ` Wang Sheng-Hui
2012-05-03  4:26     ` Benjamin Herrenschmidt
2012-05-03  4:26       ` Benjamin Herrenschmidt
2012-05-03  4:22   ` Benjamin Herrenschmidt
2012-05-03  4:22     ` Benjamin Herrenschmidt
2012-05-03  5:51     ` Wang Sheng-Hui
2012-05-03  5:51       ` Wang Sheng-Hui
2012-05-03  6:52       ` Benjamin Herrenschmidt
2012-05-03  6:52         ` Benjamin Herrenschmidt
2012-05-03  6:33     ` Wang Sheng-Hui
2012-05-03  6:33       ` Wang Sheng-Hui
2012-05-03  6:59       ` Wang Sheng-Hui
2012-05-03  6:59         ` Wang Sheng-Hui
2012-05-03  8:09         ` Benjamin Herrenschmidt
2012-05-03  8:09           ` Benjamin Herrenschmidt
2012-05-03 23:35           ` Wang Sheng-Hui
2012-05-03 23:35             ` Wang Sheng-Hui
2012-05-04  0:10             ` Benjamin Herrenschmidt
2012-05-04  0:10               ` Benjamin Herrenschmidt
2012-05-08  3:46               ` Benjamin Herrenschmidt [this message]
2012-05-08  3:46                 ` Benjamin Herrenschmidt
2012-05-10  5:37                 ` Wang Sheng-Hui
2012-05-10  5:37                   ` Wang Sheng-Hui

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1336448796.2540.187.camel@pasglop \
    --to=benh@kernel.crashing.org \
    --cc=anton@samba.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=miltonm@bga.com \
    --cc=sfr@canb.auug.org.au \
    --cc=shhuiw@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.