Re: [PATCH v2 05/13] mm/migrate_device: don't collect a range twice

["Mika Penttilä" <mpenttil@redhat.com>]

On 11/14/26 07:09, Balbir Singh wrote:
> On Fri, Nov 13, 2026 at 09:45:23PM +0000, Lorenzo Stoakes (ARM) wrote:
>> From: Josef Bacik <josef@toxicpanda.com>
>>
>> migrate_vma_collect_pmd() drops the page table lock to split a large folio
>> which it finds in the middle of a page table, and then starts over at the
>> "again" label. The entries for [start, addr) are in migrate->src[] and
>> migrate->dst[] by then.
>>
>> While the page table lock is dropped the page table can go away, in which
>> case the restarted walk invokes migrate_vma_collect_huge_pmd(). However
>> this is passed start rather than addr, so [start, addr) is collected a
>> second time.
>>
>> This can result in migrate->npages exceeding the number of pages the
>> caller asked for, and therefore in writes past the end of the
>> migrate->src[] and migrate->dst[] arrays.
>>
>> Fix this by only handing over what has not yet been collected,
>> page-by-page, and flush the TLB for those entries which have been
>> previously replaced, similar to the other returns in the function.
>>
>> Fixes: 022a12deda53 ("mm/migrate_device: handle partially mapped folios during collection")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
>> [ ljs: cleanup commit message ]
>> Tested-by: Gregory Price (Meta) <gourry@gourry.net>
>> Reviewed-by: Gregory Price (Meta) <gourry@gourry.net>
>> Signed-off-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
>> ---
>>  mm/migrate_device.c | 8 ++++++--
>>  1 file changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/mm/migrate_device.c b/mm/migrate_device.c
>> index 9e5dab8e1b77..28ed706109da 100644
>> --- a/mm/migrate_device.c
>> +++ b/mm/migrate_device.c
>> @@ -257,13 +257,17 @@ static int migrate_vma_collect_pmd(pmd_t *pmdp,
>>  	pte_t *ptep;
>>  
>>  again:
>> +	/* [start, addr) is collected if we get here after splitting a folio */
>>  	if (pmd_trans_huge(*pmdp) || !pmd_present(*pmdp)) {
>> -		int ret = migrate_vma_collect_huge_pmd(pmdp, start, end, walk, fault_folio);
>> +		int ret = migrate_vma_collect_huge_pmd(pmdp, addr, end, walk, fault_folio);
>>  
>>  		if (ret == -EAGAIN)
>>  			goto again;
>> -		if (ret == 0)
>> +		if (ret == 0) {
>> +			if (unmapped)
>> +				flush_tlb_range(walk->vma, start, end);
>>  			return 0;
>> +		}
>>  	}
>>  
>>  	ptep = pte_offset_map_lock(mm, pmdp, start, &ptl);
>>
> Is the intended race a concurrent MADV_DONTNEED reclaiming the PTE table while
> PTL is dropped for the split? That would leave pmd_none() true on retry, entering
> migrate_vma_collect_huge_pmd() through !pmd_present() and taking its hole path.

I think this is "ok" for the fix, the hole is collected from [addr, end). 

But couple of things follow. It can't be possibly collected as
MIGRATE_PFN_COMPOUND entry, only small pages. And the collected entries
in [start, addr) are wrong ones.  Finalize will recover, but the
migration will fail for those. These all are handled in the HMM walk
based migration series.

--Mika


>
> Thanks,
> Balbir
>