lkml.org 
[lkml]   [2009]   [Apr]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [PATCH mmotm] mm: alloc_large_system_hash check order
    On Wed, Apr 29, 2009 at 10:09:48PM +0100, Hugh Dickins wrote:
    > On an x86_64 with 4GB ram, tcp_init()'s call to alloc_large_system_hash(),
    > to allocate tcp_hashinfo.ehash, is now triggering an mmotm WARN_ON_ONCE on
    > order >= MAX_ORDER - it's hoping for order 11. alloc_large_system_hash()
    > had better make its own check on the order.
    >
    > Signed-off-by: Hugh Dickins <hugh@veritas.com>

    Looks good

    Reviewed-by: Mel Gorman <mel@csn.ul.ie>

    As I was looking there, it seemed that alloc_large_system_hash() should be
    using alloc_pages_exact() instead of having its own "give back the spare
    pages at the end of the buffer" logic. If alloc_pages_exact() was used, then
    the check for an order >= MAX_ORDER can be pushed down to alloc_pages_exact()
    where it may catch other unwary callers.

    How about adding the following patch on top of yours?

    ==== CUT HERE ====
    Use alloc_pages_exact() in alloc_large_system_hash() to avoid duplicated logic

    alloc_large_system_hash() has logic for freeing unused pages at the end
    of an power-of-two-pages-aligned buffer that is a duplicate of what is in
    alloc_pages_exact(). This patch converts alloc_large_system_hash() to use
    alloc_pages_exact().

    Signed-off-by: Mel Gorman <mel@csn.ul.ie>
    ---
    mm/page_alloc.c | 27 +++++----------------------
    1 file changed, 5 insertions(+), 22 deletions(-)

    diff --git a/mm/page_alloc.c b/mm/page_alloc.c
    index 1b3da0f..c94b140 100644
    --- a/mm/page_alloc.c
    +++ b/mm/page_alloc.c
    @@ -1942,6 +1942,9 @@ void *alloc_pages_exact(size_t size, gfp_t gfp_mask)
    unsigned int order = get_order(size);
    unsigned long addr;

    + if (order >= MAX_ORDER)
    + return NULL;
    +
    addr = __get_free_pages(gfp_mask, order);
    if (addr) {
    unsigned long alloc_end = addr + (PAGE_SIZE << order);
    @@ -4755,28 +4758,8 @@ void *__init alloc_large_system_hash(const char *tablename,
    table = alloc_bootmem_nopanic(size);
    else if (hashdist)
    table = __vmalloc(size, GFP_ATOMIC, PAGE_KERNEL);
    - else {
    - unsigned long order = get_order(size);
    -
    - if (order < MAX_ORDER)
    - table = (void *)__get_free_pages(GFP_ATOMIC,
    - order);
    - /*
    - * If bucketsize is not a power-of-two, we may free
    - * some pages at the end of hash table.
    - */
    - if (table) {
    - unsigned long alloc_end = (unsigned long)table +
    - (PAGE_SIZE << order);
    - unsigned long used = (unsigned long)table +
    - PAGE_ALIGN(size);
    - split_page(virt_to_page(table), order);
    - while (used < alloc_end) {
    - free_page(used);
    - used += PAGE_SIZE;
    - }
    - }
    - }
    + else
    + table = alloc_pages_exact(PAGE_ALIGN(size), GFP_ATOMIC);
    } while (!table && size > PAGE_SIZE && --log2qty);

    if (!table)

    \
     
     \ /
      Last update: 2009-04-30 15:29    [W:0.023 / U:6.084 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site