Re: [v4 PATCH 2/2] mm: vmscan: correct some vmscan counters for THP swapout

[Hillf Danton <hdanton@sina.com>]

On Thu, 23 May 2019 10:27:38 +0800 Yang Shi wrote:
> 
> @ -1642,14 +1650,14 @@ static unsigned long isolate_lru_pages(unsigned long nr_to_scan,
>  	unsigned long nr_zone_taken[MAX_NR_ZONES] = { 0 };
>  	unsigned long nr_skipped[MAX_NR_ZONES] = { 0, };
>  	unsigned long skipped = 0;
> -	unsigned long scan, total_scan, nr_pages;
> +	unsigned long scan, total_scan;
> +	unsigned long nr_pages;
Change for no earn:)

>  	LIST_HEAD(pages_skipped);
>  	isolate_mode_t mode = (sc->may_unmap ? 0 : ISOLATE_UNMAPPED);
>  
> +	total_scan = 0;
>  	scan = 0;
> -	for (total_scan = 0;
> -	     scan < nr_to_scan && nr_taken < nr_to_scan && !list_empty(src);
> -	     total_scan++) {
> +	while (scan < nr_to_scan && !list_empty(src)) {
>  		struct page *page;
AFAICS scan currently prevents us from looping for ever, while nr_taken bails
us out once we get what's expected, so I doubt it makes much sense to cut
nr_taken off.
>  
>  		page = lru_to_page(src);
> @@ -1657,9 +1665,12 @@ static unsigned long isolate_lru_pages(unsigned long nr_to_scan,
>  
>  		VM_BUG_ON_PAGE(!PageLRU(page), page);
>  
> +		nr_pages = 1 << compound_order(page);
> +		total_scan += nr_pages;
> +
>  		if (page_zonenum(page) > sc->reclaim_idx) {
>  			list_move(&page->lru, &pages_skipped);
> -			nr_skipped[page_zonenum(page)]++;
> +			nr_skipped[page_zonenum(page)] += nr_pages;
>  			continue;
>  		}
>  
> @@ -1669,10 +1680,9 @@ static unsigned long isolate_lru_pages(unsigned long nr_to_scan,
>  		 * ineligible pages.  This causes the VM to not reclaim any
>  		 * pages, triggering a premature OOM.
>  		 */
> -		scan++;
> +		scan += nr_pages;
The comment looks to defy the change if we fail to add a huge page to
the dst list; otherwise nr_taken knows how to do the right thing. What
I prefer is to let scan to do one thing a time.

>  		switch (__isolate_lru_page(page, mode)) {
>  		case 0:
> -			nr_pages = hpage_nr_pages(page);
>  			nr_taken += nr_pages;
>  			nr_zone_taken[page_zonenum(page)] += nr_pages;
>  			list_move(&page->lru, dst);
> -- 
> 1.8.3.1
> 
Best Regards
Hillf