Re: [PATCH v2] powerpc/book3s/mm: Update Oops message to print the correct translation in use

[Christophe Leroy <christophe.leroy@c-s.fr>]

Le 12/07/2019 à 08:25, Michael Ellerman a écrit :
> "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> writes:
> 
>> Avoids confusion when printing Oops message like below
>>
>>   Faulting instruction address: 0xc00000000008bdb4
>>   Oops: Kernel access of bad area, sig: 11 [#1]
>>   LE PAGE_SIZE=64K MMU=Radix MMU=Hash SMP NR_CPUS=2048 NUMA PowerNV
>>
>> This was because we never clear the MMU_FTR_HPTE_TABLE feature flag
>> even if we run with radix translation. It was discussed that we should
>> look at this feature flag as an indication of the capability to run
>> hash translation and we should not clear the flag even if we run in
>> radix translation. All the code paths check for radix_enabled() check and
>> if found true consider we are running with radix translation. Follow the
>> same sequence for finding the MMU translation string to be used in Oops
>> message.
>>
>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
>> ---
>>
>> Changes from V1:
>> * Don't clear the HPTE feature flag.
>>
>>   arch/powerpc/kernel/traps.c | 15 +++++++++++----
>>   1 file changed, 11 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
>> index 11caa0291254..b181d6860f28 100644
>> --- a/arch/powerpc/kernel/traps.c
>> +++ b/arch/powerpc/kernel/traps.c
>> @@ -250,15 +250,22 @@ static void oops_end(unsigned long flags, struct pt_regs *regs,
>>   }
>>   NOKPROBE_SYMBOL(oops_end);
>>   
>> +static char *get_mmu_str(void)
>> +{
>> +	if (early_radix_enabled())
>> +		return " MMU=Radix";
>> +	if (early_mmu_has_feature(MMU_FTR_HPTE_TABLE))
>> +		return " MMU=Hash";
>> +	return "";
>> +}
> 
> We don't change MMU once we're up, so just do this logic once and stash
> it into a static string, rather than rechecking on every oops.

Do we really have oops so often that we have to worry about that ?

Christophe

> 
> cheers
> 
>>   static int __die(const char *str, struct pt_regs *regs, long err)
>>   {
>>   	printk("Oops: %s, sig: %ld [#%d]\n", str, err, ++die_counter);
>>   
>> -	printk("%s PAGE_SIZE=%luK%s%s%s%s%s%s%s %s\n",
>> +	printk("%s PAGE_SIZE=%luK%s%s%s%s%s%s %s\n",
>>   	       IS_ENABLED(CONFIG_CPU_LITTLE_ENDIAN) ? "LE" : "BE",
>> -	       PAGE_SIZE / 1024,
>> -	       early_radix_enabled() ? " MMU=Radix" : "",
>> -	       early_mmu_has_feature(MMU_FTR_HPTE_TABLE) ? " MMU=Hash" : "",
>> +	       PAGE_SIZE / 1024, get_mmu_str(),
>>   	       IS_ENABLED(CONFIG_PREEMPT) ? " PREEMPT" : "",
>>   	       IS_ENABLED(CONFIG_SMP) ? " SMP" : "",
>>   	       IS_ENABLED(CONFIG_SMP) ? (" NR_CPUS=" __stringify(NR_CPUS)) : "",
>> -- 
>> 2.21.0