Received-SPF: softfail (google.com: domain of transitioning linux-kernel-owner@vger.kernel.org does not designate 23.128.96.19 as permitted sender) client-ip=23.128.96.19;
Message-ID: <05e816e9-255c-7180-7335-09f09c24859e@suse.cz>
Date:   Mon, 9 May 2022 15:41:50 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.8.1
Subject: Re: [v3 PATCH 4/8] mm: thp: only regular file could be THP eligible
Content-Language: en-US
To:     Yang Shi <shy828301@gmail.com>, kirill.shutemov@linux.intel.com,
        linmiaohe@huawei.com, songliubraving@fb.com, riel@surriel.com,
        willy@infradead.org, ziy@nvidia.com, tytso@mit.edu,
        akpm@linux-foundation.org
Cc:     linux-mm@kvack.org, linux-fsdevel@vger.kernel.org,
        linux-kernel@vger.kernel.org
References: <20220404200250.321455-1-shy828301@gmail.com>
 <20220404200250.321455-5-shy828301@gmail.com>
From:   Vlastimil Babka <vbabka@suse.cz>
In-Reply-To: <20220404200250.321455-5-shy828301@gmail.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 4/4/22 22:02, Yang Shi wrote:
> Since commit a4aeaa06d45e ("mm: khugepaged: skip huge page collapse for
> special files"), khugepaged just collapses THP for regular file which is
> the intended usecase for readonly fs THP.  Only show regular file as THP
> eligible accordingly.
> 
> And make file_thp_enabled() available for khugepaged too in order to remove
> duplicate code.
> 
> Acked-by: Song Liu <song@kernel.org>
> Signed-off-by: Yang Shi <shy828301@gmail.com>

Acked-by: Vlastimil Babka <vbabka@suse.cz>

> ---
>  include/linux/huge_mm.h | 14 ++++++++++++++
>  mm/huge_memory.c        | 11 ++---------
>  mm/khugepaged.c         |  9 ++-------
>  3 files changed, 18 insertions(+), 16 deletions(-)
> 
> diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h
> index 2999190adc22..62a6f667850d 100644
> --- a/include/linux/huge_mm.h
> +++ b/include/linux/huge_mm.h
> @@ -172,6 +172,20 @@ static inline bool __transparent_hugepage_enabled(struct vm_area_struct *vma)
>  	return false;
>  }
>  
> +static inline bool file_thp_enabled(struct vm_area_struct *vma)
> +{
> +	struct inode *inode;
> +
> +	if (!vma->vm_file)
> +		return false;
> +
> +	inode = vma->vm_file->f_inode;
> +
> +	return (IS_ENABLED(CONFIG_READ_ONLY_THP_FOR_FS)) &&
> +	       (vma->vm_flags & VM_EXEC) &&
> +	       !inode_is_open_for_write(inode) && S_ISREG(inode->i_mode);
> +}
> +
>  bool transparent_hugepage_active(struct vm_area_struct *vma);
>  
>  #define transparent_hugepage_use_zero_page()				\
> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> index 2fe38212e07c..183b793fd28e 100644
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -68,13 +68,6 @@ static atomic_t huge_zero_refcount;
>  struct page *huge_zero_page __read_mostly;
>  unsigned long huge_zero_pfn __read_mostly = ~0UL;
>  
> -static inline bool file_thp_enabled(struct vm_area_struct *vma)
> -{
> -	return transhuge_vma_enabled(vma, vma->vm_flags) && vma->vm_file &&
> -	       !inode_is_open_for_write(vma->vm_file->f_inode) &&
> -	       (vma->vm_flags & VM_EXEC);
> -}
> -
>  bool transparent_hugepage_active(struct vm_area_struct *vma)
>  {
>  	/* The addr is used to check if the vma size fits */
> @@ -86,8 +79,8 @@ bool transparent_hugepage_active(struct vm_area_struct *vma)
>  		return __transparent_hugepage_enabled(vma);
>  	if (vma_is_shmem(vma))
>  		return shmem_huge_enabled(vma);
> -	if (IS_ENABLED(CONFIG_READ_ONLY_THP_FOR_FS))
> -		return file_thp_enabled(vma);
> +	if (transhuge_vma_enabled(vma, vma->vm_flags) && file_thp_enabled(vma))
> +		return true;
>  
>  	return false;
>  }
> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
> index 964a4d2c942a..609c1bc0a027 100644
> --- a/mm/khugepaged.c
> +++ b/mm/khugepaged.c
> @@ -464,13 +464,8 @@ static bool hugepage_vma_check(struct vm_area_struct *vma,
>  		return false;
>  
>  	/* Only regular file is valid */
> -	if (IS_ENABLED(CONFIG_READ_ONLY_THP_FOR_FS) && vma->vm_file &&
> -	    (vm_flags & VM_EXEC)) {
> -		struct inode *inode = vma->vm_file->f_inode;
> -
> -		return !inode_is_open_for_write(inode) &&
> -			S_ISREG(inode->i_mode);
> -	}
> +	if (file_thp_enabled(vma))
> +		return true;
>  
>  	if (!vma->anon_vma || vma->vm_ops)
>  		return false;