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

["Huang, Ying" <ying.huang@linux.alibaba.com>]

"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