Re: [PATCH] mm/huge_memory: let special huge VMAs bypass the THP policy check

On 8/6/26 18:19, David Hildenbrand (Arm) wrote:
>>  From d6537260722c8741586e6295c8eea68d06087efa Mon Sep 17 00:00:00 2001
>> From: "Lorenzo Stoakes (ARM)" <[email protected]>
>> Date: Wed, 5 Aug 2026 11:35:13 +0100
>> Subject: [PATCH] ideas
>>
>> ---
>>   mm/huge_memory.c | 32 +++++++++++++++++++++++++++++---
>>   1 file changed, 29 insertions(+), 3 deletions(-)
>>
>> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
>> index abc65d608c23..5fa01364f089 100644
>> --- a/mm/huge_memory.c
>> +++ b/mm/huge_memory.c
>> @@ -111,6 +111,34 @@ static bool vma_is_special_huge(const struct vm_area_struct *vma)
>>   	return vma_test_any(vma, VMA_PFNMAP_BIT, VMA_MIXEDMAP_BIT);
>>   }
>>
>> +static bool should_obey_thp_file_tunables(const struct vm_area_struct *vma,
>> +					  bool forced_collapse)
>> +{> +	if (forced_collapse)
>> +		return false;
>> +	VM_WARN_ON_ONCE(vma_is_anonymous(vma));
>> +	/* Huge PFN mappings allocate no folios so the policy doesn't apply. */
>> +	if (vma_test(vma, VMA_PFNMAP_BIT) && vma->vm_ops->huge_fault)
>> +		return false;
>> +	return true;
>> +}
> 
> If that's sufficient, then this is is the better direction.
> 
>> +> +static bool can_thp_collapse_file(const struct vm_area_struct *vma,
>> +		vm_flags_t vm_flags, bool forced_collapse)
>> +{
>> +	/* Override THP tunables? */
>> +	if (!should_obey_thp_file_tunables(vma, forced_collapse))
>> +		return true;
>> +	/* THP=always? */
>> +	if (hugepage_global_always())
>> +		return true;
>> +	/* THP=madvise? */
>> +	if (!hugepage_global_enabled())
>> +		return false;
>> +	/* Has VMA had madvise(..., MADV_HUGEPAGE) applied to it? */
>> +	return vm_flags & VM_HUGEPAGE;
>> +}
>> +
>>   unsigned long __thp_vma_allowable_orders(struct vm_area_struct *vma,
>>   					 vm_flags_t vm_flags,
>>   					 enum tva_type type,
>> @@ -188,9 +216,7 @@ unsigned long __thp_vma_allowable_orders(struct vm_area_struct *vma,
>>   		 * Enforce THP collapse requirements as necessary. Anonymous vmas
>>   		 * were already handled in thp_vma_allowable_orders().
>>   		 */
>> -		if (!forced_collapse &&
>> -		    (!hugepage_global_enabled() || (!(vm_flags & VM_HUGEPAGE) &&
>> -						    !hugepage_global_always())))
>> +		if (!can_thp_collapse_file(vma, vm_flags, forced_collapse))
>>   			return 0;
> 
> I played a bit with that and came up with the following. Not quite happy about
> it, just for your inspiration on naming and what to split out.
> 
> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> index a00df56a68b57..8ba77608959d4 100644
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -111,6 +111,54 @@ static bool vma_is_special_huge(const struct vm_area_struct
> *vma)
>   	return vma_test_any(vma, VMA_PFNMAP_BIT, VMA_MIXEDMAP_BIT);
>   }
> 
> +static bool file_vma_honors_thp_toggles(struct vm_area_struct *vma,> +		enum tva_type type)
> +{
> +	const bool forced_collapse = type == TVA_FORCED_COLLAPSE;
> +
> +	if (forced_collapse)
> +		return false;
> +	/* Huge PFN mappings allocate no folios so the policy doesn't apply. */

So, may be rename the routine to file_vma_thp_policy_applies() ?

> +	return !(vma_test(vma, VMA_PFNMAP_BIT) && vma->vm_ops->huge_fault);
> +}
> +
> +static bool vma_thp_toggles_enabled(struct vm_area_struct *vma,

This routine could use the same 'file_vma_' prefix.

> +		vm_flags_t vm_flags)
> +{
> +	/* THP=always? */
> +	if (hugepage_global_always())
> +		return true;
> +	/* THP=madvise and actually advised? */
> +	return hugepage_global_enabled() && vm_flags & VM_HUGEPAGE;

I would add extra parentheses around 'vm_flags & VM_HUGEPAGE'

> +}
> +
> +static bool file_vma_forces_order_0(struct vm_area_struct *vma,
> +		vm_flags_t vm_flags, enum tva_type type)
> +{
> +	const bool in_pf = type == TVA_PAGEFAULT;
> +	const bool smaps = type == TVA_SMAPS;
> +
> +	/*
> +	 * Enforce THP collapse requirements as necessary. Anonymous vmas
> +	 * were already handled in thp_vma_allowable_orders().
> +	 */
> +
> +	if (file_vma_honors_thp_toggles(vma, type) &&
> +	    !vma_thp_toggles_enabled(vma, vm_flags))
> +		return true;
> +
> +	/*
> +	 * Trust that ->huge_fault() handlers know what they are doing
> +	 * in fault path.
> +	 */
> +	if (((in_pf || smaps)) && vma->vm_ops->huge_fault)

and there remove the extra parentheses.

> +		return false;
> +	/* Only regular file is valid in collapse path */
> +	if (((!in_pf || smaps)) && file_thp_enabled(vma))

there too.

Thanks,

C.

> +		return false;
> +	return true;
> +}
> +
>   unsigned long __thp_vma_allowable_orders(struct vm_area_struct *vma,
>   					 vm_flags_t vm_flags,
>   					 enum tva_type type,
> @@ -183,27 +231,8 @@ unsigned long __thp_vma_allowable_orders(struct
> vm_area_struct *vma,
>   						   vma, vma_start_pgoff(vma), 0,
>   						   forced_collapse);
> 
> -	if (!vma_is_anonymous(vma)) {
> -		/*
> -		 * Enforce THP collapse requirements as necessary. Anonymous vmas
> -		 * were already handled in thp_vma_allowable_orders().
> -		 */
> -		if (!forced_collapse &&
> -		    (!hugepage_global_enabled() || (!(vm_flags & VM_HUGEPAGE) &&
> -						    !hugepage_global_always())))
> -			return 0;
> -
> -		/*
> -		 * Trust that ->huge_fault() handlers know what they are doing
> -		 * in fault path.
> -		 */
> -		if (((in_pf || smaps)) && vma->vm_ops->huge_fault)
> -			return orders;
> -		/* Only regular file is valid in collapse path */
> -		if (((!in_pf || smaps)) && file_thp_enabled(vma))
> -			return orders;
> -		return 0;
> -	}
> +	if (!vma_is_anonymous(vma))
> +		return __file_vma_forces_order_0(vma, vm_flags, type) ? 0 : orders;
> 
>   	if (vma_is_temporary_stack(vma))
>   		return 0;
>