Re: [PATCH v2 7/7] mm/rmap: batch the unmap of large folios in try_to_migrate_one()

[Garg, Shivank]

On Wed, 2026-08-19 at 17:42 +0800, Huang, Ying wrote:
> Lance Yang <lance.yang@xxxxxxxxx> writes:
> 
> > On Tue, Aug 18, 2026 at 04:55:06PM +0800, Miaohe Lin wrote:
> > > On 2026/8/17 17:14, Lance Yang wrote:
> > > > +Cc Miaohe
> > > > 
> > > > On Thu, Aug 13, 2026 at 04:23:18AM +0000, Shivank Garg wrote:
> > > > > try_to_migrate_one() converts present PTEs to migration entries one at a
> > > > > time. For a PTE-mapped large folio, this repeat calls to ptep clear+flush,
> > > > > the migration entry build and set, folio_remove_rmap_pte() and folio_put(),
> > > > > each re-entering page_vma_mapped_walk() once per base page (256 times for
> > > > > 1M folio).
> > > > > 
> > > > > Mirror try_to_unmap_one() to introduce folio_migrate_pte_batch() to detect
> > > > > eligible batch for PTEs mapping conseuctive subpages of a large folios,
> > > > > and convert the whole batch in one shot using the batched helpers.
> > > > > 
> > > > > A side-effect of this change is trace_set_migration_pte() will record
> > > > > one event per batched run instead of earlier behavior of one per base page.
> > > > > 
> > > > > Signed-off-by: Shivank Garg <shivankg@xxxxxxx>
> > > > > ---
> > > > > mm/rmap.c | 115 ++++++++++++++++++++++++++++++++++++++++++++++----------------
> > > > > 1 file changed, 86 insertions(+), 29 deletions(-)
> > > > > 
> > > > > diff --git a/mm/rmap.c b/mm/rmap.c
> > > > > index 35752a70f3a0..63b885c0b7ef 100644
> > > > > --- a/mm/rmap.c
> > > > > +++ b/mm/rmap.c
> > > > > @@ -2675,6 +2675,44 @@ static bool try_to_migrate_hugetlb_one(struct folio *folio,
> > > > > 	return ret;
> > > > > }
> > > > > 
> > > > > +static inline unsigned int folio_migrate_pte_batch(struct folio *folio,
> > > > > +		struct page_vma_mapped_walk *pvmw, pte_t pte,
> > > > > +		struct page *subpage, bool anon_exclusive)
> > > > > +{
> > > > > +	unsigned long end_addr, addr = pvmw->address;
> > > > > +	struct vm_area_struct *vma = pvmw->vma;
> > > > > +	unsigned int max_nr, nr;
> > > > > +
> > > > > +#ifdef __HAVE_ARCH_UNMAP_ONE
> > > > > +	/* Cannot batch unmap if arch_unmap_one() is defined. */
> > > > > +	return 1;
> > > > > +#endif
> > > > > +
> > > > > +	if (!folio_test_large(folio))
> > > > > +		return 1;
> > > > > +	if (folio_is_zone_device(folio) || folio_test_has_hwpoisoned(folio))
> > > > > +		return 1;
> > > > > +	if (pte_unused(pte))
> > > > > +		return 1;
> > > > > +
> > > > > +	/* We may only batch within a single VMA and a single page table. */
> > > > > +	end_addr = pmd_addr_end(addr, vma->vm_end);
> > > > > +	max_nr = (end_addr - addr) >> PAGE_SHIFT;
> > > > 
> > > > Hmm ... can this still batch over a poisoned tail page?
> > > > 
> > > > memory_failure() sets PageHWPoison() before taking folio lock, but
> > > > cannot set PG_has_hwpoisoned until it acquires and releases that lock.
> > > > 
> > > > So tail page can already be poisoned while folio_test_has_hwpoisoned()
> > > > still returns false ... no?
> > > 
> > > When memory error hits thp pages, memory_failure() first set PG_has_hwpoisoned and
> > > then tries to split thp pages. And try_to_migrate() will be called to set migration
> > > entries for anon pages. Does folio_migrate_pte_batch() work on this case? If so, the
> > > folio_test_has_hwpoisoned() check above could catch the bad pages?
> > > 
> > > Or do you worry about the scene that meory error hits a thp while it's under migration?
> > 
> > Yeah, latter case is exactly what I meant.
> > 
> > try_to_migrate() is called with folio lock held. If migration already
> > owns the lock, memory_failure() can set PageHWPoison() on a tail page and
> > then block in folio_lock(), before reaching folio_set_has_hwpoisoned().
> > try_to_migrate_one() may meanwhile start from a healthy subpage, see
> > PageHWPoison(subpage) clear and PG_has_hwpoisoned still clear, then batch
> > across poisoned tail and install a normal migration entry for it.
> > 
> > Once memory_failure() publishes PG_has_hwpoisoned, the folio-level check
> > does stop batching, as you said. It's just this window before publication
> > that worries me ...
> 
> I agree with you that we should check PageHWPoison() for each subpage
> before batching.
> 
> However, IIUC, your suggestion can only reduce the race window instead
> of avoiding it completely. PageHWPoison() may be set even after checking
> it anyway.
> 


Thank you Lance, Miaohe, Ying for your analysis.
I agree on it.

In my understanding, following happens:

Even before  memory_failure() takes folio lock, it takes folio reference. 

memory_failure() -> get_hwpoison_page() -> bump the refcount

Now,
__migrate_folio() checks for:
	folio_ref_count(src) != folio_expected_ref_count(src) + 1

This would abort the migration.

Even if it pass that window, folio_mc_copy() does machine-check safe copy
and would catch the poison and return -EHWPOISON.

So, this limits the consequences.

For correctness, I'll have poison check for each subpage in
folio_migrate_pte_batch().

Thanks,
Shivank


> >