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

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

[Nick Piggin]

Martin J. Bligh wrote:
>> Oh that's a 32bit kernel. I don't think the 32bit NUMA has ever worked
>> anywhere but some Summit systems (at least every time I tried it it 
>> blew up on me and nobody seems to use it regularly). Maybe it would be 
>> finally time to mark it CONFIG_BROKEN though or just remove it (even 
>> by design it doesn't work very well) 
> 
> 
> Bollocks. It works fine, and is tested every single day, on every git
> release, and every -mm tree.

Whatever the case, there definitely does not appear to be sufficient
zone alignment enforced for the buddy allocator. I cannot see how it
could work if zones are not aligned on 4MB boundaries.

Maybe some architectures / subarch code naturally does this for us,
but Ingo is definitely hitting this bug because his config does not
(align, that is).

I've randomly added a couple more cc's.

-- 
SUSE Labs, Novell Inc.
Send instant messages to your online friends http://au.messenger.yahoo.com 

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

[Bob Picco]

Nick Piggin wrote:	[Tue May 02 2006, 10:25:57AM EDT]
> Martin J. Bligh wrote:
> >>Oh that's a 32bit kernel. I don't think the 32bit NUMA has ever worked
> >>anywhere but some Summit systems (at least every time I tried it it 
> >>blew up on me and nobody seems to use it regularly). Maybe it would be 
> >>finally time to mark it CONFIG_BROKEN though or just remove it (even 
> >>by design it doesn't work very well) 
> >
> >
> >Bollocks. It works fine, and is tested every single day, on every git
> >release, and every -mm tree.
> 
> Whatever the case, there definitely does not appear to be sufficient
> zone alignment enforced for the buddy allocator. I cannot see how it
> could work if zones are not aligned on 4MB boundaries.
> 
> Maybe some architectures / subarch code naturally does this for us,
> but Ingo is definitely hitting this bug because his config does not
> (align, that is).
> 
> I've randomly added a couple more cc's.
> 
The patch below isn't compile tested or correct for those cases where
alloc_remap is called or where arch code has allocated node_mem_map for
CONFIG_FLAT_NODE_MEM_MAP. It's just conveying what I believe the issue is.

Andy added code to buddy allocator which doesn't require the zone's endpoints
to be aligned to MAX_ORDER. I think the issue is that the buddy
allocator requires the node_mem_map's endpoints to be MAX_ORDER aligned. 
Otherwise __page_find_buddy could compute a buddy not in node_mem_map
for partial MAX_ORDER regions at zone's endpoints. page_is_buddy will
detect that these pages at endpoints aren't PG_buddy (they were zeroed
out by bootmem allocator and not part of zone).  Of course the negative
here is we could waste a little memory but the positive is eliminating
all the old checks for zone boundary conditions.

SPARSEMEM won't encounter this issue because of MAX_ORDER size
constraint when SPARSEMEM is configured. ia64 VIRTUAL_MEM_MAP doesn't
need the logic either because the holes and endpoints are handled
differently.  This leaves checking alloc_remap and other arches which
privately allocate for node_mem_map.

Any how I could be totally wrong but like I said this requires more
thought.

bob


Index: linux-2.6.17-rc3/mm/page_alloc.c
===================================================================
--- linux-2.6.17-rc3.orig/mm/page_alloc.c	2006-04-27 09:44:02.000000000 -0400
+++ linux-2.6.17-rc3/mm/page_alloc.c	2006-05-03 14:50:13.000000000 -0400
@@ -2123,14 +2123,23 @@ static void __init alloc_node_mem_map(st
 #ifdef CONFIG_FLAT_NODE_MEM_MAP
 	/* ia64 gets its own node_mem_map, before this, without bootmem */
 	if (!pgdat->node_mem_map) {
-		unsigned long size;
+		unsigned long size, start, end;
 		struct page *map;
 
-		size = (pgdat->node_spanned_pages + 1) * sizeof(struct page);
+		/*
+		 * 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 & ~((1 << (MAX_ORDER - 1)) - 1);
+		end = start + pgdat->node_spanned_pages;
+		end = (end + ((1 << (MAX_ORDER - 1)) - 1) &
+			~((1 << (MAX_ORDER - 1)) - 1);
+		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_mem_map = map + ( pgdat->node_start_pfn - start);
 	}
 #ifdef CONFIG_FLATMEM
 	/*

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

[Ingo Molnar]

* Bob Picco <bob.picco@hp.com> wrote:

> The patch below isn't compile tested or correct for those cases where 
> alloc_remap is called or where arch code has allocated node_mem_map 
> for CONFIG_FLAT_NODE_MEM_MAP. It's just conveying what I believe the 
> issue is.

thx. One pair of parentheses were missing i think - see the delta fix 
below. I'll try it.

	Ingo

Index: linux/mm/page_alloc.c
===================================================================
--- linux.orig/mm/page_alloc.c
+++ linux/mm/page_alloc.c
@@ -2296,7 +2296,7 @@ static void __init alloc_node_mem_map(st
 		 */
 		start = pgdat->node_start_pfn & ~((1 << (MAX_ORDER - 1)) - 1);
 		end = start + pgdat->node_spanned_pages;
-		end = (end + ((1 << (MAX_ORDER - 1)) - 1) &
+		end = (end + ((1 << (MAX_ORDER - 1)) - 1)) &
 			~((1 << (MAX_ORDER - 1)) - 1);
 		size =  (end - start) * sizeof(struct page);
 		map = alloc_remap(pgdat->node_id, size);

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

[Ingo Molnar]

* Ingo Molnar <mingo@elte.hu> wrote:

> > The patch below isn't compile tested or correct for those cases where 
> > alloc_remap is called or where arch code has allocated node_mem_map 
> > for CONFIG_FLAT_NODE_MEM_MAP. It's just conveying what I believe the 
> > issue is.
> 
> thx. One pair of parentheses were missing i think - see the delta fix 
> below. I'll try it.

the same easy crash still happens if i enable CONFIG_NUMA:

 zone c214e600 (HighMem):
 pfn: 00037d00
 zone->zone_start_pfn: 00037e00
 zone->spanned_pages: 00007e00
 zone->zone_start_pfn + zone->spanned_pages: 0003fc00

 [<c010574a>] do_invalid_op+0x63/0x93
 [<c0104a0b>] error_code+0x4f/0x54
 [<c0164d48>] get_page_from_freelist+0x13e/0x565
 [<c01651dd>] __alloc_pages+0x6e/0x325
 [<c017a6c9>] alloc_page_vma+0x80/0x86
 [<c016e2ae>] __handle_mm_fault+0x1e7/0xd00
 [<c10fe9af>] do_page_fault+0x339/0x7c5
 [<c0104a0b>] error_code+0x4f/0x54

see the debug patch below.

	Ingo

----
From: Ingo Molnar <mingo@elte.hu>

do buddy zone size checks unconditionally.

Signed-off-by: Ingo Molnar <mingo@elte.hu>

----

 mm/page_alloc.c |   31 ++++++++++++++++++++++++-------
 1 files changed, 24 insertions(+), 7 deletions(-)

Index: linux/mm/page_alloc.c
===================================================================
--- linux.orig/mm/page_alloc.c
+++ linux/mm/page_alloc.c
@@ -101,17 +101,32 @@ static int page_outside_zone_boundaries(
 			ret = 1;
 	} while (zone_span_seqretry(zone, seq));
 
+#define P(x) printk("%s: %08lx\n", #x, x)
+
+	if (ret) {
+		printk("zone %p (%s):\n", zone, zone->name);
+		P(pfn);
+		P(zone->zone_start_pfn);
+		P(zone->spanned_pages);
+		P(zone->zone_start_pfn + zone->spanned_pages);
+	}
+
 	return ret;
 }
 
 static int page_is_consistent(struct zone *zone, struct page *page)
 {
-#ifdef CONFIG_HOLES_IN_ZONE
-	if (!pfn_valid(page_to_pfn(page)))
+	if (!pfn_valid(page_to_pfn(page))) {
+		printk("BUG: pfn: %08lx, page: %p\n",
+			page_to_pfn(page), page);
+		dump_stack();
 		return 0;
-#endif
-	if (zone != page_zone(page))
+	}
+	if (zone != page_zone(page)) {
+		printk("zone: %p != %p == page_zone(%p)\n",
+			zone, page_zone(page), page);
 		return 0;
+	}
 
 	return 1;
 }
@@ -309,10 +324,12 @@ __find_combined_index(unsigned long page
  */
 static inline int page_is_buddy(struct page *page, int order)
 {
-#ifdef CONFIG_HOLES_IN_ZONE
-	if (!pfn_valid(page_to_pfn(page)))
+	if (!pfn_valid(page_to_pfn(page))) {
+		printk("BUG: pfn: %08lx, page: %p, order: %d\n",
+			page_to_pfn(page), page, order);
+		dump_stack();
 		return 0;
-#endif
+	}
 
 	if (PageBuddy(page) && page_order(page) == order) {
 		BUG_ON(page_count(page) != 0);

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

[Ingo Molnar]

* Ingo Molnar <mingo@elte.hu> wrote:

> the same easy crash still happens if i enable CONFIG_NUMA:

btw., with CONFIG_NUMA off i get this warning during bootup:

BUG: pfn: 0003fff0, page: c404d840, order: 4
 [<c0104e7f>] show_trace+0xd/0xf
 [<c0104e96>] dump_stack+0x15/0x17
 [<c0163312>] free_pages_bulk+0x207/0x370
 [<c01642f9>] free_hot_cold_page+0x127/0x17c
 [<c016438d>] free_hot_page+0xa/0xc
 [<c01643e5>] __free_pages+0x56/0x6f
 [<c0172e14>] __vunmap+0xc1/0xed
 [<c0172f02>] vfree+0x3b/0x3e
 [<c0128865>] build_sched_domains+0xaf2/0xcde
 [<c0128a6a>] arch_init_sched_domains+0x19/0x1b
 [<c1bd3a67>] sched_init_smp+0x18/0x349
 [<c01003c6>] init+0xb9/0x2cb
 [<c0102005>] kernel_thread_helper+0x5/0xb

but this is nonfatal and the system is robust afterwards. (this warning 
is not present if CONFIG_NUMA is on) [Btw., in the NUMA test i also had 
CONFIG_MIGRATION enabled.]

	Ingo

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

[Andy Whitcroft]

Bob Picco wrote:
> Nick Piggin wrote:	[Tue May 02 2006, 10:25:57AM EDT]
> 
>>Martin J. Bligh wrote:
>>
>>>>Oh that's a 32bit kernel. I don't think the 32bit NUMA has ever worked
>>>>anywhere but some Summit systems (at least every time I tried it it 
>>>>blew up on me and nobody seems to use it regularly). Maybe it would be 
>>>>finally time to mark it CONFIG_BROKEN though or just remove it (even 
>>>>by design it doesn't work very well) 
>>>
>>>
>>>Bollocks. It works fine, and is tested every single day, on every git
>>>release, and every -mm tree.
>>
>>Whatever the case, there definitely does not appear to be sufficient
>>zone alignment enforced for the buddy allocator. I cannot see how it
>>could work if zones are not aligned on 4MB boundaries.
>>
>>Maybe some architectures / subarch code naturally does this for us,
>>but Ingo is definitely hitting this bug because his config does not
>>(align, that is).
>>
>>I've randomly added a couple more cc's.
>>
> 
> The patch below isn't compile tested or correct for those cases where
> alloc_remap is called or where arch code has allocated node_mem_map for
> CONFIG_FLAT_NODE_MEM_MAP. It's just conveying what I believe the issue is.
> 
> Andy added code to buddy allocator which doesn't require the zone's endpoints
> to be aligned to MAX_ORDER. I think the issue is that the buddy
> allocator requires the node_mem_map's endpoints to be MAX_ORDER aligned. 
> Otherwise __page_find_buddy could compute a buddy not in node_mem_map
> for partial MAX_ORDER regions at zone's endpoints. page_is_buddy will
> detect that these pages at endpoints aren't PG_buddy (they were zeroed
> out by bootmem allocator and not part of zone).  Of course the negative
> here is we could waste a little memory but the positive is eliminating
> all the old checks for zone boundary conditions.

Yes this is correct.  The buddy location algorithm uses the relative pfn
number to locate the buddy.  Both the old anew new free detect
algorithms require a struct page exist for that buddy regardless of
whether the page exists in memory.  Thus the node_mem_map needs to exist
out to a MAX_ORDER boundry in both directions.  With real machines we
would likely get this as memory is mostly in larger chunks than
MAX_ORDER and generally maximally aligned.  From what I can see we could
potentially not be allocating the end correctly but the rmap allocation
would likely be larger than the request so we'd get away with it.

> 
> SPARSEMEM won't encounter this issue because of MAX_ORDER size
> constraint when SPARSEMEM is configured. ia64 VIRTUAL_MEM_MAP doesn't
> need the logic either because the holes and endpoints are handled
> differently.  This leaves checking alloc_remap and other arches which
> privately allocate for node_mem_map.
> 
> Any how I could be totally wrong but like I said this requires more
> thought.

I'll have a go at testing this to see what difference it makes.  I think
there might be a problem with the buddy merging at zone boundries as we
are anyhow (or the code commentry is incomplete), so am going to have a
look at that at the same time.

-apw

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

[Bob Picco]

Dave Hansen wrote:	[Thu May 04 2006, 11:21:06AM EDT]
> 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"?
Yes. I failed to quilt refresh before sending. You mean end should be
end = pgdat->node_start_pfn + pgdat->node_spanned_pages before rounding
up.
> 
> (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? 
Yes.
> 
> Also, it might look nicer if there was an intermediate variable
> something like this:
> 
> 	#define MAX_ORDER_NR_PAGES (1 << (MAX_ORDER - 1))
Yes.
> 
> 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 = pgdat->node_start_pfn + pgdat->node_spanned_pages;
>                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
bob
> 

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

[Dave Hansen]

On Thu, 2006-05-04 at 11:46 -0400, Bob Picco wrote:
> Dave Hansen wrote:	[Thu May 04 2006, 11:21:06AM EDT]
> > 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"?
> Yes. I failed to quilt refresh before sending. You mean end should be
> end = pgdat->node_start_pfn + pgdat->node_spanned_pages before rounding
> up.

Yep.  Looks good.

-- Dave

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

[Ingo Molnar]

* Bob Picco <bob.picco@hp.com> wrote:

> Dave Hansen wrote:	[Thu May 04 2006, 11:21:06AM EDT]
> > 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"?
>
> Yes. I failed to quilt refresh before sending. You mean end should be 
> end = pgdat->node_start_pfn + pgdat->node_spanned_pages before 
> rounding up.

do you have an updated patch i should try?

	Ingo

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

[Bob Picco]

Ingo Molnar wrote:	[Thu May 04 2006, 03:25:28PM EDT]
> 
> * Bob Picco <bob.picco@hp.com> wrote:
> 
> > Dave Hansen wrote:	[Thu May 04 2006, 11:21:06AM EDT]
> > > 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"?
> >
> > Yes. I failed to quilt refresh before sending. You mean end should be 
> > end = pgdat->node_start_pfn + pgdat->node_spanned_pages before 
> > rounding up.
> 
> do you have an updated patch i should try?
> 
> 	Ingo
You can try this but don't believe it will change your outcome. I've
booted this on ia64 with slight modification to eliminate
VIRTUAL_MEM_MAP and have only DISCONTIGMEM. Your case is failing at the
front edge of of the zone and not the ending edge which had a flaw in my
first post of the patch. I would have expected the first patch to handle
the front edge correctly.

I don't remember seeing your .config in the thread (or blind and unable
to see it). Would you please send it my way.

I'm also hoping Andy has time to look into this.

bob


Index: linux-2.6.17-rc3/mm/page_alloc.c
===================================================================
--- linux-2.6.17-rc3.orig/mm/page_alloc.c	2006-04-27 09:44:02.000000000 -0400
+++ linux-2.6.17-rc3/mm/page_alloc.c	2006-05-04 13:01:25.000000000 -0400
@@ -2123,14 +2123,22 @@ static void __init alloc_node_mem_map(st
 #ifdef CONFIG_FLAT_NODE_MEM_MAP
 	/* ia64 gets its own node_mem_map, before this, without bootmem */
 	if (!pgdat->node_mem_map) {
-		unsigned long size;
+		unsigned long size, start, end;
 		struct page *map;
 
-		size = (pgdat->node_spanned_pages + 1) * sizeof(struct page);
+		/*
+		 * 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 = pgdat->node_start_pfn + 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_mem_map = map + (pgdat->node_start_pfn - start);
 	}
 #ifdef CONFIG_FLATMEM
 	/*
Index: linux-2.6.17-rc3/include/linux/mmzone.h
===================================================================
--- linux-2.6.17-rc3.orig/include/linux/mmzone.h	2006-04-27 09:44:02.000000000 -0400
+++ linux-2.6.17-rc3/include/linux/mmzone.h	2006-05-04 13:01:39.000000000 -0400
@@ -22,6 +22,7 @@
 #else
 #define MAX_ORDER CONFIG_FORCE_MAX_ZONEORDER
 #endif
+#define MAX_ORDER_NR_PAGES (1 << (MAX_ORDER - 1))
 
 struct free_area {
 	struct list_head	free_list;

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

[Andy Whitcroft]

Bob Picco wrote:
> Ingo Molnar wrote:	[Thu May 04 2006, 03:25:28PM EDT]
> 
>>* Bob Picco <bob.picco@hp.com> wrote:
>>
>>
>>>Dave Hansen wrote:	[Thu May 04 2006, 11:21:06AM EDT]
>>>
>>>>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"?
>>>
>>>Yes. I failed to quilt refresh before sending. You mean end should be 
>>>end = pgdat->node_start_pfn + pgdat->node_spanned_pages before 
>>>rounding up.
>>
>>do you have an updated patch i should try?
>>
>>	Ingo
> 
> You can try this but don't believe it will change your outcome. I've
> booted this on ia64 with slight modification to eliminate
> VIRTUAL_MEM_MAP and have only DISCONTIGMEM. Your case is failing at the
> front edge of of the zone and not the ending edge which had a flaw in my
> first post of the patch. I would have expected the first patch to handle
> the front edge correctly.
> 
> I don't remember seeing your .config in the thread (or blind and unable
> to see it). Would you please send it my way.
> 
> I'm also hoping Andy has time to look into this.
> 
> bob

Yeah will have a look tommorrow my time.  Could you drop me the .config
too.  There is definatly some unstated requirements on alignment, which
I was testing today.  I presume its one of those thats being violated.

-apw

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

[Ingo Molnar]

* Andy Whitcroft <apw@shadowen.org> wrote:

> Yeah will have a look tommorrow my time.  Could you drop me the 
> .config too.  There is definatly some unstated requirements on 
> alignment, which I was testing today.  I presume its one of those 
> thats being violated.

the config is at:

 http://redhat.com/~mingo/misc/config

the bootlog is at:

 http://redhat.com/~mingo/misc/crash.log

	Ingo

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

[Bob Picco]

Andy Wihitcroft wrote:	[Thu May 04 2006, 05:50:29PM EDT]
> Bob Picco wrote:
> > Ingo Molnar wrote:	[Thu May 04 2006, 03:25:28PM EDT]
> > 
> >>* Bob Picco <bob.picco@hp.com> wrote:
> >>
> >>
> >>>Dave Hansen wrote:	[Thu May 04 2006, 11:21:06AM EDT]
> >>>
> >>>>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"?
> >>>
> >>>Yes. I failed to quilt refresh before sending. You mean end should be 
> >>>end = pgdat->node_start_pfn + pgdat->node_spanned_pages before 
> >>>rounding up.
> >>
> >>do you have an updated patch i should try?
> >>
> >>	Ingo
> > 
> > You can try this but don't believe it will change your outcome. I've
> > booted this on ia64 with slight modification to eliminate
> > VIRTUAL_MEM_MAP and have only DISCONTIGMEM. Your case is failing at the
> > front edge of of the zone and not the ending edge which had a flaw in my
> > first post of the patch. I would have expected the first patch to handle
> > the front edge correctly.
> > 
> > I don't remember seeing your .config in the thread (or blind and unable
> > to see it). Would you please send it my way.
> > 
> > I'm also hoping Andy has time to look into this.
> > 
> > bob
> 
> Yeah will have a look tommorrow my time.  Could you drop me the .config
> too.  There is definatly some unstated requirements on alignment, which
> I was testing today.  I presume its one of those thats being violated.
> 
> -apw
I think the problem was my not looking closely at the full email thread. 
I finally found time to read entire thread (found Ingo's config and boot logs). The patch below should fix Ingo's problem.  It's probably only required for 
ZONE_HIGHMEM.  To be safe, I think we should apply it generically. 

Not only must node_mem_map array be MAX_ORDER aligned but the the distance 
between interior zones covered by node_mem_map must satisfy this alignment. 
While in the buddy allocator before checking for a valid buddy the buddy page 
must reside in the parent's zone too. ZONE_HIGHMEM doesn't satisfy the zone 
alignment condition and requires this new check that the parent's buddy and 
parent are within by the same zone.

The other possible solution is aligning HIGHMEM zone to satisfy MAX_ORDER.
This I didn't pursue and possibly is what Andy refers to above.

Adding a printk for the line with the zonenum mismatch condition caught two
instances in boot up on my x86 which was configured similarly to Ingo's config.

bob

Index: linux-2.6.17-rc3/mm/page_alloc.c
===================================================================
--- linux-2.6.17-rc3.orig/mm/page_alloc.c	2006-04-27 09:44:02.000000000 -0400
+++ linux-2.6.17-rc3/mm/page_alloc.c	2006-05-05 07:42:40.000000000 -0400
@@ -280,6 +280,15 @@ __find_combined_index(unsigned long page
 	return (page_idx & ~(1 << order));
 }
 
+static inline int page_in_zone_hole(struct page *page)
+{
+#ifdef CONFIG_HOLES_IN_ZONE
+	if (!pfn_valid(page_to_pfn(page)))
+		return 1;
+#endif
+	return 0;
+}
+
 /*
  * This function checks whether a page is free && is the buddy
  * we can do coalesce a page and its buddy if
@@ -294,11 +303,6 @@ __find_combined_index(unsigned long page
  */
 static inline int page_is_buddy(struct page *page, int order)
 {
-#ifdef CONFIG_HOLES_IN_ZONE
-	if (!pfn_valid(page_to_pfn(page)))
-		return 0;
-#endif
-
 	if (PageBuddy(page) && page_order(page) == order) {
 		BUG_ON(page_count(page) != 0);
 		return 1;
@@ -351,7 +355,11 @@ static inline void __free_one_page(struc
 		struct page *buddy;
 
 		buddy = __page_find_buddy(page, page_idx, order);
-		if (!page_is_buddy(buddy, order))
+		if (page_in_zone_hole(buddy))
+			break;
+		else if (page_zonenum(buddy) != page_zonenum(page))
+			break;
+		else if (!page_is_buddy(buddy, order))
 			break;		/* Move the buddy up one level. */
 
 		list_del(&buddy->lru);
@@ -2123,14 +2131,22 @@ static void __init alloc_node_mem_map(st
 #ifdef CONFIG_FLAT_NODE_MEM_MAP
 	/* ia64 gets its own node_mem_map, before this, without bootmem */
 	if (!pgdat->node_mem_map) {
-		unsigned long size;
+		unsigned long size, start, end;
 		struct page *map;
 
-		size = (pgdat->node_spanned_pages + 1) * sizeof(struct page);
+		/*
+		 * 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 = pgdat->node_start_pfn + 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_mem_map = map + (pgdat->node_start_pfn - start);
 	}
 #ifdef CONFIG_FLATMEM
 	/*
Index: linux-2.6.17-rc3/include/linux/mmzone.h
===================================================================
--- linux-2.6.17-rc3.orig/include/linux/mmzone.h	2006-04-27 09:44:02.000000000 -0400
+++ linux-2.6.17-rc3/include/linux/mmzone.h	2006-05-04 13:01:39.000000000 -0400
@@ -22,6 +22,7 @@
 #else
 #define MAX_ORDER CONFIG_FORCE_MAX_ZONEORDER
 #endif
+#define MAX_ORDER_NR_PAGES (1 << (MAX_ORDER - 1))
 
 struct free_area {
 	struct list_head	free_list;

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

[Dave Hansen]

On Fri, 2006-05-05 at 09:55 -0400, Bob Picco wrote:
> -               if (!page_is_buddy(buddy, order))
> +               if (page_in_zone_hole(buddy))
> +                       break;
> +               else if (page_zonenum(buddy) != page_zonenum(page))
> +                       break;
> +               else if (!page_is_buddy(buddy, order))
>                         break;          /* Move the buddy up one level. */ 

The page_zonenum() checks look good, but I'm not sure I understand the
page_in_zone_hole() part.  If a page is in a hole in a zone, it will
still have a valid mem_map entry, right?  It should also never have been
put into the allocator, so it also won't ever be coalesced.  

I'm a bit confused. :(

BTW, I like the idea of just aligning HIGHMEM's start because it has no
runtime cost.  Buuuuut, it is still just a shift and compare of the two
page->flags, which should already be (or will soon anyway be) in the
cache.

-- Dave

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

[Bob Picco]

Dave Hansen wrote:	[Fri May 05 2006, 10:33:10AM EDT]
> On Fri, 2006-05-05 at 09:55 -0400, Bob Picco wrote:
> > -               if (!page_is_buddy(buddy, order))
> > +               if (page_in_zone_hole(buddy))
> > +                       break;
> > +               else if (page_zonenum(buddy) != page_zonenum(page))
> > +                       break;
> > +               else if (!page_is_buddy(buddy, order))
> >                         break;          /* Move the buddy up one level. */ 
> 
> The page_zonenum() checks look good, but I'm not sure I understand the
> page_in_zone_hole() part.  If a page is in a hole in a zone, it will
> still have a valid mem_map entry, right?  It should also never have been
> put into the allocator, so it also won't ever be coalesced.  
This has always been subtle and not too revealing.  It probably should
have a comment. The page_in_zone_hole check is for ia64 
VIRTUAL_MEM_MAP. You might compute a page structure which is in a hole not 
backed by memory; an unallocated page which covers pages structures. 
VIRTUAL_MEM_MAP uses a contiguous virtual region with virtual space holes
not backed by memory. Take a look at ia64_pfn_valid.
> 
> I'm a bit confused. :(
> 
> BTW, I like the idea of just aligning HIGHMEM's start because it has no
> runtime cost.  Buuuuut, it is still just a shift and compare of the two
> page->flags, which should already be (or will soon anyway be) in the
> cache.
Yes. I'll defer to Andy whether he wants the zonenum check or to align
HIGHMEM corrrectly.
> 
> -- Dave
> 
bob

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

[Dave Hansen]

On Fri, 2006-05-05 at 10:50 -0400, Bob Picco wrote:
> Dave Hansen wrote:	[Fri May 05 2006, 10:33:10AM EDT]
> > The page_zonenum() checks look good, but I'm not sure I understand the
> > page_in_zone_hole() part.  If a page is in a hole in a zone, it will
> > still have a valid mem_map entry, right?  It should also never have been
> > put into the allocator, so it also won't ever be coalesced.  
> This has always been subtle and not too revealing.  It probably should
> have a comment. The page_in_zone_hole check is for ia64 
> VIRTUAL_MEM_MAP. You might compute a page structure which is in a hole not 
> backed by memory; an unallocated page which covers pages structures. 
> VIRTUAL_MEM_MAP uses a contiguous virtual region with virtual space holes
> not backed by memory. Take a look at ia64_pfn_valid.

Ahhh.  I hadn't made the ia64 connection.  I wonder if it is worth
making CONFIG_HOLES_IN_ZONE say ia64 or something about vmem_map in it
somewhere.  Might be worth at least a comment like this:

+               if (page_in_zone_hole(buddy)) /* noop on all but ia64 */
+                       break;
+               else if (page_zonenum(buddy) != page_zonenum(page))
+                       break;
+               else if (!page_is_buddy(buddy, order))
                        break;          /* Move the buddy up one level. */

BTW, wasn't the whole idea of discontig to have holes in zones (before
NUMA) without tricks like this? ;)

-- Dave

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

[Martin J. Bligh]

> Ahhh.  I hadn't made the ia64 connection.  I wonder if it is worth
> making CONFIG_HOLES_IN_ZONE say ia64 or something about vmem_map in it
> somewhere.  Might be worth at least a comment like this:
> 
> +               if (page_in_zone_hole(buddy)) /* noop on all but ia64 */
> +                       break;
> +               else if (page_zonenum(buddy) != page_zonenum(page))
> +                       break;
> +               else if (!page_is_buddy(buddy, order))
>                         break;          /* Move the buddy up one level. */
> 
> BTW, wasn't the whole idea of discontig to have holes in zones (before
> NUMA) without tricks like this? ;)

Sparsemem should fix this - that was one of the things Andy designed it
for. Then we can remove the virtual memmap stuff (and discontig).
Indeed, I'd hope we're ready to do that real soon now ... has anyone
got an ia64 box that needed virtual memmap that they could test this
on?

M.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

[Bob Picco]

Martin J. Bligh wrote:	[Fri May 05 2006, 11:03:02AM EDT]
> >Ahhh.  I hadn't made the ia64 connection.  I wonder if it is worth
> >making CONFIG_HOLES_IN_ZONE say ia64 or something about vmem_map in it
> >somewhere.  Might be worth at least a comment like this:
> >
> >+               if (page_in_zone_hole(buddy)) /* noop on all but ia64 */
> >+                       break;
> >+               else if (page_zonenum(buddy) != page_zonenum(page))
> >+                       break;
> >+               else if (!page_is_buddy(buddy, order))
> >                        break;          /* Move the buddy up one level. */
> >
> >BTW, wasn't the whole idea of discontig to have holes in zones (before
> >NUMA) without tricks like this? ;)
> 
> Sparsemem should fix this - that was one of the things Andy designed it
> for. Then we can remove the virtual memmap stuff (and discontig).
> Indeed, I'd hope we're ready to do that real soon now ... has anyone
> got an ia64 box that needed virtual memmap that they could test this
> on?
> 
> M.
I totally agree about SPARSEMEM. I believe most ia64 boxes use
VIRTUAL_MEM_MAP. I only know of Fujitsu and myself that use SPARSEMEM
for ia64 (perhaps Andy too in his testing). Dave and I have advocated its use
more than once.

bob

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

[Bob Picco]

Dave Hansen wrote:	[Fri May 05 2006, 10:57:44AM EDT]
> On Fri, 2006-05-05 at 10:50 -0400, Bob Picco wrote:
> > Dave Hansen wrote:	[Fri May 05 2006, 10:33:10AM EDT]
> > > The page_zonenum() checks look good, but I'm not sure I understand the
> > > page_in_zone_hole() part.  If a page is in a hole in a zone, it will
> > > still have a valid mem_map entry, right?  It should also never have been
> > > put into the allocator, so it also won't ever be coalesced.  
> > This has always been subtle and not too revealing.  It probably should
> > have a comment. The page_in_zone_hole check is for ia64 
> > VIRTUAL_MEM_MAP. You might compute a page structure which is in a hole not 
> > backed by memory; an unallocated page which covers pages structures. 
> > VIRTUAL_MEM_MAP uses a contiguous virtual region with virtual space holes
> > not backed by memory. Take a look at ia64_pfn_valid.
> 
> Ahhh.  I hadn't made the ia64 connection.  I wonder if it is worth
> making CONFIG_HOLES_IN_ZONE say ia64 or something about vmem_map in it
> somewhere.  Might be worth at least a comment like this:
> 
> +               if (page_in_zone_hole(buddy)) /* noop on all but ia64 */
> +                       break;
> +               else if (page_zonenum(buddy) != page_zonenum(page))
> +                       break;
> +               else if (!page_is_buddy(buddy, order))
>                         break;          /* Move the buddy up one level. */
> 
> BTW, wasn't the whole idea of discontig to have holes in zones (before
> NUMA) without tricks like this? ;)
Sure you could boot ia64 with just DISCONTIGMEM and no VIRTUAL_MEM_MAP.
In fact that's exactly what I did to test code added in alloc_node_mem_map.
Unfortunately I was missing 1Gb from free memory after booting. The
missing 1Gb was consumed by reserved pages structures :)
> 
> -- Dave
> 
bob

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

[Nick Piggin]

Dave Hansen wrote:

> Ahhh.  I hadn't made the ia64 connection.  I wonder if it is worth
> making CONFIG_HOLES_IN_ZONE say ia64 or something about vmem_map in it
> somewhere.  Might be worth at least a comment like this:
> 
> +               if (page_in_zone_hole(buddy)) /* noop on all but ia64 */
> +                       break;
> +               else if (page_zonenum(buddy) != page_zonenum(page))
> +                       break;
> +               else if (!page_is_buddy(buddy, order))
>                         break;          /* Move the buddy up one level. */
> 
> BTW, wasn't the whole idea of discontig to have holes in zones (before
> NUMA) without tricks like this? ;)

Yes.

I don't like the patch much, because all that logic should be moved
into page_is_buddy where I put it (surely it is more readable not to
have the checks spilling out -- a page which is not in the correct
zone or is a "hole" is by definition not a buddy, right?)

So, I agree with adding the zone check if any architecture needs it.
But it would be something under CONFIG_HOLES_IN_ZONE, and the arch
needs to *either* align zones correctly (as they've always had to),
or turn this option on.

Thanks for working at this, everyone.

-- 
SUSE Labs, Novell Inc.
Send instant messages to your online friends http://au.messenger.yahoo.com 

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

[Andy Whitcroft]

Nick Piggin wrote:
> Dave Hansen wrote:
> 
>> Ahhh.  I hadn't made the ia64 connection.  I wonder if it is worth
>> making CONFIG_HOLES_IN_ZONE say ia64 or something about vmem_map in it
>> somewhere.  Might be worth at least a comment like this:
>>
>> +               if (page_in_zone_hole(buddy)) /* noop on all but ia64 */
>> +                       break;
>> +               else if (page_zonenum(buddy) != page_zonenum(page))
>> +                       break;
>> +               else if (!page_is_buddy(buddy, order))
>>                         break;          /* Move the buddy up one
>> level. */
>>
>> BTW, wasn't the whole idea of discontig to have holes in zones (before
>> NUMA) without tricks like this? ;)
> 
> 
> Yes.
> 
> I don't like the patch much, because all that logic should be moved
> into page_is_buddy where I put it (surely it is more readable not to
> have the checks spilling out -- a page which is not in the correct
> zone or is a "hole" is by definition not a buddy, right?)
> 
> So, I agree with adding the zone check if any architecture needs it.
> But it would be something under CONFIG_HOLES_IN_ZONE, and the arch
> needs to *either* align zones correctly (as they've always had to),
> or turn this option on.

I agree that there is no need for these checks to leak out of
page_is_buddy().  If its not there or in another zone, its not my buddy.
 The allocator loop is nasty enough as it is.

I think we need to do a couple of things:

1) check the alignment of the zones matches the implied alignment
constraints and correct it as we go.
2) optionally allow an architecture to say its not aligning and doesn't
want to have to align its zone -- providing a config option to add the
zone index checks

