Re: assert/crash in __rmqueue() when enabling CONFIG_NUMA

[Dave Hansen]

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@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/