1. Patchew
  2. linux
  3. View series

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

Usama Arif posted 1 patch 1 month ago
There is a newer version of this series
mm/migrate_device.c | 5 +++++
1 file changed, 5 insertions(+)
[PATCH] mm/migrate_device: document folio_get requirement before frozen PMD split
Posted by Usama Arif 1 month ago 
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@nvidia.com>
Signed-off-by: Usama Arif <usama.arif@linux.dev>
---
 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);
+	/*
+	 * take a reference, since split_huge_pmd_address() with freeze = true
+	 * drops a reference at the end.
+	 */
 	folio_get(folio);
 	split_huge_pmd_address(migrate->vma, addr, true);
 	ret = folio_split_unmapped(folio, 0);
-- 
2.47.3
Re: [PATCH] mm/migrate_device: document folio_get requirement before frozen PMD split
Posted by David Hildenbrand (Arm) 1 month ago 
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@nvidia.com>
> Signed-off-by: Usama Arif <usama.arif@linux.dev>
> ---
>  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.

> +	/*
> +	 * take a reference, since split_huge_pmd_address() with freeze = true
> +	 * drops a reference at the end.
> +	 */
>  	folio_get(folio);
>  	split_huge_pmd_address(migrate->vma, addr, true);
>  	ret = folio_split_unmapped(folio, 0);


-- 
Cheers,

David
Re: [PATCH] mm/migrate_device: document folio_get requirement before frozen PMD split
Posted by Usama Arif 1 month ago 

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@nvidia.com>
>> Signed-off-by: Usama Arif <usama.arif@linux.dev>
>> ---
>>  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.

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.

[1] https://elixir.bootlin.com/linux/v6.19.6/source/mm/huge_memory.c#L4137

> 
>> +	/*
>> +	 * take a reference, since split_huge_pmd_address() with freeze = true
>> +	 * drops a reference at the end.
>> +	 */
>>  	folio_get(folio);
>>  	split_huge_pmd_address(migrate->vma, addr, true);
>>  	ret = folio_split_unmapped(folio, 0);
> 
>
Re: [PATCH] mm/migrate_device: document folio_get requirement before frozen PMD split
Posted by David Hildenbrand (Arm) 1 month ago 
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@nvidia.com>
>>> Signed-off-by: Usama Arif <usama.arif@linux.dev>
>>> ---
>>>  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
Re: [PATCH] mm/migrate_device: document folio_get requirement before frozen PMD split
Posted by Nico Pache 1 month ago 
On Fri, Mar 6, 2026 at 3:44 AM Usama Arif <usama.arif@linux.dev> 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@nvidia.com>
> Signed-off-by: Usama Arif <usama.arif@linux.dev>

LGTM! Thank you for investigating my concerns about this. I'm glad you
found the proper answer.

Reviewed-by: Nico Pache <npache@redhat.com>

> ---
>  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);
> +       /*
> +        * take a reference, since split_huge_pmd_address() with freeze = true
> +        * drops a reference at the end.
> +        */
>         folio_get(folio);
>         split_huge_pmd_address(migrate->vma, addr, true);
>         ret = folio_split_unmapped(folio, 0);
> --
> 2.47.3
>
Re: [PATCH] mm/migrate_device: document folio_get requirement before frozen PMD split
Posted by Zi Yan 1 month ago 
On 6 Mar 2026, at 5: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@nvidia.com>
> Signed-off-by: Usama Arif <usama.arif@linux.dev>
> ---
>  mm/migrate_device.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
Thanks for fixing the logic in my suggestion,

Reviewed-by: Zi Yan <ziy@nvidia.com>

Best Regards,
Yan, Zi

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.