I think the later is valuable for these test builds and potentially for
the embedded side where megabytes mean something.

I'm testing a patch for this at the moment and will drop it out when I'm
done.

-apw

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

[Nick Piggin]

Andy Whitcroft wrote:

> I agree that there is no need for these checks to leak out of
> page_is_buddy().  If its not there or in another zone, its not my buddy.
>  The allocator loop is nasty enough as it is.

OK, glad you agree.

> 
> I think we need to do a couple of things:
> 
> 1) check the alignment of the zones matches the implied alignment
> constraints and correct it as we go.

Yes. And preferably have checks in the generic page allocator setup
code, so we can do something sane if the arch code gets it wrong.

> 2) optionally allow an architecture to say its not aligning and doesn't
> want to have to align its zone -- providing a config option to add the
> zone index checks
> 
> I think the later is valuable for these test builds and potentially for
> the embedded side where megabytes mean something.

Yes. Depends whether we fold it under the HOLES_IN_ZONE config. I guess
HOLES_IN_ZONE is potentially quite a bit more expensive than the plain
zone check, so having 2 config options may not be unreasonable.

Also, if the architecture doesn't align the ends of zones, *and* they are
not adjacent to another zone, they need either CONFIG_HOLES_IN_ZONE or
they need to provide dummy 'struct page's that never have PageBuddy set.


