Re: [PATCH RFC v3 4/4] mm: add PMD-level huge page support for remap_pfn_range()

["David Hildenbrand (Arm)" <david@kernel.org>]

On 2/28/26 08:09, Yin Tirui wrote:
> Add PMD-level huge page support to remap_pfn_range(), automatically
> creating huge mappings when prerequisites are satisfied (size, alignment,
> architecture support, etc.) and falling back to normal page mappings
> otherwise.
> 
> Implement special huge PMD splitting by utilizing the pgtable deposit/
> withdraw mechanism. When splitting is needed, the deposited pgtable is
> withdrawn and populated with individual PTEs created from the original
> huge mapping.
> 
> Signed-off-by: Yin Tirui <yintirui@huawei.com>
> ---

[...]

>  
>  	if (!vma_is_anonymous(vma)) {
>  		old_pmd = pmdp_huge_clear_flush(vma, haddr, pmd);
> +
> +		if (!vma_is_dax(vma) && vma_is_special_huge(vma)) {

These magical vma checks are really bad. This all needs a cleanup
(Lorenzo is doing some, hoping it will look better on top of that).

> +			pte_t entry;
> +
> +			if (!pmd_special(old_pmd)) {

If you are using pmd_special(), you are doing something wrong.

Hint: vm_normal_page_pmd() is usually what you want.

> +				zap_deposited_table(mm, pmd);
> +				return;
> +			}
> +			pgtable = pgtable_trans_huge_withdraw(mm, pmd);
> +			if (unlikely(!pgtable))
> +				return;
> +			pmd_populate(mm, &_pmd, pgtable);
> +			pte = pte_offset_map(&_pmd, haddr);
> +			entry = pfn_pte(pmd_pfn(old_pmd), pmd_pgprot(old_pmd));
> +			set_ptes(mm, haddr, pte, entry, HPAGE_PMD_NR);
> +			pte_unmap(pte);
> +
> +			smp_wmb(); /* make pte visible before pmd */
> +			pmd_populate(mm, pmd, pgtable);
> +			return;
> +		}
> +
>  		/*
>  		 * We are going to unmap this huge page. So
>  		 * just go ahead and zap it
>  		 */
>  		if (arch_needs_pgtable_deposit())
>  			zap_deposited_table(mm, pmd);
> -		if (!vma_is_dax(vma) && vma_is_special_huge(vma))
> -			return;
> +
>  		if (unlikely(pmd_is_migration_entry(old_pmd))) {
>  			const softleaf_t old_entry = softleaf_from_pmd(old_pmd);
>  
> diff --git a/mm/memory.c b/mm/memory.c
> index 07778814b4a8..affccf38cbcf 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -2890,6 +2890,40 @@ static int remap_pte_range(struct mm_struct *mm, pmd_t *pmd,
>  	return err;
>  }
>  
> +#ifdef CONFIG_ARCH_SUPPORTS_PMD_PFNMAP

Why exactly do we need arch support for that in form of a Kconfig.

Usually, we guard pmd support by CONFIG_TRANSPARENT_HUGEPAGE.

And then, we must check at runtime if PMD leaves are actually supported.

Luiz is working on a cleanup series:

https://lore.kernel.org/r/cover.1775679721.git.luizcap@redhat.com

pgtable_has_pmd_leaves() is what you would want to check.


> +static int remap_try_huge_pmd(struct mm_struct *mm, pmd_t *pmd,
> +			unsigned long addr, unsigned long end,
> +			unsigned long pfn, pgprot_t prot)

Use two-tab indent. (currently 3? :) )

Also, we tend to call these things now "pmd leaves". Call it
 "remap_try_pmd_leaf" or something even more expressive like

"remap_try_install_pmd_leaf()"

> +{
> +	pgtable_t pgtable;
> +	spinlock_t *ptl;
> +
> +	if ((end - addr) != PMD_SIZE)

	if (end - addr != PMD_SIZE)

Should work

> +		return 0;
> +
> +	if (!IS_ALIGNED(addr, PMD_SIZE))
> +		return 0;
> +

You could likely combine both things into a

	if (!IS_ALIGNED(addr | end, PMD_SIZE))

> +	if (!IS_ALIGNED(pfn, HPAGE_PMD_NR))

Another sign that you piggy-back on THP support ;)

> +		return 0;
> +
> +	if (pmd_present(*pmd) && !pmd_free_pte_page(pmd, addr))
> +		return 0;

Ripping out a page table?! That doesn't sound right :)

Why is that required? We shouldn't be doing that here. Gah.

Especially, without any pmd locks etc.

> +
> +	pgtable = pte_alloc_one(mm);
> +	if (unlikely(!pgtable))
> +		return 0;
> +
> +	mm_inc_nr_ptes(mm);
> +	ptl = pmd_lock(mm, pmd);
> +	set_pmd_at(mm, addr, pmd, pmd_mkspecial(pmd_mkhuge(pfn_pmd(pfn, prot))));
> +	pgtable_trans_huge_deposit(mm, pmd, pgtable);
> +	spin_unlock(ptl);
> +
> +	return 1;
> +}
> +#endif
> +
>  static inline int remap_pmd_range(struct mm_struct *mm, pud_t *pud,
>  			unsigned long addr, unsigned long end,
>  			unsigned long pfn, pgprot_t prot)
> @@ -2905,6 +2939,12 @@ static inline int remap_pmd_range(struct mm_struct *mm, pud_t *pud,
>  	VM_BUG_ON(pmd_trans_huge(*pmd));
>  	do {
>  		next = pmd_addr_end(addr, end);
> +#ifdef CONFIG_ARCH_SUPPORTS_PMD_PFNMAP
> +		if (remap_try_huge_pmd(mm, pmd, addr, next,
> +				pfn + (addr >> PAGE_SHIFT), prot)) {

Please provide a stub instead so we don't end up with ifdef in this code.

-- 
Cheers,

David