Re: [PATCH] mm: vmscan: Add warn on inadvertently reclaiming mapped page

[Yang Shi <yang.shi@linux.alibaba.com>]

On 5/26/19 2:23 PM, Hillf Danton wrote:
> In the function isolate_lru_pages(), we check scan_control::may_unmap and set
> isolation mode accordingly in order to not isolate from the lru list any page
> that does not match the isolation mode. For example, we should skip all sill
> mapped pages if isolation mode is set to be ISOLATE_UNMAPPED.
>
> So complain, while scanning the isolated pages, about the very unlikely event
> that we hit a mapped page that we should never have isolated. Note no change
> is added in the current scanning behavior without VM debug configured.
>
> And cut off one line of comment that goes stale.

Looks good to me. Reviewed-by: Yang Shi <yang.shi@linux.alibaba.com>

>
> Cc: Mel Gorman <mgorman@techsingularity.net>
> Cc: Johannes Weiner <hannes@cmpxchg.org>
> Cc: Minchan Kim <minchan@kernel.org>
> Cc: Yang Shi <yang.shi@linux.alibaba.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Signed-off-by: Hillf Danton <hdanton@sina.com>
> ---
>   mm/vmscan.c | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index d9c3e87..799ad9e 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -1134,8 +1134,10 @@ static unsigned long shrink_page_list(struct list_head *page_list,
>   		if (unlikely(!page_evictable(page)))
>   			goto activate_locked;
>   
> -		if (!sc->may_unmap && page_mapped(page))
> +		if (!sc->may_unmap && page_mapped(page)) {
> +			VM_WARN_ON(true);
>   			goto keep_locked;
> +		}
>   
>   		/* Double the slab pressure for mapped and swapcache pages */
>   		if ((page_mapped(page) || PageSwapCache(page)) &&
> @@ -1632,7 +1634,6 @@ static __always_inline void update_lru_sizes(struct lruvec *lruvec,
>    * @dst:	The temp list to put pages on to.
>    * @nr_scanned:	The number of pages that were scanned.
>    * @sc:		The scan_control struct for this reclaim session
> - * @mode:	One of the LRU isolation modes
>    * @lru:	LRU list id for isolating
>    *
>    * returns how many pages were moved onto *@dst.
> --