[PATCH 0/3] mm: cleanup thp and shmem allowable order check

[PATCH 0/3] mm: cleanup thp and shmem allowable order check

[Kefeng Wang]

Kefeng Wang (3):
  mm: huge_memory: move file_thp_enabled() into huge_memory.c
  mm: huge_memory: add thp_vma_disabled()
  mm: shmem: remove __shmem_huge_global_enabled()

 include/linux/huge_mm.h | 32 +++++++++++++++++++-------------
 mm/huge_memory.c        | 28 ++++++++++++++++------------
 mm/shmem.c              | 40 +++++++++++-----------------------------
 3 files changed, 46 insertions(+), 54 deletions(-)

-- 
2.27.0

[PATCH 1/3] mm: huge_memory: move file_thp_enabled() into huge_memory.c

[Kefeng Wang]

The file_thp_enabled() only used in __thp_vma_allowable_orders(),
so move it into huge_memory.c, also check READ_ONLY_THP_FOR_FS
ahead to avoid unnecessary code if config disabled.

Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
---
 include/linux/huge_mm.h | 13 -------------
 mm/huge_memory.c        | 15 +++++++++++++++
 2 files changed, 15 insertions(+), 13 deletions(-)

diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h
index 3eca60f3d512..795df660efa5 100644
--- a/include/linux/huge_mm.h
+++ b/include/linux/huge_mm.h
@@ -253,19 +253,6 @@ static inline unsigned long thp_vma_suitable_orders(struct vm_area_struct *vma,
 	return orders;
 }
 
