linux-kernel - Re: [PATCH 5/5] mm, vmscan: Account for skipped pages as a partial 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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <20160726082638.GD15721@js1304-P5Q-DELUXE>
Date:	Tue, 26 Jul 2016 17:26:38 +0900
From:	Joonsoo Kim <iamjoonsoo.kim@....com>
To:	Mel Gorman <mgorman@...hsingularity.net>
Cc:	Andrew Morton <akpm@...ux-foundation.org>,
	Johannes Weiner <hannes@...xchg.org>,
	Minchan Kim <minchan@...nel.org>,
	Michal Hocko <mhocko@...e.cz>,
	Vlastimil Babka <vbabka@...e.cz>,
	Linux-MM <linux-mm@...ck.org>,
	LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 5/5] mm, vmscan: Account for skipped pages as a partial
 scan

On Tue, Jul 26, 2016 at 05:16:22PM +0900, Joonsoo Kim wrote:
> On Thu, Jul 21, 2016 at 03:11:01PM +0100, Mel Gorman wrote:
> > Page reclaim determines whether a pgdat is unreclaimable by examining how
> > many pages have been scanned since a page was freed and comparing that to
> > the LRU sizes. Skipped pages are not reclaim candidates but contribute to
> > scanned. This can prematurely mark a pgdat as unreclaimable and trigger
> > an OOM kill.
> > 
> > This patch accounts for skipped pages as a partial scan so that an
> > unreclaimable pgdat will still be marked as such but by scaling the cost
> > of a skip, it'll avoid the pgdat being marked prematurely.
> > 
> > Signed-off-by: Mel Gorman <mgorman@...hsingularity.net>
> > ---
> >  mm/vmscan.c | 20 ++++++++++++++++++--
> >  1 file changed, 18 insertions(+), 2 deletions(-)
> > 
> > diff --git a/mm/vmscan.c b/mm/vmscan.c
> > index 6810d81f60c7..e5af357dd4ac 100644
> > --- a/mm/vmscan.c
> > +++ b/mm/vmscan.c
> > @@ -1424,7 +1424,7 @@ static unsigned long isolate_lru_pages(unsigned long nr_to_scan,
> >  	LIST_HEAD(pages_skipped);
> >  
> >  	for (scan = 0; scan < nr_to_scan && nr_taken < nr_to_scan &&
> > -					!list_empty(src); scan++) {
> > +					!list_empty(src);) {
> >  		struct page *page;
> >  
> >  		page = lru_to_page(src);
> > @@ -1438,6 +1438,12 @@ static unsigned long isolate_lru_pages(unsigned long nr_to_scan,
> >  			continue;
> >  		}
> >  
> > +		/*
> > +		 * Account for scanned and skipped separetly to avoid the pgdat
> > +		 * being prematurely marked unreclaimable by pgdat_reclaimable.
> > +		 */
> > +		scan++;
> > +
> 
> This logic has potential unbounded retry problem. src would not become
> empty if __isolate_lru_page() return -EBUSY since we move failed page
> to src list again in this case.

Oops.. It would not unbounded retry. It would cause needless retry but
bounded. Sorry about noise.

Thanks.