Re: mmotm 2010-10-20-15-01 uploaded

[Peter Zijlstra <a.p.zijlstra@chello.nl>]

On Thu, 2010-10-21 at 00:30 -0700, Andrew Morton wrote:
> On Thu, 21 Oct 2010 09:17:26 +0200 Peter Zijlstra <a.p.zijlstra@chello.nl> wrote:
> 
> > > It would have been clearer to do
> > > 
> > >         WARN_ON_ONCE(vaddr != __fix_to_virt(FIX_KMAP_BEGIN + idx))
> > > 
> > > but I don't see what's gone wrong here.  Peter?
> > 
> > Right, so that's the warning that the unmapped address isn't actually
> > the top-of-stack one.
> > 
> > Your version is much nicer, although I haven't looked too hard at it, I
> > probably got my head in a twist.
> > 
> > But like you, I cannot directly see what's going wrong here,
> > zero_user_segments() seems a simple enough function without any
> > kmap_atomic nesting, so I'm not at all seeing how that's going wrong.
> > 
> > /me goes find where you hide your mmotm patch-queue and stare at the
> > actual code.

OK, so found it, and its really rather embarrassing.. I really blotched
those x86 WARN_ON_ONCE()s, for some reason I got my brain in a twist and
messed those up big-time. All other architectures have the form you
suggested earlier.

The problem is that fixmaps are top-down, so the original warning ended
up trying to compare x with (-x) >> PAGE_SHIFT, which clearly won't
work.

The sad part is that I apparently only compile tested the debug code :/

With the warning fixed (and CONFIG_DEBUG_HIGHMEM=y) an otherwise
i386-defconfig seems to fully complete boot without warnings on a qemu
with 2048 MB of memory.

---
Subject: mm, x86: Fix stack based kmap_atomic debug warnings
From: Peter Zijlstra <a.p.zijlstra@chello.nl>
Date: Thu Oct 21 11:45:08 CEST 2010

Due to a massive brainfart I got the x86 kunmap_atomic debug code that
is supposed to avoid stack violations wrong.

Use the form all other architectures already use (which was also
independently suggested by Andrew).

Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
---
 arch/x86/mm/highmem_32.c |    3 +--
 arch/x86/mm/iomap_32.c   |    3 +--
 2 files changed, 2 insertions(+), 4 deletions(-)

Index: linux-2.6/arch/x86/mm/highmem_32.c
===================================================================
--- linux-2.6.orig/arch/x86/mm/highmem_32.c
+++ linux-2.6/arch/x86/mm/highmem_32.c
@@ -78,8 +78,7 @@ void __kunmap_atomic(void *kvaddr)
 		idx = type + KM_TYPE_NR * smp_processor_id();
 
 #ifdef CONFIG_DEBUG_HIGHMEM
-		WARN_ON_ONCE(idx !=
-			((vaddr - __fix_to_virt(FIX_KMAP_BEGIN)) >> PAGE_SHIFT));
+		WARN_ON_ONCE(vaddr != __fix_to_virt(FIX_KMAP_BEGIN + idx));
 #endif
 		/*
 		 * Force other mappings to Oops if they'll try to access this
Index: linux-2.6/arch/x86/mm/iomap_32.c
===================================================================
--- linux-2.6.orig/arch/x86/mm/iomap_32.c
+++ linux-2.6/arch/x86/mm/iomap_32.c
@@ -102,8 +102,7 @@ iounmap_atomic(void __iomem *kvaddr)
 		idx = type + KM_TYPE_NR * smp_processor_id();
 
 #ifdef CONFIG_DEBUG_HIGHMEM
-		WARN_ON_ONCE(idx !=
-			((vaddr - __fix_to_virt(FIX_KMAP_BEGIN)) >> PAGE_SHIFT));
+		WARN_ON_ONCE(vaddr != __fix_to_virt(FIX_KMAP_BEGIN + idx));
 #endif
 		/*
 		 * Force other mappings to Oops if they'll try to access this