Re: [PATCH v3 08/15] mm: introduce and use linear_folio_page_index()

["Lorenzo Stoakes (ARM)" <ljs@kernel.org>]

On Wed, Aug 05, 2026 at 09:26:59AM +0200, David Hildenbrand (Arm) wrote:
> On 8/3/26 16:30, Lorenzo Stoakes (ARM) wrote:
> > On Mon, Aug 03, 2026 at 01:27:09PM +0200, David Hildenbrand (Arm) wrote:
> >> On 7/29/26 18:48, Lorenzo Stoakes (ARM) wrote:
> >>> This function is, for now, a placeholder; it will be used in future to
> >>> determine whether to use the anonymous page index or not, based on whether
> >>> the folio is anonymous or not.
> >>>
> >>> Currently it simply wraps linear_page_index(), so this does not change
> >>> behaviour.
> >>>
> >>> We update callers that will, once the change is introduced to track
> >>> anonymous folios by anonymous page offset if MAP_PRIVATE file-backed, need
> >>> to determine which index to use based on folio type.
> >>>
> >>> No functional change intended.
> >>>
> >>> Signed-off-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
> >>> ---
> >>>  include/linux/pagemap.h | 18 ++++++++++++++++++
> >>>  mm/huge_memory.c        |  3 ++-
> >>>  mm/migrate.c            |  6 ++++--
> >>>  mm/userfaultfd.c        |  6 ++++--
> >>>  4 files changed, 28 insertions(+), 5 deletions(-)
> >>>
> >>> diff --git a/include/linux/pagemap.h b/include/linux/pagemap.h
> >>> index 259177544b03..6eb8d811ba4c 100644
> >>> --- a/include/linux/pagemap.h
> >>> +++ b/include/linux/pagemap.h
> >>> @@ -1143,6 +1143,24 @@ static inline pgoff_t linear_anon_page_index(const struct vm_area_struct *vma,
> >>>  	return pgoff;
> >>>  }
> >>>
> >>> +/**
> >>> + * linear_folio_page_index() - Determine the absolute page offset of
> >>> + * @address within @vma from @folio.
> >>> + * @folio: The folio whose linear page index is sought.
> >>> + * @vma: The VMA in which @address resides.
> >>> + * @address: The address whose absolute page offset is required.
> >>> + *
> >>> + * For compatibility, currently identical to linear_page_index().
> >>> + *
> >>> + * Returns: The absolute page offset of @address within @vma.
> >>> + */
> >>> +static inline pgoff_t linear_folio_page_index(const struct folio *folio,
> >>> +					      const struct vm_area_struct *vma,
> >>> +					      const unsigned long address)
> >>> +{
> >>> +	return linear_page_index(vma, address);
> >>> +}
> >>
> >>
> >> I found this to be rather confusing, given that we now have a "folio" helper that
> >> receives a folio and a "page" helper that doesn't receive a page ...
> >
> > This is just to avoid having to duplicate the if (folio_test_anon()) { ... }
> > else { ... } stuff.
> >
> > Agreed it's a bit confusing!
> >
> > Really you are figuring things out from (vma, address) - 'what is the correct
> > page offset based on the VMA'.
> >
> > And yeah it seems migrate can do it via PFN as you suggest, it really is
> > just trying to find the page offset in the folio.
> >
> > But...
> >
> >>
> >> I guess the problem is the "page" in "linear_page_index", as it
> >> reminds of legacy page->index.
> >>
> >>
> >> I wonder if it would be better to have a linear_folio_index() and
> >> force that address points at the start of the folio.
> >>
> >> Looking below, this is exactly what we want for all except one case:
> >
> > ...I don't think this is true.
> >
> > The uffd code uses this too in move_present_ptes():
> >
> > 		src_folio->index = linear_folio_page_index(src_folio, dst_vma,
> > 							   dst_addr);
> >
> > And it's now _updating_ the source folio index to the offset in the destination
> > VMA, so it doesn't even relate to the source folio's offset at all?
> >
> > (move_swap_pte() calls linear_folio_page_index() but obviously has to be
> > anon, so that can just use linear_anon_page_index() there instead, will
> > update.)
> >
> > With your change we can just eliminate the linear_folio_page_index()
> > function and open-code the uffd case.
>
> The would be even better!
>
> >
> > It's a bit of a special case anyway and is neatly the one place where you
> > actually don't know if it's anon or file-backed (well anon or shmem
> > specifically I think).
> >
> >>
> >>
> >>>  		/* pgoff is invalid for ksm pages, but they are never large */
> >>> -		if (folio_test_large(folio) && !folio_test_hugetlb(folio))
> >>> -			idx = linear_page_index(vma, pvmw.address) - pvmw.pgoff;
> >>> +		if (folio_test_large(folio) && !folio_test_hugetlb(folio)) {
> >>> +			idx += linear_folio_page_index(folio, vma, pvmw.address);
> >>> +			idx -= pvmw.pgoff;
> >>> +		}
> >>>  		new = folio_page(folio, idx);
> >>
> >> I think we could avoid this index work entirely by using the pfn, which is much
> >> clearer to me, and similar to how we handle it during other rmap operations.
> >>
> >> diff --git a/mm/migrate.c b/mm/migrate.c
> >> index 222c8c15f782f..686351d353203 100644
> >> --- a/mm/migrate.c
> >> +++ b/mm/migrate.c
> >> @@ -362,17 +362,12 @@ static bool remove_migration_pte(struct folio *folio,
> >>                 struct page *new;
> >>                 unsigned long idx = 0;
> >>
> >> -               /* pgoff is invalid for ksm pages, but they are never large */
> >> -               if (folio_test_large(folio) && !folio_test_hugetlb(folio))
> >> -                       idx = linear_page_index(vma, pvmw.address) - pvmw.pgoff;
> >> -               new = folio_page(folio, idx);
> >> -
> >>  #ifdef CONFIG_ARCH_HAS_PMD_SOFTLEAVES
> >>                 /* PMD-mapped THP migration entry */
> >>                 if (!pvmw.pte) {
> >>                         VM_BUG_ON_FOLIO(folio_test_hugetlb(folio) ||
> >>                                         !folio_test_pmd_mappable(folio), folio);
> >> -                       remove_migration_pmd(&pvmw, new);
> >> +                       remove_migration_pmd(&pvmw, folio_page(folio, idx));
> >
> > I think we'd need the idx code below to go above where the idx code is now
> > as otherwise this will be incorrect right?
> >
> > But then again, if it's a PMD softleaf it'd have to be aligned right, so
> > couldn't we just update that function to be passed a folio instead and
> > avoid the idx here at all?
>
> Right, that's what I mentioned below
>
> "remove_migration_pmd() will work for now. Later it should just receive the folio
> and do the same thing through softleaf_from_pmd() -> softleaf_to_pfn().
>
> >
> >>                         continue;
> >>                 }
> >>  #endif
> >> @@ -385,10 +380,14 @@ static bool remove_migration_pte(struct folio *folio,
> >>                     try_to_map_unused_to_zeropage(&pvmw, folio, old_pte, idx))
> >>                         continue;
> >>
> >> +               entry = softleaf_from_pte(old_pte);
> >> +               if (folio_test_large(folio) && !folio_test_hugetlb(folio))
> >> +                       idx = softleaf_to_pfn(entry) - folio_pfn(rmap_walk_arg->folio);
> >
> > Could actually be softleaf_to_pfn(entry) - pvmw.pfn even?
>
> Given that DEFINE_FOLIO_VMA_WALK() sets
>
> 	.pfn = folio_pfn(_folio);
>
> I think so.
>
> >
> >> +               new = folio_page(folio, idx);
> >> +
> >>                 folio_get(folio);
> >>                 pte = mk_pte(new, READ_ONCE(vma->vm_page_prot));
> >>
> >> -               entry = softleaf_from_pte(old_pte);
> >>                 if (!softleaf_is_migration_young(entry))
> >>                         pte = pte_mkold(pte);
> >>                 if (folio_test_dirty(folio) && softleaf_is_migration_dirty(entr
> >>
> >>
> >> remove_migration_pmd() will work for now. Later it should just receive the folio
> >> and do the same thing through softleaf_from_pmd() -> softleaf_to_pfn().
> >
> > Ah you already addressed it. But since this would move the idx code above,
> > I think I should just change it to accept a folio instead?
> >
> > Or for less churn &folio->page...
> Right, whatever you prefer.

Awesome will do all that on respin :)

Thanks for the review, this is actually really improving the series :>)

>
>
>
> --
> Cheers,
>
> David

--
Cheers, Lorenzo