[PATCH] [ARM] highmem: fix SMP preemption bug in kmap_high_l1_vipt

[PATCH] [ARM] highmem: fix SMP preemption bug in kmap_high_l1_vipt

[gking at nvidia.com]

From: Andrew Howe <ahowe@nvidia.com>

smp_processor_id() must not be called from a preemptible context (this
is checked by CONFIG_DEBUG_PREEMPT).  kmap_high_l1_vipt() was doing so.
This lead to a problem where the wrong per_cpu kmap_high_l1_vipt_depth
could be incremented, causing a BUG_ON(*depth <= 0); in
kunmap_high_l1_vipt().

The solution is to move the call to smp_processor_id() after the call
to preempt_disable().

Signed-off-by: Gary King <gking@nvidia.com>
---
 arch/arm/mm/highmem.c |   13 ++++++++-----
 1 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/arch/arm/mm/highmem.c b/arch/arm/mm/highmem.c
index 086816b..6ab2440 100644
--- a/arch/arm/mm/highmem.c
+++ b/arch/arm/mm/highmem.c
@@ -163,19 +163,22 @@ static DEFINE_PER_CPU(int, kmap_high_l1_vipt_depth);
 
 void *kmap_high_l1_vipt(struct page *page, pte_t *saved_pte)
 {
-	unsigned int idx, cpu = smp_processor_id();
-	int *depth = &per_cpu(kmap_high_l1_vipt_depth, cpu);
+	unsigned int idx, cpu;
+	int *depth;
 	unsigned long vaddr, flags;
 	pte_t pte, *ptep;
 
+	if (!in_interrupt())
+		preempt_disable();
+
+	cpu = smp_processor_id();
+	depth = &per_cpu(kmap_high_l1_vipt_depth, cpu);
+
 	idx = KM_L1_CACHE + KM_TYPE_NR * cpu;
 	vaddr = __fix_to_virt(FIX_KMAP_BEGIN + idx);
 	ptep = TOP_PTE(vaddr);
 	pte = mk_pte(page, kmap_prot);
 
-	if (!in_interrupt())
-		preempt_disable();
-
 	raw_local_irq_save(flags);
 	(*depth)++;
 	if (pte_val(*ptep) == pte_val(pte)) {
-- 
1.7.0.4


-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information.  Any unauthorized review, use, disclosure or distribution
is prohibited.  If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------

[PATCH] [ARM] highmem: fix SMP preemption bug in kmap_high_l1_vipt

[Nicolas Pitre]

On Wed, 28 Jul 2010, gking at nvidia.com wrote:

> From: Andrew Howe <ahowe@nvidia.com>
> 
> smp_processor_id() must not be called from a preemptible context (this
> is checked by CONFIG_DEBUG_PREEMPT).  kmap_high_l1_vipt() was doing so.
> This lead to a problem where the wrong per_cpu kmap_high_l1_vipt_depth
> could be incremented, causing a BUG_ON(*depth <= 0); in
> kunmap_high_l1_vipt().
> 
> The solution is to move the call to smp_processor_id() after the call
> to preempt_disable().
> 
> Signed-off-by: Gary King <gking@nvidia.com>

Acked-by: Nicolas Pitre <nico.as.pitre@linaro.org>

Please forward to RMK's patch system.


> ---
>  arch/arm/mm/highmem.c |   13 ++++++++-----
>  1 files changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/arm/mm/highmem.c b/arch/arm/mm/highmem.c
> index 086816b..6ab2440 100644
> --- a/arch/arm/mm/highmem.c
> +++ b/arch/arm/mm/highmem.c
> @@ -163,19 +163,22 @@ static DEFINE_PER_CPU(int, kmap_high_l1_vipt_depth);
>  
>  void *kmap_high_l1_vipt(struct page *page, pte_t *saved_pte)
>  {
> -	unsigned int idx, cpu = smp_processor_id();
> -	int *depth = &per_cpu(kmap_high_l1_vipt_depth, cpu);
> +	unsigned int idx, cpu;
> +	int *depth;
>  	unsigned long vaddr, flags;
>  	pte_t pte, *ptep;
>  
> +	if (!in_interrupt())
> +		preempt_disable();
> +
> +	cpu = smp_processor_id();
> +	depth = &per_cpu(kmap_high_l1_vipt_depth, cpu);
> +
>  	idx = KM_L1_CACHE + KM_TYPE_NR * cpu;
>  	vaddr = __fix_to_virt(FIX_KMAP_BEGIN + idx);
>  	ptep = TOP_PTE(vaddr);
>  	pte = mk_pte(page, kmap_prot);
>  
> -	if (!in_interrupt())
> -		preempt_disable();
> -
>  	raw_local_irq_save(flags);
>  	(*depth)++;
>  	if (pte_val(*ptep) == pte_val(pte)) {
> -- 
> 1.7.0.4
> 
> 
> -----------------------------------------------------------------------------------
> This email message is for the sole use of the intended recipient(s) and may contain
> confidential information.  Any unauthorized review, use, disclosure or distribution
> is prohibited.  If you are not the intended recipient, please contact the sender by
> reply email and destroy all copies of the original message.
> -----------------------------------------------------------------------------------
> 

[PATCH] [ARM] highmem: fix SMP preemption bug in kmap_high_l1_vipt

[Russell King - ARM Linux]

On Thu, Jul 29, 2010 at 10:59:52AM -0400, Nicolas Pitre wrote:
> On Wed, 28 Jul 2010, gking at nvidia.com wrote:
> 
> > From: Andrew Howe <ahowe@nvidia.com>
> > 
> > smp_processor_id() must not be called from a preemptible context (this
> > is checked by CONFIG_DEBUG_PREEMPT).  kmap_high_l1_vipt() was doing so.
> > This lead to a problem where the wrong per_cpu kmap_high_l1_vipt_depth
> > could be incremented, causing a BUG_ON(*depth <= 0); in
> > kunmap_high_l1_vipt().
> > 
> > The solution is to move the call to smp_processor_id() after the call
> > to preempt_disable().
> > 
> > Signed-off-by: Gary King <gking@nvidia.com>
> 
> Acked-by: Nicolas Pitre <nico.as.pitre@linaro.org>
> 
> Please forward to RMK's patch system.

Unfortunately, I suspect that none of the remaining stuff in my master
branch will make 2.6.35 (which is due this evening) especially as I've
put the fixes for the Versatile/Realview/Versatile Express MMC card
detection in there today, and we still don't know what's right for its
GPIO-based card detection.  (The change to fix the status method may
have broken the GPIO-based method.)

So I'm not sending a pull request this evening.  Hopefully people can
work out what's right for the GPIO based card detection while I'm out
tomorrow, and report back so it can be fixed in the evening if
necessary.