> 
> I'm testing a patch for this at the moment and will drop it out when I'm
> done.

Great!

-- 
SUSE Labs, Novell Inc.
Send instant messages to your online friends http://au.messenger.yahoo.com 

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH 0/3] Zone boundry alignment fixes

[Andy Whitcroft]

Ok.  Finally got my test bed working and got this lot tested.

To summarise the problem , the buddy allocator currently requires
that the boundries between zones occur at MAX_ORDER boundries.
The specific case where we were tripping up on this was in x86 with
NUMA enabled.  There we try to ensure that each node's stuct pages
are in node local memory, in order to allow them to be virtually
mapped we have to reduce the size of ZONE_NORMAL.  Here we are
rounding the remap space up to a large page size to allow large
page TLB entries to be used.  However, these are smaller than
MAX_ORDER.  This can lead to bad buddy merges.  With VM_DEBUG enabled
we detect the attempts to merge across this boundry and panic.

We have two basic options we can either apply the appropriate
alignment when we make make the NUMA remap space, or we can 'fix'
the assumption in the buddy allocator.  The fix for the buddy
allocator involves adding conditionals to the free fast path and
so it seems reasonable to at least favor realigning the remap space.

Following this email are 3 patches:

zone-init-check-and-report-unaligned-zone-boundries -- introduces
  a zone alignement helper, and uses it to add a check to zone
  initialisation for unaligned zone boundries,

