lkml.org 
[lkml]   [2010]   [Oct]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    SubjectRe: mmotm 2010-10-20-15-01 uploaded
    From
    Date
    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



    \
     
     \ /
      Last update: 2010-10-21 11:55    [W:2.410 / U:0.060 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site