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

[Kefeng Wang <wangkefeng.wang@huawei.com>]

On 6/18/2026 11:02 PM, Pedro Falcato wrote:
> On Thu, Jun 18, 2026 at 09:01:08PM +0800, Kefeng Wang wrote:
>>
>>
>> On 6/18/2026 7:49 PM, Pedro Falcato wrote:
>>> Please CC reviewers properly!
>>>
>>
>> Oh, I will put more reviewes to cc list.
>>
>>> On Thu, Jun 18, 2026 at 05:28:42PM +0800, Kefeng Wang wrote:
>>>> The do_mincore() uses walk_page_range() to walk the page table.
>>>> Fortunately, the caller always passes start/end that falls within
>>>> a single VMA, so it's safe to use the walk_page_range_vma() in
>>>> do_mincore() to eliminate an unnecessary find_vma() lookup.
>>>>
>>>> Unlike walk_page_range(), walk_page_range_vma() does not call
>>>> walk_page_test(), which handles VM_PFNMAP by invoking ->pte_hole()
>>>
>>> Why not? Can we fix that instead? I really don't like having this open
>>> coded in callers. Are there callers of walk_page_range_vma() that expect
>>> to look at PFNMAP mappings as well? From what I can see, the callers all
>>> seem to operate on folios (and/or anonymous memory).
>>
>> As you said, all the other callers don't operate VM_PFNMAP， so we don't
>> want to add walk_page_test() into walk_page_range_vma(). This hack is to
>> preserve the original behavior, but as David said[1], we could add a
>> follow-up patch to remove the special handling to see if anyone screams,
>> and this indeed changed some behaviors, so it's better to handle it with
>> another patch.
> 
> Yes, I agree, I'm 95% sure no one is invoking mincore() on PFNMAP mappings.
> 
> So, if we're keeping this check for this patch:
> 
>>>>
>>>> diff --git a/mm/mincore.c b/mm/mincore.c
>>>> index 296f2e3922b5..0c6731ae6c4d 100644
>>>> --- a/mm/mincore.c
>>>> +++ b/mm/mincore.c
>>>> @@ -259,7 +259,21 @@ static long do_mincore(unsigned long addr, unsigned long pages, unsigned char *v
>>>>    		memset(vec, 1, pages);
>>>>    		return pages;
>>>>    	}
>>>> -	err = walk_page_range(vma->vm_mm, addr, end, &mincore_walk_ops, vec);
>>>> +
>>>> +	/*
>>>> +	 * 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.
>>>> +	 */
> 
> I would rather we amend this comment to something like:
> 
> 	/* mincore (historically) reports PFNMAP mappings as non-resident. */
> 
> because we don't need to explain internal differences in walk_page_range
> functions in a random comment in mincore. And perhaps attempt a separate

Hope Andrew can fix the comments when pickup patches.

> PFNMAP check removal patch as part of the series, or as a follow up (so if
> it does matter, we can simply revert that patch instead of this conversion).
> 

I will send it separately, along with other mincore optimizations.

> In any case,
> 
> Reviewed-by: Pedro Falcato <pfalcato@suse.de>
> 

Thanks.