Re: [PATCH] arm64: mm: Use generic enum pgtable_level
From: Ryan Roberts
Date: Tue Mar 17 2026 - 11:18:32 EST
On 17/03/2026 12:47, Kevin Brodsky wrote:
> On 16/03/2026 15:45, David Hildenbrand (Arm) wrote:
>> On 3/16/26 15:38, Kevin Brodsky wrote:
>>> On 16/03/2026 15:22, Ryan Roberts wrote:
>>>>> static phys_addr_t __pgd_pgtable_alloc(struct mm_struct *mm, gfp_t gfp,
>>>>> - enum pgtable_type pgtable_type)
>>>>> + enum pgtable_level pgtable_level)
>>>>> {
>>>>> /* Page is zeroed by init_clear_pgtable() so don't duplicate effort. */
>>>>> struct ptdesc *ptdesc = pagetable_alloc(gfp & ~__GFP_ZERO, 0);
>>>>> @@ -539,40 +539,42 @@ static phys_addr_t __pgd_pgtable_alloc(struct mm_struct *mm, gfp_t gfp,
>>>>>
>>>>> pa = page_to_phys(ptdesc_page(ptdesc));
>>>>>
>>>>> - switch (pgtable_type) {
>>>>> - case TABLE_PTE:
>>>>> + switch (pgtable_level) {
>>>>> + case PGTABLE_LEVEL_PTE:
>>>>> BUG_ON(!pagetable_pte_ctor(mm, ptdesc));
>>>>> break;
>>>>> - case TABLE_PMD:
>>>>> + case PGTABLE_LEVEL_PMD:
>>>>> BUG_ON(!pagetable_pmd_ctor(mm, ptdesc));
>>>>> break;
>>>>> - case TABLE_PUD:
>>>>> + case PGTABLE_LEVEL_PUD:
>>>>> pagetable_pud_ctor(ptdesc);
>>>>> break;
>>>>> - case TABLE_P4D:
>>>>> + case PGTABLE_LEVEL_P4D:
>>>>> pagetable_p4d_ctor(ptdesc);
>>>>> break;
>>>>> + default:
>>>>> + break;
>>>> nit: I think we should either explicitly support pgd or explicitly bug/warn. Now
>>>> that the enum has PGTABLE_LEVEL_PGD it looks legit to call __pgd_pgtable_alloc()
>>>> to allocate one. But it will currently silently fail to call pagetable_pgd_ctor().
>>> I hesitated there as well, eventually I concluded that we're dealing
>>> with kernel page tables so we'll never allocate a PGD anyway...
>>>
>>>> Probably simplest just to call BUG() in the default path?
>>> ... but that's certainly fine by me :)
>> If we could force it to be inline, we could turn it into a BUILD_BUG().
>
> I don't think this would help, pgd_pgtable_alloc_init_mm() is passed
> around as a function pointer so the compiler can't know the value of
> pgtable_level at compile time.
>
>> VM_WARN_ON()
>>
>> might be good enough I guess.
Fine by me.
>
> I think so, not calling a ctor at PGD level has no real consequence (it
> wasn't done at all until recently).
>
> - Kevin