Re: [PATCH 2/6] mm: differentiate unmap for vmscan from other unmap.

[John Hubbard]

On Fri, 27 Jun 2014, JÃrÃme Glisse wrote:

> From: JÃrÃme Glisse <jglisse@xxxxxxxxxx>
> 
> New code will need to be able to differentiate between a regular unmap and
> an unmap trigger by vmscan in which case we want to be as quick as possible.
> 
> Signed-off-by: JÃrÃme Glisse <jglisse@xxxxxxxxxx>
> ---
>  include/linux/rmap.h | 15 ++++++++-------
>  mm/memory-failure.c  |  2 +-
>  mm/vmscan.c          |  4 ++--
>  3 files changed, 11 insertions(+), 10 deletions(-)
> 
> diff --git a/include/linux/rmap.h b/include/linux/rmap.h
> index be57450..eddbc07 100644
> --- a/include/linux/rmap.h
> +++ b/include/linux/rmap.h
> @@ -72,13 +72,14 @@ struct anon_vma_chain {
>  };
>  
>  enum ttu_flags {
> -	TTU_UNMAP = 1,			/* unmap mode */
> -	TTU_MIGRATION = 2,		/* migration mode */
> -	TTU_MUNLOCK = 4,		/* munlock mode */
> -
> -	TTU_IGNORE_MLOCK = (1 << 8),	/* ignore mlock */
> -	TTU_IGNORE_ACCESS = (1 << 9),	/* don't age */
> -	TTU_IGNORE_HWPOISON = (1 << 10),/* corrupted page is recoverable */
> +	TTU_VMSCAN = 1,			/* unmap for vmscan */
> +	TTU_POISON = 2,			/* unmap for poison */
> +	TTU_MIGRATION = 4,		/* migration mode */
> +	TTU_MUNLOCK = 8,		/* munlock mode */
> +
> +	TTU_IGNORE_MLOCK = (1 << 9),	/* ignore mlock */
> +	TTU_IGNORE_ACCESS = (1 << 10),	/* don't age */
> +	TTU_IGNORE_HWPOISON = (1 << 11),/* corrupted page is recoverable */

Unless there is a deeper purpose that I am overlooking, I think it would 
be better to leave the _MLOCK, _ACCESS, and _HWPOISON at their original 
values. I just can't quite see why they would need to start at bit 9 
instead of bit 8...

>  };
>  
>  #ifdef CONFIG_MMU
> diff --git a/mm/memory-failure.c b/mm/memory-failure.c
> index a7a89eb..ba176c4 100644
> --- a/mm/memory-failure.c
> +++ b/mm/memory-failure.c
> @@ -887,7 +887,7 @@ static int page_action(struct page_state *ps, struct page *p,
>  static int hwpoison_user_mappings(struct page *p, unsigned long pfn,
>  				  int trapno, int flags, struct page **hpagep)
>  {
> -	enum ttu_flags ttu = TTU_UNMAP | TTU_IGNORE_MLOCK | TTU_IGNORE_ACCESS;
> +	enum ttu_flags ttu = TTU_POISON | TTU_IGNORE_MLOCK | TTU_IGNORE_ACCESS;
>  	struct address_space *mapping;
>  	LIST_HEAD(tokill);
>  	int ret;
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index 6d24fd6..5a7d286 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -1163,7 +1163,7 @@ unsigned long reclaim_clean_pages_from_list(struct zone *zone,
>  	}
>  
>  	ret = shrink_page_list(&clean_pages, zone, &sc,
> -			TTU_UNMAP|TTU_IGNORE_ACCESS,
> +			TTU_VMSCAN|TTU_IGNORE_ACCESS,
>  			&dummy1, &dummy2, &dummy3, &dummy4, &dummy5, true);
>  	list_splice(&clean_pages, page_list);
>  	mod_zone_page_state(zone, NR_ISOLATED_FILE, -ret);
> @@ -1518,7 +1518,7 @@ shrink_inactive_list(unsigned long nr_to_scan, struct lruvec *lruvec,
>  	if (nr_taken == 0)
>  		return 0;
>  
> -	nr_reclaimed = shrink_page_list(&page_list, zone, sc, TTU_UNMAP,
> +	nr_reclaimed = shrink_page_list(&page_list, zone, sc, TTU_VMSCAN,
>  				&nr_dirty, &nr_unqueued_dirty, &nr_congested,
>  				&nr_writeback, &nr_immediate,
>  				false);
> -- 
> 1.9.0
> 
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majordomo@xxxxxxxxxx  For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>
> 

Other than that, looks good.

Reviewed-by: John Hubbard <jhubbard@xxxxxxxxxx>

thanks,
John H.