[PATCH] mm/migrate_device: Cleanup up PMD Checks and warnings

[PATCH] mm/migrate_device: Cleanup up PMD Checks and warnings

[Sunny Patel]

Remove the odd VM_WARN_ON_FOLIO(!folio, folio) usage and replace it
with a simpler VM_WARN_ON_ONCE(!folio) check.

Drop the redundant VM_WARN_ON_ONCE(!pmd_none(*pmdp) &&
!is_huge_zero_pmd(*pmdp)).

Refactor the PMD checks to use pmd_present(), making the control flow
clearer and avoiding duplicate condition checks.

Signed-off-by: Sunny Patel <nueralspacetech@gmail.com>
---
 mm/migrate_device.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/mm/migrate_device.c b/mm/migrate_device.c
index 8079676c8f1f..fe1c1999aa2d 100644
--- a/mm/migrate_device.c
+++ b/mm/migrate_device.c
@@ -807,8 +807,7 @@ static int migrate_vma_insert_huge_pmd_page(struct migrate_vma *migrate,
 	bool flush = false;
 	unsigned long i;
 
-	VM_WARN_ON_FOLIO(!folio, folio);
-	VM_WARN_ON_ONCE(!pmd_none(*pmdp) && !is_huge_zero_pmd(*pmdp));
+	VM_WARN_ON_ONCE(!folio);
 
 	if (!thp_vma_suitable_order(vma, addr, HPAGE_PMD_ORDER))
 		return -EINVAL;
@@ -865,12 +864,13 @@ static int migrate_vma_insert_huge_pmd_page(struct migrate_vma *migrate,
 	if (userfaultfd_missing(vma))
 		goto unlock_abort;
 
-	if (!pmd_none(*pmdp)) {
+	if (pmd_present(*pmdp)) {
 		if (!is_huge_zero_pmd(*pmdp))
 			goto unlock_abort;
 		flush = true;
-	} else if (!pmd_none(*pmdp))
+	} else if (!pmd_none(*pmdp)) {
 		goto unlock_abort;
+	}
 
 	add_mm_counter(vma->vm_mm, MM_ANONPAGES, HPAGE_PMD_NR);
 	folio_add_new_anon_rmap(folio, vma, addr, RMAP_EXCLUSIVE);
-- 
2.43.0

Re: [PATCH] mm/migrate_device: Cleanup up PMD Checks and warnings

[David Hildenbrand (Arm)]

On 4/14/26 16:13, Sunny Patel wrote:
> Remove the odd VM_WARN_ON_FOLIO(!folio, folio) usage and replace it
> with a simpler VM_WARN_ON_ONCE(!folio) check.
> 
> Drop the redundant VM_WARN_ON_ONCE(!pmd_none(*pmdp) &&
> !is_huge_zero_pmd(*pmdp)).
> 
> Refactor the PMD checks to use pmd_present(), making the control flow
> clearer and avoiding duplicate condition checks.
> 
> Signed-off-by: Sunny Patel <nueralspacetech@gmail.com>
> ---
>  mm/migrate_device.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/mm/migrate_device.c b/mm/migrate_device.c
> index 8079676c8f1f..fe1c1999aa2d 100644
> --- a/mm/migrate_device.c
> +++ b/mm/migrate_device.c
> @@ -807,8 +807,7 @@ static int migrate_vma_insert_huge_pmd_page(struct migrate_vma *migrate,
>  	bool flush = false;
>  	unsigned long i;
>  
> -	VM_WARN_ON_FOLIO(!folio, folio);
> -	VM_WARN_ON_ONCE(!pmd_none(*pmdp) && !is_huge_zero_pmd(*pmdp));
> +	VM_WARN_ON_ONCE(!folio);

I'd test for "page" here instead. But it doesn't matter as of today.

>  
>  	if (!thp_vma_suitable_order(vma, addr, HPAGE_PMD_ORDER))
>  		return -EINVAL;
> @@ -865,12 +864,13 @@ static int migrate_vma_insert_huge_pmd_page(struct migrate_vma *migrate,
>  	if (userfaultfd_missing(vma))
>  		goto unlock_abort;
>  
> -	if (!pmd_none(*pmdp)) {
> +	if (pmd_present(*pmdp)) {
>  		if (!is_huge_zero_pmd(*pmdp))
>  			goto unlock_abort;
>  		flush = true;
> -	} else if (!pmd_none(*pmdp))
> +	} else if (!pmd_none(*pmdp)) {
>  		goto unlock_abort;
> +	}
>  
>  	add_mm_counter(vma->vm_mm, MM_ANONPAGES, HPAGE_PMD_NR);
>  	folio_add_new_anon_rmap(folio, vma, addr, RMAP_EXCLUSIVE);

is_huge_zero_pmd() checks pmd_present(), so we didn't have a bug before.

We could also do:

if (is_huge_zero_pmd(*pmdp)) {
	flush = true;
} else if (!pmd_none(*pmdp)) {
	goto unlock_abort;
}


But I don't particularly care

Acked-by: David Hildenbrand (Arm) <david@kernel.org>

-- 
Cheers,

David

Re: [PATCH] mm/migrate_device: Cleanup up PMD Checks and warnings

[SeongJae Park]

On Thu, 16 Apr 2026 21:44:15 +0200 "David Hildenbrand (Arm)" <david@kernel.org> wrote:

> On 4/14/26 16:13, Sunny Patel wrote:
[...]
> > @@ -865,12 +864,13 @@ static int migrate_vma_insert_huge_pmd_page(struct migrate_vma *migrate,
> >  	if (userfaultfd_missing(vma))
> >  		goto unlock_abort;
> >  
> > -	if (!pmd_none(*pmdp)) {
> > +	if (pmd_present(*pmdp)) {
> >  		if (!is_huge_zero_pmd(*pmdp))
> >  			goto unlock_abort;
> >  		flush = true;
> > -	} else if (!pmd_none(*pmdp))
> > +	} else if (!pmd_none(*pmdp)) {
> >  		goto unlock_abort;
> > +	}
> >  
> >  	add_mm_counter(vma->vm_mm, MM_ANONPAGES, HPAGE_PMD_NR);
> >  	folio_add_new_anon_rmap(folio, vma, addr, RMAP_EXCLUSIVE);
> 
> is_huge_zero_pmd() checks pmd_present(), so we didn't have a bug before.
> 
> We could also do:
> 
> if (is_huge_zero_pmd(*pmdp)) {
> 	flush = true;
> } else if (!pmd_none(*pmdp)) {
> 	goto unlock_abort;
> }

Then we could even further remove the braces and reduce one more line, nice!


Thanks,
SJ

[...]

Re: [PATCH] mm/migrate_device: Cleanup up PMD Checks and warnings

[David Hildenbrand (Arm)]

On 4/17/26 01:52, SeongJae Park wrote:
> On Thu, 16 Apr 2026 21:44:15 +0200 "David Hildenbrand (Arm)" <david@kernel.org> wrote:
> 
>> On 4/14/26 16:13, Sunny Patel wrote:
> [...]
>>> @@ -865,12 +864,13 @@ static int migrate_vma_insert_huge_pmd_page(struct migrate_vma *migrate,
>>>  	if (userfaultfd_missing(vma))
>>>  		goto unlock_abort;
>>>  
>>> -	if (!pmd_none(*pmdp)) {
>>> +	if (pmd_present(*pmdp)) {
>>>  		if (!is_huge_zero_pmd(*pmdp))
>>>  			goto unlock_abort;
>>>  		flush = true;
>>> -	} else if (!pmd_none(*pmdp))
>>> +	} else if (!pmd_none(*pmdp)) {
>>>  		goto unlock_abort;
>>> +	}
>>>  
>>>  	add_mm_counter(vma->vm_mm, MM_ANONPAGES, HPAGE_PMD_NR);
>>>  	folio_add_new_anon_rmap(folio, vma, addr, RMAP_EXCLUSIVE);
>>
>> is_huge_zero_pmd() checks pmd_present(), so we didn't have a bug before.
>>
>> We could also do:
>>
>> if (is_huge_zero_pmd(*pmdp)) {
>> 	flush = true;
>> } else if (!pmd_none(*pmdp)) {
>> 	goto unlock_abort;
>> }
> 
> Then we could even further remove the braces and reduce one more line, nice!

Indeed :)

-- 
Cheers,

David

Re: [PATCH] mm/migrate_device: Cleanup up PMD Checks and warnings

[Huang, Ying]

"David Hildenbrand (Arm)" <david@kernel.org> writes:

> On 4/14/26 16:13, Sunny Patel wrote:
>> Remove the odd VM_WARN_ON_FOLIO(!folio, folio) usage and replace it
>> with a simpler VM_WARN_ON_ONCE(!folio) check.
>> 
>> Drop the redundant VM_WARN_ON_ONCE(!pmd_none(*pmdp) &&
>> !is_huge_zero_pmd(*pmdp)).
>> 
>> Refactor the PMD checks to use pmd_present(), making the control flow
>> clearer and avoiding duplicate condition checks.
>> 
>> Signed-off-by: Sunny Patel <nueralspacetech@gmail.com>
>> ---
>>  mm/migrate_device.c | 8 ++++----
>>  1 file changed, 4 insertions(+), 4 deletions(-)
>> 
>> diff --git a/mm/migrate_device.c b/mm/migrate_device.c
>> index 8079676c8f1f..fe1c1999aa2d 100644
>> --- a/mm/migrate_device.c
>> +++ b/mm/migrate_device.c
>> @@ -807,8 +807,7 @@ static int migrate_vma_insert_huge_pmd_page(struct migrate_vma *migrate,
>>  	bool flush = false;
>>  	unsigned long i;
>>  
>> -	VM_WARN_ON_FOLIO(!folio, folio);
>> -	VM_WARN_ON_ONCE(!pmd_none(*pmdp) && !is_huge_zero_pmd(*pmdp));
>> +	VM_WARN_ON_ONCE(!folio);
>
> I'd test for "page" here instead. But it doesn't matter as of today.
>
>>  
>>  	if (!thp_vma_suitable_order(vma, addr, HPAGE_PMD_ORDER))
>>  		return -EINVAL;
>> @@ -865,12 +864,13 @@ static int migrate_vma_insert_huge_pmd_page(struct migrate_vma *migrate,
>>  	if (userfaultfd_missing(vma))
>>  		goto unlock_abort;
>>  
>> -	if (!pmd_none(*pmdp)) {
>> +	if (pmd_present(*pmdp)) {
>>  		if (!is_huge_zero_pmd(*pmdp))
>>  			goto unlock_abort;
>>  		flush = true;
>> -	} else if (!pmd_none(*pmdp))
>> +	} else if (!pmd_none(*pmdp)) {
>>  		goto unlock_abort;
>> +	}
>>  
>>  	add_mm_counter(vma->vm_mm, MM_ANONPAGES, HPAGE_PMD_NR);
>>  	folio_add_new_anon_rmap(folio, vma, addr, RMAP_EXCLUSIVE);
>
> is_huge_zero_pmd() checks pmd_present(), so we didn't have a bug before.
>
> We could also do:
>
> if (is_huge_zero_pmd(*pmdp)) {
> 	flush = true;
> } else if (!pmd_none(*pmdp)) {
> 	goto unlock_abort;
> }

I think that this is better.  And because the patch is for code cleanup,
why not do it better.

> But I don't particularly care
>
> Acked-by: David Hildenbrand (Arm) <david@kernel.org>

---
Best Regards,
Huang, Ying

Re: [PATCH] mm/migrate_device: Cleanup up PMD Checks and warnings

[Zi Yan]

On 14 Apr 2026, at 10:13, Sunny Patel wrote:

> Remove the odd VM_WARN_ON_FOLIO(!folio, folio) usage and replace it
> with a simpler VM_WARN_ON_ONCE(!folio) check.
>
> Drop the redundant VM_WARN_ON_ONCE(!pmd_none(*pmdp) &&
> !is_huge_zero_pmd(*pmdp)).
>
> Refactor the PMD checks to use pmd_present(), making the control flow
> clearer and avoiding duplicate condition checks.
>
> Signed-off-by: Sunny Patel <nueralspacetech@gmail.com>
> ---
>  mm/migrate_device.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
Acked-by: Zi Yan <ziy@nvidia.com>

Best Regards,
Yan, Zi

Re: [PATCH] mm/migrate_device: Cleanup up PMD Checks and warnings

[SeongJae Park]

On Tue, 14 Apr 2026 19:43:38 +0530 Sunny Patel <nueralspacetech@gmail.com> wrote:

> Remove the odd VM_WARN_ON_FOLIO(!folio, folio) usage and replace it
> with a simpler VM_WARN_ON_ONCE(!folio) check.
> 
> Drop the redundant VM_WARN_ON_ONCE(!pmd_none(*pmdp) &&
> !is_huge_zero_pmd(*pmdp)).
> 
> Refactor the PMD checks to use pmd_present(), making the control flow
> clearer and avoiding duplicate condition checks.
> 
> Signed-off-by: Sunny Patel <nueralspacetech@gmail.com>

Acked-by: SeongJae Park <sj@kernel.org>

[...]
> @@ -865,12 +864,13 @@ static int migrate_vma_insert_huge_pmd_page(struct migrate_vma *migrate,
>  	if (userfaultfd_missing(vma))
>  		goto unlock_abort;
>  
> -	if (!pmd_none(*pmdp)) {
> +	if (pmd_present(*pmdp)) {
>  		if (!is_huge_zero_pmd(*pmdp))
>  			goto unlock_abort;
>  		flush = true;
> -	} else if (!pmd_none(*pmdp))
> +	} else if (!pmd_none(*pmdp)) {
>  		goto unlock_abort;
> +	}

I like this coding style [1] fix.

[1] https://docs.kernel.org/process/coding-style.html#placing-braces-and-spaces


Thanks,
SJ

[...]