Re: [PATCH] mm/migrate_device: avoid overflowing migrate_vma collection arrays

[Alistair Popple]

On 2026-07-14 at 01:39 +1000, Zi Yan <ziy@xxxxxxxxxx> wrote...
> On 13 Jul 2026, at 11:25, David Hildenbrand (Arm) wrote:
> 
> > On 7/13/26 17:20, Zi Yan wrote:
> >> On 13 Jul 2026, at 8:57, David Hildenbrand (Arm) wrote:
> >>
> >>> On 7/10/26 17:35, Zi Yan wrote:
> >>>>
> >>>> Not all the time. After a folio split and PTE lock is dropped, there are
> >>>> different cases:
> >>>>
> >>>> 1. no page table change, the collection grabs the PTE lock and continues;
> >>>
> >>> Yes.
> >>>
> >>>>
> >>>> 2. some PTE(s) is changed, the same as 1 and the collection continues;
> >>>
> >>> Yes.
> >>>
> >>>>
> >>>> 3. the PMD pointing to the PTE page table is changed to either a leaf
> >>>>    PMD or an invalid PMD, the collection cannot grab the PTE lock to
> >>>>    work on the remaming PTEs, since they are gone. For the collected
> >>>>    PFNs (they are mapped more than once) and folios with elevated
> >>>>    refcount (they are mapped once and unmapped here as an optimization),
> >>>>    they will be processed later in migrate_vma_unmap(). Since
> >>>>    migrate_vma_collect() is best effort, there is no need to revert and
> >>>>    try to recollect from the beginning (to get that possible large
> >>>>    folio or skip).
> >>>
> >>> Yes.
> >>>
> >>>>
> >>>> BTW, recollection will be more feasible if migrate_vma_collect() does
> >>>> not do unmap singly-mapped optimization, since in the case, no PTE is
> >>>> changed, we just need to reset migrate->->cpages and migrate->npages and
> >>>> restart from the beginning.
> >>>>
> >>>>
> >>>> Right. The PTE page table should be gone and the original PMD, pointing
> >>>> to the PTE page table, becomes a leaf PMD or an invalid PMD, since
> >>>> someone changed it when the PTE lock is dropped.
> >>>
> >>> Right, so I guess we'd have to remember how far to rollback, and rollback would
> >>> mean resetting migrate->->cpages and migrate->npages.
> >>
> >> For case 1 and 2, we do not roll back. For case 3, since the PMD is changed,
> >> we just roll back to the start address and set migrate->cpages and migrate->npages
> >> both to 0.
> >
> > Can't we walk multiple PMDs and have data already in there?
> 
> migrate_vma_collect() is the pmd_entry of struct mm_walk_ops, so it only
> walks a single PMD at a time.
> 
> >
> >>>
> >>> And for the entries we rollback, we have to decide whether to folio_put() and
> >>> whether to folio_unlock() [fault_folio != folio]. That's the confusing bit given
> >>> that the code is rather "interesting".
> >>
> >> IMHO, it might be much cleaner to remove the “unmap the mapping if the folio is
> >> only mapped once” optimization, so there is no need to roll back at all.
> >
> > I guess most folios are only mapped once. Do we have any numbers on that?
> 
> I will defer the question to Balbir, Alistair, and Matthew.

Unfortunately previous tests that I've done showed the optimisation is critical
to performance in the common case (ie. singly mapped folios).

I say unfortunately because I have similar feelings to others here about this
code :) I tested this a while ago because I was trying to clean up the collect
step by mostly removing it and replacing it with a hmm_range_fault() type thing,
but the optimisation proved too important due to unmap having to do another
rmap/page table walk.

I don't have the numbers on hand atm, but will recreate them just in case my
recollection is faulty.

 - Alistair

> BTW, for this patch, since there is no bug report on it, we would like to
> fix it properly without worrying about backport, right?
> 
> Best Regards,
> Yan, Zi