Re: [PATCH 3/5] mm/huge_memory.c: add missing read-only THP checking in transparent_hugepage_enabled()

[Yang Shi <shy828301@gmail.com>]

On Tue, Apr 27, 2021 at 6:32 AM Miaohe Lin <linmiaohe@huawei.com> wrote:
>
> Since commit 99cb0dbd47a1 ("mm,thp: add read-only THP support for
> (non-shmem) FS"), read-only THP file mapping is supported. But it
> forgot to add checking for it in transparent_hugepage_enabled().
>
> Fixes: 99cb0dbd47a1 ("mm,thp: add read-only THP support for (non-shmem) FS")
> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
> ---
>  mm/huge_memory.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> index 76ca1eb2a223..aa22a0ae9894 100644
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -74,6 +74,9 @@ bool transparent_hugepage_enabled(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) && vma->vm_file &&
> +           (vma->vm_flags & VM_DENYWRITE))
> +               return true;

I don't think this change is correct. This function is used to
indicate if allocating THP is eligible for the VMAs or not showed by
smap. And currently readonly FS THP is collapsed by khugepaged only.

So, you need check if the vma is suitable for khugepaged. Take a look
at what hugepage_vma_check() does.

And, the new patch
(https://lore.kernel.org/linux-mm/20210406000930.3455850-1-cfijalkovich@google.com/)
relax the constraints for readonly FS THP, it might be already in -mm
tree, so you need adopt the new condition as well.

>
>         return false;
>  }

> --
> 2.23.0
>
>