printk: ignore recursion_bug flag when MCE in progress
Subsystems (3)
PATCH 1/1 printk: ignore recursion_bug flag when MCE in progress
From: ShuoX Liu <[email protected]>
When MCE happens in printk, we ignore recursion_bug to make sure some MCE logs printed out. Re-use mce_entry variable.
---
I found mce_entry was introduced by commit 553f265f, but it's not used now. Why not removed?
--- arch/x86/include/asm/mce.h | 2 -- arch/x86/kernel/cpu/mcheck/mce.c | 2 -- include/linux/kernel.h | 1 + kernel/printk.c | 4 +++- 4 files changed, 4 insertions(+), 5 deletions(-)
arch/x86/include/asm/mce.h +0 -2
@@ -187,8 +187,6 @@ int mce_available(struct cpuinfo_x86 *c);DECLARE_PER_CPU (unsigned, mce_exception_count);DECLARE_PER_CPU (unsigned, mce_poll_count); -extern atomic_t mce_entry; - typedef DECLARE_BITMAP(mce_banks_t, MAX_NR_BANKS);DECLARE_PER_CPU (mce_banks_t ,mce_poll_banks );
arch/x86/kernel/cpu/mcheck/mce.c +0 -2
@@ -61,8 +61,6 @@ int mce_disabled __read_mostly;#define SPINUNIT 100/* 100ns */ -atomic_t mce_entry; -DEFINE_PER_CPU (unsigned, mce_exception_count);/*
include/linux/kernel.h +1 -0
@@ -354,6 +354,7 @@ unsigned long int_sqrt(unsigned long);extern void bust_spinlocks (int yes);extern void wake_up_klogd (void );extern int oops_in_progress;/* If set, an oops, panic(), BUG() or die() is in progress */ +extern atomic_t mce_entry;extern int panic_timeout;extern int panic_on_oops;extern int panic_on_unrecovered_nmi;
kernel/printk.c +3 -1
@@ -79,6 +79,7 @@ int console_printk[4] = {int oops_in_progress;EXPORT_SYMBOL (oops_in_progress); +atomic_t mce_entry;/* * console_sem protects the console_drivers list, and also * provides serialisation for access to the entire console
@@ -864,7 +865,8 @@ asmlinkage int vprintk(const char *fmt, va_list args)* recursion and return - but flag the recursion so that * it can be printed at the next appropriate moment: */ -if (!oops_in_progress && !lockdep_recursing (current)) { +if (!oops_in_progress && !atomic_read (&mce_entry) + && !lockdep_recursing (current)) { recursion_bug =1 ; goto out_restore_irqs; }
Signature · 1 line
1.7.1
Borislav Petkov
maintainer+ Tony
Quoted 68 lines · ShuoX Liu
From: ShuoX Liu <[email protected]>
When MCE happens in printk, we ignore
recursion_bugto make sure some MCE logs printed out. Re-usemce_entryvariable.Signed-off-by Yanmin Zhang <[email protected]>Signed-off-by ShuoX Liu <[email protected]>---
I foundmce_entrywas introduced by commit553f265f, but it's not used now. Why not removed?--- arch/x86/include/asm/mce.h | 2 -- arch/x86/kernel/cpu/mcheck/mce.c | 2 -- include/linux/kernel.h | 1 + kernel/printk.c | 4 +++- 4 files changed, 4 insertions(+), 5 deletions(-)arch/x86/include/asm/mce.h +0
-2@@ -187,8 +187,6 @@ int mce_available(struct cpuinfo_x86 *c);DECLARE_PER_CPU (unsigned, mce_exception_count);DECLARE_PER_CPU (unsigned, mce_poll_count); -extern atomic_t mce_entry; - typedef DECLARE_BITMAP(mce_banks_t, MAX_NR_BANKS);DECLARE_PER_CPU (mce_banks_t ,mce_poll_banks );arch/x86/kernel/cpu/mcheck/mce.c +0
-2@@ -61,8 +61,6 @@ int mce_disabled __read_mostly;#define SPINUNIT 100/* 100ns */ -atomic_t mce_entry; -DEFINE_PER_CPU (unsigned, mce_exception_count);/* include/linux/kernel.h +1
-0@@ -354,6 +354,7 @@ unsigned long int_sqrt(unsigned long);extern void bust_spinlocks (int yes);extern void wake_up_klogd (void );extern int oops_in_progress;/* If set, an oops, panic(), BUG() or die() is in progress */ +extern atomic_t mce_entry;extern int panic_timeout;extern int panic_on_oops;extern int panic_on_unrecovered_nmi;kernel/printk.c +1
-0@@ -79,6 +79,7 @@ int console_printk[4] = {int oops_in_progress;EXPORT_SYMBOL (oops_in_progress); +atomic_t mce_entry;/* * console_sem protects the console_drivers list, and also * provides serialisation for access to the entire console
kernel/printk.c asmlinkage int vprintk(const char *fmt, va_list args) · line 868
Quoted 5 lines
kernel/printk.c +1
-1@@ -864,7 +865,8 @@ asmlinkage int vprintk(const char *fmt, va_list args)* recursion and return - but flag the recursion so that * it can be printed at the next appropriate moment: */ -if (!oops_in_progress && !lockdep_recursing (current)) { + if (!oops_in_progress && !atomic_read(&mce_entry)
This is leaking x86-specific (MCE) stuff in generic kernel code. I think it would be more appropriate to add a in_hw_error() helper or similar and define it on each arch. I can very well imagine other architectures would like to print hw error info too...
Hmmm.
Signature · 2 lines
Regards/Gruss, Boris.
Quoted 66 lines · Borislav Petkov
+ Tony
Quoted 63 lines · ShuoX Liu
From: ShuoX Liu <[email protected]>
When MCE happens in printk, we ignore
recursion_bugto make sure some MCE logs printed out. Re-usemce_entryvariable.Signed-off-by Yanmin Zhang <[email protected]>Signed-off-by ShuoX Liu <[email protected]>---
I foundmce_entrywas introduced by commit553f265f, but it's not used now. Why not removed?--- arch/x86/include/asm/mce.h | 2 -- arch/x86/kernel/cpu/mcheck/mce.c | 2 -- include/linux/kernel.h | 1 + kernel/printk.c | 4 +++- 4 files changed, 4 insertions(+), 5 deletions(-)arch/x86/include/asm/mce.h +0
-2@@ -187,8 +187,6 @@ int mce_available(struct cpuinfo_x86 *c);DECLARE_PER_CPU (unsigned, mce_exception_count);DECLARE_PER_CPU (unsigned, mce_poll_count); -extern atomic_t mce_entry; - typedef DECLARE_BITMAP(mce_banks_t, MAX_NR_BANKS);DECLARE_PER_CPU (mce_banks_t ,mce_poll_banks );arch/x86/kernel/cpu/mcheck/mce.c +0
-2@@ -61,8 +61,6 @@ int mce_disabled __read_mostly;#define SPINUNIT 100/* 100ns */ -atomic_t mce_entry; -DEFINE_PER_CPU (unsigned, mce_exception_count);/* include/linux/kernel.h +1
-0@@ -354,6 +354,7 @@ unsigned long int_sqrt(unsigned long);extern void bust_spinlocks (int yes);extern void wake_up_klogd (void );extern int oops_in_progress;/* If set, an oops, panic(), BUG() or die() is in progress */ +extern atomic_t mce_entry;extern int panic_timeout;extern int panic_on_oops;extern int panic_on_unrecovered_nmi;kernel/printk.c +0
-0@@ -79,6 +79,7 @@ int console_printk[4] = {int oops_in_progress;EXPORT_SYMBOL (oops_in_progress);
Quoted 15 lines
kernel/printk.c asmlinkage int vprintk(const char *fmt, va_list args) · line 868
Quoted 10 lines
kernel/printk.c +2
-1+atomic_t mce_entry;/* * console_sem protects the console_drivers list, and also * provides serialisation for access to the entire console @@ -864,7 +865,8 @@ asmlinkage int vprintk(const char *fmt, va_list args)* recursion and return - but flag the recursion so that * it can be printed at the next appropriate moment: */ -if (!oops_in_progress && !lockdep_recursing (current)) { + if (!oops_in_progress && !atomic_read(&mce_entry)This is leaking x86-specific (MCE) stuff in generic kernel code. I think it would be more appropriate to add a
in_hw_error()helper or similar and define it on each arch. I can very well imagine other architectures would like to print hw error info too...
Good idea. We would do so to make it more generic.
Thanks,
Yanmin
PATCH v2 1/1 printk: ignore recursion_bug flag in HW error handle process
In reply to Borislav PetkovFrom: ShuoX Liu <[email protected]>
When MCE happens in printk, we ignore recursion_bug to make sure MCE logs printed out.
According to Boris' suggestion, we add some helper functions.
1) hw_error_enter: Call it when specific arch begins to process a hardware error.
2) hw_error_exit: Call it when specific arch finishes the processing of a hardware error.
3) in_hw_error():indicates whether HW error handling is in processing.
Each arch could call the helpers in their arch-dependent HW error handlers.
--- arch/x86/include/asm/mce.h | 2 -- arch/x86/kernel/cpu/mcheck/mce.c | 6 ++---- include/linux/cpu.h | 17 +++++++++++++++++ kernel/cpu.c | 3 +++ kernel/printk.c | 3 ++- 5 files changed, 24 insertions(+), 7 deletions(-)
arch/x86/include/asm/mce.h +0 -2
@@ -187,8 +187,6 @@ int mce_available(struct cpuinfo_x86 *c);DECLARE_PER_CPU (unsigned, mce_exception_count);DECLARE_PER_CPU (unsigned, mce_poll_count); -extern atomic_t mce_entry; - typedef DECLARE_BITMAP(mce_banks_t, MAX_NR_BANKS);DECLARE_PER_CPU (mce_banks_t ,mce_poll_banks );
arch/x86/kernel/cpu/mcheck/mce.c +2 -4
@@ -61,8 +61,6 @@ int mce_disabled __read_mostly;#define SPINUNIT 100/* 100ns */ -atomic_t mce_entry; -DEFINE_PER_CPU (unsigned, mce_exception_count);/*
@@ -1015,7 +1013,7 @@ void do_machine_check(struct pt_regs *regs, long error_code)DECLARE_BITMAP (toclear,MAX_NR_BANKS );char *msg ="Unknown" ; -atomic_inc (&mce_entry); +hw_error_enter ();this_cpu_inc (mce_exception_count);
@@ -1143,7 +1141,7 @@ void do_machine_check(struct pt_regs *regs, long error_code)mce_report_event (regs);mce_wrmsrl (MSR_IA32_MCG_STATUS ,0 ); out: -atomic_dec (&mce_entry); +hw_error_exit ();sync_core (); }EXPORT_SYMBOL_GPL (do_machine_check);
include/linux/cpu.h +17 -0
@@ -210,4 +210,21 @@ static inline int disable_nonboot_cpus(void) { return 0; }static inline void enable_nonboot_cpus (void ) {}#endif /* !CONFIG_PM_SLEEP_SMP */ +/* HW error handle status helpers */ +extern atomic_t hw_error; +static inline void hw_error_enter (void ) +{ +atomic_inc (&hw_error); +} + +static inline void hw_error_exit (void ) +{ +atomic_dec (&hw_error); +} + +static inline int in_hw_error (void ) +{ +return atomic_read (&hw_error); +} +#endif /* _LINUX_CPU_H_ */
kernel/cpu.c +3 -0
@@ -679,3 +679,6 @@ void init_cpu_online(const struct cpumask *src) {cpumask_copy (to_cpumask (cpu_online_bits), src); } + +/* hardware error status */ +atomic_t hw_error__read_mostly =ATOMIC_INIT (0 );
kernel/printk.c +2 -1
@@ -1295,7 +1295,8 @@ asmlinkage int vprintk_emit(int facility, int level,* recursion and return - but flag the recursion so that * it can be printed at the next appropriate moment: */ -if (!oops_in_progress && !lockdep_recursing (current)) { +if (!oops_in_progress && !in_hw_error () + && !lockdep_recursing (current)) { recursion_bug =1 ; goto out_restore_irqs; }
Signature · 1 line
1.7.1
Borislav Petkov
maintainerRe: [PATCH v2] printk: ignore recursion_bug flag in HW error handle process
Quoted 10 lines · ShuoX Liu
From: ShuoX Liu <[email protected]>
When MCE happens in printk, we ignore
recursion_bugto make sure MCE logs printed out.According to Boris' suggestion, we add some helper functions.
1)hw_error_enter: Call it when specific arch begins to process a hardware error.
2)hw_error_exit: Call it when specific arch finishes the processing of a hardware error.
3) in_hw_error():indicates whether HW error handling is in processing.Each arch could call the helpers in their arch-dependent HW error handlers.
Yep, looks better, thanks. Design question though:
Quoted 69 lines
Signed-off-by Yanmin Zhang <[email protected]>Signed-off-by ShuoX Liu <[email protected]>--- arch/x86/include/asm/mce.h | 2 -- arch/x86/kernel/cpu/mcheck/mce.c | 6 ++---- include/linux/cpu.h | 17 +++++++++++++++++ kernel/cpu.c | 3 +++ kernel/printk.c | 3 ++- 5 files changed, 24 insertions(+), 7 deletions(-)arch/x86/include/asm/mce.h +0
-2@@ -187,8 +187,6 @@ int mce_available(struct cpuinfo_x86 *c);DECLARE_PER_CPU (unsigned, mce_exception_count);DECLARE_PER_CPU (unsigned, mce_poll_count); -extern atomic_t mce_entry; - typedef DECLARE_BITMAP(mce_banks_t, MAX_NR_BANKS);DECLARE_PER_CPU (mce_banks_t ,mce_poll_banks );arch/x86/kernel/cpu/mcheck/mce.c +2
-4@@ -61,8 +61,6 @@ int mce_disabled __read_mostly;#define SPINUNIT 100/* 100ns */ -atomic_t mce_entry; -DEFINE_PER_CPU (unsigned, mce_exception_count);/* @@ -1015,7 +1013,7 @@ void do_machine_check(struct pt_regs *regs, long error_code)DECLARE_BITMAP (toclear,MAX_NR_BANKS );char *msg ="Unknown" ; -atomic_inc (&mce_entry); +hw_error_enter ();this_cpu_inc (mce_exception_count);@@ -1143,7 +1141,7 @@ void do_machine_check(struct pt_regs *regs, long error_code)mce_report_event (regs);mce_wrmsrl (MSR_IA32_MCG_STATUS ,0 ); out: -atomic_dec (&mce_entry); +hw_error_exit ();sync_core (); }EXPORT_SYMBOL_GPL (do_machine_check);include/linux/cpu.h +6
-0@@ -210,4 +210,21 @@ static inline int disable_nonboot_cpus(void) { return 0; }static inline void enable_nonboot_cpus (void ) {}#endif /* !CONFIG_PM_SLEEP_SMP */ +/* HW error handle status helpers */ +extern atomic_t hw_error; +static inline void hw_error_enter (void ) +{ +atomic_inc (&hw_error); +}
include/linux/cpu.h static inline int disable_nonboot_cpus(void) { return 0; } · line 228
Quoted 10 lines
include/linux/cpu.h +10
-0+ +static inline void hw_error_exit (void ) +{ +atomic_dec (&hw_error); +} + +static inline int in_hw_error (void ) +{ +return atomic_read (&hw_error); +}
Shouldn't those be generic empty functions and each arch implement their own with the stuff they want to do on the respective architecture when they get a hardware error?
Andrew, Ingo?
Thanks.
Signature · 2 lines
Regards/Gruss,
Boris.Re: [PATCH v2] printk: ignore recursion_bug flag in HW error handle process
Quoted 12 lines · On Thu, 24 May 2012 08:11:45 +0200 Borislav Petkov <[email protected]> wrote:
Quoted 12 lines
include/linux/cpu.h +12
-0+/* HW error handle status helpers */ +extern atomic_t hw_error; +static inline void hw_error_enter (void ) +{ +atomic_inc (&hw_error); +} + +static inline void hw_error_exit (void ) +{ +atomic_dec (&hw_error); +} +
Quoted 8 lines
include/linux/cpu.h static inline int disable_nonboot_cpus(void) { return 0; } · line 228
Quoted 4 lines
include/linux/cpu.h +4
-0+static inline int in_hw_error (void ) +{ +return atomic_read (&hw_error); +}Shouldn't those be generic empty functions and each arch implement their own with the stuff they want to do on the respective architecture when they get a hardware error?
This code needs documentation.
Specifically, it should clearly explain (and hence define) what a "hardware error" *is*, and for what purpose this code exists.
Because as it stands, this interface is hopelessly vague. Once one sees that it is *specifically* used for handling mce within a printk, it all makes sense.
And with that understanding comes the realisation that the interface is poorly named. It will not be used for any purpose other than adjusting printk() behavior so it should mention printk() in its name and in its comments and probably it should all be moved into printk.h.
Futhermore, this code is not really related to MCE or hardware or anything else. It is simply a way in which callers can suppress printk()'s recursion check. Callers are free to use it for reasons other than "hardware errors".
And once all that is done, and this interface becomes part of printk() then no, there is no need to add per-arch hooks. An arch can call into printk_recursion_check_disable() and printk_recursion_chack_enable() - nice and simple.
IOW, the title of this patch should be
[patch 1/2] printk: add interface for disabling recursion check [patch 2/2] x86 mce: use new printk recursion disabling interface
Re: [PATCH v2] printk: ignore recursion_bug flag in HW error handle process
Quoted 49 lines · Andrew Morton
Quoted 12 lines · On Thu, 24 May 2012 08:11:45 +0200 Borislav Petkov <[email protected]> wrote:
Quoted 12 lines
include/linux/cpu.h +12
-0+/* HW error handle status helpers */ +extern atomic_t hw_error; +static inline void hw_error_enter (void ) +{ +atomic_inc (&hw_error); +} + +static inline void hw_error_exit (void ) +{ +atomic_dec (&hw_error); +} +Quoted 8 lines
include/linux/cpu.h static inline int disable_nonboot_cpus(void) { return 0; } · line 228
Quoted 4 lines
include/linux/cpu.h +4
-0+static inline int in_hw_error (void ) +{ +return atomic_read (&hw_error); +}Shouldn't those be generic empty functions and each arch implement their own with the stuff they want to do on the respective architecture when they get a hardware error?
This code needs documentation.
Specifically, it should clearly explain (and hence define) what a "hardware error" *is*, and for what purpose this code exists.
Because as it stands, this interface is hopelessly vague. Once one sees that it is *specifically* used for handling mce within a printk, it all makes sense.
And with that understanding comes the realisation that the interface is poorly named. It will not be used for any purpose other than adjusting
printk()behavior so it should mentionprintk()in its name and in its comments and probably it should all be moved intoprintk.h.Futhermore, this code is not really related to MCE or hardware or anything else. It is simply a way in which callers can suppress printk()'s recursion check. Callers are free to use it for reasons other than "hardware errors".
And once all that is done, and this interface becomes part of
printk()then no, there is no need to add per-arch hooks. An arch can call intoprintk_recursion_check_disable()andprintk_recursion_chack_enable()- nice and simple.
Quoted 4 lines
IOW, the title of this patch should be
[patch 1/2] printk: add interface for disabling recursion check [patch 2/2] x86 mce: use new printk recursion disabling interface
Andrew,
Thanks for the detailed comment. It's more reasonable to bind it to printk. We would follow it to create new patches.
Yanmin
PATCH 1/2 printk: add interface for disabling recursion check
From: ShuoX Liu <[email protected]>
With some special scenario, such as Machine Check Exception happened in printk, we want to bypass printk recursion check to print some important information. So we add these interfaces in printk.
1) printk_recursion_check_disable() for disabling recursion check
2) printk_recursion_check_enable() for enabling recursion check
--- include/linux/printk.h | 3 +++ kernel/printk.c | 16 +++++++++++++++- 2 files changed, 18 insertions(+), 1 deletions(-)
include/linux/printk.h +3 -0
@@ -42,6 +42,9 @@ static inline void console_verbose(void) console_loglevel =15 ; } +void printk_recursion_check_disable (void ); +void printk_recursion_check_enable (void ); +struct va_format {const char *fmt;va_list *va;
kernel/printk.c +15 -1
@@ -78,6 +78,18 @@ int console_printk[4] = {int oops_in_progress;EXPORT_SYMBOL (oops_in_progress); +static atomic_t recursion_check_disabled =ATOMIC_INIT (0 ); + +void printk_recursion_check_disable (void ) +{ +atomic_inc (&recursion_check_disabled); +} + +void printk_recursion_check_enable (void ) +{ +atomic_dec (&recursion_check_disabled); +} +/* * console_sem protects the console_drivers list, and also * provides serialisation for access to the entire console
@@ -1295,7 +1307,9 @@ asmlinkage int vprintk_emit(int facility, int level,* recursion and return - but flag the recursion so that * it can be printed at the next appropriate moment: */ -if (!oops_in_progress && !lockdep_recursing (current)) { +if (!atomic_read (&recursion_check_disabled) + && !oops_in_progress + && !lockdep_recursing (current)) { recursion_bug =1 ; goto out_restore_irqs; }
Signature · 1 line
1.7.1
PATCH 2/2 x86 mce: use new printk recursion disabling interface
From: ShuoX Liu <[email protected]>
Disable printk recursion to make sure MCE logs printed out.
--- arch/x86/kernel/cpu/mcheck/mce.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)
arch/x86/kernel/cpu/mcheck/mce.c +2 -0
@@ -1015,6 +1015,7 @@ void do_machine_check(struct pt_regs *regs, long error_code)DECLARE_BITMAP (toclear,MAX_NR_BANKS );char *msg ="Unknown" ; +printk_recursion_check_disable ();atomic_inc (&mce_entry);this_cpu_inc (mce_exception_count);
@@ -1144,6 +1145,7 @@ void do_machine_check(struct pt_regs *regs, long error_code)mce_wrmsrl (MSR_IA32_MCG_STATUS ,0 ); out:atomic_dec (&mce_entry); +printk_recursion_check_enable ();sync_core (); }EXPORT_SYMBOL_GPL (do_machine_check);
Signature · 1 line
1.7.1
Borislav Petkov
maintainerRe: [PATCH 2/2] x86 mce: use new printk recursion disabling interface
Quoted 23 lines · ShuoX Liu
From: ShuoX Liu <[email protected]>
Disable printk recursion to make sure MCE logs printed out.
Signed-off-by Yanmin Zhang <[email protected]>Signed-off-by ShuoX Liu <[email protected]>--- arch/x86/kernel/cpu/mcheck/mce.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)arch/x86/kernel/cpu/mcheck/mce.c +1
-0@@ -1015,6 +1015,7 @@ void do_machine_check(struct pt_regs *regs, long error_code)DECLARE_BITMAP (toclear,MAX_NR_BANKS );char *msg ="Unknown" ; +printk_recursion_check_disable ();atomic_inc (&mce_entry);this_cpu_inc (mce_exception_count);
arch/x86/kernel/cpu/mcheck/mce.c void do_machine_check(struct pt_regs *regs, long error_code) · line 1148
Quoted 4 lines
arch/x86/kernel/cpu/mcheck/mce.c +1
-0@@ -1144,6 +1145,7 @@ void do_machine_check(struct pt_regs *regs, long error_code)mce_wrmsrl (MSR_IA32_MCG_STATUS ,0 ); out:atomic_dec (&mce_entry); +printk_recursion_check_enable ();
Looks like those should be at the beginning and the end of print_mce() - do_machine_check() could exit without printing an MCE and disabling the recursion check then is superfluous, methinks.
Thanks.
Signature · 2 lines
Regards/Gruss,
Boris.Re: [PATCH 2/2] x86 mce: use new printk recursion disabling interface
Quoted 22 lines · Borislav Petkov
Quoted 21 lines · ShuoX Liu
From: ShuoX Liu <[email protected]>
Disable printk recursion to make sure MCE logs printed out.
Signed-off-by Yanmin Zhang <[email protected]>Signed-off-by ShuoX Liu <[email protected]>--- arch/x86/kernel/cpu/mcheck/mce.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)arch/x86/kernel/cpu/mcheck/mce.c +1
-0@@ -1015,6 +1015,7 @@ void do_machine_check(struct pt_regs *regs, long error_code)DECLARE_BITMAP (toclear,MAX_NR_BANKS );char *msg ="Unknown" ; +printk_recursion_check_disable ();atomic_inc (&mce_entry);
Quoted 10 lines
arch/x86/kernel/cpu/mcheck/mce.c void do_machine_check(struct pt_regs *regs, long error_code) · line 1148
Quoted 6 lines
arch/x86/kernel/cpu/mcheck/mce.c +1
-0this_cpu_inc (mce_exception_count);@@ -1144,6 +1145,7 @@ void do_machine_check(struct pt_regs *regs, long error_code)mce_wrmsrl (MSR_IA32_MCG_STATUS ,0 ); out:atomic_dec (&mce_entry); +printk_recursion_check_enable ();Looks like those should be at the beginning and the end of
print_mce()-do_machine_check()could exit without printing an MCE and disabling the recursion check then is superfluous, methinks.
Thanks for comment. It seems you are right. I will check the code next Monday.
Quoted 3 lines
Thanks.
Re: [PATCH 2/2] x86 mce: use new printk recursion disabling interface
In reply to Borislav PetkovQuoted 22 lines · Borislav Petkov
Quoted 21 lines · ShuoX Liu
From: ShuoX Liu <[email protected]>
Disable printk recursion to make sure MCE logs printed out.
Signed-off-by Yanmin Zhang <[email protected]>Signed-off-by ShuoX Liu <[email protected]>--- arch/x86/kernel/cpu/mcheck/mce.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)arch/x86/kernel/cpu/mcheck/mce.c +1
-0@@ -1015,6 +1015,7 @@ void do_machine_check(struct pt_regs *regs, long error_code)DECLARE_BITMAP (toclear,MAX_NR_BANKS );char *msg ="Unknown" ; +printk_recursion_check_disable ();atomic_inc (&mce_entry);
Quoted 10 lines
arch/x86/kernel/cpu/mcheck/mce.c void do_machine_check(struct pt_regs *regs, long error_code) · line 1148
Quoted 6 lines
arch/x86/kernel/cpu/mcheck/mce.c +1
-0this_cpu_inc (mce_exception_count);@@ -1144,6 +1145,7 @@ void do_machine_check(struct pt_regs *regs, long error_code)mce_wrmsrl (MSR_IA32_MCG_STATUS ,0 ); out:atomic_dec (&mce_entry); +printk_recursion_check_enable ();Looks like those should be at the beginning and the end of
print_mce()-do_machine_check()could exit without printing an MCE and disabling the recursion check then is superfluous, methinks.
Boris,
I checked code and found some other functions in do_machine_check() also would printk something. Such as add_taint(). So i think we'd better place the recursion check at the beginning and the end of do_machine_check(). Also more printks later(maybe) added will benefit from this. Do you agree?
Quoted 3 lines
Thanks.
Borislav Petkov
maintainerRe: [PATCH 2/2] x86 mce: use new printk recursion disabling interface
Quoted 6 lines · ShuoX Liu
Boris,
I checked code and found some other functions indo_machine_check()also would printk something. Such asadd_taint(). So i think we'd better place the recursion check at the beginning and the end ofdo_machine_check(). Also more printks later(maybe) added will benefit from this. Do you agree?
I'm not sure we want to disable printk recursion for add_taint() - it doesn't spit out any useful information wrt MCE so we could ignore it.
Btw, I forgot to ask: this printk recursion disabling, do you have a real usecase where you don't get the MCE info in dmesg and with your patch it works or is this purely hypothetical?
Thanks.
Signature · 2 lines
Regards/Gruss,
Boris.Re: [PATCH 2/2] x86 mce: use new printk recursion disabling interface
Quoted 10 lines · Borislav Petkov
Quoted 6 lines · ShuoX Liu
Boris,
I checked code and found some other functions indo_machine_check()also would printk something. Such asadd_taint(). So i think we'd better place the recursion check at the beginning and the end ofdo_machine_check(). Also more printks later(maybe) added will benefit from this. Do you agree?I'm not sure we want to disable printk recursion for
add_taint()- it doesn't spit out any useful information wrt MCE so we could ignore it.
add_taint might be not a good case here. We could move the recursion check flag setting around mce_panic.
Quoted 4 lines
Btw, I forgot to ask: this printk recursion disabling, do you have a real usecase where you don't get the MCE info in dmesg and with your patch it works or is this purely hypothetical?
We hit it when running a MTBF testing on a Android atom mobile.
Thanks,
Yanmin
PATCH v4 1/2 printk: add interface for disabling recursion check
In reply to Borislav PetkovFrom: ShuoX Liu <[email protected]>
With some special scenario, such as Machine Check Exception happened in printk, we want to bypass printk recursion check to printk some important information. So we add these interfaces of printk.
1) printk_recursion_check_disable() for disabling recursion check
2) printk_recursion_check_enable() for enabling recursion check
--- include/linux/printk.h | 3 +++ kernel/printk.c | 17 ++++++++++++++++- 2 files changed, 19 insertions(+), 1 deletions(-)
include/linux/printk.h +3 -0
@@ -42,6 +42,9 @@ static inline void console_verbose(void) console_loglevel =15 ; } +void printk_recursion_check_disable (void ); +void printk_recursion_check_enable (void ); +struct va_format {const char *fmt;va_list *va;
kernel/printk.c +16 -1
@@ -78,6 +78,19 @@ int console_printk[4] = {int oops_in_progress;EXPORT_SYMBOL (oops_in_progress); +static atomic_t recursion_check_disabled =ATOMIC_INIT (0 ); + +void printk_recursion_check_disable (void ) +{ +atomic_inc (&recursion_check_disabled); +} + +void printk_recursion_check_enable (void ) +{ +WARN_ON (atomic_read (&recursion_check_disabled) <1 ); +atomic_dec (&recursion_check_disabled); +} +/* * console_sem protects the console_drivers list, and also * provides serialisation for access to the entire console
@@ -1295,7 +1308,9 @@ asmlinkage int vprintk_emit(int facility, int level,* recursion and return - but flag the recursion so that * it can be printed at the next appropriate moment: */ -if (!oops_in_progress && !lockdep_recursing (current)) { +if (!atomic_read (&recursion_check_disabled) + && !oops_in_progress + && !lockdep_recursing (current)) { recursion_bug =1 ; goto out_restore_irqs; }
Signature · 1 line
1.7.1
PATCH v4 2/2 x86 mce: use new printk recursion disabling interface
From: ShuoX Liu <[email protected]>
Disable printk recursion to make sure MCE logs printed out.
---
We hit it when running a MTBF testing on a Android atom mobile.
--- arch/x86/kernel/cpu/mcheck/mce.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)
arch/x86/kernel/cpu/mcheck/mce.c +2 -0
@@ -306,6 +306,7 @@ static void mce_panic(char *msg, struct mce *final, char *exp) {int i, apei_err =0 ; +printk_recursion_check_disable ();if (!fake_panic) {/* * Make sure only one CPU runs in machine check panic
@@ -360,6 +361,7 @@ static void mce_panic(char *msg, struct mce *final, char *exp)panic (msg); }else pr_emerg (HW_ERR "Fake kernel panic: %s\n" , msg); +printk_recursion_check_enable (); }/* Support code for software error injection */
Signature · 1 line
1.7.1
Re: [PATCH v4 2/2] x86 mce: use new printk recursion disabling interface
Quoted 25 lines · On Mon, 04 Jun 2012 11:07:11 +0800 ShuoX Liu <[email protected]> wrote:
From: ShuoX Liu <[email protected]>
Disable printk recursion to make sure MCE logs printed out.
Signed-off-by Yanmin Zhang <[email protected]>Signed-off-by ShuoX Liu <[email protected]>---
We hit it when running a MTBF testing on a Android atom mobile.--- arch/x86/kernel/cpu/mcheck/mce.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)arch/x86/kernel/cpu/mcheck/mce.c +1
-0@@ -306,6 +306,7 @@ static void mce_panic(char *msg, struct mce *final, char *exp) {int i, apei_err =0 ; +printk_recursion_check_disable ();if (!fake_panic) {/* * Make sure only one CPU runs in machine check panic
arch/x86/kernel/cpu/mcheck/mce.c static void mce_panic(char *msg, struct mce *final, char *exp) · line 367
Quoted 7 lines
arch/x86/kernel/cpu/mcheck/mce.c +1
-0@@ -360,6 +361,7 @@ static void mce_panic(char *msg, struct mce *final, char *exp)panic (msg); }else pr_emerg (HW_ERR "Fake kernel panic: %s\n" , msg); +printk_recursion_check_enable (); }/* Support code for software error injection */
A couple of things here.
a) mce_panic() has a "return" statement deep inside. So we return from mce_panic() with the recursion check disabled. whoops.
b) adding a nice comment is nice.
arch/x86/kernel/cpu/mcheck/mce.c renamed from arch/x86/kernel/cpu/mcheck/mce.c~x86-mce-use-new-printk-recursion-disabling-interface-fix +12 -2
@@ -303,11 +303,10 @@ static void wait_for_panic(void)panic ("Panicing machine check CPU died" ); } -static void mce_panic (char *msg,struct mce *final,char *exp) +static void __mce_panic (char *msg,struct mce *final,char *exp) {int i, apei_err =0 ; -printk_recursion_check_disable ();if (!fake_panic) {/* * Make sure only one CPU runs in machine check panic
@@ -362,6 +361,17 @@ static void mce_panic(char *msg, structpanic (msg); }else pr_emerg (HW_ERR "Fake kernel panic: %s\n" , msg); +} + +/* +* If an MCE happens to occur during the execution of a printk(), we want the +* MCE information to be displayed. But printk()'s recursion checking prevents +* that. So temporarily disable it. +*/ +static void mce_panic (char *msg,struct mce *final,char *exp) +{ +printk_recursion_check_disable (); +__mce_panic (msg, final, exp);printk_recursion_check_enable (); }
_
Re: [PATCH v4 2/2] x86 mce: use new printk recursion disabling interface
Quoted 4 lines · Andrew Morton
A couple of things here.
a)
mce_panic()has a "return" statement deep inside. So we return frommce_panic()with the recursion check disabled. whoops.
Yeah, you're staring at v4 and we fixed this later (currently discussing v7, please follow this thread :))
Quoted 1 line
b) adding a nice comment is nice.
Agreed, if the actual use case pans out and we really need this interface...
Signature · 8 lines
Regards/Gruss, Boris. Advanced Micro Devices GmbH Einsteinring 24, 85609 Dornach GM: Alberto Bozzo Reg: Dornach, Landkreis Muenchen HRB Nr. 43632 WEEE Registernr: 129 19551
RE: [PATCH 1/2] printk: add interface for disabling recursion check
In reply to ShuoX Liu+void printk_recursion_check_enable(void)
+{
+ atomic_dec(&recursion_check_disabled);
+}
Is it worth a BUG_ON() in here to check that recursion_check_disabled is >=1 before blindly decrementing it? Or is this interface so simple that nobody would ever get this wrong?
-Tony ���{.n�+�������+%�����ݶ��w��{.n�+����{��G�����{ay� ʇڙ�,j��f���h��������z_�(�階�ݢj"���m������G������?����&���~��iO��z��v�^�m���� ������?�I�
RE: [PATCH 1/2] printk: add interface for disabling recursion check
Quoted 9 lines · Tony
kernel/printk.c +4
-0+void printk_recursion_check_enable (void ) +{ +atomic_dec (&recursion_check_disabled); +}Is it worth a
BUG_ON()in here to check thatrecursion_check_disabledis >=1 before blindly decrementing it? Or is this interface so simple that nobody would ever get this wrong?
Tony,
The interface is clear and simple. But a WARN_ON checking is better to have. We would add WARN_ON.
Thanks,
Yanmin
PATCH v3 1/2 printk: add interface for disabling recursion check
From: ShuoX Liu <[email protected]>
With some special scenario, such as Machine Check Exception happened in printk, we want to bypass printk recursion check to printk some important information. So we add these interfaces of printk.
1) printk_recursion_check_disable() for disabling recursion check
2) printk_recursion_check_enable() for enabling recursion check
--- include/linux/printk.h | 3 +++ kernel/printk.c | 17 ++++++++++++++++- 2 files changed, 19 insertions(+), 1 deletions(-)
include/linux/printk.h +3 -0
@@ -42,6 +42,9 @@ static inline void console_verbose(void) console_loglevel =15 ; } +void printk_recursion_check_disable (void ); +void printk_recursion_check_enable (void ); +struct va_format {const char *fmt;va_list *va;
kernel/printk.c +16 -1
@@ -78,6 +78,19 @@ int console_printk[4] = {int oops_in_progress;EXPORT_SYMBOL (oops_in_progress); +static atomic_t recursion_check_disabled =ATOMIC_INIT (0 ); + +void printk_recursion_check_disable (void ) +{ +atomic_inc (&recursion_check_disabled); +} + +void printk_recursion_check_enable (void ) +{ +WARN_ON (atomic_read (&recursion_check_disabled) <1 ); +atomic_dec (&recursion_check_disabled); +} +/* * console_sem protects the console_drivers list, and also * provides serialisation for access to the entire console
@@ -1295,7 +1308,9 @@ asmlinkage int vprintk_emit(int facility, int level,* recursion and return - but flag the recursion so that * it can be printed at the next appropriate moment: */ -if (!oops_in_progress && !lockdep_recursing (current)) { +if (!atomic_read (&recursion_check_disabled) + && !oops_in_progress + && !lockdep_recursing (current)) { recursion_bug =1 ; goto out_restore_irqs; }
Signature · 1 line
1.7.1
PATCH v3 2/2 x86 mce: use new printk recursion disabling interface
From: ShuoX Liu <[email protected]>
Disable printk recursion to make sure MCE logs printed out.
--- arch/x86/kernel/cpu/mcheck/mce.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)
arch/x86/kernel/cpu/mcheck/mce.c +2 -0
@@ -1015,6 +1015,7 @@ void do_machine_check(struct pt_regs *regs, long error_code)DECLARE_BITMAP (toclear,MAX_NR_BANKS );char *msg ="Unknown" ; +printk_recursion_check_disable ();atomic_inc (&mce_entry);this_cpu_inc (mce_exception_count);
@@ -1144,6 +1145,7 @@ void do_machine_check(struct pt_regs *regs, long error_code)mce_wrmsrl (MSR_IA32_MCG_STATUS ,0 ); out:atomic_dec (&mce_entry); +printk_recursion_check_enable ();sync_core (); }EXPORT_SYMBOL_GPL (do_machine_check);
Signature · 1 line
1.7.1
Borislav Petkov
maintainerRe: [PATCH v3 2/2] x86 mce: use new printk recursion disabling interface
Quoted 23 lines · ShuoX Liu
From: ShuoX Liu <[email protected]>
Disable printk recursion to make sure MCE logs printed out.
Signed-off-by Yanmin Zhang <[email protected]>Signed-off-by ShuoX Liu <[email protected]>--- arch/x86/kernel/cpu/mcheck/mce.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)arch/x86/kernel/cpu/mcheck/mce.c +1
-0@@ -1015,6 +1015,7 @@ void do_machine_check(struct pt_regs *regs, long error_code)DECLARE_BITMAP (toclear,MAX_NR_BANKS );char *msg ="Unknown" ; +printk_recursion_check_disable ();atomic_inc (&mce_entry);this_cpu_inc (mce_exception_count);
arch/x86/kernel/cpu/mcheck/mce.c void do_machine_check(struct pt_regs *regs, long error_code) · line 1149
Quoted 5 lines
arch/x86/kernel/cpu/mcheck/mce.c +1
-0@@ -1144,6 +1145,7 @@ void do_machine_check(struct pt_regs *regs, long error_code)mce_wrmsrl (MSR_IA32_MCG_STATUS ,0 ); out:atomic_dec (&mce_entry); +printk_recursion_check_enable ();sync_core ();
This is still adding the recursion check things in do_machine_check instead of in print_mce.
Also, this commit message above needs more explanation why we want to disable the recursion check on an MCE, maybe an example...
Thanks.
Signature · 2 lines
Regards/Gruss, Boris.
Re: [PATCH v3 2/2] x86 mce: use new printk recursion disabling interface
Quoted 22 lines · Borislav Petkov
Quoted 21 lines · ShuoX Liu
From: ShuoX Liu <[email protected]>
Disable printk recursion to make sure MCE logs printed out.
Signed-off-by Yanmin Zhang <[email protected]>Signed-off-by ShuoX Liu <[email protected]>--- arch/x86/kernel/cpu/mcheck/mce.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)arch/x86/kernel/cpu/mcheck/mce.c +1
-0@@ -1015,6 +1015,7 @@ void do_machine_check(struct pt_regs *regs, long error_code)DECLARE_BITMAP (toclear,MAX_NR_BANKS );char *msg ="Unknown" ; +printk_recursion_check_disable ();atomic_inc (&mce_entry);
Quoted 10 lines
arch/x86/kernel/cpu/mcheck/mce.c void do_machine_check(struct pt_regs *regs, long error_code) · line 1149
Quoted 7 lines
arch/x86/kernel/cpu/mcheck/mce.c +1
-0this_cpu_inc (mce_exception_count);@@ -1144,6 +1145,7 @@ void do_machine_check(struct pt_regs *regs, long error_code)mce_wrmsrl (MSR_IA32_MCG_STATUS ,0 ); out:atomic_dec (&mce_entry); +printk_recursion_check_enable ();sync_core ();This is still adding the recursion check things in
do_machine_checkinstead of inprint_mce.
Boris,
You are seeing an old patch. The new patches V4 are:
https://lkml.org/lkml/2012/6/3/176
https://lkml.org/lkml/2012/6/3/177
The new patches change mce_panic instead of do_machine_check.
Quoted 3 lines
Also, this commit message above needs more explanation why we want to disable the recursion check on an MCE, maybe an example...
V4 doesn't have the comment. We would add it in V5.
Thanks.
Borislav Petkov
maintainerRe: [PATCH v3 2/2] x86 mce: use new printk recursion disabling interface
Quoted 5 lines · Yanmin Zhang
You are seeing an old patch. The new patches V4 are:
https://lkml.org/lkml/2012/6/3/176
https://lkml.org/lkml/2012/6/3/177The new patches change
mce_panicinstead ofdo_machine_check.
Ok.
Quoted 3 lines
Quoted 2 lines
Also, this commit message above needs more explanation why we want to disable the recursion check on an MCE, maybe an example...
V4 doesn't have the comment. We would add it in V5.
Cool, thanks.
Signature · 2 lines
Regards/Gruss,
Boris.PATCH v5 1/2 printk: add interface for disabling recursion check
From: ShuoX Liu <[email protected]>
With some special scenario, such as Machine Check Exception happened in printk, we want to bypass printk recursion check to printk some important information. So we add these interfaces of printk.
1) printk_recursion_check_disable() for disabling recursion check
2) printk_recursion_check_enable() for enabling recursion check
--- include/linux/printk.h | 3 +++ kernel/printk.c | 17 ++++++++++++++++- 2 files changed, 19 insertions(+), 1 deletions(-)
include/linux/printk.h +3 -0
@@ -42,6 +42,9 @@ static inline void console_verbose(void) console_loglevel =15 ; } +void printk_recursion_check_disable (void ); +void printk_recursion_check_enable (void ); +struct va_format {const char *fmt;va_list *va;
kernel/printk.c +16 -1
@@ -78,6 +78,19 @@ int console_printk[4] = {int oops_in_progress;EXPORT_SYMBOL (oops_in_progress); +static atomic_t recursion_check_disabled =ATOMIC_INIT (0 ); + +void printk_recursion_check_disable (void ) +{ +atomic_inc (&recursion_check_disabled); +} + +void printk_recursion_check_enable (void ) +{ +WARN_ON (atomic_read (&recursion_check_disabled) <1 ); +atomic_dec (&recursion_check_disabled); +} +/* * console_sem protects the console_drivers list, and also * provides serialisation for access to the entire console
@@ -1295,7 +1308,9 @@ asmlinkage int vprintk_emit(int facility, int level,* recursion and return - but flag the recursion so that * it can be printed at the next appropriate moment: */ -if (!oops_in_progress && !lockdep_recursing (current)) { +if (!atomic_read (&recursion_check_disabled) + && !oops_in_progress + && !lockdep_recursing (current)) { recursion_bug =1 ; goto out_restore_irqs; }
Signature · 1 line
1.7.1
PATCH v5 2/2 x86 mce: use new printk recursion disabling interface
From: ShuoX Liu <[email protected]>
On x86 machines, some times MCE happens just when kernel calls printk to output some log info to serial console, while usually MCE module in kernel is used to print out some hardware error information, such like bad cache or bad memory bank. That causes printk recursion and printk would omit MCE printk output.
We hit it when running MTBF testing on Android ATOM mobiles.
Here in mce_panic, we choose to disable printk recursion to make sure MCE logs printed out.
--- arch/x86/kernel/cpu/mcheck/mce.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)
arch/x86/kernel/cpu/mcheck/mce.c +2 -0
@@ -306,6 +306,7 @@ static void mce_panic(char *msg, struct mce *final, char *exp) {int i, apei_err =0 ; +printk_recursion_check_disable ();if (!fake_panic) {/* * Make sure only one CPU runs in machine check panic
@@ -360,6 +361,7 @@ static void mce_panic(char *msg, struct mce *final, char *exp)panic (msg); }else pr_emerg (HW_ERR "Fake kernel panic: %s\n" , msg); +printk_recursion_check_enable (); }/* Support code for software error injection */
Signature · 1 line
1.7.1
Borislav Petkov
maintainerRe: [PATCH v5 2/2] x86 mce: use new printk recursion disabling interface
Quoted 32 lines · ShuoX Liu
From: ShuoX Liu <[email protected]>
On x86 machines, some times MCE happens just when kernel calls printk to output some log info to serial console, while usually MCE module in kernel is used to print out some hardware error information, such like bad cache or bad memory bank. That causes printk recursion and printk would omit MCE printk output.
We hit it when running MTBF testing on Android ATOM mobiles.
Here in
mce_panic, we choose to disable printk recursion to make sure MCE logs printed out.Signed-off-by Yanmin Zhang <[email protected]>Signed-off-by ShuoX Liu <[email protected]>--- arch/x86/kernel/cpu/mcheck/mce.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)arch/x86/kernel/cpu/mcheck/mce.c +1
-0@@ -306,6 +306,7 @@ static void mce_panic(char *msg, struct mce *final, char *exp) {int i, apei_err =0 ; +printk_recursion_check_disable ();if (!fake_panic) {/* * Make sure only one CPU runs in machine check panic
arch/x86/kernel/cpu/mcheck/mce.c static void mce_panic(char *msg, struct mce *final, char *exp) · line 364
Quoted 4 lines
arch/x86/kernel/cpu/mcheck/mce.c +1
-0@@ -360,6 +361,7 @@ static void mce_panic(char *msg, struct mce *final, char *exp)panic (msg); }else pr_emerg (HW_ERR "Fake kernel panic: %s\n" , msg); +printk_recursion_check_enable ();
Ok, let me ask this again: why not disable the printk recursion check in the function that actually _prints_ the MCE, i.e. print_mce() instead of here in mce_panic() which does a whole bunch of other stuff and it can also return without printing any MCE to dmesg?
Are you interested in seeing the printk's from mce_panic? Why?
Thanks.
Signature · 2 lines
Regards/Gruss, Boris.
Re: [PATCH v5 2/2] x86 mce: use new printk recursion disabling interface
Quoted 28 lines · Borislav Petkov
Quoted 27 lines · ShuoX Liu
From: ShuoX Liu <[email protected]>
On x86 machines, some times MCE happens just when kernel calls printk to output some log info to serial console, while usually MCE module in kernel is used to print out some hardware error information, such like bad cache or bad memory bank. That causes printk recursion and printk would omit MCE printk output.
We hit it when running MTBF testing on Android ATOM mobiles.
Here in
mce_panic, we choose to disable printk recursion to make sure MCE logs printed out.Signed-off-by Yanmin Zhang <[email protected]>Signed-off-by ShuoX Liu <[email protected]>--- arch/x86/kernel/cpu/mcheck/mce.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)arch/x86/kernel/cpu/mcheck/mce.c +0
-0@@ -306,6 +306,7 @@ static void mce_panic(char *msg, struct mce *final, char *exp) {int i, apei_err =0 ;
Quoted 14 lines
arch/x86/kernel/cpu/mcheck/mce.c static void mce_panic(char *msg, struct mce *final, char *exp) · line 364
Quoted 9 lines
arch/x86/kernel/cpu/mcheck/mce.c +2
-0+printk_recursion_check_disable ();if (!fake_panic) {/* * Make sure only one CPU runs in machine check panic @@ -360,6 +361,7 @@ static void mce_panic(char *msg, struct mce *final, char *exp)panic (msg); }else pr_emerg (HW_ERR "Fake kernel panic: %s\n" , msg); +printk_recursion_check_enable ();Ok, let me ask this again: why not disable the printk recursion check in the function that actually _prints_ the MCE, i.e.
print_mce()instead of here inmce_panic()which does a whole bunch of other stuff and it can also return without printing any MCE to dmesg?
Sorry for forgetting the return checking.
Quoted 1 line
Are you interested in seeing the printk's from
mce_panic? Why?
How about moving the disabling checking in print_mce? It seems not clean if we disable the printk recursion checking just around every printk statement.
PATCH v6 1/2 printk: add interface for disabling recursion check
From: ShuoX Liu <[email protected]>
With some special scenario, such as Machine Check Exception happened in printk, we want to bypass printk recursion check to printk some important information. So we add these interfaces of printk.
1) printk_recursion_check_disable() for disabling recursion check
2) printk_recursion_check_enable() for enabling recursion check
--- include/linux/printk.h | 3 +++ kernel/printk.c | 17 ++++++++++++++++- 2 files changed, 19 insertions(+), 1 deletions(-)
include/linux/printk.h +3 -0
@@ -42,6 +42,9 @@ static inline void console_verbose(void) console_loglevel =15 ; } +void printk_recursion_check_disable (void ); +void printk_recursion_check_enable (void ); +struct va_format {const char *fmt;va_list *va;
kernel/printk.c +16 -1
@@ -78,6 +78,19 @@ int console_printk[4] = {int oops_in_progress;EXPORT_SYMBOL (oops_in_progress); +static atomic_t recursion_check_disabled =ATOMIC_INIT (0 ); + +void printk_recursion_check_disable (void ) +{ +atomic_inc (&recursion_check_disabled); +} + +void printk_recursion_check_enable (void ) +{ +WARN_ON (atomic_read (&recursion_check_disabled) <1 ); +atomic_dec (&recursion_check_disabled); +} +/* * console_sem protects the console_drivers list, and also * provides serialisation for access to the entire console
@@ -1295,7 +1308,9 @@ asmlinkage int vprintk_emit(int facility, int level,* recursion and return - but flag the recursion so that * it can be printed at the next appropriate moment: */ -if (!oops_in_progress && !lockdep_recursing (current)) { +if (!atomic_read (&recursion_check_disabled) + && !oops_in_progress + && !lockdep_recursing (current)) { recursion_bug =1 ; goto out_restore_irqs; }
Signature · 1 line
1.7.1
PATCH v6 2/2 x86 mce: use new printk recursion disabling interface
From: ShuoX Liu <[email protected]>
On x86 machines, some times MCE happens just when kernel calls printk to output some log info to serial console, while usually MCE module in kernel is used to print out some hardware error information, such like bad cache or bad memory bank. That causes printk recursion and printk would omit MCE printk output.
We hit it when running MTBF testing on Android ATOM mobiles.
Here in mce_panic, we choose to disable printk recursion to make sure MCE logs printed out.
---
v6: move the disabling checking in print_mce
--- arch/x86/kernel/cpu/mcheck/mce.c | 6 +++++- 1 files changed, 5 insertions(+), 1 deletions(-)
arch/x86/kernel/cpu/mcheck/mce.c +5 -1
@@ -242,6 +242,7 @@ static void print_mce(struct mce *m) {int ret =0 ; +printk_recursion_check_disable ();pr_emerg (HW_ERR "CPU %d: Machine Check Exception: %Lx Bank %d: %016Lx\n" , m->extcpu, m->mcgstatus, m->bank, m->status);
@@ -275,10 +276,13 @@ static void print_mce(struct mce *m)* (if the CPU has an implementation for that) */ ret =atomic_notifier_call_chain (&x86_mce_decoder_chain,0 , m); -if (ret ==NOTIFY_STOP ) +if (ret ==NOTIFY_STOP ) { +printk_recursion_check_enable ();return ; + }pr_emerg_ratelimited (HW_ERR "Run the above through 'mcelog --ascii'\n" ); +printk_recursion_check_enable (); }#define PANIC_TIMEOUT 5/* 5 seconds */
Signature · 1 line
1.7.1
Borislav Petkov
maintainerRe: [PATCH v6 2/2] x86 mce: use new printk recursion disabling interface
Quoted 8 lines · ShuoX Liu
From: ShuoX Liu <[email protected]>
On x86 machines, some times MCE happens just when kernel calls printk to output some log info to serial console, while usually MCE module in kernel is used to print out some hardware error information, such like bad cache or bad memory bank. That causes printk recursion and printk would omit MCE printk output.
Quoted 4 lines
We hit it when running MTBF testing on Android ATOM mobiles.
Here in
mce_panic, we choose to disable printk recursion to make sure MCE logs printed out.
Just a minor nitpick: this should say "print_mce" or you can simply remove the whole sentence - commit message is fine without it too.
Aside of that, those look ok to me.
Thanks.
Signature · 2 lines
Regards/Gruss, Boris.
Re: [PATCH v6 2/2] x86 mce: use new printk recursion disabling interface
Quoted 9 lines · Borislav Petkov
Quoted 8 lines · ShuoX Liu
From: ShuoX Liu <[email protected]>
On x86 machines, some times MCE happens just when kernel calls printk to output some log info to serial console, while usually MCE module in kernel is used to print out some hardware error information, such like bad cache or bad memory bank. That causes printk recursion and printk would omit MCE printk output.
Quoted 7 lines
Quoted 4 lines
We hit it when running MTBF testing on Android ATOM mobiles.
Here in
mce_panic, we choose to disable printk recursion to make sure MCE logs printed out.Just a minor nitpick: this should say "
print_mce" or you can simply remove the whole sentence - commit message is fine without it too.
Thanks for your very careful/detailed comments. We would make it perfect.
PATCH v7 1/2 printk: add interface for disabling recursion check
In reply to Borislav PetkovFrom: ShuoX Liu <[email protected]>
With some special scenario, such as Machine Check Exception happened in printk, we want to bypass printk recursion check to printk some important information. So we add these interfaces of printk.
1) printk_recursion_check_disable() for disabling recursion check
2) printk_recursion_check_enable() for enabling recursion check
--- include/linux/printk.h | 3 +++ kernel/printk.c | 17 ++++++++++++++++- 2 files changed, 19 insertions(+), 1 deletions(-)
include/linux/printk.h +3 -0
@@ -42,6 +42,9 @@ static inline void console_verbose(void) console_loglevel =15 ; } +void printk_recursion_check_disable (void ); +void printk_recursion_check_enable (void ); +struct va_format {const char *fmt;va_list *va;
kernel/printk.c +16 -1
@@ -78,6 +78,19 @@ int console_printk[4] = {int oops_in_progress;EXPORT_SYMBOL (oops_in_progress); +static atomic_t recursion_check_disabled =ATOMIC_INIT (0 ); + +void printk_recursion_check_disable (void ) +{ +atomic_inc (&recursion_check_disabled); +} + +void printk_recursion_check_enable (void ) +{ +WARN_ON (atomic_read (&recursion_check_disabled) <1 ); +atomic_dec (&recursion_check_disabled); +} +/* * console_sem protects the console_drivers list, and also * provides serialisation for access to the entire console
@@ -1295,7 +1308,9 @@ asmlinkage int vprintk_emit(int facility, int level,* recursion and return - but flag the recursion so that * it can be printed at the next appropriate moment: */ -if (!oops_in_progress && !lockdep_recursing (current)) { +if (!atomic_read (&recursion_check_disabled) + && !oops_in_progress + && !lockdep_recursing (current)) { recursion_bug =1 ; goto out_restore_irqs; }
Signature · 1 line
1.7.1
PATCH v7 2/2 x86 mce: use new printk recursion disabling interface
From: ShuoX Liu <[email protected]>
On x86 machines, some times MCE happens just when kernel calls printk to output some log info to serial console, while usually MCE module in kernel is used to print out some hardware error information, such like bad cache or bad memory bank. That causes printk recursion and printk would omit MCE printk output.
We hit it when running MTBF testing on Android ATOM mobiles.
Here in print_mce, we choose to disable printk recursion to make sure MCE logs printed out.
--- arch/x86/kernel/cpu/mcheck/mce.c | 6 +++++- 1 files changed, 5 insertions(+), 1 deletions(-)
arch/x86/kernel/cpu/mcheck/mce.c +5 -1
@@ -242,6 +242,7 @@ static void print_mce(struct mce *m) {int ret =0 ; +printk_recursion_check_disable ();pr_emerg (HW_ERR "CPU %d: Machine Check Exception: %Lx Bank %d: %016Lx\n" , m->extcpu, m->mcgstatus, m->bank, m->status);
@@ -275,10 +276,13 @@ static void print_mce(struct mce *m)* (if the CPU has an implementation for that) */ ret =atomic_notifier_call_chain (&x86_mce_decoder_chain,0 , m); -if (ret ==NOTIFY_STOP ) +if (ret ==NOTIFY_STOP ) { +printk_recursion_check_enable ();return ; + }pr_emerg_ratelimited (HW_ERR "Run the above through 'mcelog --ascii'\n" ); +printk_recursion_check_enable (); }#define PANIC_TIMEOUT 5/* 5 seconds */
Signature · 1 line
1.7.1
Re: [PATCH v7 2/2] x86 mce: use new printk recursion disabling interface
Quoted 8 lines · ShuoX Liu
From: ShuoX Liu <[email protected]>
On x86 machines, some times MCE happens just when kernel calls printk to output some log info to serial console, while usually MCE module in kernel is used to print out some hardware error information, such like bad cache or bad memory bank. That causes printk recursion and printk would omit MCE printk output.
Quoted 7 lines
We hit it when running MTBF testing on Android ATOM mobiles.
Here in
print_mce, we choose to disable printk recursion to make sure MCE logs printed out.Signed-off-by Yanmin Zhang <[email protected]>Signed-off-by ShuoX Liu <[email protected]>
Ok,
those patches have to go in hand in hand and since the first one touches generic code, maybe Andrew is the right guy for the job of picking them up, provided there are no other objections against them.
Thanks.
Signature · 2 lines
Regards/Gruss, Boris.
Re: [PATCH v7 2/2] x86 mce: use new printk recursion disabling interface
In reply to ShuoX LiuQuoted 33 lines · ShuoX Liu
From: ShuoX Liu <[email protected]>
On x86 machines, some times MCE happens just when kernel calls printk to output some log info to serial console, while usually MCE module in kernel is used to print out some hardware error information, such like bad cache or bad memory bank. That causes printk recursion and printk would omit MCE printk output.
We hit it when running MTBF testing on Android ATOM mobiles.
Here in
print_mce, we choose to disable printk recursion to make sure MCE logs printed out.Signed-off-by Yanmin Zhang <[email protected]>Signed-off-by ShuoX Liu <[email protected]>--- arch/x86/kernel/cpu/mcheck/mce.c | 6 +++++- 1 files changed, 5 insertions(+), 1 deletions(-)arch/x86/kernel/cpu/mcheck/mce.c +1
-0@@ -242,6 +242,7 @@ static void print_mce(struct mce *m) {int ret =0 ; +printk_recursion_check_disable ();pr_emerg (HW_ERR "CPU %d: Machine Check Exception: %Lx Bank %d: %016Lx\n" , m->extcpu, m->mcgstatus, m->bank, m->status);@@ -275,10 +276,13 @@ static void print_mce(struct mce *m)* (if the CPU has an implementation for that)
arch/x86/kernel/cpu/mcheck/mce.c static void print_mce(struct mce *m) · line 285
Quoted 10 lines
arch/x86/kernel/cpu/mcheck/mce.c +4
-1*/ ret =atomic_notifier_call_chain (&x86_mce_decoder_chain,0 , m); -if (ret ==NOTIFY_STOP ) +if (ret ==NOTIFY_STOP ) { +printk_recursion_check_enable ();return ; + }pr_emerg_ratelimited (HW_ERR "Run the above through 'mcelog --ascii'\n" ); +printk_recursion_check_enable ();
Ok, this looks useful and it solves a real problem, but I'd prefer a better interface: instead of exposing the guts of printk to drivers in an unsafe manner (and allowing them to keep printk in an unsafe state indefinitely), shouldn't we instead introduce printk_emergency() (and variants) that just disable the recursion check, do the printk and then enable them?
That way drivers cannot possibly leave the recursion check disabled permanently and it would also be much more obvious *which* actual printk sites are affected by this exception.
In theory this could be achieved via a new, super-high-prio printk level: KERN_CRASH or so, which the core printk code could check - without introducing a new side-facility.
Thanks,
Ingo
Re: [PATCH v7 2/2] x86 mce: use new printk recursion disabling interface
Quoted 43 lines · Ingo Molnar
Quoted 41 lines · ShuoX Liu
From: ShuoX Liu <[email protected]>
On x86 machines, some times MCE happens just when kernel calls printk to output some log info to serial console, while usually MCE module in kernel is used to print out some hardware error information, such like bad cache or bad memory bank. That causes printk recursion and printk would omit MCE printk output.
We hit it when running MTBF testing on Android ATOM mobiles.
Here in
print_mce, we choose to disable printk recursion to make sure MCE logs printed out.Signed-off-by Yanmin Zhang <[email protected]>Signed-off-by ShuoX Liu <[email protected]>--- arch/x86/kernel/cpu/mcheck/mce.c | 6 +++++- 1 files changed, 5 insertions(+), 1 deletions(-)arch/x86/kernel/cpu/mcheck/mce.c +4
-1@@ -242,6 +242,7 @@ static void print_mce(struct mce *m) {int ret =0 ; +printk_recursion_check_disable ();pr_emerg (HW_ERR "CPU %d: Machine Check Exception: %Lx Bank %d: %016Lx\n" , m->extcpu, m->mcgstatus, m->bank, m->status);@@ -275,10 +276,13 @@ static void print_mce(struct mce *m)* (if the CPU has an implementation for that) */ ret =atomic_notifier_call_chain (&x86_mce_decoder_chain,0 , m); -if (ret ==NOTIFY_STOP ) +if (ret ==NOTIFY_STOP ) { +printk_recursion_check_enable ();return ; + }
Quoted 9 lines
arch/x86/kernel/cpu/mcheck/mce.c static void print_mce(struct mce *m) · line 285
Quoted 2 lines
arch/x86/kernel/cpu/mcheck/mce.c +1
-0pr_emerg_ratelimited (HW_ERR "Run the above through 'mcelog --ascii'\n" ); +printk_recursion_check_enable ();Ok, this looks useful and it solves a real problem, but I'd prefer a better interface: instead of exposing the guts of printk to drivers in an unsafe manner (and allowing them to keep printk in an unsafe state indefinitely), shouldn't we instead introduce
printk_emergency()(and variants) that just disable the recursion check, do the printk and then enable them?
I really don't see why you'd do anything like the above. For one the oops_in_progress thing is exported, so you could simply use that, you're going to panic the machine here anyway, right?
Furthermore the whole recursion stuff is broken anyway, see:
http://marc.info/?l=linux-kernel&m=132446646923333
That also introduces recursive_printk() which you could (ab)use if needed.
That said, its not entirely clear to me what context you're in when you're calling this print_mce(), it looks like its only used from mce_panic().
That already does bust_spinlocks() which already increments the oops_in_progress thing, so WTF are you doing anyway?
Re: [PATCH v7 2/2] x86 mce: use new printk recursion disabling interface
Quoted 2 lines · Peter Zijlstra
That already does
bust_spinlocks()which already increments theoops_in_progressthing, so WTF are you doing anyway?
Hmm, that's interesting. So Shuox said they hit this in some sort of "MTBF testing on Android ATOM mobiles":
http://marc.info/?l=linux-kernel&m=133903834107577&w=2
And I'm guessing they want this probably because they set fake_panic so that we don't bust_spinlocks() in mce_panic().
Shoux, Yanmin, what's the dealio here?
Thanks.
Signature · 8 lines
Regards/Gruss, Boris. Advanced Micro Devices GmbH Einsteinring 24, 85609 Dornach GM: Alberto Bozzo Reg: Dornach, Landkreis Muenchen HRB Nr. 43632 WEEE Registernr: 129 19551
Re: [PATCH v7 1/2] printk: add interface for disabling recursion check
In reply to ShuoX LiuQuoted 68 lines · ShuoX Liu
From: ShuoX Liu <[email protected]>
With some special scenario, such as Machine Check Exception happened in printk, we want to bypass printk recursion check to printk some important information. So we add these interfaces of printk.
1)
printk_recursion_check_disable()for disabling recursion check
2)printk_recursion_check_enable()for enabling recursion checkSigned-off-by Yanmin Zhang <[email protected]>Signed-off-by ShuoX Liu <[email protected]>---
include/linux/printk.h | 3 +++
kernel/printk.c | 17 ++++++++++++++++-2 files changed, 19 insertions(+), 1 deletions(-)include/linux/printk.h +3
-0@@ -42,6 +42,9 @@ static inline void console_verbose(void) console_loglevel = 15; } +void printk_recursion_check_disable (void ); +void printk_recursion_check_enable (void ); + struct va_format { const char *fmt; va_list *va;kernel/printk.c +16
-2@@ -78,6 +78,19 @@ int console_printk[4] = { int oops_in_progress; EXPORT_SYMBOL(oops_in_progress); +static atomic_t recursion_check_disabled =ATOMIC_INIT (0 ); + +void printk_recursion_check_disable (void ) +{ + atomic_inc(&recursion_check_disabled); +} + +void printk_recursion_check_enable (void ) +{ + WARN_ON(atomic_read(&recursion_check_disabled) < 1); + atomic_dec(&recursion_check_disabled); +} + /** console_sem protects the console_drivers list, and also * provides serialisation for access to the entire console @@ -1295,7 +1308,9 @@ asmlinkage int vprintk_emit(int facility, int level,* recursion and return - but flag the recursion so that * it can be printed at the next appropriate moment: */ - if (!oops_in_progress && !lockdep_recursing(current)) { + if (!atomic_read(&recursion_check_disabled) + && !oops_in_progress + && !lockdep_recursing(current)) { recursion_bug = 1; goto out_restore_irqs; } --1.7.1
Quoted 5 lines
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Hi Shuo,
Should the two function be export by EXPORT_SYMBOL? The the other
module can use it.
Signature · 2 lines
Best Regards, Bing
Borislav Petkov
maintainerRe: [PATCH v7 1/2] printk: add interface for disabling recursion check
Quoted 2 lines · bing deng
Should the two function be export by
EXPORT_SYMBOL? The the other module can use it.
Not needed right now - functions are only used in built-in code and not in modules.
Signature · 2 lines
Regards/Gruss, Boris.
Re: [PATCH v7 1/2] printk: add interface for disabling recursion check
In reply to ShuoX LiuQuoted 11 lines · ShuoX Liu
From: ShuoX Liu <[email protected]>
With some special scenario, such as Machine Check Exception happened in printk, we want to bypass printk recursion check to printk some important information. So we add these interfaces of printk.
1)
printk_recursion_check_disable()for disabling recursion check
2)printk_recursion_check_enable()for enabling recursion checkSigned-off-by Yanmin Zhang <[email protected]>Signed-off-by ShuoX Liu <[email protected]>
Signature · 2 lines
Regards/Gruss, Boris.