Re: [PATCH 4/5] mm/page_vma_mapped: use huge_ptep_get() for hugetlb
From: Dev Jain
Date: Sat Jun 27 2026 - 03:13:52 EST
On 26/06/26 10:16 pm, Lance Yang wrote:
>
>
> On 2026/6/26 23:26, Dev Jain wrote:
>>
>>
>> On 26/06/26 7:40 pm, Lance Yang wrote:
>>>
>>> On Fri, Jun 26, 2026 at 06:53:10PM +0530, Dev Jain wrote:
>>>>
>>>>
>>>> On 26/06/26 1:18 pm, Lance Yang wrote:
>>>>>
>>>>> On Thu, Jun 25, 2026 at 11:29:53AM +0000, Dev Jain wrote:
>>>>>> check_pte() is the final validation step in page_vma_mapped_walk().
>>>>>> It reads pvmw->pte with ptep_get() to decide whether the entry maps
>>>>>> the PFN range being walked. For hugetlb VMAs, that pointer refers
>>>>>> to a hugetlb entry.
>>>>>>
>>>>>> On arches which provide their own huge_ptep_get() to dereference a huge
>>>>>> pte pointer, accessing via ptep_get() would cause pte_pfn(),
>>>>>> pte_present() etc to misbehave.
>>>>>>
>>>>>> It is not clear whether this has a trivially visible effect to userspace.
>>>>>>
>>>>>> Use huge_ptep_get() to dereference a huge pte pointer.
>>>>>>
>>>>>> Fixes: ace71a19cec5 ("mm: introduce page_vma_mapped_walk()")
>>>>>> Cc: stable@xxxxxxxxxxxxxxx
>>>>>> Signed-off-by: Dev Jain <dev.jain@xxxxxxx>
>>>>>> ---
>>>>>> mm/page_vma_mapped.c | 8 +++++++-
>>>>>> 1 file changed, 7 insertions(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/mm/page_vma_mapped.c b/mm/page_vma_mapped.c
>>>>>> index 2ccbabfb2cc17..18e1d341f463c 100644
>>>>>> --- a/mm/page_vma_mapped.c
>>>>>> +++ b/mm/page_vma_mapped.c
>>>>>> @@ -107,7 +107,13 @@ static bool map_pte(struct page_vma_mapped_walk *pvmw, pmd_t *pmdvalp,
>>>>>> static bool check_pte(struct page_vma_mapped_walk *pvmw, unsigned long pte_nr)
>>>>>> {
>>>>>> unsigned long pfn;
>>>>>> - pte_t ptent = ptep_get(pvmw->pte);
>>>>>> + pte_t ptent;
>>>>>> +
>>>>>> + if (is_vm_hugetlb_page(pvmw->vma))
>>>>>> + ptent = huge_ptep_get(pvmw->vma->vm_mm, pvmw->address,
>>>>>> + pvmw->pte);
>>>>>
>>>>> I think check_pte() can pass a wrong address to huge_ptep_get() ...
>>>>
>>>> Won't this be handled by rmap_walk_anon/rmap_walk_file - they are the ones
>>>> performing the rmap traversal and passing address to try_to_unmap_one/folio_referenced_one
>>>> etc ...
>>>
>>> Right, that should cover the rmap callbacks. The bit I was worried about
>>> is page_mapped_in_vma() though.
>>>
>>>>>
>>>>> Not sure that is wrong in the first place. For memory failure,
>>>>> page_mapped_in_vma() can be called with a poisoned tail page of a hugetlb
>>>>> folio. In that case, pvmw->address need not be hugepage-aligned.
>>>>>
>>>>> @Miaohe
>>>
>>> For hugetlb memory failure we start with the poisoned PFN:
>>>
>>> static int try_memory_failure_hugetlb(unsigned long pfn, int flags)
>>> {
>>> ...
>>> struct page *p = pfn_to_page(pfn);
>>> struct folio *folio;
>>> ...
>>>
>>> folio = page_folio(p);
>>>
>>> ...
>>>
>>> if (!hwpoison_user_mappings(folio, p, pfn, flags)) {
>>> ...
>>> }
>>>
>>> ...
>>> }
>>>
>>> and pass the same p down:
>>>
>>> static bool hwpoison_user_mappings(struct folio *folio, struct page *p,
>>> unsigned long pfn, int flags)
>>> {
>>> ...
>>>
>>> collect_procs(folio, p, &tokill, flags & MF_ACTION_REQUIRED);
>>>
>>> ...
>>> }
>>>
>>> static void collect_procs(const struct folio *folio, const struct page *page,
>>> struct list_head *tokill, int force_early)
>>> {
>>> ...
>>>
>>> if (unlikely(folio_test_ksm(folio)))
>>> ...
>>> else if (folio_test_anon(folio))
>>> collect_procs_anon(folio, page, tokill, force_early);
>>> else
>>> ...
>>> }
>>>
>>> So collect_procs_anon() still gets the poisoned page, not &folio->page:
>>>
>>> static void collect_procs_anon(const struct folio *folio,
>>> const struct page *page, struct list_head *to_kill,
>>> int force_early)
>>> {
>>> ...
>>>
>>> pgoff = page_pgoff(folio, page);
>>> rcu_read_lock();
>>> for_each_process(tsk) {
>>> ...
>>>
>>> anon_vma_interval_tree_foreach(vmac, &av->rb_root,
>>> pgoff, pgoff) {
>>> ...
>>> addr = page_mapped_in_vma(page, vma);
>>> ...
>>> }
>>> }
>>> rcu_read_unlock();
>>> anon_vma_unlock_read(av);
>>> }
>>>
>>> page_mapped_in_vma() then builds pvmw for that page:
>>>
>>> unsigned long page_mapped_in_vma(const struct page *page,
>>> struct vm_area_struct *vma)
>>> {
>>> const struct folio *folio = page_folio(page);
>>> struct page_vma_mapped_walk pvmw = {
>>> .pfn = page_to_pfn(page),
>>> .nr_pages = 1,
>>> .vma = vma,
>>> .flags = PVMW_SYNC,
>>> };
>>>
>>> pvmw.address = vma_address(vma, page_pgoff(folio, page), 1);
>>> ...
>>> }
>>>
>>> and page_pgoff() includes the subpage index:
>>>
>>> static inline pgoff_t page_pgoff(const struct folio *folio,
>>> const struct page *page)
>>> {
>>> return folio->index + folio_page_idx(folio, page);
>>> }
>>>
>>> So if the poisoned PFN points to a tail page, pvmw->address can be offset
>>> from the start of the hugetlb mapping by
>>>
>>> folio_page_idx(folio, page) << PAGE_SHIFT
>>>
>>> Should check_pte() pass the hugepage-aligned address to huge_ptep_get()
>>> for that case?
>>
>> Thanks! This looks correct.
>>
>> I can indeed fix this up in check_pte. But in the memory-failure path
>> it has always been confusing to me for hugetlb folios why we are bothering
>> with the tail page. I am sure that area can also be simplified. But for
>> now I'll just do a simple fix here itself.
>
> Just thinking out loud: given that huge_ptep_get() already assumes that
> addr matches the huge pte, at least on arm64, would it make sense to
> have a small hugetlb wrapper around it that takes hstate and aligns
> the address before calling the arch helper?
>
> Might make the rule clearer, and a bit harder to get wrong again :)
Are you suggesting something like:
diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h
index fdb7bdf7645c..xxxxxxxxxxxx 100644
--- a/include/linux/hugetlb.h
+++ b/include/linux/hugetlb.h
@@ -825,6 +825,15 @@ static inline struct folio *filemap_lock_hugetlb_folio(struct hstate *h,
#include <asm/hugetlb.h>
+static inline pte_t hugetlb_ptep_get(struct vm_area_struct *vma,
+ unsigned long addr, pte_t *ptep)
+{
+ struct hstate *h = hstate_vma(vma);
+
+ return huge_ptep_get(vma->vm_mm, addr & huge_page_mask(h), ptep);
+}
+
#ifndef is_hugepage_only_range
static inline int is_hugepage_only_range(struct mm_struct *mm,
unsigned long addr, unsigned long len)
diff --git a/mm/page_vma_mapped.c b/mm/page_vma_mapped.c
index 18e1d341f463..xxxxxxxxxxxx 100644
--- a/mm/page_vma_mapped.c
+++ b/mm/page_vma_mapped.c
@@ -110,8 +110,7 @@ static bool check_pte(struct page_vma_mapped_walk *pvmw, unsigned long pte_nr)
pte_t ptent;
if (is_vm_hugetlb_page(pvmw->vma))
- ptent = huge_ptep_get(pvmw->vma->vm_mm, pvmw->address,
- pvmw->pte);
+ ptent = hugetlb_ptep_get(pvmw->vma, pvmw->address, pvmw->pte);
else
ptent = ptep_get(pvmw->pte);
>
> Thanks, Lance
>
>>
>>>
>>> Cheers, Lance
>>>
>>>>>
>>>>> For arm64, CONT_PMD_SIZE is one supported HugeTLB size. With such a VMA,
>>>>> page_vma_mapped_walk() passes that size to hugetlb_walk():
>>>>>
>>>>> bool page_vma_mapped_walk(struct page_vma_mapped_walk *pvmw)
>>>>> {
>>>>> ...
>>>>> if (unlikely(is_vm_hugetlb_page(vma))) {
>>>>> ...
>>>>> pvmw->pte = hugetlb_walk(vma, pvmw->address, size);
>>>>> ...
>>>>> }
>>>>> ...
>>>>> }
>>>>>
>>>>> hugetlb_walk() then calls arm64 huge_pte_offset(mm, addr, sz). For
>>>>> sz == CONT_PMD_SIZE, huge_pte_offset() aligns its local addr before
>>>>> calculating pmdp:
>>>>>
>>>>> pte_t *huge_pte_offset(struct mm_struct *mm,
>>>>> unsigned long addr, unsigned long sz)
>>>>> {
>>>>> ...
>>>>> if (sz == CONT_PMD_SIZE)
>>>>> addr &= CONT_PMD_MASK;
>>>>>
>>>>> pmdp = pmd_offset(pudp, addr);
>>>>> pmd = READ_ONCE(*pmdp);
>>>>> ...
>>>>> }
>>>>>
>>>>> So for that case, pvmw->pte is calculated from the aligned addr, not
>>>>> necessarily from the original pvmw->address. But check_pte() passes the
>>>>> original address together with pvmw->pte:
>>>>>
>>>>> + ptent = huge_ptep_get(pvmw->vma->vm_mm, pvmw->address,
>>>>> + pvmw->pte);
>>>>>
>>>>> arm64 then uses that addr again to choose ncontig:
>>>>>
>>>>> pte_t huge_ptep_get(struct mm_struct *mm, unsigned long addr, pte_t *ptep)
>>>>> {
>>>>> ...
>>>>> ncontig = find_num_contig(mm, addr, ptep, &pgsize);
>>>>> for (i = 0; i < ncontig; i++, ptep++) {
>>>>> ...
>>>>> }
>>>>> return orig_pte;
>>>>> }
>>>>>
>>>>> static int find_num_contig(struct mm_struct *mm, unsigned long addr,
>>>>> pte_t *ptep, size_t *pgsize)
>>>>> {
>>>>> pgd_t *pgdp = pgd_offset(mm, addr);
>>>>> p4d_t *p4dp;
>>>>> pud_t *pudp;
>>>>> pmd_t *pmdp;
>>>>>
>>>>> *pgsize = PAGE_SIZE;
>>>>> p4dp = p4d_offset(pgdp, addr);
>>>>> pudp = pud_offset(p4dp, addr);
>>>>> pmdp = pmd_offset(pudp, addr);
>>>>> if ((pte_t *)pmdp == ptep) {
>>>>> *pgsize = PMD_SIZE;
>>>>> return CONT_PMDS;
>>>>> }
>>>>> return CONT_PTES;
>>>>> }
>>>>>
>>>>> With a tail address, pmdp may no longer point at pvmw->pte, so
>>>>> find_num_contig() can return CONT_PTES for a CONT_PMD HugeTLB mapping.
>>>>>
>>>>> On 16K arm64, that changes ncontig from 32 to 128. So huge_ptep_get()
>>>>> can walk past the CONT_PMD entries, and possibly past the PMD table.
>>>>>
>>>>> Should check_pte() pass the address matching pvmw->pte, sth like:
>>>>>
>>>>> ---8<---
>>>>> diff --git a/mm/page_vma_mapped.c b/mm/page_vma_mapped.c
>>>>> index 406fd50bbd8f..58463493bd3d 100644
>>>>> --- a/mm/page_vma_mapped.c
>>>>> +++ b/mm/page_vma_mapped.c
>>>>> @@ -109,11 +109,14 @@ static bool check_pte(struct page_vma_mapped_walk *pvmw, unsigned long pte_nr)
>>>>> unsigned long pfn;
>>>>> pte_t ptent;
>>>>>
>>>>> - if (is_vm_hugetlb_page(pvmw->vma))
>>>>> - ptent = huge_ptep_get(pvmw->vma->vm_mm, pvmw->address,
>>>>> - pvmw->pte);
>>>>> - else
>>>>> + if (is_vm_hugetlb_page(pvmw->vma)) {
>>>>> + struct hstate *hstate = hstate_vma(pvmw->vma);
>>>>> + unsigned long haddr = pvmw->address & huge_page_mask(hstate);
>>>>> +
>>>>> + ptent = huge_ptep_get(pvmw->vma->vm_mm, haddr, pvmw->pte);
>>>>> + } else {
>>>>> ptent = ptep_get(pvmw->pte);
>>>>> + }
>>>>>
>>>>> if (pvmw->flags & PVMW_MIGRATION) {
>>>>> const softleaf_t entry = softleaf_from_pte(ptent);
>>>>> --
>>>>>
>>>>> while leaving pvmw->address unchanged for page_mapped_in_vma()?
>>>>>
>>>>> Cheers, Lance
>>>>>
>>>>>> + else
>>>>>> + ptent = ptep_get(pvmw->pte);
>>>>>>
>>>>>> if (pvmw->flags & PVMW_MIGRATION) {
>>>>>> const softleaf_t entry = softleaf_from_pte(ptent);
>>>>>> --
>>>>>> 2.43.0
>>>>>>
>>>>>>
>>>>
>>>>
>>>
>>
>
>