20 Dec, 2010

1 commit

  • Since commit 3e4d3af501 "mm: stack based kmap_atomic()", it is no longer
    necessary to carry an ad hoc version of kmap_atomic() added in commit
    7e5a69e83b "ARM: 6007/1: fix highmem with VIPT cache and DMA" to cope
    with reentrancy.

    In fact, it is now actively wrong to rely on fixed kmap type indices
    (namely KM_L1_CACHE) as kmap_atomic() totally ignores them now and a
    concurrent instance of it may reuse any slot for any purpose.

    Signed-off-by: Nicolas Pitre

    Nicolas Pitre
     

28 Oct, 2010

1 commit

  • Christoph reported a nice splat which illustrated a race in the new stack
    based kmap_atomic implementation.

    The problem is that we pop our stack slot before we're completely done
    resetting its state -- in particular clearing the PTE (sometimes that's
    CONFIG_DEBUG_HIGHMEM). If an interrupt happens before we actually clear
    the PTE used for the last slot, that interrupt can reuse the slot in a
    dirty state, which triggers a BUG in kmap_atomic().

    Fix this by introducing kmap_atomic_idx() which reports the current slot
    index without actually releasing it and use that to find the PTE and delay
    the _pop() until after we're completely done.

    Signed-off-by: Peter Zijlstra
    Reported-by: Christoph Hellwig
    Acked-by: Rik van Riel
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Peter Zijlstra
     

27 Oct, 2010

