Re: [Patch]Fix spanned_pages is not updated at a case of memory hot-add.

[Dave Hansen <haveblue@us.ibm.com>]

On Tue, 2006-05-23 at 17:29 +0900, Yasunori Goto wrote:
> Hello.
> 
> I found there is a bug in grow_zone_span() and grow_pgdat_span().
...
> @@ -95,16 +95,18 @@ EXPORT_SYMBOL_GPL(__add_pages);
>  static void grow_zone_span(struct zone *zone,
>  		unsigned long start_pfn, unsigned long end_pfn)
>  {
> -	unsigned long old_zone_end_pfn;
> +	unsigned long new_zone_end_pfn;
>  
>  	zone_span_writelock(zone);
>  
> -	old_zone_end_pfn = zone->zone_start_pfn + zone->spanned_pages;
> +	new_zone_end_pfn = zone->zone_start_pfn + zone->spanned_pages;

I really don't like the idea of having this variable called "new_"
something.  That implies that this is what the new end_pfn is going to
be.  The *new* one.  In reality, it is what it _might_ have been.  How
about "tmp_zone_end_pfn"?

This practice of dealing with spanned_pages is a real pain. 

I generally try to avoid max/min in code, but this struck me as possibly
being useful.  Do you find this easier to read, or your patch?

diff -puN mm/memory_hotplug.c~fix-spanned-pages mm/memory_hotplug.c
--- work/mm/memory_hotplug.c~fix-spanned-pages	2006-05-23 09:04:31.000000000 -0700
+++ work-dave/mm/memory_hotplug.c	2006-05-23 09:22:18.000000000 -0700
@@ -91,8 +91,8 @@ static void grow_zone_span(struct zone *
 	if (start_pfn < zone->zone_start_pfn)
 		zone->zone_start_pfn = start_pfn;
 
-	if (end_pfn > old_zone_end_pfn)
-		zone->spanned_pages = end_pfn - zone->zone_start_pfn;
+	zone->spanned_pages = max(old_zone_end_pfn, end_pfn) -
+				zone->zone_start_pfn);
 
 	zone_span_writeunlock(zone);
 }
@@ -106,8 +106,8 @@ static void grow_pgdat_span(struct pglis
 	if (start_pfn < pgdat->node_start_pfn)
 		pgdat->node_start_pfn = start_pfn;
 
-	if (end_pfn > old_pgdat_end_pfn)
-		pgdat->node_spanned_pages = end_pfn - pgdat->node_start_pfn;
+	pgdat->node_spanned_pages = max(old_pgdat_end_pfn, end_pfn) -
+					pgdat->node_start_pfn;
 }
 
 int online_pages(unsigned long pfn, unsigned long nr_pages)
_


-- 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/