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

[David Hildenbrand (Arm)]

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?

>>
>> 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?

-- 
Cheers,

David