Re: [PATCH 2/3] mm: huge_memory: add thp_vma_disabled()

[Kefeng Wang <wangkefeng.wang@huawei.com>]

On 2024/10/10 22:53, David Hildenbrand wrote:
> On 10.10.24 16:41, David Hildenbrand wrote:
>> On 10.10.24 08:10, Kefeng Wang wrote:
>>> Add thp_vma_disabled() helper to shared by shmem_allowable_huge_orders()
>>> and __thp_vma_allowable_orders().
>>>
>>> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
>>> ---
>>>    include/linux/huge_mm.h | 19 +++++++++++++++++++
>>>    mm/huge_memory.c        | 13 +------------
>>>    mm/shmem.c              |  7 +------
>>>    3 files changed, 21 insertions(+), 18 deletions(-)
>>>
>>> diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h
>>> index 795df660efa5..d77891332b35 100644
>>> --- a/include/linux/huge_mm.h
>>> +++ b/include/linux/huge_mm.h
>>> @@ -309,6 +309,25 @@ struct thpsize {
>>>        (transparent_hugepage_flags &                    \
>>>         (1<<TRANSPARENT_HUGEPAGE_USE_ZERO_PAGE_FLAG))
>>> +static inline bool thp_vma_disabled(struct vm_area_struct *vma,
>>> +                    unsigned long vm_flags)
>>> +{
>>>
>>
>> I might need a patch like this for an independent fix, and the more I
>> look at this the more I hate the separate vm_flags and the optional vma
>> argument.

Yes, it is a little strange when made this changes,  a separate vm_flags
and another vma argument, most vm_flags is just vma->vm_flags(madvise is
a special case).

>>
>> Let me try to improve things.
>>
>> Long term we can get rid of the vm_flags, it just needs some madvise()
>> massaging.
> 

Thanks for your improvement, it is more accurate.

> For the time being I suggest this:
> 
>  From 318c25742380cdf15c8c807e5e8a52cabc217ef4 Mon Sep 17 00:00:00 2001
> From: Kefeng Wang <wangkefeng.wang@huawei.com>
> Date: Thu, 10 Oct 2024 14:10:23 +0800
> Subject: [PATCH] mm: huge_memory: add vma_thp_disabled() and
>   thp_disabled_by_hw()
> 
> Add vma_thp_disabled() and thp_disabled_by_hw() helpers to be shared by
> shmem_allowable_huge_orders() and __thp_vma_allowable_orders().
> 
> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
> [ rename to vma_thp_disabled(), split out thp_disabled_by_hw() ]
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>   include/linux/huge_mm.h | 18 ++++++++++++++++++
>   mm/huge_memory.c        | 13 +------------
>   mm/shmem.c              |  7 +------
>   3 files changed, 20 insertions(+), 18 deletions(-)
> 
> diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h
> index 67d0ab3c3bba..57b62fd1ccb4 100644
> --- a/include/linux/huge_mm.h
> +++ b/include/linux/huge_mm.h
> @@ -322,6 +322,24 @@ struct thpsize {
>       (transparent_hugepage_flags &                    \
>        (1<<TRANSPARENT_HUGEPAGE_USE_ZERO_PAGE_FLAG))
> 
> +static inline bool vma_thp_disabled(struct vm_area_struct *vma,
> +        unsigned long vm_flags)
> +{
> +    /*
> +     * Explicitly disabled through madvise or prctl, or some
> +     * architectures may disable THP for some mappings, for
> +     * example, s390x kvm.
> +     */
> +    return (vm_flags & VM_NOHUGEPAGE) ||
> +           test_bit(MMF_DISABLE_THP, &vma->vm_mm->flags);
> +}
> +
> +static inline bool thp_disabled_by_hw(void)
> +{
> +    /* If the hardware/firmware marked hugepage support disabled. */
> +    return transparent_hugepage_flags & (1 << 
> TRANSPARENT_HUGEPAGE_UNSUPPORTED);
> +}
> +
>   unsigned long thp_get_unmapped_area(struct file *filp, unsigned long 
> addr,
>           unsigned long len, unsigned long pgoff, unsigned long flags);
>   unsigned long thp_get_unmapped_area_vmflags(struct file *filp, 
> unsigned long addr,
> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> index 3ca89e0279a7..ffbf0add2a82 100644
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -109,18 +109,7 @@ unsigned long __thp_vma_allowable_orders(struct 
> vm_area_struct *vma,
>       if (!vma->vm_mm)        /* vdso */
>           return 0;
> 
> -    /*
> -     * Explicitly disabled through madvise or prctl, or some
> -     * architectures may disable THP for some mappings, for
> -     * example, s390 kvm.
> -     * */
> -    if ((vm_flags & VM_NOHUGEPAGE) ||
> -        test_bit(MMF_DISABLE_THP, &vma->vm_mm->flags))
> -        return 0;
> -    /*
> -     * If the hardware/firmware marked hugepage support disabled.
> -     */
> -    if (transparent_hugepage_flags & (1 << 
> TRANSPARENT_HUGEPAGE_UNSUPPORTED))
> +    if (thp_disabled_by_hw() || vma_thp_disabled(vma, vm_flags))
>           return 0;
> 
>       /* khugepaged doesn't collapse DAX vma, but page fault is fine. */
> diff --git a/mm/shmem.c b/mm/shmem.c
> index 4f11b5506363..c5adb987b23c 100644
> --- a/mm/shmem.c
> +++ b/mm/shmem.c
> @@ -1664,12 +1664,7 @@ unsigned long shmem_allowable_huge_orders(struct 
> inode *inode,
>       loff_t i_size;
>       int order;
> 
> -    if (vma && ((vm_flags & VM_NOHUGEPAGE) ||
> -        test_bit(MMF_DISABLE_THP, &vma->vm_mm->flags)))
> -        return 0;
> -
> -    /* If the hardware/firmware marked hugepage support disabled. */
> -    if (transparent_hugepage_flags & (1 << 
> TRANSPARENT_HUGEPAGE_UNSUPPORTED))
> +    if (thp_disabled_by_hw() || (vma && vma_thp_disabled(vma, vm_flags)))
>           return 0;
> 
>       global_huge = shmem_huge_global_enabled(inode, index, write_end,