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

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) <[email protected]>
>>> ---
>>>  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.



-- 
Cheers,

David