x86-align-highmem-zone-boundries-with-NUMA -- uses the zone alignment
  helper to align the end of ZONE_NORMAL after the remap space has
  been reserved, and

zone-allow-unaligned-zone-boundries -- modifies the buddy allocator
  so that we can allow unaligned zone boundries.  A new configuration
  option is added to enable this functionality.

The first two are the fixes for alignement in x86, these fix the
panics thrown when VM_DEBUG is enabled.

The last is a patch to support unaligned zone boundries.  As this
(re)introduces a zone check into the free hot path it seems
reasonable to only enable this should it be needed; for example
we never need this if we have a single zone.  I have tested the
failing system with this patch enabled and it also fixes the panic.
I am inclined to suggest that it be included as it very clearly
documents the alignment requirements for the buddy allocator.

Comments.

-apw

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH 1/3] zone init check and report unaligned zone boundries

[Andy Whitcroft]

zone init check and report unaligned zone boundries

We have a number of strict constraints on the layout of struct
page's for use with the buddy allocator.  One of which is that zone
boundries must occur at MAX_ORDER page boundries.  Add a check for
this during init.

Signed-off-by: Andy Whitcroft <apw@shadowen.org>
---
 include/linux/mmzone.h |    5 +++++
 mm/page_alloc.c        |    4 ++++
 2 files changed, 9 insertions(+)
