From: Ryan Roberts <ryan.roberts@arm.com>
To: Anshuman Khandual <anshuman.khandual@arm.com>
Cc: linux-arm-kernel@lists.infradead.org,
Catalin Marinas <catalin.marinas@arm.com>,
Will Deacon <will@kernel.org>,
Mark Rutland <mark.rutland@arm.com>,
Lorenzo Stoakes <ljs@kernel.org>,
Andrew Morton <akpm@linux-foundation.org>,
David Hildenbrand <david@kernel.org>,
Mike Rapoport <rppt@kernel.org>,
Linu Cherian <linu.cherian@arm.com>,
linux-kernel@vger.kernel.org, linux-mm@kvack.org,
Mark Rtland <mark.rtland@arm.com>,
linx-arm-kernel@lists.infradead.org, linx-kernel@vger.kernel.org,
kasan-dev@googlegrops.com
Subject: Re: [PATCH V2 1/4] arm64/mm: Use pmdp_get() for PMD accesses
Date: Wed, 23 Sep 2026 12:31:42 +0100 [thread overview]
Message-ID: <bce043cc-0f69-4297-8f31-92dcfbc0061d@arm.com> (raw)
In-Reply-To: <m72rpspdg5cv7xptent4tglgd25zehx462nfmcx5uyjwi2ag7p@bc6hvhosgtfb>
On 23/09/2026 12:01, Anshuman Khandual wrote:
> On Wed, Sep 23, 2026 at 09:32:17AM +0100, Ryan Roberts wrote:
>> On 22/09/2026 07:16, Anshuman Khandual wrote:
>>> Replace READ_ONCE() with pmdp_get() for PMD accesses in preparation for
>>> supporting both D64 and D128 translation table formats.
>>>
>>> READ_ONCE() cannot currently be used for 128-bit page table entries on
>>> arm64 because it does not provide the required 128-bit single-copy
>>> atomicity, causing builds to fail for accesses wider than 64 bits.
>>>
>>> Although LDP/STP provide the required atomicity when FEAT_LSE is
>>> available (as required by FEAT_D128), extending READ_ONCE() to support
>>> 128-bit accesses is undesirable. READ_ONCE() is a general-purpose API,
>>> so doing so could encourage other 128-bit users that would either fail
>>> to build in configurations without D128 support or, if D128 becomes a
>>> runtime option, silently permit tearing on systems without the required
>>> hardware support.
>>>
>>> Instead, standardize PMD accesses on the existing page-table helpers.
>>> These can be overridden on arm64 to provide 128-bit single-copy
>>> atomicity when required. No functional change intended.
>>
>> I notice you have an unconverted READ_ONCE(*pmdp) in
>> pmdp_test_and_clear_young(). Is that intentional?
>
> Converting the above READ_ONCE() instances as pmdp_get() cuases a build
> failure as pmpd_get() which is defined in generic pgtable header is not
> visible inside platform specific pgtable header, even with a forward
> declaration.
>
> In D128 V2 series these conversions were performed later in the series,
> when platform specific pxdp_get() overrides were added via ptval_get().
>
> But to avoid this, we could just move pmdp_test_and_clear_young() inside
> arch/arm64/mm/mmu.c file instead. Folding in the following change builds.
I'm not sure if/how this might affect performance? Probably not significant
since they are for higher levels, so not called as frequently as pte helpers.
Another option would be to implement arm64-specific pxdp_get() helpers in
asm/pgtable.h to override the generic versions?
Thanks,
Ryan
>
> diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
> index ae41946c54f26..13ca85ec89f6a 100644
> --- a/arch/arm64/include/asm/pgtable.h
> +++ b/arch/arm64/include/asm/pgtable.h
> @@ -1289,14 +1289,8 @@ static inline bool __ptep_clear_flush_young(struct vm_area_struct *vma,
>
> #if defined(CONFIG_TRANSPARENT_HUGEPAGE) || defined(CONFIG_ARCH_HAS_NONLEAF_PMD_YOUNG)
> #define __HAVE_ARCH_PMDP_TEST_AND_CLEAR_YOUNG
> -static inline bool pmdp_test_and_clear_young(struct vm_area_struct *vma,
> - unsigned long address, pmd_t *pmdp)
> -{
> - /* Operation applies to PMD table entry only if FEAT_HAFT is enabled */
> - VM_WARN_ON(pmd_table(READ_ONCE(*pmdp)) && !system_supports_haft());
> - return __ptep_test_and_clear_young(vma, address, (pte_t *)pmdp);
> -}
> -#endif /* CONFIG_TRANSPARENT_HUGEPAGE || CONFIG_ARCH_HAS_NONLEAF_PMD_YOUNG */
> +bool pmdp_test_and_clear_young(struct vm_area_struct *vma, unsigned long address, pmd_t *pmdp);
> +#endif
>
> static inline pte_t __ptep_get_and_clear_anysz(struct mm_struct *mm,
> unsigned long address,
> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
> index 76d8d320aeedf..b3fd06a623db5 100644
> --- a/arch/arm64/mm/mmu.c
> +++ b/arch/arm64/mm/mmu.c
> @@ -2440,4 +2440,15 @@ void __check_safe_pte_update(struct mm_struct *mm, pte_t *ptep, pte_t pte)
> __func__, pte_str_old, pte_str);
> }
> #endif /* CONFIG_DEBUG_VM */
> +
> +#if defined(CONFIG_TRANSPARENT_HUGEPAGE) || defined(CONFIG_ARCH_HAS_NONLEAF_PMD_YOUNG)
> +#define __HAVE_ARCH_PMDP_TEST_AND_CLEAR_YOUNG
> +bool pmdp_test_and_clear_young(struct vm_area_struct *vma, unsigned long address, pmd_t *pmdp)
> +{
> + /* Operation applies to PMD table entry only if FEAT_HAFT is enabled */
> + VM_WARN_ON(pmd_table(pmdp_get(pmdp)) && !system_supports_haft());
> + return __ptep_test_and_clear_young(vma, address, (pte_t *)pmdp);
> +}
> +#endif /* CONFIG_TRANSPARENT_HUGEPAGE || CONFIG_ARCH_HAS_NONLEAF_PMD_YOUNG */
> +
> #endif
>
>>
>> Thanks,
>> Ryan
>>
>>
>>>
>>> Cc: Catalin Marinas <catalin.marinas@arm.com>
>>> Cc: Will Deacon <will@kernel.org>
>>> Cc: Ryan Roberts <ryan.roberts@arm.com>
>>> Cc: Mark Rtland <mark.rtland@arm.com>
>>> Cc: linx-arm-kernel@lists.infradead.org
>>> Cc: linx-kernel@vger.kernel.org
>>> Cc: kasan-dev@googlegrops.com
>>> Reviewed-by: Ryan Roberts <ryan.roberts@arm.com>
>>> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
>>> ---
>>> arch/arm64/include/asm/pgtable.h | 3 ++-
>>> arch/arm64/mm/fault.c | 2 +-
>>> arch/arm64/mm/fixmap.c | 2 +-
>>> arch/arm64/mm/hugetlbpage.c | 2 +-
>>> arch/arm64/mm/kasan_init.c | 4 ++--
>>> arch/arm64/mm/mmu.c | 22 +++++++++++-----------
>>> arch/arm64/mm/pageattr.c | 2 +-
>>> arch/arm64/mm/trans_pgd.c | 2 +-
>>> 8 files changed, 20 insertions(+), 19 deletions(-)
>>>
>>> diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
>>> index 763c5a411d64..b953ea0ba3ed 100644
>>> --- a/arch/arm64/include/asm/pgtable.h
>>> +++ b/arch/arm64/include/asm/pgtable.h
>>> @@ -811,7 +811,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))
>>>
>>> #define pte_set_fixmap(addr) ((pte_t *)set_fixmap_offset(FIX_PTE, addr))
>>> #define pte_set_fixmap_offset(pmd, addr) pte_set_fixmap(pte_offset_phys(pmd, addr))
>>> diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
>>> index 2cecf6ba6df7..2757ee0c4300 100644
>>> --- a/arch/arm64/mm/fault.c
>>> +++ b/arch/arm64/mm/fault.c
>>> @@ -188,7 +188,7 @@ static void show_pte(unsigned long addr)
>>> break;
>>>
>>> pmdp = pmd_offset_lockless(pudp, pud, addr);
>>> - pmd = READ_ONCE(*pmdp);
>>> + pmd = pmdp_get(pmdp);
>>> ptval_to_str(pxd_str, pmd_val(pmd));
>>> pr_cont(", pmd=%s", pxd_str);
>>> if (pmd_none(pmd) || pmd_bad(pmd))
>>> diff --git a/arch/arm64/mm/fixmap.c b/arch/arm64/mm/fixmap.c
>>> index f66a0016dd02..3cdac8021d4f 100644
>>> --- a/arch/arm64/mm/fixmap.c
>>> +++ b/arch/arm64/mm/fixmap.c
>>> @@ -42,7 +42,7 @@ static inline pte_t *fixmap_pte(unsigned long addr)
>>>
>>> static void __init early_fixmap_init_pte(pmd_t *pmdp, unsigned long addr)
>>> {
>>> - pmd_t pmd = READ_ONCE(*pmdp);
>>> + pmd_t pmd = pmdp_get(pmdp);
>>> pte_t *ptep;
>>>
>>> if (pmd_none(pmd)) {
>>> diff --git a/arch/arm64/mm/hugetlbpage.c b/arch/arm64/mm/hugetlbpage.c
>>> index 8e799c1fe0aa..cdaa4500faf9 100644
>>> --- a/arch/arm64/mm/hugetlbpage.c
>>> +++ b/arch/arm64/mm/hugetlbpage.c
>>> @@ -304,7 +304,7 @@ pte_t *huge_pte_offset(struct mm_struct *mm,
>>> addr &= CONT_PMD_MASK;
>>>
>>> pmdp = pmd_offset(pudp, addr);
>>> - pmd = READ_ONCE(*pmdp);
>>> + pmd = pmdp_get(pmdp);
>>> if (!(sz == PMD_SIZE || sz == CONT_PMD_SIZE) &&
>>> pmd_none(pmd))
>>> return NULL;
>>> diff --git a/arch/arm64/mm/kasan_init.c b/arch/arm64/mm/kasan_init.c
>>> index 45fbdce684c8..7ca833c5de5e 100644
>>> --- a/arch/arm64/mm/kasan_init.c
>>> +++ b/arch/arm64/mm/kasan_init.c
>>> @@ -62,7 +62,7 @@ static phys_addr_t __init kasan_alloc_raw_page(int node)
>>> static pte_t *__init kasan_pte_offset(pmd_t *pmdp, unsigned long addr, int node,
>>> bool early)
>>> {
>>> - if (pmd_none(READ_ONCE(*pmdp))) {
>>> + if (pmd_none(pmdp_get(pmdp))) {
>>> phys_addr_t pte_phys = early ?
>>> __pa_symbol(kasan_early_shadow_pte)
>>> : kasan_alloc_zeroed_page(node);
>>> @@ -138,7 +138,7 @@ static void __init kasan_pmd_populate(pud_t *pudp, unsigned long addr,
>>> do {
>>> next = pmd_addr_end(addr, end);
>>> kasan_pte_populate(pmdp, addr, next, node, early);
>>> - } while (pmdp++, addr = next, addr != end && pmd_none(READ_ONCE(*pmdp)));
>>> + } while (pmdp++, addr = next, addr != end && pmd_none(pmdp_get(pmdp)));
>>> }
>>>
>>> static void __init kasan_pud_populate(p4d_t *p4dp, unsigned long addr,
>>> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
>>> index e589fb00d1e3..95621913679e 100644
>>> --- a/arch/arm64/mm/mmu.c
>>> +++ b/arch/arm64/mm/mmu.c
>>> @@ -200,7 +200,7 @@ static int alloc_init_cont_pte(pmd_t *pmdp, unsigned long addr,
>>> int flags)
>>> {
>>> unsigned long next;
>>> - pmd_t pmd = READ_ONCE(*pmdp);
>>> + pmd_t pmd = pmdp_get(pmdp);
>>> pte_t *ptep;
>>>
>>> BUG_ON(pmd_leaf(pmd));
>>> @@ -257,7 +257,7 @@ static int init_pmd(pmd_t *pmdp, unsigned long addr, unsigned long end,
>>> unsigned long next;
>>>
>>> do {
>>> - pmd_t old_pmd = READ_ONCE(*pmdp);
>>> + pmd_t old_pmd = pmdp_get(pmdp);
>>>
>>> next = pmd_addr_end(addr, end);
>>>
>>> @@ -272,7 +272,7 @@ static int init_pmd(pmd_t *pmdp, unsigned long addr, unsigned long end,
>>> * only allow updates to the permission attributes.
>>> */
>>> BUG_ON(!pgattr_change_is_safe(pmd_val(old_pmd),
>>> - READ_ONCE(pmd_val(*pmdp))));
>>> + pmd_val(pmdp_get(pmdp))));
>>> } else {
>>> int ret;
>>>
>>> @@ -282,7 +282,7 @@ static int init_pmd(pmd_t *pmdp, unsigned long addr, unsigned long end,
>>> return ret;
>>>
>>> VM_WARN_ON_ONCE(pmd_val(old_pmd) != 0 &&
>>> - pmd_val(old_pmd) != READ_ONCE(pmd_val(*pmdp)));
>>> + pmd_val(old_pmd) != pmd_val(pmdp_get(pmdp)));
>>> }
>>> phys += next - addr;
>>> } while (pmdp++, addr = next, addr != end);
>>> @@ -293,7 +293,7 @@ static int init_pmd(pmd_t *pmdp, unsigned long addr, unsigned long end,
>>> static bool pmd_range_has_valid_noncont(pmd_t *pmdp)
>>> {
>>> for (int i = 0; i < CONT_PMDS; i++) {
>>> - pte_t pte = pmd_pte(READ_ONCE(pmdp[i]));
>>> + pte_t pte = pmd_pte(pmdp_get(pmdp + i));
>>>
>>> if (pte_valid(pte) && !pte_cont(pte))
>>> return true;
>>> @@ -1553,7 +1553,7 @@ static void unmap_hotplug_pmd_range(pud_t *pudp, unsigned long addr,
>>> do {
>>> next = pmd_addr_end(addr, end);
>>> pmdp = pmd_offset(pudp, addr);
>>> - pmd = READ_ONCE(*pmdp);
>>> + pmd = pmdp_get(pmdp);
>>> if (pmd_none(pmd))
>>> continue;
>>>
>>> @@ -1708,7 +1708,7 @@ static void free_empty_pmd_table(pud_t *pudp, unsigned long addr,
>>> do {
>>> next = pmd_addr_end(addr, end);
>>> pmdp = pmd_offset(pudp, addr);
>>> - pmd = READ_ONCE(*pmdp);
>>> + pmd = pmdp_get(pmdp);
>>> if (pmd_none(pmd))
>>> continue;
>>>
>>> @@ -1729,7 +1729,7 @@ static void free_empty_pmd_table(pud_t *pudp, unsigned long addr,
>>> */
>>> pmdp = pmd_offset(pudp, 0UL);
>>> for (i = 0; i < PTRS_PER_PMD; i++) {
>>> - if (!pmd_none(READ_ONCE(pmdp[i])))
>>> + if (!pmd_none(pmdp_get(pmdp + i)))
>>> return;
>>> }
>>>
>>> @@ -1881,7 +1881,7 @@ int pmd_set_huge(pmd_t *pmdp, phys_addr_t phys, pgprot_t prot)
>>> pmd_t new_pmd = pfn_pmd(__phys_to_pfn(phys), mk_pmd_sect_prot(prot));
>>>
>>> /* Only allow permission changes for now */
>>> - if (!pgattr_change_is_safe(READ_ONCE(pmd_val(*pmdp)),
>>> + if (!pgattr_change_is_safe(pmd_val(pmdp_get(pmdp)),
>>> pmd_val(new_pmd)))
>>> return 0;
>>>
>>> @@ -1906,7 +1906,7 @@ int pud_clear_huge(pud_t *pudp)
>>>
>>> int pmd_clear_huge(pmd_t *pmdp)
>>> {
>>> - if (!pmd_leaf(READ_ONCE(*pmdp)))
>>> + if (!pmd_leaf(pmdp_get(pmdp)))
>>> return 0;
>>> pmd_clear(pmdp);
>>> return 1;
>>> @@ -1917,7 +1917,7 @@ int pmd_free_pte_page(pmd_t *pmdp, unsigned long addr)
>>> pte_t *table;
>>> pmd_t pmd;
>>>
>>> - pmd = READ_ONCE(*pmdp);
>>> + pmd = pmdp_get(pmdp);
>>>
>>> if (!pmd_table(pmd)) {
>>> VM_WARN_ON(1);
>>> diff --git a/arch/arm64/mm/pageattr.c b/arch/arm64/mm/pageattr.c
>>> index bbe98ac9ad8c..0ca07bd5ded9 100644
>>> --- a/arch/arm64/mm/pageattr.c
>>> +++ b/arch/arm64/mm/pageattr.c
>>> @@ -414,7 +414,7 @@ bool kernel_page_present(struct page *page)
>>> return pud_valid(pud);
>>>
>>> pmdp = pmd_offset(pudp, addr);
>>> - pmd = READ_ONCE(*pmdp);
>>> + pmd = pmdp_get(pmdp);
>>> if (pmd_none(pmd))
>>> return false;
>>> if (pmd_leaf(pmd))
>>> diff --git a/arch/arm64/mm/trans_pgd.c b/arch/arm64/mm/trans_pgd.c
>>> index cca9706a875c..b27b2d2c20c3 100644
>>> --- a/arch/arm64/mm/trans_pgd.c
>>> +++ b/arch/arm64/mm/trans_pgd.c
>>> @@ -74,7 +74,7 @@ static int copy_pmd(struct trans_pgd_info *info, pud_t *dst_pudp,
>>>
>>> src_pmdp = pmd_offset(src_pudp, start);
>>> do {
>>> - pmd_t pmd = READ_ONCE(*src_pmdp);
>>> + pmd_t pmd = pmdp_get(src_pmdp);
>>>
>>> next = pmd_addr_end(addr, end);
>>> if (pmd_none(pmd))
>>
next prev parent reply other threads:[~2026-09-23 11:31 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-22 6:16 [PATCH V2 0/4] arm64/mm: Convert pgtable READ_ONCE() as pxdp_get() Anshuman Khandual
2026-09-22 6:16 ` [PATCH V2 1/4] arm64/mm: Use pmdp_get() for PMD accesses Anshuman Khandual
2026-09-23 8:32 ` Ryan Roberts
2026-09-23 11:01 ` Anshuman Khandual
2026-09-23 11:31 ` Ryan Roberts [this message]
2026-09-23 12:22 ` Anshuman Khandual
2026-09-23 12:50 ` Ryan Roberts
2026-09-22 6:16 ` [PATCH V2 2/4] arm64/mm: Use pudp_get() for PUD accesses Anshuman Khandual
2026-09-22 6:16 ` [PATCH V2 3/4] arm64/mm: Use p4dp_get() for P4D accesses Anshuman Khandual
2026-09-23 8:30 ` Ryan Roberts
2026-09-22 6:16 ` [PATCH V2 4/4] arm64/mm: Use pgdp_get() for PGD accesses Anshuman Khandual
2026-09-23 8:31 ` Ryan Roberts
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=bce043cc-0f69-4297-8f31-92dcfbc0061d@arm.com \
--to=ryan.roberts@arm.com \
--cc=akpm@linux-foundation.org \
--cc=anshuman.khandual@arm.com \
--cc=catalin.marinas@arm.com \
--cc=david@kernel.org \
--cc=kasan-dev@googlegrops.com \
--cc=linu.cherian@arm.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=linx-arm-kernel@lists.infradead.org \
--cc=linx-kernel@vger.kernel.org \
--cc=ljs@kernel.org \
--cc=mark.rtland@arm.com \
--cc=mark.rutland@arm.com \
--cc=rppt@kernel.org \
--cc=will@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®