[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

[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

[...]

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

[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

[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

[Sunny Patel]

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!

is_huge_zero_pmd() didn't check for pmd_present as of now as per the current implementation of it so additional check require for pmd_present(). 
Please let me know if anything needs to add in here.

Thanks,
Sunny Patel

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

[David Hildenbrand (Arm)]

On 4/18/26 19:18, Sunny Patel wrote:
> 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:
>>
>> [...]
>>>
>>> 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!
> 
> is_huge_zero_pmd() didn't check for pmd_present as of now as per the current implementation of it so additional check require for pmd_present().

I don't know what you mean. Here is the code in the tree:
static inline bool is_huge_zero_pmd(pmd_t pmd)
{
	return pmd_present(pmd) && is_huge_zero_pfn(pmd_pfn(pmd));
}

-- 
Cheers,

David

[PATCH v2] 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, making the control flow
clearer and avoiding duplicate condition checks.

Signed-off-by: Sunny Patel <nueralspacetech@gmail.com>
---

Changes in v2:
  - Removed additional check for pmd_present() as is_huge_zero_pmd()
  already checks for pmd_present().

Missed it in first version of this patch.

 mm/migrate_device.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/mm/migrate_device.c b/mm/migrate_device.c
index 8079676c8f1f..2c7e89a6d1b1 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,11 +864,9 @@ static int migrate_vma_insert_huge_pmd_page(struct migrate_vma *migrate,
 	if (userfaultfd_missing(vma))
 		goto unlock_abort;
 
-	if (!pmd_none(*pmdp)) {
-		if (!is_huge_zero_pmd(*pmdp))
-			goto unlock_abort;
+	if (is_huge_zero_pmd(*pmdp))
 		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);
-- 
2.43.0

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

[Zi Yan]

On 19 Apr 2026, at 13:47, 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, making the control flow
> clearer and avoiding duplicate condition checks.
>
> Signed-off-by: Sunny Patel <nueralspacetech@gmail.com>
> ---
>
> Changes in v2:
>   - Removed additional check for pmd_present() as is_huge_zero_pmd()
>   already checks for pmd_present().
>
> Missed it in first version of this patch.
>
>  mm/migrate_device.c | 9 +++------
>  1 file changed, 3 insertions(+), 6 deletions(-)
>
LGTM.

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

--
Best Regards,
Yan, Zi

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

[Sunny Patel]

got it. thanks for the pointer. 
Sent the new version of this patch. please check on it.

Thanks,
Sunny Patel

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

[Huang, Ying]

Sunny Patel <nueralspacetech@gmail.com> writes:

> 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, making the control flow
> clearer and avoiding duplicate condition checks.
>
> Signed-off-by: Sunny Patel <nueralspacetech@gmail.com>
> ---
>
> Changes in v2:
>   - Removed additional check for pmd_present() as is_huge_zero_pmd()
>   already checks for pmd_present().
>
> Missed it in first version of this patch.
>
>  mm/migrate_device.c | 9 +++------
>  1 file changed, 3 insertions(+), 6 deletions(-)
>
> diff --git a/mm/migrate_device.c b/mm/migrate_device.c
> index 8079676c8f1f..2c7e89a6d1b1 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,11 +864,9 @@ static int migrate_vma_insert_huge_pmd_page(struct migrate_vma *migrate,
>  	if (userfaultfd_missing(vma))
>  		goto unlock_abort;
>  
> -	if (!pmd_none(*pmdp)) {
> -		if (!is_huge_zero_pmd(*pmdp))
> -			goto unlock_abort;
> +	if (is_huge_zero_pmd(*pmdp))
>  		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);

LGTM, feel free to add my

Reviewed-by: Huang Ying <ying.huang@linux.alibaba.com>

in the future versions.

---
Best Regards,
Huang, Ying

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

[David Hildenbrand (Arm)]

On 4/19/26 19:47, 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, making the control flow
> clearer and avoiding duplicate condition checks.
> 
> Signed-off-by: Sunny Patel <nueralspacetech@gmail.com>
> ---

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

-- 
Cheers,

David

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

[Andrew Morton]

On Sun, 19 Apr 2026 23:17:43 +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, making the control flow
> clearer and avoiding duplicate condition checks.

AI review might have found a bug in the surrounding code:
	https://sashiko.dev/#/patchset/20260419174747.10701-1-nueralspacetech@gmail.com

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

[Sunny Patel]

On Sun, 19 Apr 2026 23:17:43 +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, making the control flow
> clearer and avoiding duplicate condition checks.

AI review might have found a bug in the surrounding code:
        https://sashiko.dev/#/patchset/20260419174747.10701-1-nueralspacetech@gmail.com

After looking into the AI comment found two potential leak.

1. The goto should be unlock_abort instead of abort to ensure 
the spinlock is released.

2. The pgtable allocation at the top of the function
is indeed not freed on the unlock_abort path. Will add a
pte_free(vma->vm_mm, pgtable). 

Since this patch is more of Cleanup on PMD checks and
Warnings Is it ok if I send different patch to address
this potential leak issues or need it in this revision
itself?

Thanks,
Sunny Patel

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

[Sunny Patel]

On Sun, 19 Apr 2026 23:17:43 +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, making the control flow
> clearer and avoiding duplicate condition checks.

AI review might have found a bug in the surrounding code:
        https://sashiko.dev/#/patchset/20260419174747.10701-1-nueralspacetech@gmail.com

NOTE: Ignore the previous message was sent on different 
Subject Header by mistake.

After looking into the AI comment found two potential leak.

1. The goto should be unlock_abort instead of abort to ensure 
the spinlock is released.

2. The pgtable allocation at the top of the function
is indeed not freed on the unlock_abort path. Will add a
pte_free(vma->vm_mm, pgtable). 

Since this patch is more of Cleanup on PMD checks and
Warnings Is it ok if I send different patch to address
this potential leak issues or need it in this revision
itself?

Thanks,
Sunny Patel

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

[David Hildenbrand (Arm)]

On 4/24/26 19:33, Sunny Patel wrote:
> On Sun, 19 Apr 2026 23:17:43 +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, making the control flow
>> clearer and avoiding duplicate condition checks.
> 
> AI review might have found a bug in the surrounding code:
>         https://sashiko.dev/#/patchset/20260419174747.10701-1-nueralspacetech@gmail.com
> 
> NOTE: Ignore the previous message was sent on different 
> Subject Header by mistake.
> 
> After looking into the AI comment found two potential leak.
> 
> 1. The goto should be unlock_abort instead of abort to ensure 
> the spinlock is released.
> 
> 2. The pgtable allocation at the top of the function
> is indeed not freed on the unlock_abort path. Will add a
> pte_free(vma->vm_mm, pgtable). 
> 
> Since this patch is more of Cleanup on PMD checks and
> Warnings Is it ok if I send different patch to address
> this potential leak issues or need it in this revision
> itself?

No, independent issues -> independent patches :)

-- 
Cheers,

David

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

[Andrew Morton]

On Fri, 24 Apr 2026 22:52:08 +0530 Sunny Patel <nueralspacetech@gmail.com> wrote:

> On Sun, 19 Apr 2026 23:17:43 +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, making the control flow
> > clearer and avoiding duplicate condition checks.
> 
> AI review might have found a bug in the surrounding code:
>         https://sashiko.dev/#/patchset/20260419174747.10701-1-nueralspacetech@gmail.com
> 
> After looking into the AI comment found two potential leak.
> 
> 1. The goto should be unlock_abort instead of abort to ensure 
> the spinlock is released.
> 
> 2. The pgtable allocation at the top of the function
> is indeed not freed on the unlock_abort path. Will add a
> pte_free(vma->vm_mm, pgtable). 
> 
> Since this patch is more of Cleanup on PMD checks and
> Warnings Is it ok if I send different patch to address
> this potential leak issues or need it in this revision
> itself?

A separate patch, please.

It's best to prepare the bugfix patch against current Linus mainline,
then any cleanups come on top of the bugfix.

Thanks.

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

[Sunny Patel]

On Fri, 24 Apr 2026 20:25:06 +0200, David Hildenbrand wrote:
> No, independent issues -> independent patches :)

Sent two individual patches for review as discussed.

1. https://lore.kernel.org/linux-mm/20260425134453.23769-1-nueralspacetech@gmail.com/T/#u
2. https://lore.kernel.org/linux-mm/20260425133537.17463-1-nueralspacetech@gmail.com/T/#u

Thanks,
Sunny Patel