Re: [PATCH v1] mm: use ptep_get() instead of directly dereferencing pte_t*

[Qi Zheng <zhengqi.arch@bytedance.com>]

On 3/11/25 12:31 AM, Ryan Roberts wrote:
> On 10/03/2025 15:51, Qi Zheng wrote:
>> Hi Ryan,
>>
>> On 3/10/25 10:04 PM, Ryan Roberts wrote:
>>> It is best practice for all pte accesses to go via the arch helpers, to
>>> ensure non-torn values and to allow the arch to intervene where needed
>>> (contpte for arm64 for example). While in this case it was probably safe
>>> to directly dereference, let's tidy it up for consistency.
>>>
>>> Signed-off-by: Ryan Roberts <ryan.roberts@arm.com>
>>> ---
>>>    mm/migrate.c | 2 +-
>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> This looks good to me. So
>>
>> Reviewed-by: Qi Zheng <zhengqi.arch@bytedance.com>
> 
> Thanks!
> 
>>
>> BTW, there are many other places in the kernel that directly
>> dereference pmd_t* and pud_t*, etc.
> 
> It's all a little bit murky. For now, from arm64's perspective at least, there
> is only a hard requirement for ptes to be accessed through the arch helper. This
> is because arm64's contpte layer may apply a transform when reading the pte.
> 
> In general there are also potential issues with tearing, if you don't at least
> read with READ_ONCE(). But often to consumer of the value is tolerant to tearing
> (e.g. pmd_none(), etc). Also, in practice on arm64 the compiler will emit
> instructions that ensure single-copy-atomicity for direct dereferences, so it
> all works out.
> 
> That said, Anshuman (cc'ed) has been looking at supporting FEAT_D128 (128 bit
> page table descriptors) on arm64. The compiler does not emit single-copy-atomic
> loads for direct dereferences of 128 bit data, so he has been working on
> converting the other levels to use the accessors for that reason.
> 
> But that has some potentially problematic interactions with level folding that
> need to be solved. Some arches rely on the compiler optimizing away the direct
> dereferences when folded. But it can't do that for a READ_ONCE().
> 
> I believe Anshuman is aiming to post a series to do this at some point in the
> future.

Got it. Thanks!