diff -upN reference/include/linux/mmzone.h current/include/linux/mmzone.h
--- reference/include/linux/mmzone.h
+++ current/include/linux/mmzone.h
@@ -388,6 +388,11 @@ static inline int is_dma(struct zone *zo
 	return zone == zone->zone_pgdat->node_zones + ZONE_DMA;
 }
 
+static inline unsigned long zone_boundry_align_pfn(unsigned long pfn)
+{
+	return pfn & ~((1 << MAX_ORDER) - 1);
+}
+
 /* These two functions are used to setup the per zone pages min values */
 struct ctl_table;
 struct file;
diff -upN reference/mm/page_alloc.c current/mm/page_alloc.c
--- reference/mm/page_alloc.c
+++ current/mm/page_alloc.c
@@ -2078,6 +2078,10 @@ static void __init free_area_init_core(s
 		struct zone *zone = pgdat->node_zones + j;
 		unsigned long size, realsize;
 
+		if (zone_boundry_align_pfn(zone_start_pfn) != zone_start_pfn)
+			printk(KERN_CRIT "node %d zone %s missaligned "
+					"start pfn\n", nid, zone_names[j]);
+
 		realsize = size = zones_size[j];
 		if (zholes_size)
 			realsize -= zholes_size[j];

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 1/3] zone init check and report unaligned zone boundries

