Re: [PATCH] mm/migrate_device: document folio_get requirement before frozen PMD split

[David Hildenbrand (Arm)]

On 3/9/26 20:11, Usama Arif wrote:
> 
> 
> On 09/03/2026 18:18, David Hildenbrand (Arm) wrote:
>> On 3/6/26 11:44, Usama Arif wrote:
>>> split_huge_pmd_address() with freeze=true splits a PMD migration entry
>>> into PTE migration entries, consuming one folio reference in the
>>> process.  The folio_get() before it provides this reference.
>>>
>>> Add a comment explaining this relationship and a VM_WARN_ON_ONCE to
>>> catch an unexpected refcount != 1 entry state.
>>>
>>> Suggested-by: Zi Yan <ziy@xxxxxxxxxx>
>>> Signed-off-by: Usama Arif <usama.arif@xxxxxxxxx>
>>> ---
>>>  mm/migrate_device.c | 5 +++++
>>>  1 file changed, 5 insertions(+)
>>>
>>> diff --git a/mm/migrate_device.c b/mm/migrate_device.c
>>> index 78c7acf024615..6fa2878848a7e 100644
>>> --- a/mm/migrate_device.c
>>> +++ b/mm/migrate_device.c
>>> @@ -908,6 +908,11 @@ static int migrate_vma_split_unmapped_folio(struct migrate_vma *migrate,
>>>  	unsigned long flags;
>>>  	int ret = 0;
>>>  
>>> +	VM_WARN_ON_ONCE(folio_ref_count(folio) != 1);
>>
>> Can't we have speculative references here? In general, asserting that
>> the refcount has an exact value (besides 0) is often shaky.
> 
> 
> I hope not at this point in code.
> 
> At this point, the folio is locked and unmapped (both done in migrate_vma_collect_huge_pmd()),
> and the present PMD was set to migration entry. It is isolated from LRU in
> migrate_device_unmap(). So the folio should not be visible to GUP or reclaim/compaction.
> Only anon, non-swapcache folios should reach here. So it won't run into any folio_try_get
> in page cache or swap cache.

We have other pfn walkers that can just temporary grab a reference, to
immediately back off. So it's not very reliable to depend on that.

> 
> The folio_get() done in migrate_vma_split_unmapped_folio() is consumed by
> split_huge_pmd_address(), and folio_split_unmapped() expects a folio_reference
> of 1 after this [1].
> 
> If its not considered good to assert a non zero refcount value, I can change the
> warning to a comment, but I think refcount should be 1 at this point, otherwise
> folio_split_unmapped will fail.

Well, yes. folio_split_unmapped() will handle this gracefully (as
documented), without triggering a warning.

So best to remove the VM_WARN_ON_ONCE().

-- 
Cheers,

David