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

[Balbir Singh]

On 7/14/26 8:24 AM, Alistair Popple wrote:
> 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.
> 

I had a patch to add tracepoints to the code and some of that instrumentation
included this data. A combination of the trace points + hmm_tests will help us
get some data to start with.

Balbir

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