Re: [PATCH 6/9] mm: convert PTE table entry to pte
Muhammad Usama Anjum <[email protected]>
| Newsgroups | org.kernel.vger.linux-arch,dev.linux.lists.damon,org.freedesktop.lists.dri-devel,org.freedesktop.lists.intel-gfx,org.kernel.vger.bpf,org.kernel.vger.linux-fsdevel,org.kernel.vger.linux-kernel,org.kernel.vger.linux-parisc,org.kernel.vger.linux-perf-users,org.kernel.vger.linux-trace-kernel,org.kvack.linux-mm,org.xenproject.lists.xen-devel |
|---|---|
| Message-ID | <[email protected]> |
On 10/08/2026 7:44 am, Alexander Gordeev wrote: > On Fri, Aug 07, 2026 at 05:26:04PM +0100, Muhammad Usama Anjum wrote: >> On 07/08/2026 7:58 am, Alexander Gordeev wrote: >>> On Thu, Aug 06, 2026 at 09:38:44AM +0100, Muhammad Usama Anjum wrote: >>>> The non-MMU stub receives hw_pte_t but returns a logical pte_t >>>> value. Convert the stored entry through __pte_from_hw() before >>>> returning. >>>> >>>> Signed-off-by: Muhammad Usama Anjum <[email protected]> >>>> --- >>>> include/linux/hugetlb.h | 2 +- >>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>> >>>> diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h >>>> index bc0b9c65aa1d0..9e8b391aa4bc9 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 __pte_from_hw(*ptep); >>> >>> But this is a direct dereferencing, which breaks the whole point, isn't it? >> 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. >> >>> >>> What about introducing something like pte_t ptep_get_sw(hw_pte_t *ptep) >>> to be used in exactly situations like this? With that the semantics of >>> hw_pte_t pointers becomes straightforward and closes the still ongoing >>> "storage vs lifetime" discussion: >>> >>> hw_pte_t* points to HW-formatted page table entries >>> >>> ptep_get() is used to obtain HW-linked/attached entries, and may wire >>> extra code like [1] or [2] >>> >>> ptep_get_sw() is used to obtain HW-unlinked/unattached entries and in >>> most cases is just a direct dereference >> 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) { -- Thanks, Usama