-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)) &&
-	       !inode_is_open_for_write(inode) && S_ISREG(inode->i_mode);
-}
-
 unsigned long __thp_vma_allowable_orders(struct vm_area_struct *vma,
 					 unsigned long vm_flags,
 					 unsigned long tva_flags,
diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index cc346c771921..d23e4aab7511 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -83,6 +83,21 @@ unsigned long huge_anon_orders_madvise __read_mostly;
 unsigned long huge_anon_orders_inherit __read_mostly;
 static bool anon_orders_configured __initdata;
 
+static inline bool file_thp_enabled(struct vm_area_struct *vma)
+{
+	struct inode *inode;
+
+	if (!IS_ENABLED(CONFIG_READ_ONLY_THP_FOR_FS))
+		return false;
+
+	if (!vma->vm_file)
+		return false;
+
+	inode = file_inode(vma->vm_file);
+
+	return !inode_is_open_for_write(inode) && S_ISREG(inode->i_mode);
+}
+
 unsigned long __thp_vma_allowable_orders(struct vm_area_struct *vma,
 					 unsigned long vm_flags,
 					 unsigned long tva_flags,
-- 
2.27.0

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

[Kefeng Wang]

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)
+{
+	/*
+	 * Explicitly disabled through madvise or prctl, or some
+	 * architectures may disable THP for some mappings, for
+	 * example, s390 kvm.
+	 */
+	if (vma && ((vm_flags & VM_NOHUGEPAGE) ||
+	     test_bit(MMF_DISABLE_THP, &vma->vm_mm->flags)))
+		return true;
+
+	/* If the hardware/firmware marked hugepage support disabled. */
+	if (transparent_hugepage_flags & (1 << TRANSPARENT_HUGEPAGE_UNSUPPORTED))
+		return true;
+
+	return false;
+}
+
 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 d23e4aab7511..30912a93f7dc 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -123,18 +123,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_vma_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 0a2f78c2b919..34a31e7e527c 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -1683,12 +1683,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_vma_disabled(vma, vm_flags))
 		return 0;
 
 	global_huge = shmem_huge_global_enabled(inode, index, write_end,
-- 
2.27.0

[PATCH 3/3] mm: shmem: remove __shmem_huge_global_enabled()

[Kefeng Wang]

Remove __shmem_huge_global_enabled() since only one caller,
and remove repeated check of VM_NOHUGEPAGE/MMF_DISABLE_THP
as they are checked in shmem_allowable_huge_orders(), also
remove unnecessary vma parameter.

Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
---
 mm/shmem.c | 33 ++++++++++-----------------------
 1 file changed, 10 insertions(+), 23 deletions(-)

diff --git a/mm/shmem.c b/mm/shmem.c
index 34a31e7e527c..36ac51d55867 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -548,17 +548,15 @@ static bool shmem_confirm_swap(struct address_space *mapping,
 
 static int shmem_huge __read_mostly = SHMEM_HUGE_NEVER;
 
-static bool __shmem_huge_global_enabled(struct inode *inode, pgoff_t index,
-					loff_t write_end, bool shmem_huge_force,
-					struct vm_area_struct *vma,
-					unsigned long vm_flags)
+static bool shmem_huge_global_enabled(struct inode *inode, pgoff_t index,
+				      loff_t write_end, bool shmem_huge_force,
+				      unsigned long vm_flags)
 {
-	struct mm_struct *mm = vma ? vma->vm_mm : NULL;
 	loff_t i_size;
 
-	if (!S_ISREG(inode->i_mode))
+	if (HPAGE_PMD_ORDER > MAX_PAGECACHE_ORDER)
 		return false;
-	if (mm && ((vm_flags & VM_NOHUGEPAGE) || test_bit(MMF_DISABLE_THP, &mm->flags)))
+	if (!S_ISREG(inode->i_mode))
 		return false;
 	if (shmem_huge == SHMEM_HUGE_DENY)
 		return false;
@@ -576,7 +574,7 @@ static bool __shmem_huge_global_enabled(struct inode *inode, pgoff_t index,
 			return true;
 		fallthrough;
 	case SHMEM_HUGE_ADVISE:
-		if (mm && (vm_flags & VM_HUGEPAGE))
+		if (vm_flags & VM_HUGEPAGE)
 			return true;
 		fallthrough;
 	default:
@@ -584,17 +582,6 @@ static bool __shmem_huge_global_enabled(struct inode *inode, pgoff_t index,
 	}
 }
 
-static bool shmem_huge_global_enabled(struct inode *inode, pgoff_t index,
-		   loff_t write_end, bool shmem_huge_force,
-		   struct vm_area_struct *vma, unsigned long vm_flags)
-{
-	if (HPAGE_PMD_ORDER > MAX_PAGECACHE_ORDER)
-		return false;
-
-	return __shmem_huge_global_enabled(inode, index, write_end,
-					   shmem_huge_force, vma, vm_flags);
-}
-
 #if defined(CONFIG_SYSFS)
 static int shmem_parse_huge(const char *str)
 {
@@ -772,8 +759,8 @@ static unsigned long shmem_unused_huge_shrink(struct shmem_sb_info *sbinfo,
 }
 
 static bool shmem_huge_global_enabled(struct inode *inode, pgoff_t index,
-		loff_t write_end, bool shmem_huge_force,
-		struct vm_area_struct *vma, unsigned long vm_flags)
+				      loff_t write_end, bool shmem_huge_force,
+				      unsigned long vm_flags)
 {
 	return false;
 }
@@ -1170,7 +1157,7 @@ static int shmem_getattr(struct mnt_idmap *idmap,
 	generic_fillattr(idmap, request_mask, inode, stat);
 	inode_unlock_shared(inode);
 
-	if (shmem_huge_global_enabled(inode, 0, 0, false, NULL, 0))
+	if (shmem_huge_global_enabled(inode, 0, 0, false, 0))
 		stat->blksize = HPAGE_PMD_SIZE;
 
 	if (request_mask & STATX_BTIME) {
@@ -1687,7 +1674,7 @@ unsigned long shmem_allowable_huge_orders(struct inode *inode,
 		return 0;
 
 	global_huge = shmem_huge_global_enabled(inode, index, write_end,
-					shmem_huge_force, vma, vm_flags);
+						shmem_huge_force, vm_flags);
 	if (!vma || !vma_is_anon_shmem(vma)) {
 		/*
 		 * For tmpfs, we now only support PMD sized THP if huge page
-- 
2.27.0

Re: [PATCH 1/3] mm: huge_memory: move file_thp_enabled() into huge_memory.c

[David Hildenbrand]

On 10.10.24 08:10, Kefeng Wang wrote:
> The file_thp_enabled() only used in __thp_vma_allowable_orders(),
> so move it into huge_memory.c, also check READ_ONLY_THP_FOR_FS
> ahead to avoid unnecessary code if config disabled.

I would hope the compiler is smart enough. But this way is clearer


Acked-by: David Hildenbrand <david@redhat.com>


-- 
Cheers,

David / dhildenb

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

[David Hildenbrand]

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)
> +{
> +	/*
> +	 * Explicitly disabled through madvise or prctl, or some
> +	 * architectures may disable THP for some mappings, for
> +	 * example, s390 kvm.
> +	 */
> +	if (vma && ((vm_flags & VM_NOHUGEPAGE) ||
> +	     test_bit(MMF_DISABLE_THP, &vma->vm_mm->flags)))
> +		return true;
> +
> +	/* If the hardware/firmware marked hugepage support disabled. */
> +	if (transparent_hugepage_flags & (1 << TRANSPARENT_HUGEPAGE_UNSUPPORTED))
> +		return true;
> +
> +	return false;
> +}
> +

Should we call this "vma_thp_disabled()" ?

Also, I wonder if it would be more natural to check for the opposite ... 
  "vma_thp_enabled()", like we test for "allowed" and "suitable".

-- 
Cheers,

David / dhildenb

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

[David Hildenbrand]

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.

Let me try to improve things.

Long term we can get rid of the vm_flags, it just needs some madvise() 
massaging.

-- 
Cheers,

David / dhildenb

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

[David Hildenbrand]

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.
> 
> Let me try to improve things.
> 
> Long term we can get rid of the vm_flags, it just needs some madvise()
> massaging.

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,
-- 
2.46.1


-- 
Cheers,

David / dhildenb

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

[Kefeng Wang]

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,

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

[David Hildenbrand]

On 11.10.24 02:40, Kefeng Wang wrote:
> 
> 
> 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).
> 

Yes, we should be able to handle the madvise stuff in a better way 
(simply check after the vma->vm_flags where modified).

>>>
>>> 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.

I will include that in my next small fix-series and CC you.

-- 
Cheers,

David / dhildenb

Re: [PATCH 1/3] mm: huge_memory: move file_thp_enabled() into huge_memory.c

[Baolin Wang]

On 2024/10/10 14:10, Kefeng Wang wrote:
> The file_thp_enabled() only used in __thp_vma_allowable_orders(),
> so move it into huge_memory.c, also check READ_ONLY_THP_FOR_FS
> ahead to avoid unnecessary code if config disabled.
> 
> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>

LGTM.
Reviewed-by: Baolin Wang <baolin.wang@linux.alibaba.com>

> ---
>   include/linux/huge_mm.h | 13 -------------
>   mm/huge_memory.c        | 15 +++++++++++++++
>   2 files changed, 15 insertions(+), 13 deletions(-)
> 
> diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h
> index 3eca60f3d512..795df660efa5 100644
> --- a/include/linux/huge_mm.h
> +++ b/include/linux/huge_mm.h
> @@ -253,19 +253,6 @@ static inline unsigned long thp_vma_suitable_orders(struct vm_area_struct *vma,
>   	return orders;
>   }
>   
> -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)) &&
> -	       !inode_is_open_for_write(inode) && S_ISREG(inode->i_mode);
> -}
> -
>   unsigned long __thp_vma_allowable_orders(struct vm_area_struct *vma,
>   					 unsigned long vm_flags,
>   					 unsigned long tva_flags,
> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> index cc346c771921..d23e4aab7511 100644
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -83,6 +83,21 @@ unsigned long huge_anon_orders_madvise __read_mostly;
>   unsigned long huge_anon_orders_inherit __read_mostly;
>   static bool anon_orders_configured __initdata;
>   
> +static inline bool file_thp_enabled(struct vm_area_struct *vma)
> +{
> +	struct inode *inode;
> +
> +	if (!IS_ENABLED(CONFIG_READ_ONLY_THP_FOR_FS))
> +		return false;
> +
> +	if (!vma->vm_file)
> +		return false;
> +
> +	inode = file_inode(vma->vm_file);
> +
> +	return !inode_is_open_for_write(inode) && S_ISREG(inode->i_mode);
> +}
> +
>   unsigned long __thp_vma_allowable_orders(struct vm_area_struct *vma,
>   					 unsigned long vm_flags,
>   					 unsigned long tva_flags,

Re: [PATCH 3/3] mm: shmem: remove __shmem_huge_global_enabled()

[Baolin Wang]

On 2024/10/10 14:10, Kefeng Wang wrote:
> Remove __shmem_huge_global_enabled() since only one caller,
> and remove repeated check of VM_NOHUGEPAGE/MMF_DISABLE_THP
> as they are checked in shmem_allowable_huge_orders(), also
> remove unnecessary vma parameter.
> 
> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>

LGTM. Thanks.
Reviewed-by: Baolin Wang <baolin.wang@linux.alibaba.com>

> ---
>   mm/shmem.c | 33 ++++++++++-----------------------
>   1 file changed, 10 insertions(+), 23 deletions(-)
> 
> diff --git a/mm/shmem.c b/mm/shmem.c
> index 34a31e7e527c..36ac51d55867 100644
> --- a/mm/shmem.c
> +++ b/mm/shmem.c
> @@ -548,17 +548,15 @@ static bool shmem_confirm_swap(struct address_space *mapping,
>   
>   static int shmem_huge __read_mostly = SHMEM_HUGE_NEVER;
>   
> -static bool __shmem_huge_global_enabled(struct inode *inode, pgoff_t index,
> -					loff_t write_end, bool shmem_huge_force,
> -					struct vm_area_struct *vma,
> -					unsigned long vm_flags)
> +static bool shmem_huge_global_enabled(struct inode *inode, pgoff_t index,
> +				      loff_t write_end, bool shmem_huge_force,
> +				      unsigned long vm_flags)
>   {
> -	struct mm_struct *mm = vma ? vma->vm_mm : NULL;
>   	loff_t i_size;
>   
> -	if (!S_ISREG(inode->i_mode))
> +	if (HPAGE_PMD_ORDER > MAX_PAGECACHE_ORDER)
>   		return false;
> -	if (mm && ((vm_flags & VM_NOHUGEPAGE) || test_bit(MMF_DISABLE_THP, &mm->flags)))
> +	if (!S_ISREG(inode->i_mode))
>   		return false;
>   	if (shmem_huge == SHMEM_HUGE_DENY)
>   		return false;
> @@ -576,7 +574,7 @@ static bool __shmem_huge_global_enabled(struct inode *inode, pgoff_t index,
>   			return true;
>   		fallthrough;
>   	case SHMEM_HUGE_ADVISE:
> -		if (mm && (vm_flags & VM_HUGEPAGE))
> +		if (vm_flags & VM_HUGEPAGE)
>   			return true;
>   		fallthrough;
>   	default:
> @@ -584,17 +582,6 @@ static bool __shmem_huge_global_enabled(struct inode *inode, pgoff_t index,
>   	}
>   }
>   
> -static bool shmem_huge_global_enabled(struct inode *inode, pgoff_t index,
> -		   loff_t write_end, bool shmem_huge_force,
> -		   struct vm_area_struct *vma, unsigned long vm_flags)
> -{
> -	if (HPAGE_PMD_ORDER > MAX_PAGECACHE_ORDER)
> -		return false;
> -
> -	return __shmem_huge_global_enabled(inode, index, write_end,
> -					   shmem_huge_force, vma, vm_flags);
> -}
> -
>   #if defined(CONFIG_SYSFS)
>   static int shmem_parse_huge(const char *str)
>   {
> @@ -772,8 +759,8 @@ static unsigned long shmem_unused_huge_shrink(struct shmem_sb_info *sbinfo,
>   }
>   
>   static bool shmem_huge_global_enabled(struct inode *inode, pgoff_t index,
> -		loff_t write_end, bool shmem_huge_force,
> -		struct vm_area_struct *vma, unsigned long vm_flags)
> +				      loff_t write_end, bool shmem_huge_force,
> +				      unsigned long vm_flags)
>   {
>   	return false;
>   }
> @@ -1170,7 +1157,7 @@ static int shmem_getattr(struct mnt_idmap *idmap,
>   	generic_fillattr(idmap, request_mask, inode, stat);
>   	inode_unlock_shared(inode);
>   
> -	if (shmem_huge_global_enabled(inode, 0, 0, false, NULL, 0))
> +	if (shmem_huge_global_enabled(inode, 0, 0, false, 0))
>   		stat->blksize = HPAGE_PMD_SIZE;
>   
>   	if (request_mask & STATX_BTIME) {
> @@ -1687,7 +1674,7 @@ unsigned long shmem_allowable_huge_orders(struct inode *inode,
>   		return 0;
>   
>   	global_huge = shmem_huge_global_enabled(inode, index, write_end,
> -					shmem_huge_force, vma, vm_flags);
> +						shmem_huge_force, vm_flags);
>   	if (!vma || !vma_is_anon_shmem(vma)) {
>   		/*
>   		 * For tmpfs, we now only support PMD sized THP if huge page