Re: [PATCH 17/17] mm/compaction: use zone_end_pfn()

[Dave Hansen]

On 01/15/2013 04:24 PM, Cody P Schafer wrote:
> diff --git a/mm/compaction.c b/mm/compaction.c
> index 1b52528..ea66be3 100644
> --- a/mm/compaction.c
> +++ b/mm/compaction.c
> @@ -85,7 +85,7 @@ static inline bool isolation_suitable(struct compact_control *cc,
>  static void __reset_isolation_suitable(struct zone *zone)
>  {
>  	unsigned long start_pfn = zone->zone_start_pfn;
> -	unsigned long end_pfn = zone->zone_start_pfn + zone->spanned_pages;
> +	unsigned long end_pfn = zone_end_pfn(zone);
>  	unsigned long pfn;
> 
>  	zone->compact_cached_migrate_pfn = start_pfn;
> @@ -663,7 +663,7 @@ static void isolate_freepages(struct zone *zone,
>  	 */
>  	high_pfn = min(low_pfn, pfn);
> 
> -	z_end_pfn = zone->zone_start_pfn + zone->spanned_pages;
> +	z_end_pfn = zone_end_pfn(zone);
> 
>  	/*
>  	 * Isolate free pages until enough are available to migrate the
> @@ -920,7 +920,7 @@ static int compact_zone(struct zone *zone, struct compact_control *cc)
>  {
>  	int ret;
>  	unsigned long start_pfn = zone->zone_start_pfn;
> -	unsigned long end_pfn = zone->zone_start_pfn + zone->spanned_pages;
> +	unsigned long end_pfn = zone_end_pfn(zone);
> 
>  	ret = compaction_suitable(zone, cc->order);
>  	switch (ret) {

I do think theses are a _wee_ bit _too_ broken out.  In this case, it's
highly beneficial to just be able to look in the same email to make sure
that, "yeah, zone_end_pfn() is the same as the code that it replaces".
The fact that it was defined 15 patches ago makes it a bit harder to
review.  It's much nicer to review if there's _one_ patch that does the
"define this new function and do all of the replacements".

Anyway, the series looks good.  Feel free to add my:

Reviewed-by: Dave Hansen <dave@xxxxxxxxxxxxxxxxxx>

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