Re: [v3 PATCH 8/8] mm: mmap: register suitable readonly file vmas for khugepaged

[Vlastimil Babka <vbabka@suse.cz>]

On 4/4/22 22:02, Yang Shi wrote:
> The readonly FS THP relies on khugepaged to collapse THP for suitable
> vmas.  But it is kind of "random luck" for khugepaged to see the
> readonly FS vmas (https://lore.kernel.org/linux-mm/00f195d4-d039-3cf2-d3a1-a2c88de397a0@suse.cz/)
> since currently the vmas are registered to khugepaged when:
>   - Anon huge pmd page fault
>   - VMA merge
>   - MADV_HUGEPAGE
>   - Shmem mmap
> 
> If the above conditions are not met, even though khugepaged is enabled
> it won't see readonly FS vmas at all.  MADV_HUGEPAGE could be specified
> explicitly to tell khugepaged to collapse this area, but when khugepaged
> mode is "always" it should scan suitable vmas as long as VM_NOHUGEPAGE
> is not set.
> 
> So make sure readonly FS vmas are registered to khugepaged to make the
> behavior more consistent.
> 
> Registering suitable vmas in common mmap path, that could cover both
> readonly FS vmas and shmem vmas, so removed the khugepaged calls in
> shmem.c.
> 
> Still need to keep the khugepaged call in vma_merge() since vma_merge()
> is called in a lot of places, for example, madvise, mprotect, etc.
> 
> Reported-by: Vlastimil Babka <vbabka@suse.cz>
> Signed-off-by: Yang Shi <shy828301@gmail.com>

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

> ---
>  mm/mmap.c  | 6 ++++++
>  mm/shmem.c | 4 ----
>  2 files changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/mm/mmap.c b/mm/mmap.c
> index 604c8dece5dd..616ebbc2d052 100644
> --- a/mm/mmap.c
> +++ b/mm/mmap.c
> @@ -1842,6 +1842,12 @@ unsigned long mmap_region(struct file *file, unsigned long addr,
>  	}
>  
>  	vma_link(mm, vma, prev, rb_link, rb_parent);
> +
> +	/*
> +	 * vma_merge() calls khugepaged_enter_vma() either, the below
> +	 * call covers the non-merge case.
> +	 */
> +	khugepaged_enter_vma(vma, vma->vm_flags);
>  	/* Once vma denies write, undo our temporary denial count */
>  unmap_writable:
>  	if (file && vm_flags & VM_SHARED)
> diff --git a/mm/shmem.c b/mm/shmem.c
> index 92eca974771d..0c448080d210 100644
> --- a/mm/shmem.c
> +++ b/mm/shmem.c
> @@ -34,7 +34,6 @@
>  #include <linux/export.h>
>  #include <linux/swap.h>
>  #include <linux/uio.h>
> -#include <linux/khugepaged.h>
>  #include <linux/hugetlb.h>
>  #include <linux/fs_parser.h>
>  #include <linux/swapfile.h>
> @@ -2239,7 +2238,6 @@ static int shmem_mmap(struct file *file, struct vm_area_struct *vma)
>  
>  	file_accessed(file);
>  	vma->vm_ops = &shmem_vm_ops;
> -	khugepaged_enter_vma(vma, vma->vm_flags);
>  	return 0;
>  }
>  
> @@ -4132,8 +4130,6 @@ int shmem_zero_setup(struct vm_area_struct *vma)
>  	vma->vm_file = file;
>  	vma->vm_ops = &shmem_vm_ops;
>  
> -	khugepaged_enter_vma(vma, vma->vm_flags);
> -
>  	return 0;
>  }
>