[PATCH] break out zone free list initialization

[PATCH] break out zone free list initialization

[Dave Hansen]

[-- Attachment #1: Type: text/plain, Size: 802 bytes --]

The following patch removes the individual free area initialization from
free_area_init_core(), and puts it in a new function
zone_init_free_lists().  It also creates pages_to_bitmap_size(), which
is then used in zone_init_free_lists() as well as several times in my
free area bitmap resizing patch.  

First of all, I think it looks nicer this way, but it's also necessary
to have this if you want to initialize a zone after system boot, like if
a NUMA node was hot-added.  In any case, it should be functionally
equivalent to the old code.

Compiles and boots on x86.  I've been running with this for a few weeks,
and haven't seen any problems with it yet.

 page_alloc.c |   86 ++++++++++++++++++++++++++++++++---------------------------
 1 files changed, 48 insertions(+), 38 deletions(-)

-- Dave

[-- Attachment #2: zoneinit_cleanup-2.6.8-rc1-mm1-0.patch --]
[-- Type: text/x-patch, Size: 3043 bytes --]

--- mm/page_alloc.c.orig	2004-07-28 09:37:46.000000000 -0700
+++ mm/page_alloc.c	2004-07-28 09:45:18.000000000 -0700
@@ -1413,6 +1413,52 @@
 	}
 }
 
+/*
+ * Page buddy system uses "index >> (i+1)", where "index" is 
+ * at most "size-1".
+ *
+ * The extra "+3" is to round down to byte size (8 bits per byte
+ * assumption). Thus we get "(size-1) >> (i+4)" as the last byte
+ * we can access.
+ *
+ * The "+1" is because we want to round the byte allocation up 
+ * rather than down. So we should have had a "+7" before we shifted
+ * down by three. Also, we have to add one as we actually _use_ the
+ * last bit (it's [0,n] inclusive, not [0,n[).
+ *
+ * So we actually had +7+1 before we shift down by 3. But 
+ * (n+8) >> 3 == (n >> 3) + 1 (modulo overflows, which we do not have).
+ *
+ * Finally, we LONG_ALIGN because all bitmap operations are on longs.
+ */
+unsigned long pages_to_bitmap_size(unsigned long order, unsigned long nr_pages)
+{
+	unsigned long bitmap_size;
+
+	bitmap_size = (nr_pages-1) >> (order+4);
+	bitmap_size = LONG_ALIGN(bitmap_size+1);
+
+	return bitmap_size;
+}
+
+void zone_init_free_lists(struct pglist_data *pgdat, struct zone *zone, unsigned long size)
+{
+	int order;
+	for (order = 0; ; order++) {
+		unsigned long bitmap_size;
+
+		INIT_LIST_HEAD(&zone->free_area[order].free_list);
+		if (order == MAX_ORDER-1) {
+			zone->free_area[order].map = NULL;
+			break;
+		}
+
+		bitmap_size = pages_to_bitmap_size(order, size);
+		zone->free_area[order].map = 
+		  (unsigned long *) alloc_bootmem_node(pgdat, bitmap_size);
+	}
+}
+
 #ifndef __HAVE_ARCH_MEMMAP_INIT
 #define memmap_init(start, size, nid, zone, start_pfn) \
 	memmap_init_zone((start), (size), (nid), (zone), (start_pfn))
@@ -1528,44 +1574,8 @@
 
 		zone_start_pfn += size;
 		lmem_map += size;
