Re: [PATCH] mm: fix unevictable page reclaim when calling madvise_pageout

[zhong jiang <zhongjiang@huawei.com>]

On 2019/10/31 1:22, Johannes Weiner wrote:
> On Wed, Oct 30, 2019 at 09:52:39AM -0700, Minchan Kim wrote:
>> @@ -1468,7 +1468,7 @@ static long check_and_migrate_cma_pages(struct task_struct *tsk,
>>  					drain_allow = false;
>>  				}
>>  
>> -				if (!isolate_lru_page(head)) {
>> +				if (!isolate_lru_page(head, false)) {
>>  					list_add_tail(&head->lru, &cma_page_list);
>>  					mod_node_page_state(page_pgdat(head),
>>  							    NR_ISOLATED_ANON +
> It's not clear what that argument means at the callsite, and every
> caller needs to pass it to support one niche usecase. Let's not do
> that.
>
> I think there are better options. Personally, I think it's a good idea
> to keep the sanity check in shrink_page_list() because the mlock LRU
> handling is quite tricky. madvise() is really the odd one out here
> because it isolates LRU pages through page tables and then sends them
> through the regular reclaim path, so IMO it should be the madvise
> proper that handles the exceptional situation.
>
> Why not just this? Maybe with a comment that points out that we're
> coming from the page tables instead of a specific LRU list, and so
> need to filter out the unevictable lru pages from our end.
>
> diff --git a/mm/madvise.c b/mm/madvise.c
> index 99dd06fecfa9..63e130800570 100644
> --- a/mm/madvise.c
> +++ b/mm/madvise.c
> @@ -363,8 +363,12 @@ static int madvise_cold_or_pageout_pte_range(pmd_t *pmd,
>  		ClearPageReferenced(page);
>  		test_and_clear_page_young(page);
>  		if (pageout) {
> -			if (!isolate_lru_page(page))
> -				list_add(&page->lru, &page_list);
> +			if (!isolate_lru_page(page)) {
> +				if (PageUnevictable(page))
> +					putback_lru_page(page);
> +				else
> +					list_add(&page->lru, &page_list);
> +			}
>  		} else
>  			deactivate_page(page);
>  huge_unlock:
> @@ -441,8 +445,12 @@ static int madvise_cold_or_pageout_pte_range(pmd_t *pmd,
>  		ClearPageReferenced(page);
>  		test_and_clear_page_young(page);
>  		if (pageout) {
> -			if (!isolate_lru_page(page))
> -				list_add(&page->lru, &page_list);
> +			if (!isolate_lru_page(page)) {
> +				if (PageUnevictable(page))
> +					putback_lru_page(page);
> +				else
> +					list_add(&page->lru, &page_list);
> +			}
>  		} else
>  			deactivate_page(page);
>  	}
>
> .
>
It seems to filter the Unevictable page is the correct fix.  Even  though I am not very clear how
an little race window to result in an PageMlocked in vmscan. 

I will repost the patch in v2 as you has proposed.  Thanks

Sincerely,
zhong jiang