Distirbute the clear operation of mces_seen to Per-CPU rather than only monarch CPU
Subsystems (2)
PATCH v3 1/1 Distirbute the clear operation of mces_seen to Per-CPU rather than only monarch CPU
mces_seen is a Per-CPU variable which should only be accessed by Per-CPU as possible. So the clear operation of mces_seen should also be local to Per-CPU rather than monarch CPU.
Meanwhile, there is also a potential risk that mces_seen will not be be cleared if a timeout occurs in mce_end for monarch CPU. As a result, the stale value of mces_seen will reappear on the next mce.
Based on the above reasons, this patch distribute the clear operation of mces_seen to Per-CPU rather than only monarch CPU.
* From v1
* Add Reviewed-by: Andi Kleen
* Put the clear operation of mces_seen before the MCG_STATUS write.
* From v2
* Fix some misspelling in the patch message.
--- arch/x86/kernel/cpu/mcheck/mce.c | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-)
arch/x86/kernel/cpu/mcheck/mce.c +7 -11
@@ -785,13 +785,6 @@ static void mce_reign(void)*/ if (global_worst <=MCE_KEEP_SEVERITY && mca_cfg.tolerant <3 )mce_panic ("Machine check from unknown source" ,NULL ,NULL ); - -/* -* Now clear all the mces_seen so that they don't reappear on -* the next mce. -*/ -for_each_possible_cpu (cpu) -memset (&per_cpu (mces_seen, cpu),0 ,sizeof (struct mce )); }static atomic_t global_nwo;
@@ -1137,9 +1130,6 @@ void do_machine_check(struct pt_regs *regs, long error_code) } } -/* mce_clear_state will clear *final, save locally for use later */ - m = *final; -if (!no_way_out)mce_clear_state (toclear);
@@ -1161,7 +1151,7 @@ void do_machine_check(struct pt_regs *regs, long error_code)mce_panic ("Fatal machine check on current CPU" , &m, msg);if (worst ==MCE_AR_SEVERITY ) {/* schedule action before return to userland */ -mce_save_info (m.addr, m.mcgstatus &MCG_STATUS_RIPV ); +mce_save_info (final->addr, final->mcgstatus &MCG_STATUS_RIPV );set_thread_flag (TIF_MCE_NOTIFY ); }else if (kill_it) {force_sig (SIGBUS , current);
@@ -1170,6 +1160,12 @@ void do_machine_check(struct pt_regs *regs, long error_code)if (worst >0 )mce_report_event (regs); + +/* +* Now clear the mces_seen of current CPU -*final - so that it does not +* reappear on the next mce. +*/ +memset (final,0 ,sizeof (struct mce ));mce_wrmsrl (MSR_IA32_MCG_STATUS ,0 ); out:atomic_dec (&mce_entry);
Signature · 10 lines
1.7.10.4 -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Quoted 14 lines · Chen Yucong
mces_seenis a Per-CPU variable which should only be accessed by Per-CPU as possible. So the clear operation ofmces_seenshould also be local to Per-CPU rather than monarch CPU.Meanwhile, there is also a potential risk that
mces_seenwill not be be cleared if a timeout occurs inmce_endfor monarch CPU. As a result, the stale value ofmces_seenwill reappear on the next mce.Based on the above reasons, this patch distribute the clear operation of
mces_seento Per-CPU rather than only monarch CPU.* From v1
* Add Reviewed-by: Andi Kleen
* Put the clear operation ofmces_seenbefore theMCG_STATUSwrite.
Quoted 5 lines
* From v2
* Fix some misspelling in the patch message.Reviewed-by Andi Kleen <[email protected]>Signed-off-by Chen Yucong <[email protected]>
This patch is purely x86_64 MCE related matter, so you need add proper recipients to be properly reviewed.
Personally I'm OK for this code change (it reduces the processing time of monarch CPU somewhat, I think it's a small benefit), but you got complainments in ver.2, so please make some consensus.
And please fix misspelling in subject s/Distirbute/Distribute/.
Thanks,
Naoya Horiguchi
Quoted 60 lines
--- arch/x86/kernel/cpu/mcheck/mce.c | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-)arch/x86/kernel/cpu/mcheck/mce.c +7
-11@@ -785,13 +785,6 @@ static void mce_reign(void)*/ if (global_worst <=MCE_KEEP_SEVERITY && mca_cfg.tolerant <3 )mce_panic ("Machine check from unknown source" ,NULL ,NULL ); - -/* -* Now clear all the mces_seen so that they don't reappear on -* the next mce. -*/ -for_each_possible_cpu (cpu) -memset (&per_cpu (mces_seen, cpu),0 ,sizeof (struct mce )); }static atomic_t global_nwo;@@ -1137,9 +1130,6 @@ void do_machine_check(struct pt_regs *regs, long error_code) } } -/* mce_clear_state will clear *final, save locally for use later */ - m = *final; -if (!no_way_out)mce_clear_state (toclear);@@ -1161,7 +1151,7 @@ void do_machine_check(struct pt_regs *regs, long error_code)mce_panic ("Fatal machine check on current CPU" , &m, msg);if (worst ==MCE_AR_SEVERITY ) {/* schedule action before return to userland */ -mce_save_info (m.addr, m.mcgstatus &MCG_STATUS_RIPV ); +mce_save_info (final->addr, final->mcgstatus &MCG_STATUS_RIPV );set_thread_flag (TIF_MCE_NOTIFY ); }else if (kill_it) {force_sig (SIGBUS , current);@@ -1170,6 +1160,12 @@ void do_machine_check(struct pt_regs *regs, long error_code)if (worst >0 )mce_report_event (regs); + +/* +* Now clear the mces_seen of current CPU -*final - so that it does not +* reappear on the next mce. +*/ +memset (final,0 ,sizeof (struct mce ));mce_wrmsrl (MSR_IA32_MCG_STATUS ,0 ); out:atomic_dec (&mce_entry);Signature · 1 line
1.7.10.4
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>