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

[Huang, Ying]

Sunny Patel <nueralspacetech@xxxxxxxxx> 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@xxxxxxxxx>
> ---
>
> 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@xxxxxxxxxxxxxxxxx>

in the future versions.

---
Best Regards,
Huang, Ying