Re: [PATCH v3] powerpc/mce: Remove per cpu variables from MCE handlers

[Ganesh <ganeshgr@linux.ibm.com>]

On 1/19/21 9:28 AM, Nicholas Piggin wrote:

> Excerpts from Ganesh Goudar's message of January 15, 2021 10:58 pm:
>> Access to per-cpu variables requires translation to be enabled on
>> pseries machine running in hash mmu mode, Since part of MCE handler
>> runs in realmode and part of MCE handling code is shared between ppc
>> architectures pseries and powernv, it becomes difficult to manage
>> these variables differently on different architectures, So have
>> these variables in paca instead of having them as per-cpu variables
>> to avoid complications.
> Seems okay.
>
>> Maximum recursive depth of MCE is 4, Considering the maximum depth
>> allowed reduce the size of event to 10 from 100.
> Could you make this a separate patch, with memory saving numbers?
> "Delayed" MCEs are not necessarily the same as recursive (several
> sequential MCEs can occur before the first event is processed).
> But I agree 100 is pretty overboard (as is 4 recursive MCEs really).

Sure.

>> Signed-off-by: Ganesh Goudar <ganeshgr@linux.ibm.com>
>> ---
>> v2: Dynamically allocate memory for machine check event info
>>
>> v3: Remove check for hash mmu lpar, use memblock_alloc_try_nid
>>      to allocate memory.
>> ---
>>   arch/powerpc/include/asm/mce.h     | 21 ++++++++-
>>   arch/powerpc/include/asm/paca.h    |  4 ++
>>   arch/powerpc/kernel/mce.c          | 76 +++++++++++++++++-------------
>>   arch/powerpc/kernel/setup-common.c |  2 +-
>>   4 files changed, 69 insertions(+), 34 deletions(-)
>>
>> diff --git a/arch/powerpc/include/asm/mce.h b/arch/powerpc/include/asm/mce.h
>> index e6c27ae843dc..8d6e3a7a9f37 100644
>> --- a/arch/powerpc/include/asm/mce.h
>> +++ b/arch/powerpc/include/asm/mce.h
>> @@ -204,7 +204,18 @@ struct mce_error_info {
>>   	bool			ignore_event;
>>   };
>>   
>> -#define MAX_MC_EVT	100
>> +#define MAX_MC_EVT	10
>> +
>> +struct mce_info {
>> +	int mce_nest_count;
>> +	struct machine_check_event mce_event[MAX_MC_EVT];
>> +	/* Queue for delayed MCE events. */
>> +	int mce_queue_count;
>> +	struct machine_check_event mce_event_queue[MAX_MC_EVT];
>> +	/* Queue for delayed MCE UE events. */
>> +	int mce_ue_count;
>> +	struct machine_check_event  mce_ue_event_queue[MAX_MC_EVT];
>> +};
>>   
>>   /* Release flags for get_mce_event() */
>>   #define MCE_EVENT_RELEASE	true
>> @@ -233,5 +244,13 @@ long __machine_check_early_realmode_p7(struct pt_regs *regs);
>>   long __machine_check_early_realmode_p8(struct pt_regs *regs);
>>   long __machine_check_early_realmode_p9(struct pt_regs *regs);
>>   long __machine_check_early_realmode_p10(struct pt_regs *regs);
>> +#define get_mce_info() local_paca->mce_info
> I don't think this adds anything. Could you open code it?

ok

> Thanks,
> Nick