Re: [PATCH] arm64: mm: Use generic enum pgtable_level

["David Hildenbrand (Arm)" <david@kernel.org>]

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().

	VM_WARN_ON()

might be good enough I guess.

-- 
Cheers,

David