Re: [RFC V1 07/16] arm64/mm: Convert READ_ONCE() as p4dp_get() while accessing P4D

David Hildenbrand (Arm)
2026-04-08T12:17:35Z

On 2/24/26 06:11, Anshuman Khandual wrote:
> Convert all READ_ONCE() based P4D accesses as p4dp_get() instead which will
> support both D64 and D128 translation regime going forward.
>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will@kernel.org>
> Cc: Ryan Roberts <ryan.roberts@arm.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-kernel@vger.kernel.org
> Cc: kasan-dev@googlegroups.com
> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
> ---

[...]

> static void __init kasan_pgd_populate(unsigned long addr, unsigned long end,
> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
> index a80d06db4de6..16ae11b29f66 100644
> --- a/arch/arm64/mm/mmu.c
> +++ b/arch/arm64/mm/mmu.c
> @@ -354,7 +354,7 @@ static int alloc_init_pud(p4d_t *p4dp, unsigned long addr, unsigned long end,
> {
> int ret = 0;
> unsigned long next;
> - p4d_t p4d = READ_ONCE(*p4dp);
> + p4d_t p4d = p4dp_get(p4dp);
> pud_t *pudp;
>
> if (p4d_none(p4d)) {
> @@ -443,7 +443,7 @@ static int alloc_init_p4d(pgd_t *pgdp, unsigned long addr, unsigned long end,
> }
>
> do {
> - p4d_t old_p4d = READ_ONCE(*p4dp);
> + p4d_t old_p4d = p4dp_get(p4dp);
>
> next = p4d_addr_end(addr, end);
>
> @@ -453,7 +453,7 @@ static int alloc_init_p4d(pgd_t *pgdp, unsigned long addr, unsigned long end,
> goto out;
>
> BUG_ON(p4d_val(old_p4d) != 0 &&
> - p4d_val(old_p4d) != READ_ONCE(p4d_val(*p4dp)));
> + p4d_val(old_p4d) != (p4d_val(p4dp_get(p4dp))));

Same here, while at it remove the BUG_ON. (see below)

>
> phys += next - addr;
> } while (p4dp++, addr = next, addr != end);
> @@ -1541,7 +1541,7 @@ static void unmap_hotplug_p4d_range(pgd_t *pgdp, unsigned long addr,
> do {
> next = p4d_addr_end(addr, end);
> p4dp = p4d_offset(pgdp, addr);
> - p4d = READ_ONCE(*p4dp);
> + p4d = p4dp_get(p4dp);
> if (p4d_none(p4d))
> continue;
>
> @@ -1703,7 +1703,7 @@ static void free_empty_p4d_table(pgd_t *pgdp, unsigned long addr,
> do {
> next = p4d_addr_end(addr, end);
> p4dp = p4d_offset(pgdp, addr);
> - p4d = READ_ONCE(*p4dp);
> + p4d = p4dp_get(p4dp);
> if (p4d_none(p4d))
> continue;
>
> @@ -1724,7 +1724,7 @@ static void free_empty_p4d_table(pgd_t *pgdp, unsigned long addr,
> */
> p4dp = p4d_offset(pgdp, 0UL);
> for (i = 0; i < PTRS_PER_P4D; i++) {
> - if (!p4d_none(READ_ONCE(p4dp[i])))
> + if (!p4d_none(p4dp_get(p4dp + i)))
> return;
> }
>
> @@ -2258,4 +2258,21 @@ int pmdp_test_and_clear_young(struct vm_area_struct *vma,
> }
> #endif /* CONFIG_TRANSPARENT_HUGEPAGE || CONFIG_ARCH_HAS_NONLEAF_PMD_YOUNG */
>
> +#if CONFIG_PGTABLE_LEVELS > 3
> +phys_addr_t pud_offset_phys(p4d_t *p4dp, unsigned long addr)
> +{
> + p4d_t p4d = p4dp_get(p4dp);
> +
> + BUG_ON(!pgtable_l4_enabled());

Heh, while at it, convert that to a VM_WARN_ON_ONCE() or anything else
that is not a BUG.

I strongly assume CONFIG_DEBUG_VM checks are sufficient.

> +
> + return p4d_page_paddr(p4d) + pud_index(addr) * sizeof(pud_t);
> +}
> +

--
Cheers,

David