All of lore.kernel.org
 help / color / mirror / Atom feed
From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
To: Valentine Barshak <vbarshak@ru.mvista.com>
Cc: olof@lixom.net, linuxppc-dev@ozlabs.org, paulus@samba.org
Subject: [PATCH v3] powerpc/ppc64: Use preempt_schedule_irq instead of preempt_schedule
Date: Tue, 27 Oct 2009 16:41:17 +1100	[thread overview]
Message-ID: <1256622077.11607.85.camel@pasglop> (raw)
In-Reply-To: <1256601324.2076.49.camel@pasglop>


> So I _think_ that the irqs on/off accounting for lockdep isn't quite
> right. What do you think of this slightly modified version ? I've only
> done a quick boot test on a G5 with lockdep enabled and a played a bit,
> nothing shows up so far but it's definitely not conclusive.
> 
> The main difference is that I call trace_hardirqs_off to "advertise"
> the fact that we are soft-disabling (it could be a dup, but at this
> stage this is no big deal, but it's not always, like in syscall return
> the kernel thinks we have interrupts enabled and could thus get out
> of sync without that).
> 
> I also mark the PACA hard disable to reflect the MSR:EE state before
> calling into preempt_schedule_irq().

Allright, second thought :-)

It's probably simpler to just keep hardirqs off. Code is smaller and
simpler and the scheduler will re-enable them soon enough anyways.

This version of the patch also spaces the code a bit and adds comments
which makes them (the code and the patch) more readable.

Cheers,
Ben.
 
From: Benjamin Herrenschmidt <benh@kernel.crashing.org>

[PATCH v3] powerpc/ppc64: Use preempt_schedule_irq instead of preempt_schedule

Based on an original patch by Valentine Barshak <vbarshak@ru.mvista.com>

Use preempt_schedule_irq to prevent infinite irq-entry and
eventual stack overflow problems with fast-paced IRQ sources.

This kind of problems has been observed on the PASemi Electra IDE
controller. We have to make sure we are soft-disabled before calling
preempt_schedule_irq and hard disable interrupts after that
to avoid unrecoverable exceptions.

This patch also moves the "clrrdi r9,r1,THREAD_SHIFT" out of
the #ifdef CONFIG_PPC_BOOK3E scope, since r9 is clobbered
and has to be restored in both cases.
---
 arch/powerpc/kernel/entry_64.S |   41 ++++++++++++++++++++-------------------
 1 files changed, 21 insertions(+), 20 deletions(-)

diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S
index f9fd54b..9763267 100644
--- a/arch/powerpc/kernel/entry_64.S
+++ b/arch/powerpc/kernel/entry_64.S
@@ -658,42 +658,43 @@ do_work:
 	cmpdi	r0,0
 	crandc	eq,cr1*4+eq,eq
 	bne	restore
-	/* here we are preempting the current task */
-1:
-#ifdef CONFIG_TRACE_IRQFLAGS
-	bl	.trace_hardirqs_on
-	/* Note: we just clobbered r10 which used to contain the previous
-	 * MSR before the hard-disabling done by the caller of do_work.
-	 * We don't have that value anymore, but it doesn't matter as
-	 * we will hard-enable unconditionally, we can just reload the
-	 * current MSR into r10
+
+	/* Here we are preempting the current task.
+	 *
+	 * Ensure interrupts are soft-disabled. We also properly mark
+	 * the PACA to reflect the fact that they are hard-disabled
+	 * and trace the change
 	 */
-	mfmsr	r10
-#endif /* CONFIG_TRACE_IRQFLAGS */
-	li	r0,1
+	li	r0,0
 	stb	r0,PACASOFTIRQEN(r13)
 	stb	r0,PACAHARDIRQEN(r13)
+	TRACE_DISABLE_INTS
+
+	/* Call the scheduler with soft IRQs off */
+1:	bl	.preempt_schedule_irq
+
+	/* Hard-disable interrupts again (and update PACA) */
 #ifdef CONFIG_PPC_BOOK3E
-	wrteei	1
-	bl	.preempt_schedule
 	wrteei	0
 #else
-	ori	r10,r10,MSR_EE
-	mtmsrd	r10,1		/* reenable interrupts */
-	bl	.preempt_schedule
 	mfmsr	r10
-	clrrdi	r9,r1,THREAD_SHIFT
-	rldicl	r10,r10,48,1	/* disable interrupts again */
+	rldicl	r10,r10,48,1
 	rotldi	r10,r10,16
 	mtmsrd	r10,1
 #endif /* CONFIG_PPC_BOOK3E */
+	li	r0,0
+	stb	r0,PACAHARDIRQEN(r13)
+
+	/* Re-test flags and eventually loop */
+	clrrdi	r9,r1,THREAD_SHIFT
 	ld	r4,TI_FLAGS(r9)
 	andi.	r0,r4,_TIF_NEED_RESCHED
 	bne	1b
 	b	restore
 
 user_work:
-#endif
+#endif /* CONFIG_PREEMPT */
+
 	/* Enable interrupts */
 #ifdef CONFIG_PPC_BOOK3E
 	wrteei	1
-- 
1.6.1.2.14.gf26b5

  reply	other threads:[~2009-10-27  5:41 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-10-19 18:28 [PATCH] [RFC] PowerPC64: Use preempt_schedule_irq instead of preempt_schedule when returning from exceptions Valentine Barshak
2009-10-26 23:55 ` Benjamin Herrenschmidt
2009-10-27  5:41   ` Benjamin Herrenschmidt [this message]
2009-10-28 19:19     ` [PATCH v3] powerpc/ppc64: Use preempt_schedule_irq instead of preempt_schedule Valentine
2009-10-28 20:30       ` Benjamin Herrenschmidt
2009-10-28 21:28         ` Valentine
2009-10-28 21:37           ` Benjamin Herrenschmidt
2009-10-28 22:49             ` Valentine
2009-10-29  0:49               ` Benjamin Herrenschmidt
2009-11-06 22:38                 ` Valentine
2009-11-06 22:49                   ` Benjamin Herrenschmidt

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=1256622077.11607.85.camel@pasglop \
    --to=benh@kernel.crashing.org \
    --cc=linuxppc-dev@ozlabs.org \
    --cc=olof@lixom.net \
    --cc=paulus@samba.org \
    --cc=vbarshak@ru.mvista.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.