ACPI, APEI, GHES: Remove strict check for memory error handling
PATCH 1/2 ACPI, APEI, GHES: Remove strict check for memory error handling
Usually SCI is employed to handle corrected error, especially for memory corrected error but in fact SCI still can be used to handle any error like memory uncorrected error if BIOS enable it. For this situation, memory uncorrected error should be logged as corrected error does, too.
--- arch/x86/include/asm/mce.h | 3 +-- arch/x86/kernel/cpu/mcheck/mce-apei.c | 6 ++---- drivers/acpi/apei/ghes.c | 3 +-- 3 files changed, 4 insertions(+), 8 deletions(-)
arch/x86/include/asm/mce.h +1 -2
@@ -244,7 +244,6 @@ static inline void mcheck_intel_therm_init(void) { }*/ struct cper_sec_mem_err ; -extern void apei_mce_report_mem_error (int corrected, -struct cper_sec_mem_err *mem_err); +extern void apei_mce_report_mem_error (struct cper_sec_mem_err *mem_err);#endif /* _ASM_X86_MCE_H */
arch/x86/kernel/cpu/mcheck/mce-apei.c +2 -4
@@ -37,13 +37,11 @@#include "mce-internal.h" -void apei_mce_report_mem_error (int corrected,struct cper_sec_mem_err *mem_err) +void apei_mce_report_mem_error (struct cper_sec_mem_err *mem_err) {struct mce m; -/* Only corrected MC is reported */ -if (!corrected || !(mem_err->validation_bits & -CPER_MEM_VALID_PHYSICAL_ADDRESS )) +if (!(mem_err->validation_bits &CPER_MEM_VALID_PHYSICAL_ADDRESS ))return ;mce_setup (&m);
drivers/acpi/apei/ghes.c +1 -2
@@ -453,8 +453,7 @@ static void ghes_do_proc(struct ghes *ghes,ghes_edac_report_mem_error (ghes, sev, mem_err);#ifdef CONFIG_X86_MCE -apei_mce_report_mem_error (sev ==GHES_SEV_CORRECTED , - mem_err); +apei_mce_report_mem_error (mem_err);#endif ghes_handle_memory_failure (gdata, sev); }
Signature · 1 line
1.8.4.rc3
PATCH 2/2 ACPI, APEI, GHES: Cleanup ghes codes for memory error handling
Cleanup the logic for function ghes_handle_memory_failure. Just make it simpler and cleaner.
--- drivers/acpi/apei/ghes.c | 36 ++++++++++++++++++++---------------- 1 file changed, 20 insertions(+), 16 deletions(-)
drivers/acpi/apei/ghes.c +20 -16
@@ -413,27 +413,31 @@ static void ghes_handle_memory_failure(struct acpi_hest_generic_data *gdata, int {#ifdef CONFIG_ACPI_APEI_MEMORY_FAILURE unsigned long pfn; +int flags =-1 ;int sec_sev =ghes_severity (gdata->error_severity);struct cper_sec_mem_err *mem_err; mem_err = (struct cper_sec_mem_err *)(gdata +1 ); -if (sec_sev ==GHES_SEV_CORRECTED && - (gdata->flags &CPER_SEC_ERROR_THRESHOLD_EXCEEDED ) && - (mem_err->validation_bits &CPER_MEM_VALID_PHYSICAL_ADDRESS )) { - pfn = mem_err->physical_addr >>PAGE_SHIFT ; -if (pfn_valid (pfn)) -memory_failure_queue (pfn,0 ,MF_SOFT_OFFLINE ); -else if (printk_ratelimit ()) - pr_warn(FW_WARN GHES_PFX -"Invalid address in generic error data: %#llx\n" , - mem_err->physical_addr); - } -if (sev ==GHES_SEV_RECOVERABLE && - sec_sev ==GHES_SEV_RECOVERABLE && - mem_err->validation_bits &CPER_MEM_VALID_PHYSICAL_ADDRESS ) { - pfn = mem_err->physical_addr >>PAGE_SHIFT ; -memory_failure_queue (pfn,0 ,0 ); +if (!(mem_err->validation_bits &CPER_MEM_VALID_PHYSICAL_ADDRESS )) +return ; + + pfn = mem_err->physical_addr >>PAGE_SHIFT ; +if (!pfn_valid (pfn)) { +pr_warn_ratelimited (GHES_PFX +"Invalid address in generic error data: %#llx\n" , + mem_err->physical_addr); +return ; } + +/* ensure iff events by strictly check can be handled */ +if (sec_sev ==GHES_SEV_CORRECTED && + (gdata->flags &CPER_SEC_ERROR_THRESHOLD_EXCEEDED )) + flags =MF_SOFT_OFFLINE ; +if (sev ==GHES_SEV_RECOVERABLE && sec_sev ==GHES_SEV_RECOVERABLE ) + flags =0 ; + +if (flag !=-1 ) +memory_failure_queue (pfn,0 , flags);#endif }
Signature · 1 line
1.8.4.rc3
Re: [PATCH 2/2] ACPI, APEI, GHES: Cleanup ghes codes for memory error handling
Quoted 33 lines · Chen Gong
Cleanup the logic for function
ghes_handle_memory_failure. Just make it simpler and cleaner.Signed-off-by Chen, Gong <[email protected]>--- drivers/acpi/apei/ghes.c | 36 ++++++++++++++++++++---------------- 1 file changed, 20 insertions(+), 16 deletions(-)drivers/acpi/apei/ghes.c +1
-12@@ -413,27 +413,31 @@ static void ghes_handle_memory_failure(struct acpi_hest_generic_data *gdata, int {#ifdef CONFIG_ACPI_APEI_MEMORY_FAILURE unsigned long pfn; +int flags =-1 ;int sec_sev =ghes_severity (gdata->error_severity);struct cper_sec_mem_err *mem_err; mem_err = (struct cper_sec_mem_err *)(gdata +1 ); -if (sec_sev ==GHES_SEV_CORRECTED && - (gdata->flags &CPER_SEC_ERROR_THRESHOLD_EXCEEDED ) && - (mem_err->validation_bits &CPER_MEM_VALID_PHYSICAL_ADDRESS )) { - pfn = mem_err->physical_addr >>PAGE_SHIFT ; -if (pfn_valid (pfn)) -memory_failure_queue (pfn,0 ,MF_SOFT_OFFLINE ); -else if (printk_ratelimit ()) - pr_warn(FW_WARN GHES_PFX -"Invalid address in generic error data: %#llx\n" , - mem_err->physical_addr); - } - if (sev == GHES_SEV_RECOVERABLE &&
drivers/acpi/apei/ghes.c static void ghes_handle_memory_failure(struct acpi_hest_generic_data *gdata, int · line 426
Quoted 10 lines
drivers/acpi/apei/ghes.c +6
-4- sec_sev == GHES_SEV_RECOVERABLE && - mem_err->validation_bits & CPER_MEM_VALID_PHYSICAL_ADDRESS) { - pfn = mem_err->physical_addr >>PAGE_SHIFT ; -memory_failure_queue (pfn,0 ,0 ); +if (!(mem_err->validation_bits &CPER_MEM_VALID_PHYSICAL_ADDRESS )) +return ; + + pfn = mem_err->physical_addr >>PAGE_SHIFT ; +if (!pfn_valid (pfn)) { + pr_warn_ratelimited(GHES_PFX
You also need FW_WARN so we point to the right source of the error.
drivers/acpi/apei/ghes.c static void ghes_handle_memory_failure(struct acpi_hest_generic_data *gdata, int · line 432
Quoted 6 lines
drivers/acpi/apei/ghes.c +5
-0+"Invalid address in generic error data: %#llx\n" , + mem_err->physical_addr); +return ; } + +/* ensure iff events by strictly check can be handled */
Comment can probably be removed - not sure it conveys anything useful.
drivers/acpi/apei/ghes.c static void ghes_handle_memory_failure(struct acpi_hest_generic_data *gdata, int · line 439
Quoted 7 lines
drivers/acpi/apei/ghes.c +7
-0+if (sec_sev ==GHES_SEV_CORRECTED && + (gdata->flags &CPER_SEC_ERROR_THRESHOLD_EXCEEDED )) + flags =MF_SOFT_OFFLINE ; +if (sev ==GHES_SEV_RECOVERABLE && sec_sev ==GHES_SEV_RECOVERABLE ) + flags =0 ; + +if (flag !=-1 )
Wonder if this even compiles! That should be 'flags'.
- Naveen
Quoted 12 lines
drivers/acpi/apei/ghes.c +1
-0+memory_failure_queue (pfn,0 , flags);#endif }Signature · 6 lines
1.8.4.rc3 -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] ACPI, APEI, GHES: Cleanup ghes codes for memory error handling
Quoted 5 lines · Naveen N. Rao
Review
Quoted 2 lines
(quoted diff) +2
-0+ + /* ensure iff events by strictly check can be handled */Comment can probably be removed - not sure it conveys anything useful.
I think comments are always meaningful only if it is correct. BTW, this comment is used to tell only following two scenarios can be handled properly.
Quoted 10 lines
drivers/acpi/apei/ghes.c static void ghes_handle_memory_failure(struct acpi_hest_generic_data *gdata, int · line 439
Quoted 7 lines
drivers/acpi/apei/ghes.c +7
-0+if (sec_sev ==GHES_SEV_CORRECTED && + (gdata->flags &CPER_SEC_ERROR_THRESHOLD_EXCEEDED )) + flags =MF_SOFT_OFFLINE ; +if (sev ==GHES_SEV_RECOVERABLE && sec_sev ==GHES_SEV_RECOVERABLE ) + flags =0 ; + +if (flag !=-1 )Wonder if this even compiles! That should be 'flags'.
I'm sorry for that. I must forget to select CONFIG_ACPI_APEI_MEMORY_FAILURE. Thanks for your careful observation.
Quoted 34 lines · Chen Gong
Usually SCI is employed to handle corrected error, especially for memory corrected error but in fact SCI still can be used to handle any error like memory uncorrected error if BIOS enable it. For this situation, memory uncorrected error should be logged as corrected error does, too.
Signed-off-by Chen, Gong <[email protected]>--- arch/x86/include/asm/mce.h | 3 +-- arch/x86/kernel/cpu/mcheck/mce-apei.c | 6 ++---- drivers/acpi/apei/ghes.c | 3 +-- 3 files changed, 4 insertions(+), 8 deletions(-)arch/x86/include/asm/mce.h +1
-2@@ -244,7 +244,6 @@ static inline void mcheck_intel_therm_init(void) { }*/ struct cper_sec_mem_err ; -extern void apei_mce_report_mem_error (int corrected, -struct cper_sec_mem_err *mem_err); +extern void apei_mce_report_mem_error (struct cper_sec_mem_err *mem_err);#endif /* _ASM_X86_MCE_H */ arch/x86/kernel/cpu/mcheck/mce-apei.c +0
-0@@ -37,13 +37,11 @@#include "mce-internal.h"
arch/x86/kernel/cpu/mcheck/mce-apei.c line 45
Quoted 10 lines
arch/x86/kernel/cpu/mcheck/mce-apei.c +2
-4-void apei_mce_report_mem_error (int corrected,struct cper_sec_mem_err *mem_err) +void apei_mce_report_mem_error (struct cper_sec_mem_err *mem_err) {struct mce m; -/* Only corrected MC is reported */ -if (!corrected || !(mem_err->validation_bits & -CPER_MEM_VALID_PHYSICAL_ADDRESS )) +if (!(mem_err->validation_bits &CPER_MEM_VALID_PHYSICAL_ADDRESS ))return ;
This won't be enough. Further down, you'll see that all memory errors get logged as corrected errors due to the hardcoded MCE status. A lot more work will be needed if we have to log GHES errors through mcelog properly.
- Naveen
Quoted 24 lines
arch/x86/kernel/cpu/mcheck/mce-apei.c +0
-0mce_setup (&m);drivers/acpi/apei/ghes.c +1
-2@@ -453,8 +453,7 @@ static void ghes_do_proc(struct ghes *ghes,ghes_edac_report_mem_error (ghes, sev, mem_err);#ifdef CONFIG_X86_MCE -apei_mce_report_mem_error (sev ==GHES_SEV_CORRECTED , - mem_err); +apei_mce_report_mem_error (mem_err);#endif ghes_handle_memory_failure (gdata, sev); }Signature · 6 lines
1.8.4.rc3 -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Quoted 59 lines · Naveen N. Rao
Date: Mon, 25 Nov 2013 11:38:23 +0530
From: "Naveen N. Rao" <[email protected]>
To: "Chen, Gong" <[email protected]>
Cc: [email protected], [email protected], [email protected]
Subject: Re: [PATCH 1/2] ACPI, APEI, GHES: Remove strict check for memory
error handling
User-Agent: Mutt/1.5.21 (2010-09-15)Quoted 34 lines · Chen Gong
Usually SCI is employed to handle corrected error, especially for memory corrected error but in fact SCI still can be used to handle any error like memory uncorrected error if BIOS enable it. For this situation, memory uncorrected error should be logged as corrected error does, too.
Signed-off-by Chen, Gong <[email protected]>--- arch/x86/include/asm/mce.h | 3 +-- arch/x86/kernel/cpu/mcheck/mce-apei.c | 6 ++---- drivers/acpi/apei/ghes.c | 3 +-- 3 files changed, 4 insertions(+), 8 deletions(-)arch/x86/include/asm/mce.h +1
-2@@ -244,7 +244,6 @@ static inline void mcheck_intel_therm_init(void) { }*/ struct cper_sec_mem_err ; -extern void apei_mce_report_mem_error (int corrected, -struct cper_sec_mem_err *mem_err); +extern void apei_mce_report_mem_error (struct cper_sec_mem_err *mem_err);#endif /* _ASM_X86_MCE_H */ arch/x86/kernel/cpu/mcheck/mce-apei.c +0
-0@@ -37,13 +37,11 @@#include "mce-internal.h" arch/x86/kernel/cpu/mcheck/mce-apei.c line 45
Quoted 10 lines
arch/x86/kernel/cpu/mcheck/mce-apei.c +2
-4-void apei_mce_report_mem_error (int corrected,struct cper_sec_mem_err *mem_err) +void apei_mce_report_mem_error (struct cper_sec_mem_err *mem_err) {struct mce m; -/* Only corrected MC is reported */ -if (!corrected || !(mem_err->validation_bits & -CPER_MEM_VALID_PHYSICAL_ADDRESS )) +if (!(mem_err->validation_bits &CPER_MEM_VALID_PHYSICAL_ADDRESS ))return ;This won't be enough. Further down, you'll see that all memory errors get logged as corrected errors due to the hardcoded MCE status. A lot more work will be needed if we have to log GHES errors through mcelog properly.
Quoted 2 lines
- Naveen
Sure. In fact, the most valuable information from CPER is physical address and most other data are faked or only little values. But at least we can make it more precisely.