Re: [PATCH 07/12] mm: hugetlb_vmemmap: simplify shared tail page handling

[Muchun Song]

On 2026/10/23 10:20, Lance Yang wrote:
> On Tue, Oct 20, 2026 at 07:58:19PM +0800, Muchun Song wrote:
> > The remap walker currently carries a shared tail page selected by its
> > caller, even though the optimize and restore callbacks can derive the
> > same information from the folio being processed.  This adds redundant
> > state to the walker and couples its interface to caller-side preparation.
> >
> > Derive the shared mapping state in the callbacks instead, keeping the
> > mapping policy with the PTE being processed and simplifying the remap
> > interface.
> >
> > No functional change intended.
> >
> > Signed-off-by: Muchun Song <songmuchun@xxxxxxxxxxxxx>
> > ---
> > include/linux/vmemmap-optimization.h |  6 +-
> > mm/hugetlb_vmemmap.c                 | 89 ++++++++++++----------------
> > mm/mm_init.c                         |  6 +-
> > mm/sparse-vmemmap.c                  |  5 +-
> > 4 files changed, 49 insertions(+), 57 deletions(-)
> >
> > diff --git a/include/linux/vmemmap-optimization.h b/include/linux/vmemmap-optimization.h
> > index 065840f3c241..858311a71b0c 100644
> > --- a/include/linux/vmemmap-optimization.h
> > +++ b/include/linux/vmemmap-optimization.h
> > @@ -91,11 +91,13 @@ static inline struct page *vmemmap_shared_tail_page(unsigned int order,
> > }
> > #endif /* CONFIG_VMEMMAP_OPTIMIZATION */
> >
> > -static inline bool vmemmap_optimizable_pfn(unsigned long pfn)
> > +static inline bool vmemmap_optimizable_pfn(unsigned long pfn, unsigned int order)
> > {
> > -	const unsigned int order = pfn_to_section_compound_order(pfn);
> > 	const unsigned long nr_pages = 1UL << order;
> >
> > +	if (!IS_ENABLED(CONFIG_VMEMMAP_OPTIMIZATION))
> > +		return false;
> > +
> > 	if (!is_power_of_2(sizeof(struct page)))
> > 		return false;
> >
> > diff --git a/mm/hugetlb_vmemmap.c b/mm/hugetlb_vmemmap.c
> > index 18b72d553955..8fac61b7d74a 100644
> > --- a/mm/hugetlb_vmemmap.c
> > +++ b/mm/hugetlb_vmemmap.c
> > @@ -24,22 +24,18 @@
> >   * struct vmemmap_remap_walk - walk vmemmap page table
> >   *
> >   * @remap_pte:		called for each lowest-level entry (PTE).
> > - * @nr_walked:		the number of walked pte.
> > - * @vmemmap_tail:	the page to be installed as non-first in the vmemmap range
> > + * @nr_walked:		the number of PTEs successfully remapped.
> >   * @vmemmap_pages:	the list head of the vmemmap pages that can be freed
> >   *			or is mapped from.
> >   * @flags:		used to modify behavior in vmemmap page table walking
> >   *			operations.
> >   */
> > struct vmemmap_remap_walk {
> > -	void			(*remap_pte)(pte_t *pte, unsigned long addr,
> > +	int			(*remap_pte)(pte_t *pte, unsigned long addr,
> > 					     struct vmemmap_remap_walk *walk);
> > -
> > 	unsigned long		nr_walked;
> > -	struct page		*vmemmap_tail;
> > 	struct list_head	*vmemmap_pages;
> >
> > -
> > /* Skip the TLB flush while updating the vmemmap page tables. */
> > #define VMEMMAP_REMAP_NO_TLB_FLUSH	BIT(0)
> > 	unsigned long		flags;
> > @@ -140,12 +136,14 @@ static int vmemmap_pmd_entry(pmd_t *pmd, unsigned long addr,
> > static int vmemmap_pte_entry(pte_t *pte, unsigned long addr,
> > 			     unsigned long next, struct mm_walk *walk)
> > {
> > +	int ret;
> > 	struct vmemmap_remap_walk *vmemmap_walk = walk->private;
> >
> > -	vmemmap_walk->remap_pte(pte, addr, vmemmap_walk);
> > -	vmemmap_walk->nr_walked++;
> > +	ret = vmemmap_walk->remap_pte(pte, addr, vmemmap_walk);
> > +	if (!ret)
> > +		vmemmap_walk->nr_walked++;
> >
> > -	return 0;
> > +	return ret;
> > }
> >
> > static const struct mm_walk_ops vmemmap_remap_ops = {
> > @@ -213,51 +211,51 @@ static struct page *vmemmap_dup_page_from_list(const struct page *src,
> > 	return dst;
> > }
> >
> > -static void vmemmap_optimize_pte(pte_t *pte, unsigned long addr,
> > -				 struct vmemmap_remap_walk *walk)
> > +static inline struct page *folio_vmemmap_shared_tail_page(const struct folio *folio)
> > {
> > -	struct page *page = pte_page(ptep_get(pte));
> > -	pte_t entry;
> > -
> > -	/* Remapping the head page requires r/w */
> > -	if (unlikely(walk->nr_walked == 0)) {
> > -		struct page *head;
> > +	return vmemmap_shared_tail_page(folio_order(folio), folio_zone(folio));
> > +}
> >
> > -		VM_WARN_ON_ONCE(!PageHead((const struct page *)addr));
> > +static int vmemmap_optimize_pte(pte_t *pte, unsigned long addr,
> > +				struct vmemmap_remap_walk *walk)
> > +{
> > +	pte_t entry;
> > +	struct page *src = pte_page(ptep_get(pte)), *dst;
> > +	const struct page *page = (const struct page *)addr;
> > +	const struct folio *folio = page_folio(page);
> >
> > -		head = vmemmap_dup_page_from_list(page, walk->vmemmap_pages);
> > -		entry = mk_pte(head, PAGE_KERNEL);
> > +	if (likely(vmemmap_optimizable_pfn(page_to_pfn(page), folio_order(folio)))) {
> > +		dst = folio_vmemmap_shared_tail_page(folio);
> > +		if (!dst)
> > +			return -ENOMEM;
> One thing caught my eye here ... shared tail allocation can now fail after
> we've already remapped the head.

Right. Once the shared page has been allocated successfully, this
shouldn't be a common case.

> IIUC, nothing to worry about there, though :) since rollback keeps the
> new head mapped and flushes before clearing the optimized flag or freeing
> the old backing page.

Yes.

> [...]
>
> Reviewed-by: Lance Yang <lance.yang@xxxxxxxxx>

Thanks.