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

[Zi Yan]

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.

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