Re: [PATCH v2 1/4] mm: mincore: use walk_page_range_vma() in do_mincore()

["David Hildenbrand (Arm)" <david@kernel.org>]

On 6/26/26 17:21, Kefeng Wang wrote:
> 
> 
> On 6/25/2026 10:38 AM, Andrew Morton wrote:
>> On Mon, 22 Jun 2026 11:23:11 +0800 Kefeng Wang <wangkefeng.wang@huawei.com>
>> wrote:
>>
>>>
>>> Hope Andrew can fix the comments when pickup patches.
>>
>> I added this:
>>
>> --- a/mm/mincore.c~mm-mincore-use-walk_page_range_vma-in-do_mincore-fix
>> +++ a/mm/mincore.c
>> @@ -261,12 +261,7 @@ static long do_mincore(unsigned long add
>>       }
>>         /*
>> -     * walk_page_range_vma() does not call walk_page_test(), which
>> -     * handles VM_PFNMAP VMA by invoking ->pte_hole() to skip the
>> -     * page table walk. Without this check, PFNMAP PTEs would be
>> -     * treated as present by mincore_pte_range(), changing the returned
>> -     * residency status from the historical "not resident" to "resident".
>> -     * Handle VM_PFNMAP explicitly to preserve the original behavior.
>> +     * mincore historically reports PFNMAP mappings as non-resident.
>>        */
>>       if (vma->vm_flags & VM_PFNMAP) {
>>           __mincore_unmapped_range(addr, end, vma, vec);
>> _
>>
> 
> Thanks.
> 
>>
>> Sashiko is OK with your patchset, but it might have found four(!)
>> pre-existing issues:
>>
>>     https://sashiko.dev/#/patchset/20260618092845.3905740-1-
>> wangkefeng.wang@huawei.com
>>
> 
> Sorry for the late, this is pre-existing issue, I am not familiar with
> zone devices, so just focus on my changes and the subsequent
> changes related to mincore.. After a quick review, most of them are not
> issues based on my understanding.
> 
> + CC Alistair and Gregroy for migrate_device.c, also Zi could you help
> to check it?
> 
> 1. mlock.c, ACTION_AGAIN
> 2. migrate_device.c, goto again array overrun / double counting
> 3. migrate_device.c, softleaf_to_folio VM_WARN
> 4. migrate_device.c, pte_pfn / pte_dirty on non-present PTE
> 
> I think issue 4 is real issue, a quick fix is
> 
> diff --git a/mm/migrate_device.c b/mm/migrate_device.c
> index 554754eb26ff..3ccc4fc1015c 100644
> --- a/mm/migrate_device.c
> +++ b/mm/migrate_device.c
> @@ -401,7 +401,8 @@ static int migrate_vma_collect_pmd(pmd_t *pmdp,
>                         bool anon_exclusive;
>                         pte_t swp_pte;
> 
> -                       flush_cache_page(vma, addr, pte_pfn(pte));
> +                       if (pte_present(pte))
> +                               flush_cache_page(vma, addr, pte_pfn(pte));
>                         anon_exclusive = folio_test_anon(folio) &&
>                                           PageAnonExclusive(page);

That looks reasonable.

>                         if (anon_exclusive) {
> @@ -422,8 +423,12 @@ static int migrate_vma_collect_pmd(pmd_t *pmdp,
>                         migrate->cpages++;
> 
>                         /* Set the dirty flag on the folio now the pte is gone. */
> -                       if (pte_dirty(pte))
> +                       if (pte_present(pte)) {
> +                               if (pte_dirty(pte))
> +                                       folio_mark_dirty(folio);
> +                       } else if (softleaf_is_device_private_write(entry)) {
>                                 folio_mark_dirty(folio);
> +                       }

The pte_presnt() check looks reasonable. But dirty handling is really
responsibility of the device side user.

So likely just

if (pte_present(pte) && pte_dirty(pte))
	folio_mark_dirty(folio);

-- 
Cheers,

David