linux-kernel - Re: [PATCH 3/8] mm: check if we isolated a compound page during lumpy scan

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 for Android: free password hash cracker in your pocket

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

Message-ID: <20111121115131.GC19415@suse.de>
Date:	Mon, 21 Nov 2011 11:51:31 +0000
From:	Mel Gorman <mgorman@...e.de>
To:	Andrea Arcangeli <aarcange@...hat.com>
Cc:	linux-mm@...ck.org, Minchan Kim <minchan.kim@...il.com>,
	Jan Kara <jack@...e.cz>, Andy Isaacson <adi@...apodia.org>,
	Johannes Weiner <jweiner@...hat.com>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 3/8] mm: check if we isolated a compound page during
 lumpy scan

On Sat, Nov 19, 2011 at 08:54:15PM +0100, Andrea Arcangeli wrote:
> Properly take into account if we isolated a compound page during the
> lumpy scan in reclaim and break the loop if we've isolated enough.
> 
> Signed-off-by: Andrea Arcangeli <aarcange@...hat.com>
>  1 files changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index a1893c0..3421746 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -1183,13 +1183,16 @@ static unsigned long isolate_lru_pages(unsigned long nr_to_scan,
>  				break;
>  
>  			if (__isolate_lru_page(cursor_page, mode, file) == 0) {
> +				unsigned int isolated_pages;
>  				list_move(&cursor_page->lru, dst);
>  				mem_cgroup_del_lru(cursor_page);
> -				nr_taken += hpage_nr_pages(page);

nr_taken was already being updated correctly.

> -				nr_lumpy_taken++;
> +				isolated_pages = hpage_nr_pages(page);
> +				nr_taken += isolated_pages;
> +				nr_lumpy_taken += isolated_pages;

nr_lumpy_taken was not, and this patch corrects it.

>  				if (PageDirty(cursor_page))
> -					nr_lumpy_dirty++;
> +					nr_lumpy_dirty += isolated_pages;

nr_lumpy_dirty was not, and this patch corrects it.

However, the nr_lumpy_* variables here are not of critical importance
as they only are used by a trace point. Fixing them is nice but
functionally changes nothing.

>  				scan++;
> +				pfn += isolated_pages-1;

This is more important. With the current code the next page encountered
after a THP page is isolated will be a tail page, not on the LRU
and will cause the loop to break as __isolate_lru_page will return
EINVAL. With this change, the next PFN encountered will really be
the next PFN of interest.

That said, the impact of this change is low. For THP allocations,
pfn += isolated_pages-1 will bring pfn past end_pfn so with or without
the page, we break the loop after isolating a THP. For lower-order
allocations, pfn+= isolated_pages-1 will also bring pfn past end_pfn.
This patch does help the case where the allocation is for a page larger
than a THP but that is very rare.

Hence, while I think the patch is correct, the changelog is misleading
as it does not have a large impact on breaking the loop if we've
isolated enough. To really break if we've isolated enough, you'd also
need 

scan += isolated_pages-1

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