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

[Minchan Kim <minchan@kernel.org>]

On Wed, Oct 30, 2019 at 01:22:41PM -0400, 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.

Totally, agree.

> 
> 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);
>  	}
>