lkml.org 
[lkml]   [2006]   [May]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: assert/crash in __rmqueue() when enabling CONFIG_NUMA
    From
    Date
    I haven't thought through it completely, but these two lines worry me:

    > + start = pgdat->node_start_pfn & ~((1 << (MAX_ORDER - 1)) - 1);
    > + end = start + pgdat->node_spanned_pages;

    Should the "end" be based off of the original "start", or the aligned
    "start"?

    (using decimal math to make it easy) ...

    Let's say that MAX_ORDER comes out to be 10 pages. node_start_pfn is 9,
    and the node's end pfn is 21. node_spanned_pages will be 12. "start"
    will get rounded down to 0. "end" will be "start" (0) +
    node_spanned_pages (12), so 12. "end" then gets rounded up to 20.
    However, this is not sufficient space for the mem_map as the node
    *actually* ended at 21.

    I think that "end" needs to be calculated without rounding down the
    start_pfn, or the node_spanned_pages number needs to be rounded up in
    the same way that "end" is.

    Does that sound right?

    Also, it might look nicer if there was an intermediate variable
    something like this:

    #define MAX_ORDER_NR_PAGES (1 << (MAX_ORDER - 1))

    Take a look at the loop below, I've also used ALIGN() from kernel.h for
    the "end" alignment. I think it is just a drop-in replacement.

    /* ia64 gets its own node_mem_map, before this, without bootmem */
    if (!pgdat->node_mem_map) {
    unsigned long size, start, end;
    struct page *map;

    /*
    * The zone's endpoints aren't required to be MAX_ORDER
    * aligned but the node_mem_map endpoints must be in order
    * for the buddy allocator to function correctly.
    */
    start = pgdat->node_start_pfn & ~(MAX_ORDER_NR_PAGES - 1);
    end = start + pgdat->node_spanned_pages;
    end = ALIGN(end, MAX_ORDER_NR_PAGES);
    size = (end - start) * sizeof(struct page);
    map = alloc_remap(pgdat->node_id, size);
    if (!map)
    map = alloc_bootmem_node(pgdat, size);
    pgdat->node_mem_map = map + (pgdat->node_start_pfn - start);
    }

    -- Dave

    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2006-05-04 17:24    [W:4.943 / U:0.048 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site