Re: [PATCH 2/2] mm,thp,shm: limit gfp mask to no more than specified

[Hillf Danton <hdanton@sina.com>]

On Thu,  5 Nov 2020 14:15:08 -0500
> 
> Matthew Wilcox pointed out that the i915 driver opportunistically
> allocates tmpfs memory, but will happily reclaim some of its
> pool if no memory is available.
> 
> Make sure the gfp mask used to opportunistically allocate a THP
> is always at least as restrictive as the original gfp mask.
> 
> Signed-off-by: Rik van Riel <riel@surriel.com>
> Suggested-by: Matthew Wilcox <willy@infradead.org>
> ---
>  mm/shmem.c | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
> 
> diff --git a/mm/shmem.c b/mm/shmem.c
> index 6c3cb192a88d..ee3cea10c2a4 100644
> --- a/mm/shmem.c
> +++ b/mm/shmem.c
> @@ -1531,6 +1531,26 @@ static struct page *shmem_swapin(swp_entry_t swap, gfp_t gfp,
>  	return page;
>  }
>  
> +/*
> + * Make sure huge_gfp is always more limited than limit_gfp.
> + * Some of the flags set permissions, while others set limitations.
> + */
> +static gfp_t limit_gfp_mask(gfp_t huge_gfp, gfp_t limit_gfp)
> +{
> +	gfp_t allowflags = __GFP_IO | __GFP_FS | __GFP_RECLAIM;
> +	gfp_t denyflags = __GFP_NOWARN | __GFP_NORETRY;
> +	gfp_t result = huge_gfp & ~allowflags;
> +
> +	/*
> +	 * Minimize the result gfp by taking the union with the deny flags,
> +	 * and the intersection of the allow flags.
> +	 */
> +	result |= (limit_gfp & denyflags);

Currently NORETRY is always set regardless of i915 and if it's
determined in 1/2 then the i915 thing can be done like

	return huge_gfp | (limit_gfp & __GFP_RECLAIM);

in assumption that 1/2 gives the baseline gfp shmem needs.

> +	result |= (huge_gfp & limit_gfp) & allowflags;
> +
> +	return result;
> +}
> +
>  static struct page *shmem_alloc_hugepage(gfp_t gfp,
>  		struct shmem_inode_info *info, pgoff_t index)
>  {
> @@ -1889,6 +1909,7 @@ static int shmem_getpage_gfp(struct inode *inode, pgoff_t index,
>  
>  alloc_huge:
>  	huge_gfp = vma_thp_gfp_mask(vma);
> +	huge_gfp = limit_gfp_mask(huge_gfp, gfp);
>  	page = shmem_alloc_and_acct_page(huge_gfp, inode, index, true);
>  	if (IS_ERR(page)) {
>  alloc_nohuge:
> -- 
> 2.25.4