1 commit

  • Keep the current interface but ignore the KM_type and use a stack based
    approach.

    The advantage is that we get rid of crappy code like:

    #define __KM_PTE \
    (in_nmi() ? KM_NMI_PTE : \
    in_irq() ? KM_IRQ_PTE : \
    KM_PTE0)

    and in general can stop worrying about what context we're in and what kmap
    slots might be appropriate for that.

    The downside is that FRV kmap_atomic() gets more expensive.

    For now we use a CPP trick suggested by Andrew:

    #define kmap_atomic(page, args...) __kmap_atomic(page)

    to avoid having to touch all kmap_atomic() users in a single patch.

    [ not compiled on:
    - mn10300: the arch doesn't actually build with highmem to begin with ]

    [akpm@linux-foundation.org: coding-style fixes]
    [akpm@linux-foundation.org: fix up drivers/gpu/drm/i915/intel_overlay.c]
    Acked-by: Rik van Riel
    Signed-off-by: Peter Zijlstra
    Acked-by: Chris Metcalf
    Cc: David Howells
    Cc: Hugh Dickins
    Cc: Ingo Molnar
    Cc: Thomas Gleixner
    Cc: "H. Peter Anvin"
    Cc: Steven Rostedt
    Cc: Russell King
    Cc: Ralf Baechle
    Cc: David Miller
    Cc: Paul Mackerras
    Cc: Benjamin Herrenschmidt
    Cc: Dave Airlie
    Cc: Li Zefan
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Peter Zijlstra
     

10 Aug, 2010

1 commit

  • kunmap_atomic() is currently at level -4 on Rusty's "Hard To Misuse"
    list[1] ("Follow common convention and you'll get it wrong"), except in
    some architectures when CONFIG_DEBUG_HIGHMEM is set[2][3].

    kunmap() takes a pointer to a struct page; kunmap_atomic(), however, takes
    takes a pointer to within the page itself. This seems to once in a while
    trip people up (the convention they are following is the one from
    kunmap()).

    Make it much harder to misuse, by moving it to level 9 on Rusty's list[4]
    ("The compiler/linker won't let you get it wrong"). This is done by
    refusing to build if the type of its first argument is a pointer to a
    struct page.

    The real kunmap_atomic() is renamed to kunmap_atomic_notypecheck()
    (which is what you would call in case for some strange reason calling it
    with a pointer to a struct page is not incorrect in your code).

    The previous version of this patch was compile tested on x86-64.

    [1] http://ozlabs.org/~rusty/index.cgi/tech/2008-04-01.html
    [2] In these cases, it is at level 5, "Do it right or it will always
    break at runtime."
    [3] At least mips and powerpc look very similar, and sparc also seems to
    share a common ancestor with both; there seems to be quite some
    degree of copy-and-paste coding here. The include/asm/highmem.h file
    for these three archs mention x86 CPUs at its top.
    [4] http://ozlabs.org/~rusty/index.cgi/tech/2008-03-30.html
    [5] As an aside, could someone tell me why mn10300 uses unsigned long as
    the first parameter of kunmap_atomic() instead of void *?

    Signed-off-by: Cesar Eduardo Barros
    Cc: Russell King (arch/arm)
    Cc: Ralf Baechle (arch/mips)
    Cc: David Howells (arch/frv, arch/mn10300)
    Cc: Koichi Yasutake (arch/mn10300)
    Cc: Kyle McMartin (arch/parisc)
    Cc: Helge Deller (arch/parisc)
    Cc: "James E.J. Bottomley" (arch/parisc)
    Cc: Benjamin Herrenschmidt (arch/powerpc)
    Cc: Paul Mackerras (arch/powerpc)
    Cc: "David S. Miller" (arch/sparc)
    Cc: Thomas Gleixner (arch/x86)
    Cc: Ingo Molnar (arch/x86)
    Cc: "H. Peter Anvin" (arch/x86)
    Cc: Arnd Bergmann (include/asm-generic)
    Cc: Rusty Russell ("Hard To Misuse" list)
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Cesar Eduardo Barros
     

31 Jul, 2010

1 commit

  • 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

    Signed-off-by: Gary King
    Acked-by: Nicolas Pitre
    Signed-off-by: Russell King

    Gary King
     

09 Jun, 2010

1 commit

  • When CONFIG_DEBUG_HIGHMEM is used, the fixmap entry used for a highmem page
    by kmap_atomic() is always cleared by kunmap_atomic(). This helps find
    bad usages such as dereferences after the unmap, or overflow into the
    adjacent fixmap areas.

    But this debugging aid is completely bypassed when a kmap for the same
    page already exists as the kmap is reused instead. ON VIVT systems we
    have no choice but to reuse that kmap due to cache coherency issues,
    but on non VIVT systems we should always force the fixmap usage when
    debugging is active.

    Signed-off-by: Nicolas Pitre
    Signed-off-by: Russell King

    Nicolas Pitre
     

14 Apr, 2010

1 commit

  • The VIVT cache of a highmem page is always flushed before the page
    is unmapped. This cache flush is explicit through flush_cache_kmaps()
    in flush_all_zero_pkmaps(), or through __cpuc_flush_dcache_area() in
    kunmap_atomic(). There is also an implicit flush of those highmem pages
    that were part of a process that just terminated making those pages free
    as the whole VIVT cache has to be flushed on every task switch. Hence
    unmapped highmem pages need no cache maintenance in that case.

    However unmapped pages may still be cached with a VIPT cache because the
    cache is tagged with physical addresses. There is no need for a whole
    cache flush during task switching for that reason, and despite the
    explicit cache flushes in flush_all_zero_pkmaps() and kunmap_atomic(),
    some highmem pages that were mapped in user space end up still cached
    even when they become unmapped.

    So, we do have to perform cache maintenance on those unmapped highmem
    pages in the context of DMA when using a VIPT cache. Unfortunately,
    it is not possible to perform that cache maintenance using physical
    addresses as all the L1 cache maintenance coprocessor functions accept
    virtual addresses only. Therefore we have no choice but to set up a
    temporary virtual mapping for that purpose.

    And of course the explicit cache flushing when unmapping a highmem page
    on a system with a VIPT cache now can go, which should increase
    performance.

    While at it, because the code in __flush_dcache_page() has to be modified
    anyway, let's also make sure the mapped highmem pages are pinned with
    kmap_high_get() for the duration of the cache maintenance operation.
    Because kunmap() does unmap highmem pages lazily, it was reported by
    Gary King that those pages ended up being unmapped
    during cache maintenance on SMP causing segmentation faults.

    Signed-off-by: Nicolas Pitre
    Signed-off-by: Russell King

    Nicolas Pitre
     

14 Dec, 2009

1 commit


11 Oct, 2009

1 commit


05 Sep, 2009

1 commit

  • Let's suppose a highmem page is kmap'd with kmap(). A pkmap entry is
    used, the page mapped to it, and the virtual cache is dirtied. Then
    kunmap() is used which does virtually nothing except for decrementing a
    usage count.

    Then, let's suppose the _same_ page gets mapped using kmap_atomic().
    It is therefore mapped onto a fixmap entry instead, which has a
    different virtual address unaware of the dirty cache data for that page
    sitting in the pkmap mapping.

    Fortunately it is easy to know if a pkmap mapping still exists for that
    page and use it directly with kmap_atomic(), thanks to kmap_high_get().

    And actual testing with a printk in the added code path shows that this
    condition is actually met *extremely* frequently. Seems that we've been
    quite lucky that things have worked so well with highmem so far.

    Cc: stable@kernel.org
    Signed-off-by: Nicolas Pitre
    Signed-off-by: Russell King

    Nicolas Pitre
     

16 Mar, 2009

1 commit

  • The kmap virtual area borrows a 2MB range at the top of the 16MB area
    below PAGE_OFFSET currently reserved for kernel modules and/or the
    XIP kernel. This 2MB corresponds to the range covered by 2 consecutive
    second-level page tables, or a single pmd entry as seen by the Linux
    page table abstraction. Because XIP kernels are unlikely to be seen
    on systems needing highmem support, there shouldn't be any shortage of
    VM space for modules (14 MB for modules is still way more than twice the
    typical usage).

    Because the virtual mapping of highmem pages can go away at any moment
    after kunmap() is called on them, we need to bypass the delayed cache
    flushing provided by flush_dcache_page() in that case.

    The atomic kmap versions are based on fixmaps, and
    __cpuc_flush_dcache_page() is used directly in that case.

    Signed-off-by: Nicolas Pitre

    Nicolas Pitre