linux-kernel - Re: [PATCH] mm: vmscam: check page order in isolating lru pages

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: <20120105133248.GH28031@suse.de>
Date:	Thu, 5 Jan 2012 13:32:48 +0000
From:	Mel Gorman <mgorman@...e.de>
To:	Hillf Danton <dhillf@...il.com>
Cc:	KOSAKI Motohiro <kosaki.motohiro@...il.com>, linux-mm@...ck.org,
	David Rientjes <rientjes@...gle.com>,
	Hugh Dickins <hughd@...gle.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	KAMEZAWA Hiroyuki <kamezawa.hiroyu@...fujitsu.com>,
	Michal Hocko <mhocko@...e.cz>,
	LKML <linux-kernel@...r.kernel.org>,
	Andrea Arcangeli <aarcange@...hat.com>
Subject: Re: [PATCH] mm: vmscam: check page order in isolating lru pages

On Sat, Dec 31, 2011 at 10:55:22PM +0800, Hillf Danton wrote:
> From: Hillf Danton <dhillf@...il.com>
> Subject: [PATCH] mm: vmscam: check page order in isolating lru pages
> 
> Before try to isolate physically contiguous pages, check for page order is
> added, and if it is not regular page, we should give up the attempt.
> 
> Signed-off-by: Hillf Danton <dhillf@...il.com>
> Acked-by: KOSAKI Motohiro <kosaki.motohiro@...fujitsu.com>
> Cc: Michal Hocko <mhocko@...e.cz>
> Cc: KAMEZAWA Hiroyuki <kamezawa.hiroyu@...fujitsu.com>
> Cc: Andrew Morton <akpm@...ux-foundation.org>
> Cc: David Rientjes <rientjes@...gle.com>
> Cc: Hugh Dickins <hughd@...gle.com>
> Cc: Andrea Arcangeli <aarcange@...hat.com>
> Cc: Mel Gorman <mgorman@...e.de>
> ---
> 
> --- a/mm/vmscan.c	Thu Dec 29 20:20:16 2011
> +++ b/mm/vmscan.c	Sat Dec 31 22:44:16 2011
> @@ -1162,6 +1162,7 @@ static unsigned long isolate_lru_pages(u
>  		unsigned long end_pfn;
>  		unsigned long page_pfn;
>  		int zone_id;
> +		unsigned int isolated_pages = 1;
> 
>  		page = lru_to_page(src);
>  		prefetchw_prev_lru_page(page, src, flags);
> @@ -1172,7 +1173,7 @@ static unsigned long isolate_lru_pages(u
>  		case 0:
>  			mem_cgroup_lru_del(page);
>  			list_move(&page->lru, dst);
> -			nr_taken += hpage_nr_pages(page);
> +			isolated_pages = hpage_nr_pages(page);
>  			break;
> 
>  		case -EBUSY:
> @@ -1184,8 +1185,12 @@ static unsigned long isolate_lru_pages(u
>  			BUG();
>  		}
> 
> +		nr_taken += isolated_pages;
>  		if (!order)
>  			continue;
> +		/* try pfn-based isolation only for regular page */
> +		if (isolated_pages != 1)
> +			continue;
> 

Please put more detail in your changelogs explaining the intention
of your patch.  Judging from it, this is a marginal performance
improvement when THPs are being isolated from the LRU by bypassing
lumpy reclaim.

However, basing the check on "isolated_pages" is obscure and it also
disables lumpy reclaim for the cases where order > HPAGE_SHIFT . This
is very rare (might never even happen) but it's still broken. Minimally
the check should have been something like

if (!order || isolated_pages >= (1 << order))
	continue;

with a comment explaining that there is no point taking pages around
a naturally-aligned region if we just isolated a page larger than it.
This would look better, avoid reusing isolated_pages, be less obscure
and still work for cases where the requested order is larger than
a THP.

Nak to this version.

-- 
Mel Gorman
SUSE Labs
--
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/