Re: [PATCH 1/8] mm/huge_memory: simplify vma_is_specal_huge()

[Qi Zheng <qi.zheng@linux.dev>]

On 3/19/26 4:39 AM, Lorenzo Stoakes (Oracle) wrote:
> This function is confused - it overloads the term 'special' yet again,
> checks for DAX but in many cases the code explicitly excludes DAX before
> invoking the predicate.
> 
> It also unnecessarily checks for vma->vm_file - this has to be present for
> a driver to have set VMA_MIXEDMAP_BIT or VMA_PFNMAP_BIT.
> 
> In fact, a far simpler form of this is to reverse the DAX predicate and
> return false if DAX is set.
> 
> This makes sense from the point of view of 'special' as in
> vm_normal_page(), as DAX actually does potentially have retrievable folios.
> 
> Also there's no need to have this in mm.h so move it to huge_memory.c.
> 
> Signed-off-by: Lorenzo Stoakes (Oracle) <ljs@kernel.org>
> ---
>   include/linux/huge_mm.h |  4 ++--
>   include/linux/mm.h      | 16 ----------------
>   mm/huge_memory.c        | 30 +++++++++++++++++++++++-------
>   3 files changed, 25 insertions(+), 25 deletions(-)
> 
> diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h
> index bd7f0e1d8094..61fda1672b29 100644
> --- a/include/linux/huge_mm.h
> +++ b/include/linux/huge_mm.h
> @@ -83,7 +83,7 @@ extern struct kobj_attribute thpsize_shmem_enabled_attr;
>    * file is never split and the MAX_PAGECACHE_ORDER limit does not apply to
>    * it.  Same to PFNMAPs where there's neither page* nor pagecache.
>    */
> -#define THP_ORDERS_ALL_SPECIAL		\
> +#define THP_ORDERS_ALL_SPECIAL_DAX	\

As mentioned in the comments, the pfnmap case is also include in the
'special' case, right?

