| Message ID | 1385099825-31765-2-git-send-email-gong.chen@linux.intel.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
On 2013/11/22 12:57AM, Chen Gong wrote: > Cleanup the logic for function ghes_handle_memory_failure. Just > make it simpler and cleaner. > > Signed-off-by: Chen, Gong <gong.chen@linux.intel.com> > --- > drivers/acpi/apei/ghes.c | 36 ++++++++++++++++++++---------------- > 1 file changed, 20 insertions(+), 16 deletions(-) > > diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c > index 039c23c..121462b 100644 > --- a/drivers/acpi/apei/ghes.c > +++ b/drivers/acpi/apei/ghes.c > @@ -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 You also need FW_WARN so we point to the right source of the error. > + "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. > + 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 > + memory_failure_queue(pfn, 0, flags); > #endif > } > > -- > 1.8.4.rc3 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-acpi" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Nov 25, 2013 at 11:42:09AM +0530, Naveen N. Rao wrote: > > + > > + /* 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. > > + 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.
diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c index 039c23c..121462b 100644 --- a/drivers/acpi/apei/ghes.c +++ b/drivers/acpi/apei/ghes.c @@ -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 }
Cleanup the logic for function ghes_handle_memory_failure. Just make it simpler and cleaner. Signed-off-by: Chen, Gong <gong.chen@linux.intel.com> --- drivers/acpi/apei/ghes.c | 36 ++++++++++++++++++++---------------- 1 file changed, 20 insertions(+), 16 deletions(-)