Re: [PATCH] mm/debug_vm_pgtable: Fix BUG_ON with pud advanced test

["Aneesh Kumar K.V" <aneesh.kumar@kernel.org>]

On 1/29/24 11:52 AM, Anshuman Khandual wrote:
> 
> 
> On 1/29/24 11:30, Aneesh Kumar K.V (IBM) wrote:
>> Architectures like powerpc add debug checks to ensure we find only devmap
>> PUD pte entries. These debug checks are only done with CONFIG_DEBUG_VM.
>> This patch marks the ptes used for PUD advanced test devmap pte entries
>> so that we don't hit on debug checks on architecture like ppc64 as
>> below.
>>
>> WARNING: CPU: 2 PID: 1 at arch/powerpc/mm/book3s64/radix_pgtable.c:1382 radix__pud_hugepage_update+0x38/0x138
>> ....
>> NIP [c0000000000a7004] radix__pud_hugepage_update+0x38/0x138
>> LR [c0000000000a77a8] radix__pudp_huge_get_and_clear+0x28/0x60
>> Call Trace:
>> [c000000004a2f950] [c000000004a2f9a0] 0xc000000004a2f9a0 (unreliable)
>> [c000000004a2f980] [000d34c100000000] 0xd34c100000000
>> [c000000004a2f9a0] [c00000000206ba98] pud_advanced_tests+0x118/0x334
>> [c000000004a2fa40] [c00000000206db34] debug_vm_pgtable+0xcbc/0x1c48
>> [c000000004a2fc10] [c00000000000fd28] do_one_initcall+0x60/0x388
>>
>> Also
>>
>>  kernel BUG at arch/powerpc/mm/book3s64/pgtable.c:202!
>>  ....
>>
>>  NIP [c000000000096510] pudp_huge_get_and_clear_full+0x98/0x174
>>  LR [c00000000206bb34] pud_advanced_tests+0x1b4/0x334
>>  Call Trace:
>>  [c000000004a2f950] [000d34c100000000] 0xd34c100000000 (unreliable)
>>  [c000000004a2f9a0] [c00000000206bb34] pud_advanced_tests+0x1b4/0x334
>>  [c000000004a2fa40] [c00000000206db34] debug_vm_pgtable+0xcbc/0x1c48
>>  [c000000004a2fc10] [c00000000000fd28] do_one_initcall+0x60/0x388
>>
>> Fixes: 27af67f35631 ("powerpc/book3s64/mm: enable transparent pud hugepage")
>> Signed-off-by: Aneesh Kumar K.V (IBM) <aneesh.kumar@kernel.org>
>> ---
>>  mm/debug_vm_pgtable.c | 8 ++++++++
>>  1 file changed, 8 insertions(+)
>>
>> diff --git a/mm/debug_vm_pgtable.c b/mm/debug_vm_pgtable.c
>> index 5662e29fe253..65c19025da3d 100644
>> --- a/mm/debug_vm_pgtable.c
>> +++ b/mm/debug_vm_pgtable.c
>> @@ -362,6 +362,12 @@ static void __init pud_advanced_tests(struct pgtable_debug_args *args)
>>  	vaddr &= HPAGE_PUD_MASK;
>>  
>>  	pud = pfn_pud(args->pud_pfn, args->page_prot);
>> +	/*
>> +	 * Some architectures have debug checks to make sure
>> +	 * huge pud mapping are only found with devmap entries
>> +	 * For now test with only devmap entries.
>> +	 */
> Do you see this behaviour to be changed in powerpc anytime soon ? Otherwise
> these pud_mkdevmap() based work arounds, might be required to stick around
> for longer just to prevent powerpc specific triggers. Given PUD transparent
> huge pages i.e HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD are just supported on x86
> and powerpc platforms, could not this problem be solved in a more uniform
> manner.
> 


IIUC pud level transparent hugepages are only supported with devmap entries even
on x86. We don't do anonymous pud hugepage.

>> +	pud = pud_mkdevmap(pud);
>>  	set_pud_at(args->mm, vaddr, args->pudp, pud);
>>  	flush_dcache_page(page);
>>  	pudp_set_wrprotect(args->mm, vaddr, args->pudp);
>> @@ -374,6 +380,7 @@ static void __init pud_advanced_tests(struct pgtable_debug_args *args)
>>  	WARN_ON(!pud_none(pud));
>>  #endif /* __PAGETABLE_PMD_FOLDED */
>>  	pud = pfn_pud(args->pud_pfn, args->page_prot);
>> +	pud = pud_mkdevmap(pud);
>>  	pud = pud_wrprotect(pud);
>>  	pud = pud_mkclean(pud);
>>  	set_pud_at(args->mm, vaddr, args->pudp, pud);
>> @@ -391,6 +398,7 @@ static void __init pud_advanced_tests(struct pgtable_debug_args *args)
>>  #endif /* __PAGETABLE_PMD_FOLDED */
>>  
>>  	pud = pfn_pud(args->pud_pfn, args->page_prot);
>> +	pud = pud_mkdevmap(pud);
>>  	pud = pud_mkyoung(pud);
>>  	set_pud_at(args->mm, vaddr, args->pudp, pud);
>>  	flush_dcache_page(page);


-aneesh