All of lore.kernel.org
 help / color / mirror / Atom feed
From: rabin@rab.in (Rabin Vincent)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] ARM: cache-v7: Disable preemption when reading CCSIDR
Date: Tue, 14 Feb 2012 19:45:24 +0530	[thread overview]
Message-ID: <20120214141524.GB3310@debian> (raw)
In-Reply-To: <20120213232901.GG25655@n2100.arm.linux.org.uk>

On Mon, Feb 13, 2012 at 11:29:01PM +0000, Russell King - ARM Linux wrote:
> On Mon, Feb 13, 2012 at 02:23:29PM -0800, Stephen Boyd wrote:
> > On 02/13/12 10:15, Russell King - ARM Linux wrote:
> > > On Mon, Feb 13, 2012 at 10:13:27AM -0800, Stephen Boyd wrote:
> > >> Thanks. Russell has already merged the original patch to the fixes
> > >> branch. Hopefully he can fold this one in.
> > > Nope, I've asked Linus to pull it.
> > >
> > > So do we conclude that the original patch wasn't properly tested? :P
> > 
> > Sigh. Lockdep strikes again! I promise I tested it with lockdep disabled.
> > 
> > It looks like Linus' hasn't pulled yet but maybe he just hasn't
> > published it.
> 
> It's not nice to change something after you've sent a pull request -
> there's no way of knowing when Linus actually pulls it before he's
> published it, and if he gets something different then it can raise
> questions.
> 
> So, it's gone in as-is, and, as I'm now intending asking for another
> pull request soo soon after my previous one, this is something that
> we will have to live with probably for the remainder of the week.

OK, since it can't be folded in, here is a proper patch:

8<---------
>From 26f02624a20a61ed1997a4e8648e4c766a54d91d Mon Sep 17 00:00:00 2001
From: Rabin Vincent <rabin@rab.in>
Date: Tue, 14 Feb 2012 19:22:07 +0530
Subject: [PATCH] ARM: fix v7 boot with lockdep enabled

Bootup with lockdep enabled has been broken on v7 since b46c0f74657d
("ARM: 7321/1: cache-v7: Disable preemption when reading CCSIDR").

This is because v7_setup (which is called very early during boot) calls
v7_flush_dcache_all, and the save_and_disable_irqs added by that patch
ends up attempting to call into lockdep C code (trace_hardirqs_off())
when we are in no position to execute it (no stack, MMU off).

Fix this by using a notrace variant of save_and_disable_irqs.  The code
already uses the notrace variant of restore_irqs.

Cc: Stephen Boyd <sboyd@codeaurora.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Nicolas Pitre <nico@linaro.org>
Cc: stable at vger.kernel.org
Signed-off-by: Rabin Vincent <rabin@rab.in>
---
 arch/arm/include/asm/assembler.h |    5 +++++
 arch/arm/mm/cache-v7.S           |    2 +-
 2 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/arch/arm/include/asm/assembler.h b/arch/arm/include/asm/assembler.h
index 62f8095..23371b1 100644
--- a/arch/arm/include/asm/assembler.h
+++ b/arch/arm/include/asm/assembler.h
@@ -137,6 +137,11 @@
 	disable_irq
 	.endm
 
