Re: [PATCH 3/6] mm: Remove CONFIG_TRANSPARENT_HUGE_PAGECACHE

[Alexander Duyck <alexander.duyck@gmail.com>]

On Mon, Mar 2, 2020 at 8:11 PM Matthew Wilcox <willy@infradead.org> wrote:
>
> From: "Matthew Wilcox (Oracle)" <willy@infradead.org>
>
> Commit e496cf3d7821 ("thp: introduce CONFIG_TRANSPARENT_HUGE_PAGECACHE")
> notes that it should be reverted when the PowerPC problem was fixed.
> The commit fixing the PowerPC problem (953c66c2b22a) did not revert
> the commit; instead setting CONFIG_TRANSPARENT_HUGE_PAGECACHE to the
> same as CONFIG_TRANSPARENT_HUGEPAGE.  Checking with Kirill and Aneesh,
> this was an oversight, so remove the Kconfig symbol and undo the work
> of commit e496cf3d7821.
>
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> Cc: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> Cc: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
> ---
>  include/linux/shmem_fs.h | 10 +---------
>  mm/Kconfig               |  6 +-----
>  mm/huge_memory.c         |  2 +-
>  mm/khugepaged.c          | 10 ++--------
>  mm/memory.c              |  5 ++---
>  mm/rmap.c                |  2 +-
>  mm/shmem.c               | 36 ++++++++++++++++++------------------
>  7 files changed, 26 insertions(+), 45 deletions(-)
>
> diff --git a/include/linux/shmem_fs.h b/include/linux/shmem_fs.h
> index d56fefef8905..7a35a6901221 100644
> --- a/include/linux/shmem_fs.h
> +++ b/include/linux/shmem_fs.h
> @@ -78,6 +78,7 @@ extern void shmem_truncate_range(struct inode *inode, loff_t start, loff_t end);
>  extern int shmem_unuse(unsigned int type, bool frontswap,
>                        unsigned long *fs_pages_to_unuse);
>
> +extern bool shmem_huge_enabled(struct vm_area_struct *vma);
>  extern unsigned long shmem_swap_usage(struct vm_area_struct *vma);
>  extern unsigned long shmem_partial_swap_usage(struct address_space *mapping,
>                                                 pgoff_t start, pgoff_t end);
> @@ -114,15 +115,6 @@ static inline bool shmem_file(struct file *file)
>  extern bool shmem_charge(struct inode *inode, long pages);
>  extern void shmem_uncharge(struct inode *inode, long pages);
>
> -#ifdef CONFIG_TRANSPARENT_HUGE_PAGECACHE
> -extern bool shmem_huge_enabled(struct vm_area_struct *vma);
> -#else
> -static inline bool shmem_huge_enabled(struct vm_area_struct *vma)
> -{
> -       return false;
> -}
> -#endif
> -
>  #ifdef CONFIG_SHMEM
>  extern int shmem_mcopy_atomic_pte(struct mm_struct *dst_mm, pmd_t *dst_pmd,
>                                   struct vm_area_struct *dst_vma,

<snip>

> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
> index b679908743cb..f99ac65271aa 100644
> --- a/mm/khugepaged.c
> +++ b/mm/khugepaged.c
> @@ -416,8 +416,6 @@ static bool hugepage_vma_check(struct vm_area_struct *vma,
>             (IS_ENABLED(CONFIG_READ_ONLY_THP_FOR_FS) &&
>              vma->vm_file &&
>              (vm_flags & VM_DENYWRITE))) {
> -               if (!IS_ENABLED(CONFIG_TRANSPARENT_HUGE_PAGECACHE))
> -                       return false;
>                 return IS_ALIGNED((vma->vm_start >> PAGE_SHIFT) - vma->vm_pgoff,
>                                 HPAGE_PMD_NR);
>         }
> @@ -1260,7 +1258,7 @@ static void collect_mm_slot(struct mm_slot *mm_slot)
>         }
>  }
>
> -#if defined(CONFIG_SHMEM) && defined(CONFIG_TRANSPARENT_HUGE_PAGECACHE)
> +#ifdef CONFIG_SHMEM
>  /*
>   * Notify khugepaged that given addr of the mm is pte-mapped THP. Then
>   * khugepaged should try to collapse the page table.
> @@ -1986,14 +1984,10 @@ static unsigned int khugepaged_scan_mm_slot(unsigned int pages,
>                                   khugepaged_scan.address + HPAGE_PMD_SIZE >
>                                   hend);
>                         if (IS_ENABLED(CONFIG_SHMEM) && vma->vm_file) {
> -                               struct file *file;
> +                               struct file *file = get_file(vma->vm_file);
>                                 pgoff_t pgoff = linear_page_index(vma,
>                                                 khugepaged_scan.address);
>
> -                               if (shmem_file(vma->vm_file)
> -                                   && !shmem_huge_enabled(vma))
> -                                       goto skip;
> -                               file = get_file(vma->vm_file);
>                                 up_read(&mm->mmap_sem);
>                                 ret = 1;
>                                 khugepaged_scan_file(mm, file, pgoff, hpage);

In the code above you didn't eliminate shmem_huge_enabled, it still
exists and has multiple paths that can return false. Are we guaranteed
that it will return true or is it that it can be ignored here?

All the other changes look correct.