Re: [PATCH 5/5] mm, vmscan: Account for skipped pages as a partial scan

[Joonsoo Kim]

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@xxxxxxxxxxxxxxxxxxx>
> > ---
> >  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.