On 11/08/2026 1:11 pm, David Hildenbrand (Arm) wrote:
> On 8/10/26 13:06, Muhammad Usama Anjum wrote:
>> On 10/08/2026 7:44 am, Alexander Gordeev wrote:
>>> On Fri, Aug 07, 2026 at 05:26:04PM +0100, Muhammad Usama Anjum wrote:
>>>> Yes, this is particular line is for non MMU. In this case, 
>>>> CONIFG_ARCH_HAS_HW_PTE
>>>> would never be defined. Hence hw_pte_t is just pte_t and direct 
>>>> dereference is
>>>> allowed. I'd thought a lot about it; is better to leave direct dereference 
>>>> here
>>>> or use some helper. Then used __pte_from_hw() was already being used in 
>>>> generic
>>>> ptep_get().
>>>
>>> But in case CONIFG_ARCH_HAS_HW_PTE=n __pte_from_hw() is still gets called.
>>> That looks inconsistent to me. Why not just call ptep_deref() (see below)?
>>
>> Agreed. Calling __pte_from_hw() directly exposes the representation
>> conversion at the call site. I will introduce ptep_deref() and use it
>> here.
>>
>>>
>>>> There are only two users of __pte_from_hw() at this time. 
>>>>
>>>> ptep_get_sw() or ptep_get_deref() is better name here?
>>>
>>> ptep_deref() would be it.
>>>
>>> Do you agree to the suggested API requirements?
>>
>> Yes. hw_pte_t * identifies storage containing hardware-formatted PTEs,
>> regardless of whether it is attached. ptep_get() is used for attached
>> entries and may provide additional architecture-specific handling.
>> ptep_deref() is used for unattached entries and performs only the raw
>> storage-to-value conversion.
>>
>> For review, this patch would become:
>>
>> diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h
>> index bc0b9c65aa1d0..ce900d2652d91 100644
>> --- a/include/linux/hugetlb.h
>> +++ b/include/linux/hugetlb.h
>> @@ -1283,7 +1283,7 @@ static inline pte_t huge_ptep_clear_flush(struct 
>> vm_area_struct *vma,
>>  #ifdef CONFIG_MMU
>>      return ptep_get(ptep);
>>  #else
>> -    return *ptep;
>> +    return ptep_deref(ptep);
>>  #endif
>>  }
>>  
>> diff --git a/include/linux/pgtable.h b/include/linux/pgtable.h
>> index 1768421755a9c..08613593f3320 100644
>> --- a/include/linux/pgtable.h
>> +++ b/include/linux/pgtable.h
>> @@ -490,6 +490,13 @@ static inline int pudp_set_access_flags(struct 
>> vm_area_struct *vma,
>>  #endif /* CONFIG_TRANSPARENT_HUGEPAGE */
>>  #endif
>>  
>> +#ifndef ptep_deref
>> +static inline pte_t ptep_deref(hw_pte_t *ptep)
>> +{
>> +    return __pte_from_hw(*ptep);
>> +}
>> +#endif
>> +
>>  #ifndef ptep_get
>>  static inline pte_t ptep_get(hw_pte_t *ptep)
>>  {
>>
> 
> I mean, how many such users do we expect? 1? :)
> 
> Why have a helper for that then, that seems to encourage it's use, when really
> people should be using ptep_get() ?

There is only 1 direct dereference case and even that is for non-MMU case.
That's really good point. I'll keep using __pte_from_hw() and put a comment
in huge_ptep_clear_flush() that use of this must be avoided at all cost. An
API can be introduced in case more users arrive.

-- 
Thanks,
Usama


Reply via email to