Re: [PATCH v2 3/7] mm/migrate: split remove_migration_pte_hugetlb() out of remove_migration_pte()

[Garg, Shivank]

On Wed, 2026-08-19 at 12:24 +0100, Karim Manaouil wrote:
> Hi Shivank,
> 
> On Thu, Aug 13, 2026 at 04:23:14AM +0000, Shivank Garg wrote:
> > remove_migration_pte() interleaves hugetlb handling with the regular
> > folio path. Move it into a dedicated callback selected by
> > remove_migration_ptes(), leaving the generic callback focused on regular
> > folios ahead of PTE batching.
> > 
> > With hugetlb folios routed separately, simplify the PMD mapping check to
> > a one-time warning.
> > 
> > Signed-off-by: Shivank Garg <shivankg@xxxxxxx>
> > ---
> >  mm/migrate.c | 97 +++++++++++++++++++++++++++++++++++++++---------------------
> >  1 file changed, 63 insertions(+), 34 deletions(-)
> > 
> > diff --git a/mm/migrate.c b/mm/migrate.c
> > index a3362cc9ef66..ee1b8a55a2a4 100644
> > --- a/mm/migrate.c
> > +++ b/mm/migrate.c
> > @@ -375,6 +375,57 @@ static pte_t migration_entry_to_pte(struct folio *folio, struct page *new,
> >  	return pte;
> >  }
> >  
> > +/*
> > + * Restore a potential migration pte to a working pte entry for hugetlb folios.
> > + */
> > +#ifdef CONFIG_HUGETLB_PAGE
> > +static bool remove_migration_pte_hugetlb(struct folio *folio,
> > +		struct vm_area_struct *vma, unsigned long addr, void *arg)
> > +{
> > +	struct rmap_walk_arg *rmap_walk_arg = arg;
> > +	DEFINE_FOLIO_VMA_WALK(pvmw, rmap_walk_arg->folio, vma, addr, PVMW_SYNC | PVMW_MIGRATION);
> > +	struct hstate *h = hstate_vma(vma);
> > +	unsigned int shift = huge_page_shift(h);
> > +	unsigned long psize = huge_page_size(h);
> > +	struct page *new = folio_page(folio, 0);
> > +	rmap_t rmap_flags = RMAP_NONE;
> > +	pte_t old_pte, pte;
> > +	softleaf_t entry;
> > +
> > +	/* There is only a single mapping in a VMA. */
> 
> I don't think this comment is necessary. page_vma_mapped_walk() clearly
> explains what happens with HugeTLB entries.
> 
> > +	if (!page_vma_mapped_walk(&pvmw))
> > +		return true;
> > +

I carried this over from try_to_unmap_poisoned_hugetlb_one(), which has asimilar check. Dropping it here while retaining it at the other call site
would seem odd, so I kept it. I have no strong opinion, though.


> > +	old_pte = huge_ptep_get(vma->vm_mm, pvmw.address, pvmw.pte);
> > +	entry = softleaf_from_pte(old_pte);
> > +	folio_get(folio);
> > +	pte = migration_entry_to_pte(folio, new, entry, old_pte, vma,
&rmap_flags);
> > +	pte = arch_make_huge_pte(pte, shift, vma->vm_flags);
> > +	if (folio_test_anon(folio))
> > +		hugetlb_add_anon_rmap(folio, vma, pvmw.address,
rmap_flags);
> > +	else
> > +		hugetlb_add_file_rmap(folio);
> > +	set_huge_pte_at(vma->vm_mm, pvmw.address, pvmw.pte, pte, psize);
> 
> I know nothing in HugeTLB, but can't they be mapped as 4KiB entries? In
> which case we have to use
> 
> 	while (page_vma_mapped_walk(&pvmw)) {
> 		...
> 	}
> 


The code/comment in page_vma_mapped_walk() show that it handles the only
possible mapping and returns not_found() on the next iteration. Therefore,
even if we used a while loop, it would run only once.

> It seems like it's not the case from my little investigation, but
> I'll keep it here just to be sure.



Thanks,
Shivank