[Nick Piggin]

Patchset looks nice, Andy. Page allocator stuff looks fine, just
a couple of questions about the setup and alignment stuff:

Andy Whitcroft wrote:
> zone init check and report unaligned zone boundries
> 
> We have a number of strict constraints on the layout of struct
> page's for use with the buddy allocator.  One of which is that zone
> boundries must occur at MAX_ORDER page boundries.  Add a check for
> this during init.
> 
> Signed-off-by: Andy Whitcroft <apw@shadowen.org>
> ---
>  include/linux/mmzone.h |    5 +++++
>  mm/page_alloc.c        |    4 ++++
>  2 files changed, 9 insertions(+)
> diff -upN reference/include/linux/mmzone.h current/include/linux/mmzone.h
> --- reference/include/linux/mmzone.h
> +++ current/include/linux/mmzone.h
> @@ -388,6 +388,11 @@ static inline int is_dma(struct zone *zo
>  	return zone == zone->zone_pgdat->node_zones + ZONE_DMA;
>  }
>  
> +static inline unsigned long zone_boundry_align_pfn(unsigned long pfn)
> +{
> +	return pfn & ~((1 << MAX_ORDER) - 1);
> +}
> +
>  /* These two functions are used to setup the per zone pages min values */
>  struct ctl_table;
>  struct file;
> diff -upN reference/mm/page_alloc.c current/mm/page_alloc.c
> --- reference/mm/page_alloc.c
> +++ current/mm/page_alloc.c
> @@ -2078,6 +2078,10 @@ static void __init free_area_init_core(s
>  		struct zone *zone = pgdat->node_zones + j;
>  		unsigned long size, realsize;
>  
> +		if (zone_boundry_align_pfn(zone_start_pfn) != zone_start_pfn)
> +			printk(KERN_CRIT "node %d zone %s missaligned "
> +					"start pfn\n", nid, zone_names[j]);
> +


We also need to align the end of the zone I think? I think we should
try to force alignment by reducing the size here, or if that is not
possible, then panic?

If we don't either fix it or panic, then we're allowing users' machines
to run in an unstable state.

-- 
SUSE Labs, Novell Inc.
Send instant messages to your online friends http://au.messenger.yahoo.com 

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH 2/3] x86 align highmem zone boundries with NUMA

[Andy Whitcroft]

x86 align highmem zone boundries with NUMA

When in x86 NUMA mode we allocate struct pages's node local and map
them into the kernel virtual address space in the remap space.
This space cuts into the end of ZONE_NORMAL.  When we round
ZONE_NORMAL down we must ensure we maintain the zone boundry
constraint, we must round to MAX_ORDER.

Signed-off-by: Andy Whitcroft <apw@shadowen.org>
---
 discontig.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletion(-)
diff -upN reference/arch/i386/mm/discontig.c current/arch/i386/mm/discontig.c
--- reference/arch/i386/mm/discontig.c
+++ current/arch/i386/mm/discontig.c
@@ -304,10 +304,13 @@ unsigned long __init setup_memory(void)
 	/* partially used pages are not usable - thus round upwards */
 	system_start_pfn = min_low_pfn = PFN_UP(init_pg_tables_end);
 
-	system_max_low_pfn = max_low_pfn = find_max_low_pfn() - reserve_pages;
+	max_low_pfn = find_max_low_pfn() - reserve_pages;
 	printk("reserve_pages = %ld find_max_low_pfn() ~ %ld\n",
 			reserve_pages, max_low_pfn + reserve_pages);
 	printk("max_pfn = %ld\n", max_pfn);
+
+	system_max_low_pfn = max_low_pfn = zone_boundry_align_pfn(max_low_pfn);
+
 #ifdef CONFIG_HIGHMEM
 	highstart_pfn = highend_pfn = max_pfn;
 	if (max_pfn > system_max_low_pfn)

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH 3/3] zone allow unaligned zone boundries

[Andy Whitcroft]

zone allow unaligned zone boundries

Currently the buddy allocator requires that zone boundries be
at MAX_ORDER boundries.  This may not always be desirable or
possible.  Add a config option to allow these boundies
to be arbitrary.

Signed-off-by: Andy Whitcroft <apw@shadowen.org>
---
 include/linux/mm.h     |    7 +++++--
 include/linux/mmzone.h |    4 ++++
 mm/page_alloc.c        |   21 ++++++++++++++-------
 3 files changed, 23 insertions(+), 9 deletions(-)
diff -upN reference/include/linux/mm.h current/include/linux/mm.h
--- reference/include/linux/mm.h
+++ current/include/linux/mm.h
@@ -466,10 +466,13 @@ static inline unsigned long page_zonenum
 struct zone;
 extern struct zone *zone_table[];
 
+static inline int page_zone_id(struct page *page)
+{
+	return (page->flags >> ZONETABLE_PGSHIFT) & ZONETABLE_MASK;
+}
 static inline struct zone *page_zone(struct page *page)
 {
-	return zone_table[(page->flags >> ZONETABLE_PGSHIFT) &
-			ZONETABLE_MASK];
+	return zone_table[page_zone_id(page)];
 }
 
 static inline unsigned long page_to_nid(struct page *page)
