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

Anshuman Khandual
2026-04-10T05:05:42Z

On 08/04/26 5:47 PM, David Hildenbrand (Arm) wrote:
> 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.

There are multiple similar BUG_ON() instances

arch/arm64/include/asm/pgtable.h: BUG_ON(!pgtable_l4_enabled());
arch/arm64/include/asm/pgtable.h: BUG_ON(!pgtable_l5_enabled());

arch/arm64/mm/mmu.c: BUG_ON(pmd_val(old_pmd) != 0 &&
arch/arm64/mm/mmu.c: BUG_ON(pud_val(old_pud) != 0 &&
arch/arm64/mm/mmu.c: BUG_ON(p4d_val(old_p4d) != 0 &&

Shall we convert all of them as VM_WARN_ON_ONCE() in a separate patch
as a pre-requisite ?

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