Messages in this thread Patch in this message | | | Subject | Re: mmotm 2010-10-20-15-01 uploaded | From | Peter Zijlstra <> | Date | Thu, 21 Oct 2010 11:52:17 +0200 |
| |
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
| |