diff -upN reference/include/linux/mmzone.h current/include/linux/mmzone.h
--- reference/include/linux/mmzone.h
+++ current/include/linux/mmzone.h
@@ -390,7 +390,11 @@ static inline int is_dma(struct zone *zo
 
 static inline unsigned long zone_boundry_align_pfn(unsigned long pfn)
 {
+#ifdef CONFIG_UNALIGNED_ZONE_BOUNDRIES
+	return pfn;
+#else
 	return pfn & ~((1 << MAX_ORDER) - 1);
+#endif
 }
 
 /* These two functions are used to setup the per zone pages min values */
diff -upN reference/mm/page_alloc.c current/mm/page_alloc.c
--- reference/mm/page_alloc.c
+++ current/mm/page_alloc.c
@@ -285,22 +285,28 @@ __find_combined_index(unsigned long page
  * we can do coalesce a page and its buddy if
  * (a) the buddy is not in a hole &&
  * (b) the buddy is in the buddy system &&
- * (c) a page and its buddy have the same order.
+ * (c) a page and its buddy have the same order &&
+ * (d) a page and its buddy are in the same zone.
  *
  * For recording whether a page is in the buddy system, we use PG_buddy.
  * Setting, clearing, and testing PG_buddy is serialized by zone->lock.
  *
  * For recording page's order, we use page_private(page).
  */
-static inline int page_is_buddy(struct page *page, int order)
+static inline int page_is_buddy(struct page *page, struct page *buddy,
+								int order)
 {
 #ifdef CONFIG_HOLES_IN_ZONE
-	if (!pfn_valid(page_to_pfn(page)))
+	if (!pfn_valid(page_to_pfn(buddy)))
+		return 0;
+#endif
+#ifdef CONFIG_UNALIGNED_ZONE_BOUNDRIES
+	if (page_zone_id(page) != page_zone_id(buddy))
 		return 0;
 #endif
 
-	if (PageBuddy(page) && page_order(page) == order) {
-		BUG_ON(page_count(page) != 0);
+	if (PageBuddy(buddy) && page_order(buddy) == order) {
+		BUG_ON(page_count(buddy) != 0);
 		return 1;
 	}
 	return 0;
@@ -351,7 +357,7 @@ static inline void __free_one_page(struc
 		struct page *buddy;
 
 		buddy = __page_find_buddy(page, page_idx, order);
-		if (!page_is_buddy(buddy, order))
+		if (!page_is_buddy(page, buddy, order))
 			break;		/* Move the buddy up one level. */
 
 		list_del(&buddy->lru);
@@ -2080,7 +2086,8 @@ static void __init free_area_init_core(s
 
 		if (zone_boundry_align_pfn(zone_start_pfn) != zone_start_pfn)
 			printk(KERN_CRIT "node %d zone %s missaligned "
-					"start pfn\n", nid, zone_names[j]);
+				"start pfn, enable UNALIGNED_ZONE_BOUNDRIES\n",
+							nid, zone_names[j]);
 
 		realsize = size = zones_size[j];
 		if (zholes_size)

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 0/3] Zone boundry alignment fixes

[Andrew Morton]

Andy Whitcroft <apw@shadowen.org> wrote:
>
> Ok.  Finally got my test bed working and got this lot tested.
> 
> To summarise the problem , the buddy allocator currently requires
> that the boundries between zones occur at MAX_ORDER boundries.
> The specific case where we were tripping up on this was in x86 with
> NUMA enabled.  There we try to ensure that each node's stuct pages
> are in node local memory, in order to allow them to be virtually
> mapped we have to reduce the size of ZONE_NORMAL.  Here we are
> rounding the remap space up to a large page size to allow large
> page TLB entries to be used.  However, these are smaller than
> MAX_ORDER.  This can lead to bad buddy merges.  With VM_DEBUG enabled
> we detect the attempts to merge across this boundry and panic.
> 
> We have two basic options we can either apply the appropriate
> alignment when we make make the NUMA remap space, or we can 'fix'
> the assumption in the buddy allocator.  The fix for the buddy
> allocator involves adding conditionals to the free fast path and
> so it seems reasonable to at least favor realigning the remap space.
> 
> Following this email are 3 patches:
> 
> zone-init-check-and-report-unaligned-zone-boundries -- introduces
>   a zone alignement helper, and uses it to add a check to zone
>   initialisation for unaligned zone boundries,
> 
> x86-align-highmem-zone-boundries-with-NUMA -- uses the zone alignment
>   helper to align the end of ZONE_NORMAL after the remap space has
>   been reserved, and
> 
> zone-allow-unaligned-zone-boundries -- modifies the buddy allocator
>   so that we can allow unaligned zone boundries.  A new configuration
>   option is added to enable this functionality.
> 
> The first two are the fixes for alignement in x86, these fix the
> panics thrown when VM_DEBUG is enabled.
> 
> The last is a patch to support unaligned zone boundries.  As this
> (re)introduces a zone check into the free hot path it seems
> reasonable to only enable this should it be needed; for example
> we never need this if we have a single zone.  I have tested the
> failing system with this patch enabled and it also fixes the panic.
> I am inclined to suggest that it be included as it very clearly
> documents the alignment requirements for the buddy allocator.

There's some possibility here of interaction with Mel's "patchset to size
zones and memory holes in an architecture-independent manner." I jammed
them together - let's see how it goes.

I also fixed the spelling of "boundary" in about 1.5 zillion places ;)

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 0/3] Zone boundry alignment fixes

[Ingo Molnar]

* Andrew Morton <akpm@osdl.org> wrote:

> There's some possibility here of interaction with Mel's "patchset to 
> size zones and memory holes in an architecture-independent manner." I 
> jammed them together - let's see how it goes.

update: Andy's 3 patches, applied to 2.6.17-rc3-mm1, fixed all the 
crashes and asserts i saw. NUMA-on-x86 is now rock-solid on my testbox. 
Great work Andy!

	Ingo

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 0/3] Zone boundry alignment fixes

[Nick Piggin]

Ingo Molnar wrote:
> * Andrew Morton <akpm@osdl.org> wrote:
> 
> 
>>There's some possibility here of interaction with Mel's "patchset to 
>>size zones and memory holes in an architecture-independent manner." I 
>>jammed them together - let's see how it goes.
> 
> 
> update: Andy's 3 patches, applied to 2.6.17-rc3-mm1, fixed all the 
> crashes and asserts i saw. NUMA-on-x86 is now rock-solid on my testbox. 
> Great work Andy!

Excellent. I think these should get into 2.6.17, and possibly even the
-stable series.

-- 
SUSE Labs, Novell Inc.
Send instant messages to your online friends http://au.messenger.yahoo.com 

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH 0/2] Zone boundary alignment fixes cleanups

[Andy Whitcroft]

[Sorry for the delay, we've been busy looking to see what is
responsible for the ia64 issues with architecture independant
zone sizing.]

Following this email are two cleanup patches for the
UNALIGNED_ZONE_BOUNDARIES support in -mm.

zone-init-check-and-report-unaligned-zone-boundaries-fix --
  we currently will pointlessly report zones as missaligned even
  though they are empty and will report the first zone which can
  never be missaligned assuming node_mem_map is aligned correctly.

zone-allow-unaligned-zone-boundaries-spelling-fix -- when the
  spelling errors in zone-allow-unaligned-zone-boundaries-spelling
  were fixed the configuration options were not updated.

Both of the above patches slot into the linux-2.6.17-rc4-mm1 patch
set next to their main patches.  Amazingly, they will also apply
on top of linux-2.6.17-rc4-mm1, I don't know what patch has been
taking but it rocks.

-apw

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH 1/2] zone init check and report unaligned zone boundaries fix

[Andy Whitcroft]

zone init check and report unaligned zone boundaries fix

We are reporting bad boundaries for the first zone which is allowed
to be missaligned because nodes are not allowed to be missaligned,
and zones which have zero size.  Cull them.

Signed-off-by: Andy Whitcroft <apw@shadowen.org>
---
 page_alloc.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletion(-)
diff -upN reference/mm/page_alloc.c current/mm/page_alloc.c
--- reference/mm/page_alloc.c
+++ current/mm/page_alloc.c
@@ -2223,7 +2223,8 @@ static void __meminit free_area_init_cor
 		struct zone *zone = pgdat->node_zones + j;
 		unsigned long size, realsize;
 
-		if (zone_boundary_align_pfn(zone_start_pfn) != zone_start_pfn)
+		if (zone_boundary_align_pfn(zone_start_pfn) !=
+					zone_start_pfn && j != 0 && size != 0)
 			printk(KERN_CRIT "node %d zone %s missaligned "
 					"start pfn\n", nid, zone_names[j]);
 

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH 2/2] zone allow unaligned zone boundaries spelling fix

[Andy Whitcroft]

zone allow unaligned zone boundaries spelling fix

When the spelling of boundary was sorted out the config options
got missed.

Signed-off-by: Andy Whitcroft <apw@shadowen.org>
---
 include/linux/mmzone.h |    2 +-
 mm/page_alloc.c        |    4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)