-
-		for (i = 0; ; i++) {
-			unsigned long bitmap_size;
-
-			INIT_LIST_HEAD(&zone->free_area[i].free_list);
-			if (i == MAX_ORDER-1) {
-				zone->free_area[i].map = NULL;
-				break;
-			}
-
-			/*
-			 * Page buddy system uses "index >> (i+1)",
-			 * where "index" is at most "size-1".
-			 *
-			 * The extra "+3" is to round down to byte
-			 * size (8 bits per byte assumption). Thus
-			 * we get "(size-1) >> (i+4)" as the last byte
-			 * we can access.
-			 *
-			 * The "+1" is because we want to round the
-			 * byte allocation up rather than down. So
-			 * we should have had a "+7" before we shifted
-			 * down by three. Also, we have to add one as
-			 * we actually _use_ the last bit (it's [0,n]
-			 * inclusive, not [0,n[).
-			 *
-			 * So we actually had +7+1 before we shift
-			 * down by 3. But (n+8) >> 3 == (n >> 3) + 1
-			 * (modulo overflows, which we do not have).
-			 *
-			 * Finally, we LONG_ALIGN because all bitmap
-			 * operations are on longs.
-			 */
-			bitmap_size = (size-1) >> (i+4);
-			bitmap_size = LONG_ALIGN(bitmap_size+1);
-			zone->free_area[i].map = 
-			  (unsigned long *) alloc_bootmem_node(pgdat, bitmap_size);
-		}
+		
+		zone_init_free_lists(pgdat, zone, zone->spanned_pages);
 	}
 }
 

Re: [PATCH] break out zone free list initialization

[Dave Hansen]

[-- Attachment #1: Type: text/plain, Size: 1010 bytes --]

On Wed, 2004-07-28 at 10:09, Dave Hansen wrote:
> The following patch removes the individual free area initialization from
> free_area_init_core(), and puts it in a new function
> zone_init_free_lists().  It also creates pages_to_bitmap_size(), which
> is then used in zone_init_free_lists() as well as several times in my
> free area bitmap resizing patch.  
> 
> First of all, I think it looks nicer this way, but it's also necessary
> to have this if you want to initialize a zone after system boot, like if
> a NUMA node was hot-added.  In any case, it should be functionally
> equivalent to the old code.
> 
> Compiles and boots on x86.  I've been running with this for a few weeks,
> and haven't seen any problems with it yet.

OK, I suck.  Here's one that applies with -p1 properly and doesn't add
whitespace on the line above the zone_init_free_lists() call.

 page_alloc.c |   84 +++++++++++++++++++++++++++++++++--------------------------
 1 files changed, 47 insertions(+), 37 deletions(-)

-- Dave

[-- Attachment #2: zoneinit_cleanup-2.6.8-rc1-mm1-1.patch --]
[-- Type: text/x-patch, Size: 3088 bytes --]

--- linux-2.6.8-rc1-mm1.work/mm/page_alloc.c.orig	2004-07-28 10:04:56.000000000 -0700
+++ linux-2.6.8-rc1-mm1.work/mm/page_alloc.c		2004-07-28 10:09:09.000000000 -0700
@@ -1413,6 +1413,52 @@
 	}
 }
 
+/*
+ * Page buddy system uses "index >> (i+1)", where "index" is 
+ * at most "size-1".
+ *
+ * The extra "+3" is to round down to byte size (8 bits per byte
+ * assumption). Thus we get "(size-1) >> (i+4)" as the last byte
+ * we can access.
+ *
+ * The "+1" is because we want to round the byte allocation up 
+ * rather than down. So we should have had a "+7" before we shifted
+ * down by three. Also, we have to add one as we actually _use_ the
+ * last bit (it's [0,n] inclusive, not [0,n[).
+ *
+ * So we actually had +7+1 before we shift down by 3. But 
+ * (n+8) >> 3 == (n >> 3) + 1 (modulo overflows, which we do not have).
+ *
+ * Finally, we LONG_ALIGN because all bitmap operations are on longs.
+ */
+unsigned long pages_to_bitmap_size(unsigned long order, unsigned long nr_pages)
+{
+	unsigned long bitmap_size;
+
+	bitmap_size = (nr_pages-1) >> (order+4);
+	bitmap_size = LONG_ALIGN(bitmap_size+1);
+
+	return bitmap_size;
+}
+
+void zone_init_free_lists(struct pglist_data *pgdat, struct zone *zone, unsigned long size)
+{
+	int order;
+	for (order = 0; ; order++) {
+		unsigned long bitmap_size;
+
+		INIT_LIST_HEAD(&zone->free_area[order].free_list);
+		if (order == MAX_ORDER-1) {
+			zone->free_area[order].map = NULL;
+			break;
+		}
+
+		bitmap_size = pages_to_bitmap_size(order, size);
+		zone->free_area[order].map = 
+		  (unsigned long *) alloc_bootmem_node(pgdat, bitmap_size);
+	}
+}
+
 #ifndef __HAVE_ARCH_MEMMAP_INIT
 #define memmap_init(start, size, nid, zone, start_pfn) \
 	memmap_init_zone((start), (size), (nid), (zone), (start_pfn))
