Re: [PATCH v2] mm/page_vma_mapped_walk: Use ptep_get_lockless() for lockless access

["David Hildenbrand (Arm)" <david@kernel.org>]

On 5/8/26 07:15, Alexander Gordeev wrote:
> On Fri, May 08, 2026 at 01:00:40AM +0000, Wei Yang wrote:
>> On Thu, May 07, 2026 at 12:32:09PM +0200, Alexander Gordeev wrote:
>>>
>>> If I read the code correctly map_pte() might take the lock, but also
>>> might not take it. If it took the lock and uses ptep_get_lockless(),
>>> then it is fine. But if it did not take the lock and uses ptep_get(),
>>> then it is an issue.
>>>
>>
>> So the rule here is:
>>
>>   * ptep_get_lockless() could be used for locked and not locked
>>   * ptep_get() only used when locked
>>
>> Right?
> 
> Yes, this is my assumption.

I agree, ptep_get_lockless() simply makes sense to return something sensible if
there are concurrent modifications (which cannot happen when the PTL is held).

That's why only 32bit with 64bit PTEs and arm64 even has to special-case it.


We should clarify in the patch description that in the do-while loop, we might
or might not hold the PTL, and that calling ptep_get_lockless() with the PTL
held is OK.

I wonder if it's more efficient and clearer, to use the correct variant, though?


diff --git a/mm/page_vma_mapped.c b/mm/page_vma_mapped.c
index a4d52fdb3056..36d97661a4e5 100644
--- a/mm/page_vma_mapped.c
+++ b/mm/page_vma_mapped.c
@@ -187,6 +187,7 @@ bool page_vma_mapped_walk(struct page_vma_mapped_walk *pvmw)
        p4d_t *p4d;
        pud_t *pud;
        pmd_t pmde;
+       pte_t pteval;

        /* The only possible pmd mapping has been handled on last iteration */
        if (pvmw->pmd && !pvmw->pte)
@@ -310,7 +311,11 @@ bool page_vma_mapped_walk(struct page_vma_mapped_walk *pvmw)
                                goto restart;
                        }
                        pvmw->pte++;
-               } while (pte_none(ptep_get(pvmw->pte)));
+                       if (!pvmw->ptl)
+                               pteval = ptep_get_lockless(pvmw->pte);
+                       else
+                               pteval = ptep_get(pvmw->pte);
+               } while (pte_none(pteval));

                if (!pvmw->ptl) {
                        spin_lock(ptl);


-- 
Cheers,

David