>   	(BIT(PMD_ORDER) | BIT(PUD_ORDER))
>   #define THP_ORDERS_ALL_FILE_DEFAULT	\
>   	((BIT(MAX_PAGECACHE_ORDER + 1) - 1) & ~BIT(0))
> @@ -92,7 +92,7 @@ extern struct kobj_attribute thpsize_shmem_enabled_attr;
>    * Mask of all large folio orders supported for THP.
>    */
>   #define THP_ORDERS_ALL	\
> -	(THP_ORDERS_ALL_ANON | THP_ORDERS_ALL_SPECIAL | THP_ORDERS_ALL_FILE_DEFAULT)
> +	(THP_ORDERS_ALL_ANON | THP_ORDERS_ALL_SPECIAL_DAX | THP_ORDERS_ALL_FILE_DEFAULT)
>   
>   enum tva_type {
>   	TVA_SMAPS,		/* Exposing "THPeligible:" in smaps. */
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 6f0a3edb24e1..50d68b092204 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -5077,22 +5077,6 @@ long copy_folio_from_user(struct folio *dst_folio,
>   			   const void __user *usr_src,
>   			   bool allow_pagefault);
>   
> -/**
> - * vma_is_special_huge - Are transhuge page-table entries considered special?
> - * @vma: Pointer to the struct vm_area_struct to consider
> - *
> - * Whether transhuge page-table entries are considered "special" following
> - * the definition in vm_normal_page().
> - *
> - * Return: true if transhuge page-table entries should be considered special,
> - * false otherwise.
> - */
> -static inline bool vma_is_special_huge(const struct vm_area_struct *vma)
> -{
> -	return vma_is_dax(vma) || (vma->vm_file &&
> -				   (vma->vm_flags & (VM_PFNMAP | VM_MIXEDMAP)));
> -}
> -
>   #endif /* CONFIG_TRANSPARENT_HUGEPAGE || CONFIG_HUGETLBFS */
>   
>   #if MAX_NUMNODES > 1
> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> index 3fc02913b63e..f76edfa91e96 100644
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -100,6 +100,14 @@ static inline bool file_thp_enabled(struct vm_area_struct *vma)
>   	return !inode_is_open_for_write(inode) && S_ISREG(inode->i_mode);
>   }
>   
> +/* If returns true, we are unable to access the VMA's folios. */
> +static bool vma_is_special_huge(struct vm_area_struct *vma)
> +{
> +	if (vma_is_dax(vma))
> +		return false;
> +	return vma_test_any(vma, VMA_PFNMAP_BIT, VMA_MIXEDMAP_BIT);
> +}
> +
>   unsigned long __thp_vma_allowable_orders(struct vm_area_struct *vma,
>   					 vm_flags_t vm_flags,
>   					 enum tva_type type,
> @@ -113,8 +121,8 @@ unsigned long __thp_vma_allowable_orders(struct vm_area_struct *vma,
>   	/* Check the intersection of requested and supported orders. */
>   	if (vma_is_anonymous(vma))
>   		supported_orders = THP_ORDERS_ALL_ANON;
> -	else if (vma_is_special_huge(vma))
> -		supported_orders = THP_ORDERS_ALL_SPECIAL;
> +	else if (vma_is_dax(vma) || vma_is_special_huge(vma))
> +		supported_orders = THP_ORDERS_ALL_SPECIAL_DAX;
>   	else
>   		supported_orders = THP_ORDERS_ALL_FILE_DEFAULT;
>   
> @@ -2431,7 +2439,7 @@ int zap_huge_pmd(struct mmu_gather *tlb, struct vm_area_struct *vma,
>   						tlb->fullmm);
>   	arch_check_zapped_pmd(vma, orig_pmd);
>   	tlb_remove_pmd_tlb_entry(tlb, pmd, addr);
> -	if (!vma_is_dax(vma) && vma_is_special_huge(vma)) {
> +	if (vma_is_special_huge(vma)) {
>   		if (arch_needs_pgtable_deposit())
>   			zap_deposited_table(tlb->mm, pmd);
>   		spin_unlock(ptl);
> @@ -2933,7 +2941,7 @@ int zap_huge_pud(struct mmu_gather *tlb, struct vm_area_struct *vma,
>   	orig_pud = pudp_huge_get_and_clear_full(vma, addr, pud, tlb->fullmm);
>   	arch_check_zapped_pud(vma, orig_pud);
>   	tlb_remove_pud_tlb_entry(tlb, pud, addr);
> -	if (!vma_is_dax(vma) && vma_is_special_huge(vma)) {
> +	if (vma_is_special_huge(vma)) {
>   		spin_unlock(ptl);
>   		/* No zero page support yet */
>   	} else {
> @@ -3084,7 +3092,7 @@ static void __split_huge_pmd_locked(struct vm_area_struct *vma, pmd_t *pmd,
>   		 */
>   		if (arch_needs_pgtable_deposit())
>   			zap_deposited_table(mm, pmd);
> -		if (!vma_is_dax(vma) && vma_is_special_huge(vma))
> +		if (vma_is_special_huge(vma))
>   			return;
>   		if (unlikely(pmd_is_migration_entry(old_pmd))) {
>   			const softleaf_t old_entry = softleaf_from_pmd(old_pmd);
> @@ -4645,8 +4653,16 @@ static void split_huge_pages_all(void)
>   
>   static inline bool vma_not_suitable_for_thp_split(struct vm_area_struct *vma)
>   {
> -	return vma_is_special_huge(vma) || (vma->vm_flags & VM_IO) ||
> -		    is_vm_hugetlb_page(vma);
> +	if (vma_is_dax(vma))
> +		return true;
> +	if (vma_is_special_huge(vma))
> +		return true;
> +	if (vma_test(vma, VMA_IO_BIT))
> +		return true;
> +	if (is_vm_hugetlb_page(vma))
> +		return true;
> +
> +	return false;
>   }
>   
>   static int split_huge_pages_pid(int pid, unsigned long vaddr_start,