linux-kernel - Re: [v3 PATCH 4/8] mm: thp: only regular file could be THP eligible

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <05e816e9-255c-7180-7335-09f09c24859e@suse.cz>
Date:   Mon, 9 May 2022 15:41:50 +0200
From:   Vlastimil Babka <vbabka@...e.cz>
To:     Yang Shi <shy828301@...il.com>, kirill.shutemov@...ux.intel.com,
        linmiaohe@...wei.com, songliubraving@...com, riel@...riel.com,
        willy@...radead.org, ziy@...dia.com, tytso@....edu,
        akpm@...ux-foundation.org
Cc:     linux-mm@...ck.org, linux-fsdevel@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [v3 PATCH 4/8] mm: thp: only regular file could be THP eligible

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@...nel.org>
> Signed-off-by: Yang Shi <shy828301@...il.com>

Acked-by: Vlastimil Babka <vbabka@...e.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;