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
