linux-kernel - Re: [PATCH 17/17] mm/compaction: use zone_end

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <50F604D4.407@linux.vnet.ibm.com>
Date:	Tue, 15 Jan 2013 17:39:32 -0800
From:	Dave Hansen <dave@...ux.vnet.ibm.com>
To:	Cody P Schafer <cody@...ux.vnet.ibm.com>
CC:	Linux MM <linux-mm@...ck.org>, LKML <linux-kernel@...r.kernel.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Catalin Marinas <catalin.marinas@....com>
Subject: Re: [PATCH 17/17] mm/compaction: use zone_end_pfn()

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@...ux.vnet.ibm.com>

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/