diff -upN reference/include/linux/mmzone.h current/include/linux/mmzone.h
--- reference/include/linux/mmzone.h
+++ current/include/linux/mmzone.h
@@ -393,7 +393,7 @@ static inline int is_dma(struct zone *zo
 
 static inline unsigned long zone_boundary_align_pfn(unsigned long pfn)
 {
-#ifdef CONFIG_UNALIGNED_ZONE_BOUNDRIES
+#ifdef CONFIG_UNALIGNED_ZONE_BOUNDARIES
 	return pfn;
 #else
 	return pfn & ~((1 << MAX_ORDER) - 1);
diff -upN reference/mm/page_alloc.c current/mm/page_alloc.c
--- reference/mm/page_alloc.c
+++ current/mm/page_alloc.c
@@ -315,7 +315,7 @@ static inline int page_is_buddy(struct p
 	if (!pfn_valid(page_to_pfn(buddy)))
 		return 0;
 #endif
-#ifdef CONFIG_UNALIGNED_ZONE_BOUNDRIES
+#ifdef CONFIG_UNALIGNED_ZONE_BOUNDARIES
 	if (page_zone_id(page) != page_zone_id(buddy))
 		return 0;
 #endif
@@ -2232,7 +2232,7 @@ static void __meminit free_area_init_cor
 		if (zone_boundary_align_pfn(zone_start_pfn) !=
 					zone_start_pfn && j != 0 && size != 0)
 			printk(KERN_CRIT "node %d zone %s missaligned "
-				"start pfn, enable UNALIGNED_ZONE_BOUNDRIES\n",
+				"start pfn, enable UNALIGNED_ZONE_BOUNDARIES\n",
 							nid, zone_names[j]);
 
 		realsize = size = zones_size[j];

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 2/2] zone allow unaligned zone boundaries spelling fix

[Andy Whitcroft]

Andy Whitcroft wrote:
> zone allow unaligned zone boundaries spelling fix
> 
> When the spelling of boundary was sorted out the config options
> got missed.
> 
> Signed-off-by: Andy Whitcroft <apw@shadowen.org>
> ---
>  include/linux/mmzone.h |    2 +-
>  mm/page_alloc.c        |    4 ++--
>  2 files changed, 3 insertions(+), 3 deletions(-)
> diff -upN reference/include/linux/mmzone.h current/include/linux/mmzone.h
> --- reference/include/linux/mmzone.h
> +++ current/include/linux/mmzone.h
> @@ -393,7 +393,7 @@ static inline int is_dma(struct zone *zo
>  
>  static inline unsigned long zone_boundary_align_pfn(unsigned long pfn)
>  {
> -#ifdef CONFIG_UNALIGNED_ZONE_BOUNDRIES
> +#ifdef CONFIG_UNALIGNED_ZONE_BOUNDARIES
>  	return pfn;
>  #else
>  	return pfn & ~((1 << MAX_ORDER) - 1);
> diff -upN reference/mm/page_alloc.c current/mm/page_alloc.c
> --- reference/mm/page_alloc.c
> +++ current/mm/page_alloc.c
> @@ -315,7 +315,7 @@ static inline int page_is_buddy(struct p
>  	if (!pfn_valid(page_to_pfn(buddy)))
>  		return 0;
>  #endif
> -#ifdef CONFIG_UNALIGNED_ZONE_BOUNDRIES
> +#ifdef CONFIG_UNALIGNED_ZONE_BOUNDARIES
>  	if (page_zone_id(page) != page_zone_id(buddy))
>  		return 0;
>  #endif
> @@ -2232,7 +2232,7 @@ static void __meminit free_area_init_cor
>  		if (zone_boundary_align_pfn(zone_start_pfn) !=
>  					zone_start_pfn && j != 0 && size != 0)
>  			printk(KERN_CRIT "node %d zone %s missaligned "
> -				"start pfn, enable UNALIGNED_ZONE_BOUNDRIES\n",
> +				"start pfn, enable UNALIGNED_ZONE_BOUNDARIES\n",
>  							nid, zone_names[j]);
>  
>  		realsize = size = zones_size[j];

Ignore this patch.  Somehow the wrong version has escaped here.  Clearly
wrong.

-apw

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH 0/2] Zone boundary alignment fixes, cleanups v2

[Andy Whitcroft]

[Lets try that again, here is the correct patch this time.]

[Sorry for the delay, we've been busy looking to see what is
responsible for the ia64 issues with architecture independant
zone sizing.]

Following this email are two cleanup patches for the
UNALIGNED_ZONE_BOUNDARIES support in -mm.

zone-init-check-and-report-unaligned-zone-boundaries-fix --
  we currently will pointlessly report zones as missaligned even
  though they are empty and will report the first zone which can
  never be missaligned assuming node_mem_map is aligned correctly.

zone-allow-unaligned-zone-boundaries-spelling-fix -- when the
  spelling errors in zone-allow-unaligned-zone-boundaries-spelling
  were fixed the configuration options were not updated.

Both of the above patches slot into the linux-2.6.17-rc4-mm1 patch
set next to their main patches.  Amazingly, they will also apply
on top of linux-2.6.17-rc4-mm1, I don't know what patch has been
taking but it rocks.

-apw

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH 1/2] zone init check and report unaligned zone boundaries fix

[Andy Whitcroft]

zone init check and report unaligned zone boundaries fix v2

We are reporting bad boundaries for the first zone which is allowed
to be missaligned because nodes are not allowed to be missaligned,
and zones which have zero size.  Cull them.

Signed-off-by: Andy Whitcroft <apw@shadowen.org>
---
 page_alloc.c |    9 +++++----
 1 files changed, 5 insertions(+), 4 deletions(-)
diff -upN reference/mm/page_alloc.c current/mm/page_alloc.c
--- reference/mm/page_alloc.c
+++ current/mm/page_alloc.c
@@ -2223,10 +2223,6 @@ static void __meminit free_area_init_cor
 		struct zone *zone = pgdat->node_zones + j;
 		unsigned long size, realsize;
 
-		if (zone_boundary_align_pfn(zone_start_pfn) != zone_start_pfn)
-			printk(KERN_CRIT "node %d zone %s missaligned "
-					"start pfn\n", nid, zone_names[j]);
-
 		realsize = size = zones_size[j];
 		if (zholes_size)
 			realsize -= zholes_size[j];
@@ -2235,6 +2231,11 @@ static void __meminit free_area_init_cor
 			nr_kernel_pages += realsize;
 		nr_all_pages += realsize;
 
+		if (zone_boundary_align_pfn(zone_start_pfn) !=
+					zone_start_pfn && j != 0 && size != 0)
+			printk(KERN_CRIT "node %d zone %s missaligned "
+					"start pfn\n", nid, zone_names[j]);
+
 		zone->spanned_pages = size;
 		zone->present_pages = realsize;
 		zone->name = zone_names[j];

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH 2/2] zone allow unaligned zone boundaries spelling fix

[Andy Whitcroft]

zone allow unaligned zone boundaries spelling fix

When the spelling of boundary was sorted out the config options
got missed.

Signed-off-by: Andy Whitcroft <apw@shadowen.org>
---
 include/linux/mmzone.h |    2 +-
 mm/page_alloc.c        |    4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)
diff -upN reference/include/linux/mmzone.h current/include/linux/mmzone.h
--- reference/include/linux/mmzone.h
+++ current/include/linux/mmzone.h
@@ -393,7 +393,7 @@ static inline int is_dma(struct zone *zo
 
 static inline unsigned long zone_boundary_align_pfn(unsigned long pfn)
 {
-#ifdef CONFIG_UNALIGNED_ZONE_BOUNDRIES
+#ifdef CONFIG_UNALIGNED_ZONE_BOUNDARIES
 	return pfn;
 #else
 	return pfn & ~((1 << MAX_ORDER) - 1);
diff -upN reference/mm/page_alloc.c current/mm/page_alloc.c
--- reference/mm/page_alloc.c
+++ current/mm/page_alloc.c
@@ -315,7 +315,7 @@ static inline int page_is_buddy(struct p
 	if (!pfn_valid(page_to_pfn(buddy)))
 		return 0;
 #endif
-#ifdef CONFIG_UNALIGNED_ZONE_BOUNDRIES
+#ifdef CONFIG_UNALIGNED_ZONE_BOUNDARIES
 	if (page_zone_id(page) != page_zone_id(buddy))
 		return 0;
 #endif
@@ -2232,7 +2232,7 @@ static void __meminit free_area_init_cor
 		if (zone_boundary_align_pfn(zone_start_pfn) !=
 					zone_start_pfn && j != 0 && size != 0)
 			printk(KERN_CRIT "node %d zone %s missaligned "
-				"start pfn, enable UNALIGNED_ZONE_BOUNDRIES\n",
+				"start pfn, enable UNALIGNED_ZONE_BOUNDARIES\n",
 							nid, zone_names[j]);
 
 		realsize = size = zones_size[j];

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>