Patchwork [7/7] sparc64: Add function graph tracer support.

login
register
mail settings
Submitter David Miller
Date April 20, 2010, 7:50 a.m.
Message ID <20100420.005051.40113282.davem@davemloft.net>
Download mbox | patch
Permalink /patch/50517/
State Accepted
Delegated to: David Miller
Headers show

Comments

David Miller - April 20, 2010, 7:50 a.m.
From: David Miller <davem@davemloft.net>
Date: Mon, 19 Apr 2010 22:51:05 -0700 (PDT)

> I have to think about why I might have done things this way to
> make sure I fix this right.

Ok, I don't get crashes any more.  There are all kinds of issues
with the sw watchdog triggering because cpus get slogged in
the scheduler rebalance code, but that's a seperate set of
issues.

Here is the final fix I'll commit.

sparc64: Fix hardirq tracing in trap return path.

We can overflow the hardirq stack if we set the %pil here
so early, just let the normal control flow do it.

This is fine as we are allowed to do the actual IRQ enable
at any point after we call trace_hardirqs_on.

Signed-off-by: David S. Miller <davem@davemloft.net>
---
 arch/sparc/kernel/rtrap_64.S |   12 +++++++++++-
 1 files changed, 11 insertions(+), 1 deletions(-)
Frédéric Weisbecker - April 20, 2010, 1:58 p.m.
On Tue, Apr 20, 2010 at 12:50:51AM -0700, David Miller wrote:
> From: David Miller <davem@davemloft.net>
> Date: Mon, 19 Apr 2010 22:51:05 -0700 (PDT)
> 
> > I have to think about why I might have done things this way to
> > make sure I fix this right.
> 
> Ok, I don't get crashes any more.  There are all kinds of issues
> with the sw watchdog triggering because cpus get slogged in
> the scheduler rebalance code, but that's a seperate set of
> issues.



I merged your latest sparc tree and the crashes are gone.

I'm really happy you've solved this. Thanks a lot for having spent
so much time on it!

--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller - April 20, 2010, 9:17 p.m.
From: Frederic Weisbecker <fweisbec@gmail.com>
Date: Tue, 20 Apr 2010 15:58:13 +0200

> I merged your latest sparc tree and the crashes are gone.
> 
> I'm really happy you've solved this. Thanks a lot for having spent
> so much time on it!

Thanks for testing and suffering through this bug hunt :-)
--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Steven Rostedt - April 20, 2010, 10:52 p.m.
On Tue, 2010-04-20 at 14:17 -0700, David Miller wrote:
> From: Frederic Weisbecker <fweisbec@gmail.com>
> Date: Tue, 20 Apr 2010 15:58:13 +0200
> 
> > I merged your latest sparc tree and the crashes are gone.
> > 
> > I'm really happy you've solved this. Thanks a lot for having spent
> > so much time on it!
> 
> Thanks for testing and suffering through this bug hunt :-)

Wow, I was having fun reading this CSI report ;-)

Good job guys!

-- Steve


--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/arch/sparc/kernel/rtrap_64.S b/arch/sparc/kernel/rtrap_64.S
index 83f1873..090b9e9 100644
--- a/arch/sparc/kernel/rtrap_64.S
+++ b/arch/sparc/kernel/rtrap_64.S
@@ -130,7 +130,17 @@  rtrap_xcall:
 		 nop
 		call			trace_hardirqs_on
 		 nop
-		wrpr			%l4, %pil
+		/* Do not actually set the %pil here.  We will do that
+		 * below after we clear PSTATE_IE in the %pstate register.
+		 * If we re-enable interrupts here, we can recurse down
+		 * the hardirq stack potentially endlessly, causing a
+		 * stack overflow.
+		 *
+		 * It is tempting to put this test and trace_hardirqs_on
+		 * call at the 'rt_continue' label, but that will not work
+		 * as that path hits unconditionally and we do not want to
+		 * execute this in NMI return paths, for example.
+		 */
 #endif
 rtrap_no_irq_enable:
 		andcc			%l1, TSTATE_PRIV, %l3