Re: [PATCH 11/18] hugetlb_vmemmap: Use try_populate_vmemmap_pmd for replacing in-use PMDs

[Catalin Marinas]

On Wed, Jul 08, 2026 at 03:11:21AM +0000, James Houghton wrote:
> diff --git a/arch/arm64/include/asm/pgalloc.h b/arch/arm64/include/asm/pgalloc.h
> index 1b4509d3382c..c8946250d431 100644
> --- a/arch/arm64/include/asm/pgalloc.h
> +++ b/arch/arm64/include/asm/pgalloc.h
> @@ -121,4 +121,13 @@ pmd_populate(struct mm_struct *mm, pmd_t *pmdp, pgtable_t ptep)
>  		       PMD_TYPE_TABLE | PMD_TABLE_AF | PMD_TABLE_PXN);
>  }
>  
> +#define __HAVE_ARCH_TRY_POPULATE_VMEMMAP_PMD
> +static inline int try_populate_vmemmap_pmd(pmd_t *pmdp, pte_t *pgtable,
> +					   unsigned long addr)
> +{
> +	/* BBML2_NOABORT is required. Its presence has been checked. */
> +	pmd_populate_kernel(&init_mm, pmdp, pgtable);
> +	return 0;
> +}

This patch adds the same function in 4 different arch files. Can we not
have a single generic implementation in linux/pgalloc.h, bracketed by
some ARCH_WANTS_* (macro or config)?

I think the same goes for patch 4 with try_update_vmemmap_pte(), we can
reduce the code duplication.

> diff --git a/include/linux/pgalloc.h b/include/linux/pgalloc.h
> index 9174fa59bbc5..ed446d95ca37 100644
> --- a/include/linux/pgalloc.h
> +++ b/include/linux/pgalloc.h
> @@ -26,4 +26,24 @@
>  			arch_sync_kernel_mappings(addr, addr);		\
>  	} while (0)
>  
> +#ifndef __HAVE_ARCH_TRY_POPULATE_VMEMMAP_PMD
> +/*
> + * try_populate_vmemmap_pmd - Populate a PMD that is in use by the vmemmap.
> + * @addr: Base address of the remapped PMD.
> + * @pmdp: Page table pointer to be overwritten.
> + * @pgtable: Pointer to the page table that the new PMD will point to.
> + *
> + * This function is only to be used to update PMDs that map the vmemmap to
> + * point to a page of already-populated PTEs that map the same pages.
> + *
> + * Implementations of this function must ensure that, while the update is taking
> + * place, CPUs will not fault on the remapped virtual address range.
> + */
> +static inline int try_populate_vmemmap_pmd(pmd_t *pmdp, pte_t *pgtable,
> +					   unsigned long addr)

Nit: if we follow try_update_vmemmap_pte(), we could place 'addr' first.
It matches pgd_populate_kernel() as well in this file.

> +{
> +	return -EOPNOTSUPP;
> +}
> +#endif
> +
>  #endif /* _LINUX_PGALLOC_H */
> diff --git a/mm/hugetlb_vmemmap.c b/mm/hugetlb_vmemmap.c
> index 977249e22ed0..b445febac0d2 100644
> --- a/mm/hugetlb_vmemmap.c
> +++ b/mm/hugetlb_vmemmap.c
> @@ -55,6 +55,7 @@ static int vmemmap_split_pmd(pmd_t *pmd, struct page *head, unsigned long start,
>  			     struct vmemmap_remap_walk *walk)
>  {
>  	pmd_t __pmd;
> +	int ret;
>  	int i;
>  	unsigned long addr = start;
>  	pte_t *pgtable;
> @@ -74,8 +75,15 @@ static int vmemmap_split_pmd(pmd_t *pmd, struct page *head, unsigned long start,
>  		set_pte_at(&init_mm, addr, pte, entry);
>  	}
>  
> +	ret = 0;
>  	spin_lock(&init_mm.page_table_lock);
>  	if (likely(pmd_leaf(*pmd))) {
> +		/* Make pte visible before pmd. See comment in pmd_install(). */
> +		smp_wmb();
> +		ret = try_populate_vmemmap_pmd(pmd, pgtable, start);
> +		if (ret)
> +			goto free;
> +
>  		/*
>  		 * Higher order allocations from buddy allocator must be able to
>  		 * be treated as independent small pages (as they can be freed
> @@ -84,21 +92,17 @@ static int vmemmap_split_pmd(pmd_t *pmd, struct page *head, unsigned long start,
>  		if (!PageReserved(head))
>  			split_page(head, get_order(PMD_SIZE));
>  
> -		/* Make pte visible before pmd. See comment in pmd_install(). */
> -		smp_wmb();
> -		/*
> -		 * On arm64, this requires BBML2_NOABORT. Its support has
> -		 * already been checked.
> -		 */
> -		pmd_populate_kernel(&init_mm, pmd, pgtable);
>  		if (!(walk->flags & VMEMMAP_SPLIT_NO_TLB_FLUSH))
>  			flush_tlb_kernel_range(start, start + PMD_SIZE);
> -	} else {
> -		pte_free_kernel(&init_mm, pgtable);
> -	}
> -	spin_unlock(&init_mm.page_table_lock);
> +	} else
> +		goto free;
>  
> -	return 0;
> +out:
> +	spin_unlock(&init_mm.page_table_lock);
> +	return ret;
> +free:
> +	pte_free_kernel(&init_mm, pgtable);
> +	goto out;
>  }

Would 'free' first with fall through 'out' read better?

-- 
Catalin