@@ -1529,43 +1575,7 @@
 		zone_start_pfn += size;
 		lmem_map += size;
 
-		for (i = 0; ; i++) {
-			unsigned long bitmap_size;
-
-			INIT_LIST_HEAD(&zone->free_area[i].free_list);
-			if (i == MAX_ORDER-1) {
-				zone->free_area[i].map = NULL;
-				break;
-			}
-
-			/*
-			 * Page buddy system uses "index >> (i+1)",
-			 * where "index" is at most "size-1".
-			 *
-			 * The extra "+3" is to round down to byte
-			 * size (8 bits per byte assumption). Thus
-			 * we get "(size-1) >> (i+4)" as the last byte
-			 * we can access.
-			 *
-			 * The "+1" is because we want to round the
-			 * byte allocation up rather than down. So
-			 * we should have had a "+7" before we shifted
-			 * down by three. Also, we have to add one as
-			 * we actually _use_ the last bit (it's [0,n]
-			 * inclusive, not [0,n[).
-			 *
-			 * So we actually had +7+1 before we shift
-			 * down by 3. But (n+8) >> 3 == (n >> 3) + 1
-			 * (modulo overflows, which we do not have).
-			 *
-			 * Finally, we LONG_ALIGN because all bitmap
-			 * operations are on longs.
-			 */
-			bitmap_size = (size-1) >> (i+4);
-			bitmap_size = LONG_ALIGN(bitmap_size+1);
-			zone->free_area[i].map = 
-			  (unsigned long *) alloc_bootmem_node(pgdat, bitmap_size);
-		}
+		zone_init_free_lists(pgdat, zone, zone->spanned_pages);
 	}
 }
 

Re: [PATCH] break out zone free list initialization

[Andrew Morton]

Dave Hansen <haveblue@us.ibm.com> wrote:
>
> The following patch removes the individual free area initialization from
>  free_area_init_core(), and puts it in a new function
>  zone_init_free_lists().  It also creates pages_to_bitmap_size(), which
>  is then used in zone_init_free_lists() as well as several times in my
>  free area bitmap resizing patch.  

This causes my very ordinary p4 testbox to crash very early in boot.  It's
quite pretty, with nice colourful stripes of blinking text on the display.

--
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:"aart@kvack.org"> aart@kvack.org </a>

Re: [PATCH] break out zone free list initialization

[Dave Hansen]

On Thu, 2004-08-05 at 01:24, Andrew Morton wrote:
> Dave Hansen <haveblue@us.ibm.com> wrote:
> >
> > The following patch removes the individual free area initialization from
> >  free_area_init_core(), and puts it in a new function
> >  zone_init_free_lists().  It also creates pages_to_bitmap_size(), which
> >  is then used in zone_init_free_lists() as well as several times in my
> >  free area bitmap resizing patch.  
> 
> This causes my very ordinary p4 testbox to crash very early in boot.  It's
> quite pretty, with nice colourful stripes of blinking text on the display.

Just for a sanity check: are there other machines that it booted on
without any problems for you?

I'll go steal someone's P4 desktop and retest.

-- 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:"aart@kvack.org"> aart@kvack.org </a>