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

[Pedro Falcato <pfalcato@suse.de>]

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
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).

In any case,

Reviewed-by: Pedro Falcato <pfalcato@suse.de>

-- 
Pedro