Re: [PATCH v2 2/6] mm/rmap: use huge_ptep_get() in try_to_unmap_one()

[Muchun Song <muchun.song@linux.dev>]

> On Jul 2, 2026, at 17:08, Dev Jain <dev.jain@arm.com> wrote:
> 
> 
> 
> On 02/07/26 2:17 pm, Muchun Song wrote:
>> 
>> 
>> On 2026/7/2 13:13, Dev Jain wrote:
>>> try_to_unmap_one() handles hugetlb folios when memory failure needs
>>> to replace a poisoned hugetlb mapping with a hwpoison entry. In that
>>> case page_vma_mapped_walk() returns the pte pointer to the hugetlb folio
>>> in pvmw.pte, but the code reads it with ptep_get().
>>> 
>>> 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.
>>> 
>>> Just use huge_ptep_get() for dereferencing a huge pte pointer.
>>> 
>>> Fixes: c7ab0d2fdc84 ("mm: convert try_to_unmap_one() to use page_vma_mapped_walk()")
>>> Cc: stable@vger.kernel.org
>>> Reported-by: David Hildenbrand <david@kernel.org>
>>> Signed-off-by: Dev Jain <dev.jain@arm.com>
>>> ---
>>>   include/linux/hugetlb.h |  3 +++
>>>   mm/rmap.c               | 16 ++++++++++------
>>>   2 files changed, 13 insertions(+), 6 deletions(-)
>>> 
>>> diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h
>>> index 2abaf99321e90..fdb7bdf7645c5 100644
>>> --- a/include/linux/hugetlb.h
>>> +++ b/include/linux/hugetlb.h
>>> @@ -1261,6 +1261,9 @@ static inline void hugetlb_count_sub(long l, struct mm_struct *mm)
>>>   {
>>>   }
>>>   +pte_t huge_ptep_get(struct mm_struct *mm, unsigned long addr,
>>> +            pte_t *ptep);
>>> +
>> 
>> Maybe I didn't express my thoughts clearly in the first version, let me
>> explain in more detail.
>> 
>> We should define this stub as a no-op for !CONFIG_HUGETLB_PAGE (like
>> set_huge_pte_at, that is why I mentioned 5d4af6195c87c6 for your reference
>> in your previous version). Currently, you've added a declaration, but the
>> function itself doesn't actually exist, which seems quite strange to me.
> 
> https://lore.kernel.org/all/a4fe8ba6-2ecd-4bb9-95a9-27f9f1e87d2e@kernel.org/
> 
> David suggested this. Honestly I quite like David's suggestion, what do you
> think?

Thanks for pointing that out, I missed it earlier. That said, looking at
hugetlb.h, it already contains quite a few no-op stubs. To keep things
consistent, I'd personally prefer a stub here. Since David suggested this,
I’d love to hear his thoughts on this as well.

Muchun,
Thanks

> 
> 
>> 
>> Muchun,
>> Thanks.
>>>   static inline pte_t huge_ptep_clear_flush(struct vm_area_struct *vma,
>>>                         unsigned long addr, pte_t *ptep)
>>>   {
>>> diff --git a/mm/rmap.c b/mm/rmap.c
>>> index 1c77d5dc06e9f..aa8a254efaecc 100644
>>> --- a/mm/rmap.c
>>> +++ b/mm/rmap.c
>>> @@ -2095,11 +2095,16 @@ static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma,
>>>           /* Unexpected PMD-mapped THP? */
>>>           VM_BUG_ON_FOLIO(!pvmw.pte, folio);
>>>   -        /*
>>> -         * Handle PFN swap PTEs, such as device-exclusive ones, that
>>> -         * actually map pages.
>>> -         */
>>> -        pteval = ptep_get(pvmw.pte);
>>> +        address = pvmw.address;
>>> +        if (folio_test_hugetlb(folio)) {
>>> +            pteval = huge_ptep_get(mm, address, pvmw.pte);
>>> +        } else {
>>> +            /*
>>> +             * Handle PFN swap PTEs, such as device-exclusive ones,
>>> +             * that actually map pages.
>>> +             */
>>> +            pteval = ptep_get(pvmw.pte);
>>> +        }
>>>           if (likely(pte_present(pteval))) {
>>>               pfn = pte_pfn(pteval);
>>>           } else {
>>> @@ -2110,7 +2115,6 @@ static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma,
>>>           }
>>>             subpage = folio_page(folio, pfn - folio_pfn(folio));
>>> -        address = pvmw.address;
>>>           anon_exclusive = folio_test_anon(folio) &&
>>>                    PageAnonExclusive(subpage);