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

Sunny Patel posted 1 patch 1 month, 3 weeks ago
mm/migrate_device.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
[PATCH v2] mm/migrate_device: Cleanup up PMD Checks and warnings
Posted by Sunny Patel 1 month, 3 weeks ago
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
Posted by Andrew Morton 1 month, 3 weeks ago
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 v2] mm/migrate_device: Cleanup up PMD Checks and warnings
Posted by Sunny Patel 1 month, 3 weeks ago
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
Posted by David Hildenbrand (Arm) 1 month, 3 weeks ago
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 v2] mm/migrate_device: Cleanup up PMD Checks and warnings
Posted by Sunny Patel 1 month, 3 weeks ago
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
Re: [PATCH] mm/migrate_device: Cleanup up PMD Checks and warnings
Posted by Sunny Patel 1 month, 3 weeks ago
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] mm/migrate_device: Cleanup up PMD Checks and warnings
Posted by Andrew Morton 1 month, 3 weeks ago
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
Posted by David Hildenbrand (Arm) 1 month, 3 weeks ago
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
Posted by Huang, Ying 1 month, 3 weeks ago
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
Posted by Zi Yan 1 month, 3 weeks ago
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