Re: [PATCH V3 1/4] arm64/mm: Use pmdp_get() for PMD accesses
From: Catalin Marinas
Date: Mon Oct 12 2026 - 07:04:38 EST
On Wed, Oct 07, 2026 at 08:11:52AM +0530, Anshuman Khandual wrote:
> On Mon, Oct 05, 2026 at 04:31:28PM +0100, Catalin Marinas wrote:
> > On Thu, Sep 24, 2026 at 11:52:11AM +0530, Anshuman Khandual wrote:
> > > diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
> > > index 763c5a411d64..9ad2d03e86ad 100644
> > > --- a/arch/arm64/include/asm/pgtable.h
> > > +++ b/arch/arm64/include/asm/pgtable.h
> > > @@ -84,6 +84,12 @@ static inline void arch_leave_lazy_mmu_mode(void)
> > > arch_flush_lazy_mmu_mode();
> > > }
> > >
> > > +#define pmdp_get pmdp_get
> > > +static inline pmd_t pmdp_get(pmd_t *pmdp)
> > > +{
> > > + return READ_ONCE(*pmdp);
> > > +}
> >
> > This is the same as the generic version. Does it need redefining?
>
> Without this pmdp_get() override, replacing READ_ONCE() in pmdp_test_and_clear_young()
> with pmdp_get() causes a build failure as pmpd_get() which is defined in the generic
> pgtable header is not visible from inside platform specific pgtable header, even with
> a forward declaration.
>
> Earlier had moved pmdp_test_and_clear_young() inside arch/arm64/mm/mmu.c to avoid the
> above described dependency problem. But then realised that pmdp_get() gets eventually
> overridden on arm64 platform for D128, why not just redefine here it now as standard
> READ_ONCE() to avoid the current build problem and the code movement.
>
> Please find some context here from earlier discussions during V2.
>
> https://lore.kernel.org/all/212067fe-1b16-4b87-87a7-4ce50847eb24@xxxxxxx/
Thanks for link summary of the past discussion. I agree, for D128 we'll
have to define our p*d_get() but I haven't decided D128 is worth merging
yet (it depends on future hardware availability, I need to get more
visibility into this).
For the time being, I'd keep the p*d_get() accessors generic and move
pmdp_test_and_clear_young() to mmu.c.
> > > #ifdef CONFIG_TRANSPARENT_HUGEPAGE
> > > #define __HAVE_ARCH_FLUSH_PMD_TLB_RANGE
> > >
> > > @@ -811,7 +817,8 @@ static inline unsigned long pmd_page_vaddr(pmd_t pmd)
> > > }
> > >
> > > /* Find an entry in the third-level page table. */
> > > -#define pte_offset_phys(dir,addr) (pmd_page_paddr(READ_ONCE(*(dir))) + pte_index(addr) * sizeof(pte_t))
> > > +#define pte_offset_phys(dir, addr) (pmd_page_paddr(pmdp_get(dir)) + \
> > > + pte_index(addr) * sizeof(pte_t))
> >
> > Why not make this change in the generic version?
>
> Could you please elaborate on this ? pte_offset_phys() is arm64 specific helper which
> is also local to this header file. AFAICT there is no generic version for this helper.
You are right, I got confused.
Anyway, I'd push this series post -rc1. It depends on the mm tree and
it's getting pretty close to the merging window.
--
Catalin