+	.macro	save_and_disable_irqs_notrace, oldcpsr
+	mrs	\oldcpsr, cpsr
+	disable_irq_notrace
+	.endm
+
 /*
  * Restore interrupt state previously stored in a register.  We don't
  * guarantee that this will preserve the flags.
diff --git a/arch/arm/mm/cache-v7.S b/arch/arm/mm/cache-v7.S
index 7a24d396..a655d3d 100644
--- a/arch/arm/mm/cache-v7.S
+++ b/arch/arm/mm/cache-v7.S
@@ -55,7 +55,7 @@ loop1:
 	cmp	r1, #2				@ see what cache we have at this level
 	blt	skip				@ skip if no cache, or just i-cache
 #ifdef CONFIG_PREEMPT
-	save_and_disable_irqs r9		@ make cssr&csidr read atomic
+	save_and_disable_irqs_notrace r9	@ make cssr&csidr read atomic
 #endif
 	mcr	p15, 2, r10, c0, c0, 0		@ select current cache level in cssr
 	isb					@ isb to sych the new cssr&csidr
-- 
1.7.9

WARNING: multiple messages have this Message-ID (diff)
From: Rabin Vincent <rabin@rab.in>
To: Russell King - ARM Linux <linux@arm.linux.org.uk>
Cc: Stephen Boyd <sboyd@codeaurora.org>,
	Nicolas Pitre <nico@fluxnic.net>,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org,
	Catalin Marinas <catalin.marinas@arm.com>
Subject: Re: [PATCH] ARM: cache-v7: Disable preemption when reading CCSIDR
Date: Tue, 14 Feb 2012 19:45:24 +0530	[thread overview]
Message-ID: <20120214141524.GB3310@debian> (raw)
In-Reply-To: <20120213232901.GG25655@n2100.arm.linux.org.uk>

On Mon, Feb 13, 2012 at 11:29:01PM +0000, Russell King - ARM Linux wrote:
> On Mon, Feb 13, 2012 at 02:23:29PM -0800, Stephen Boyd wrote:
> > On 02/13/12 10:15, Russell King - ARM Linux wrote:
> > > On Mon, Feb 13, 2012 at 10:13:27AM -0800, Stephen Boyd wrote:
> > >> Thanks. Russell has already merged the original patch to the fixes
> > >> branch. Hopefully he can fold this one in.
> > > Nope, I've asked Linus to pull it.
> > >
> > > So do we conclude that the original patch wasn't properly tested? :P
> > 
> > Sigh. Lockdep strikes again! I promise I tested it with lockdep disabled.
> > 
> > It looks like Linus' hasn't pulled yet but maybe he just hasn't
> > published it.
> 
> It's not nice to change something after you've sent a pull request -
> there's no way of knowing when Linus actually pulls it before he's
> published it, and if he gets something different then it can raise
> questions.
> 
> So, it's gone in as-is, and, as I'm now intending asking for another
> pull request soo soon after my previous one, this is something that
> we will have to live with probably for the remainder of the week.

OK, since it can't be folded in, here is a proper patch:

8<---------
>From 26f02624a20a61ed1997a4e8648e4c766a54d91d Mon Sep 17 00:00:00 2001
From: Rabin Vincent <rabin@rab.in>
Date: Tue, 14 Feb 2012 19:22:07 +0530
Subject: [PATCH] ARM: fix v7 boot with lockdep enabled

Bootup with lockdep enabled has been broken on v7 since b46c0f74657d
("ARM: 7321/1: cache-v7: Disable preemption when reading CCSIDR").

This is because v7_setup (which is called very early during boot) calls
v7_flush_dcache_all, and the save_and_disable_irqs added by that patch
ends up attempting to call into lockdep C code (trace_hardirqs_off())
when we are in no position to execute it (no stack, MMU off).

Fix this by using a notrace variant of save_and_disable_irqs.  The code
already uses the notrace variant of restore_irqs.

Cc: Stephen Boyd <sboyd@codeaurora.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Nicolas Pitre <nico@linaro.org>
Cc: stable@vger.kernel.org
Signed-off-by: Rabin Vincent <rabin@rab.in>
---
 arch/arm/include/asm/assembler.h |    5 +++++
 arch/arm/mm/cache-v7.S           |    2 +-
 2 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/arch/arm/include/asm/assembler.h b/arch/arm/include/asm/assembler.h
index 62f8095..23371b1 100644
--- a/arch/arm/include/asm/assembler.h
+++ b/arch/arm/include/asm/assembler.h
@@ -137,6 +137,11 @@
 	disable_irq
 	.endm
 
+	.macro	save_and_disable_irqs_notrace, oldcpsr
+	mrs	\oldcpsr, cpsr
+	disable_irq_notrace
+	.endm
+
 /*
  * Restore interrupt state previously stored in a register.  We don't
  * guarantee that this will preserve the flags.
diff --git a/arch/arm/mm/cache-v7.S b/arch/arm/mm/cache-v7.S
index 7a24d396..a655d3d 100644
--- a/arch/arm/mm/cache-v7.S
+++ b/arch/arm/mm/cache-v7.S
@@ -55,7 +55,7 @@ loop1:
 	cmp	r1, #2				@ see what cache we have at this level
 	blt	skip				@ skip if no cache, or just i-cache
 #ifdef CONFIG_PREEMPT
-	save_and_disable_irqs r9		@ make cssr&csidr read atomic
+	save_and_disable_irqs_notrace r9	@ make cssr&csidr read atomic
 #endif
 	mcr	p15, 2, r10, c0, c0, 0		@ select current cache level in cssr
 	isb					@ isb to sych the new cssr&csidr
-- 
1.7.9

  reply	other threads:[~2012-02-14 14:15 UTC|newest]

Thread overview: 51+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-02-02 19:24 [PATCH] ARM: cache-v7: Disable preemption when reading CCSIDR Stephen Boyd
2012-02-02 19:24 ` Stephen Boyd
2012-02-02 20:44 ` Russell King - ARM Linux
2012-02-02 20:44   ` Russell King - ARM Linux
2012-02-02 21:38   ` Nicolas Pitre
2012-02-02 21:38     ` Nicolas Pitre
2012-02-02 23:36     ` Stephen Boyd
2012-02-02 23:36       ` Stephen Boyd
2012-02-03  0:36       ` Russell King - ARM Linux
2012-02-03  0:36         ` Russell King - ARM Linux
2012-02-03  0:49         ` Stephen Boyd
2012-02-03  0:49           ` Stephen Boyd
2012-02-03  1:18           ` Nicolas Pitre
2012-02-03  1:18             ` Nicolas Pitre
2012-02-03  1:18             ` Nicolas Pitre
2012-02-03  2:03             ` Stephen Boyd
2012-02-03  2:03               ` Stephen Boyd
2012-02-03  2:35               ` Nicolas Pitre
2012-02-03  2:35                 ` Nicolas Pitre
2012-02-03  2:37                 ` Stephen Boyd
2012-02-03  2:37                   ` Stephen Boyd
2012-02-03  3:04                   ` Nicolas Pitre
2012-02-03  3:04                     ` Nicolas Pitre
2012-02-03 11:15                     ` Sergei Shtylyov
2012-02-03 11:15                       ` Sergei Shtylyov
2012-02-04 18:00               ` Catalin Marinas
2012-02-04 18:00                 ` Catalin Marinas
2012-02-13 17:54               ` Rabin Vincent
2012-02-13 17:54                 ` Rabin Vincent
2012-02-13 18:09                 ` Nicolas Pitre
2012-02-13 18:09                   ` Nicolas Pitre
2012-02-13 18:13                   ` Stephen Boyd
2012-02-13 18:13                     ` Stephen Boyd
2012-02-13 18:15                     ` Russell King - ARM Linux
2012-02-13 18:15                       ` Russell King - ARM Linux
2012-02-13 22:23                       ` Stephen Boyd
2012-02-13 22:23                         ` Stephen Boyd
2012-02-13 23:29                         ` Russell King - ARM Linux
2012-02-13 23:29                           ` Russell King - ARM Linux
2012-02-14 14:15                           ` Rabin Vincent [this message]
2012-02-14 14:15                             ` Rabin Vincent
2012-02-14 17:30                             ` Nicolas Pitre
2012-02-14 17:30                               ` Nicolas Pitre
2012-02-14 18:07                             ` Stephen Boyd
2012-02-14 18:07                               ` Stephen Boyd
2012-02-03  1:16         ` Nicolas Pitre
2012-02-03  1:16           ` Nicolas Pitre
2012-02-07  3:34         ` Saravana Kannan
2012-02-07  3:34           ` Saravana Kannan
2012-02-07 17:42           ` Stephen Boyd
2012-02-07 17:42             ` Stephen Boyd

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=20120214141524.GB3310@debian \
    --to=rabin@rab.in \
    --cc=linux-arm-kernel@lists.infradead.org \
    /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.