Re: [PATCH 1/4] powerpc/64s: Add DEBUG_PAGEALLOC for radix

[Christophe Leroy <christophe.leroy@csgroup.eu>]

Le 19/09/2022 à 09:00, Michael Ellerman a écrit :
> Nicholas Miehlbradt <nicholas@linux.ibm.com> writes:
>> There is support for DEBUG_PAGEALLOC on hash but not on radix.
>> Add support on radix.
>>
>> Signed-off-by: Nicholas Miehlbradt <nicholas@linux.ibm.com>
>> ---
>>   arch/powerpc/mm/book3s64/radix_pgtable.c | 16 +++++++++++++++-
>>   1 file changed, 15 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/powerpc/mm/book3s64/radix_pgtable.c b/arch/powerpc/mm/book3s64/radix_pgtable.c
>> index db2f3d193448..483c99bfbde5 100644
>> --- a/arch/powerpc/mm/book3s64/radix_pgtable.c
>> +++ b/arch/powerpc/mm/book3s64/radix_pgtable.c
>> @@ -30,6 +30,7 @@
>>   #include <asm/trace.h>
>>   #include <asm/uaccess.h>
>>   #include <asm/ultravisor.h>
>> +#include <asm/set_memory.h>
>>   
>>   #include <trace/events/thp.h>
>>   
>> @@ -503,6 +504,9 @@ static unsigned long __init radix_memory_block_size(void)
>>   {
>>   	unsigned long mem_block_size = MIN_MEMORY_BLOCK_SIZE;
>>   
>> +	if (debug_pagealloc_enabled())
>> +		return PAGE_SIZE;
>> +
>>   	/*
>>   	 * OPAL firmware feature is set by now. Hence we are ok
>>   	 * to test OPAL feature.
>> @@ -519,6 +523,9 @@ static unsigned long __init radix_memory_block_size(void)
>>   
>>   static unsigned long __init radix_memory_block_size(void)
>>   {
>> +	if (debug_pagealloc_enabled())
>> +		return PAGE_SIZE;
>> +
>>   	return 1UL * 1024 * 1024 * 1024;
>>   }
>    
> This value ends up in radix_mem_block_size, which is returned by
> pnv_memory_block_size(), which is wired up as ppc_md.memory_block_size,
> and that's called by memory_block_size_bytes().
> 
> And I thought that value had to be >= MIN_MEMORY_BLOCK_SIZE.
> 
> #define MIN_MEMORY_BLOCK_SIZE     (1UL << SECTION_SIZE_BITS)
> #define SECTION_SIZE_BITS       24
> 
> 
> I would expect us to hit the panic in memory_dev_init().
> 
> So that's odd.
> 
> I suspect you need to leave radix_memory_block_size() alone, or at least
> make sure you return MIN_MEMORY_BLOCK_SIZE when debug page alloc is
> enabled.
> 
> We probably need a separate variable that holds the max page size used
> for the linear mapping, and that would then be 1G in the normal case or
> PAGE_SIZE in the debug page alloc case.
> 

I don't know the details of PPC64, but as you mention linear mapping, be 
aware that you don't need to map everything with pages. Only data. You 
can keep text mapped by blocks, it's what is done on 8xx and book3s/32.

Christophe