diff mbox

[v2,2/2] ACPI, APEI, GHES: Cleanup ghes codes for memory error handling

Message ID 1385363701-12387-2-git-send-email-gong.chen@linux.intel.com (mailing list archive)
State Not Applicable, archived
Headers show

Commit Message

Chen Gong Nov. 25, 2013, 7:15 a.m. UTC
Cleanup the logic for function ghes_handle_memory_failure. Just
make it simpler and cleaner.

v2 -> v1: fix a compile error & some minor changes.

Signed-off-by: Chen, Gong <gong.chen@linux.intel.com>
---
 drivers/acpi/apei/ghes.c | 36 ++++++++++++++++++++----------------
 1 file changed, 20 insertions(+), 16 deletions(-)

Comments

Chen Gong Nov. 26, 2013, 6:54 a.m. UTC | #1
On Mon, Nov 25, 2013 at 02:15:01AM -0500, Chen, Gong wrote:
> Date: Mon, 25 Nov 2013 02:15:01 -0500
> From: "Chen, Gong" <gong.chen@linux.intel.com>
> To: tony.luck@intel.com, bp@alien8.de, naveen.n.rao@linux.vnet.ibm.com
> Cc: linux-acpi@vger.kernel.org, "Chen, Gong" <gong.chen@linux.intel.com>
> Subject: [PATCH v2 2/2] ACPI, APEI, GHES: Cleanup ghes codes for memory
>  error handling
> X-Mailer: git-send-email 1.8.4.3
> 
> Cleanup the logic for function ghes_handle_memory_failure. Just
> make it simpler and cleaner.
> 
> v2 -> v1: fix a compile error & some minor changes.
> 
> 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 ce3683d..46766ef 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_generic_data *gdata, int sev)
>  {
>  #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_PA)) {
> -		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_PA) {
> -		pfn = mem_err->physical_addr >> PAGE_SHIFT;
> -		memory_failure_queue(pfn, 0, 0);
> +	if (!(mem_err->validation_bits & CPER_MEM_VALID_PA))
> +		return;
> +
> +	pfn = mem_err->physical_addr >> PAGE_SHIFT;
> +	if (!pfn_valid(pfn)) {
> +		pr_warn_ratelimited(FW_WARN GHES_PFX
> +		"Invalid address in generic error data: %#llx\n",
> +		mem_err->physical_addr);
> +		return;
>  	}
> +
> +	/* iff following two events can be handled properly by now */
> +	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 (flags != -1)
> +		memory_failure_queue(pfn, 0, flags);
>  #endif
>  }
>  

Hi, Boris

In this patch so-called cleanup includes an implied PFN check for UC error
but missed in current codes.
Borislav Petkov Nov. 26, 2013, 7:23 a.m. UTC | #2
On Tue, Nov 26, 2013 at 01:54:57AM -0500, Chen, Gong wrote:
> In this patch so-called cleanup includes an implied PFN check for UC
> error but missed in current codes.

Right, I was about to look at it. You probably should add this to the
commit message so that it is clear.

Thanks.
Naveen N. Rao Nov. 26, 2013, 9:04 a.m. UTC | #3
On 11/25/2013 12:45 PM, Chen, Gong wrote:
> Cleanup the logic for function ghes_handle_memory_failure. Just
> make it simpler and cleaner.
>
> v2 -> v1: fix a compile error & some minor changes.
>
> Signed-off-by: Chen, Gong <gong.chen@linux.intel.com>

Acked-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.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 ce3683d..46766ef 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_generic_data *gdata, int sev)
>   {
>   #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_PA)) {
> -		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_PA) {
> -		pfn = mem_err->physical_addr >> PAGE_SHIFT;
> -		memory_failure_queue(pfn, 0, 0);
> +	if (!(mem_err->validation_bits & CPER_MEM_VALID_PA))
> +		return;
> +
> +	pfn = mem_err->physical_addr >> PAGE_SHIFT;
> +	if (!pfn_valid(pfn)) {
> +		pr_warn_ratelimited(FW_WARN GHES_PFX
> +		"Invalid address in generic error data: %#llx\n",
> +		mem_err->physical_addr);
> +		return;
>   	}
> +
> +	/* iff following two events can be handled properly by now */
> +	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 (flags != -1)
> +		memory_failure_queue(pfn, 0, flags);
>   #endif
>   }
>

--
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
Chen Gong Nov. 27, 2013, 2:15 a.m. UTC | #4
On Tue, Nov 26, 2013 at 08:23:35AM +0100, Borislav Petkov wrote:
> Date: Tue, 26 Nov 2013 08:23:35 +0100
> From: Borislav Petkov <bp@alien8.de>
> To: "Chen, Gong" <gong.chen@linux.intel.com>
> Cc: tony.luck@intel.com, naveen.n.rao@linux.vnet.ibm.com,
>  linux-acpi@vger.kernel.org
> Subject: Re: [PATCH v2 2/2] ACPI, APEI, GHES: Cleanup ghes codes for memory
>  error handling
> User-Agent: Mutt/1.5.21 (2010-09-15)
> 
> On Tue, Nov 26, 2013 at 01:54:57AM -0500, Chen, Gong wrote:
> > In this patch so-called cleanup includes an implied PFN check for UC
> > error but missed in current codes.
> 
> Right, I was about to look at it. You probably should add this to the
> commit message so that it is clear.
> 

How about this:

Add proper PFN validity check for UC error and cleanup the code logic
to make it simpler and cleaner.

If OK and reasonable for this patch, would you mind helping to update the
introduction in the patch before merging it?
Chen Gong Dec. 14, 2013, 1:42 p.m. UTC | #5
On Tue, Nov 26, 2013 at 09:15:42PM -0500, Chen, Gong wrote:
> Date:	Tue, 26 Nov 2013 21:15:42 -0500
> From: "Chen, Gong" <gong.chen@linux.intel.com>
> To: Borislav Petkov <bp@alien8.de>
> Cc: tony.luck@intel.com, naveen.n.rao@linux.vnet.ibm.com,
>  linux-acpi@vger.kernel.org
> Subject: Re: [PATCH v2 2/2] ACPI, APEI, GHES: Cleanup ghes codes for memory
>  error handling
> User-Agent: Mutt/1.5.21 (2010-09-15)
> 
> On Tue, Nov 26, 2013 at 08:23:35AM +0100, Borislav Petkov wrote:
> > Date: Tue, 26 Nov 2013 08:23:35 +0100
> > From: Borislav Petkov <bp@alien8.de>
> > To: "Chen, Gong" <gong.chen@linux.intel.com>
> > Cc: tony.luck@intel.com, naveen.n.rao@linux.vnet.ibm.com,
> >  linux-acpi@vger.kernel.org
> > Subject: Re: [PATCH v2 2/2] ACPI, APEI, GHES: Cleanup ghes codes for memory
> >  error handling
> > User-Agent: Mutt/1.5.21 (2010-09-15)
> > 
> > On Tue, Nov 26, 2013 at 01:54:57AM -0500, Chen, Gong wrote:
> > > In this patch so-called cleanup includes an implied PFN check for UC
> > > error but missed in current codes.
> > 
> > Right, I was about to look at it. You probably should add this to the
> > commit message so that it is clear.
> > 
> 
> How about this:
> 
> Add proper PFN validity check for UC error and cleanup the code logic
> to make it simpler and cleaner.
> 
> If OK and reasonable for this patch, would you mind helping to update the
> introduction in the patch before merging it?

Hi, Boris

Will you pick up this patch in your RAS request pull?
Borislav Petkov Dec. 21, 2013, 12:41 p.m. UTC | #6
On Tue, Nov 26, 2013 at 02:34:51PM +0530, Naveen N. Rao wrote:
> On 11/25/2013 12:45 PM, Chen, Gong wrote:
> >Cleanup the logic for function ghes_handle_memory_failure. Just
> >make it simpler and cleaner.
> >
> >v2 -> v1: fix a compile error & some minor changes.
> >
> >Signed-off-by: Chen, Gong <gong.chen@linux.intel.com>
> 
> Acked-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>

Applied, thanks.
diff mbox

Patch

diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
index ce3683d..46766ef 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_generic_data *gdata, int sev)
 {
 #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_PA)) {
-		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_PA) {
-		pfn = mem_err->physical_addr >> PAGE_SHIFT;
-		memory_failure_queue(pfn, 0, 0);
+	if (!(mem_err->validation_bits & CPER_MEM_VALID_PA))
+		return;
+
+	pfn = mem_err->physical_addr >> PAGE_SHIFT;
+	if (!pfn_valid(pfn)) {
+		pr_warn_ratelimited(FW_WARN GHES_PFX
+		"Invalid address in generic error data: %#llx\n",
+		mem_err->physical_addr);
+		return;
 	}
+
+	/* iff following two events can be handled properly by now */
+	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 (flags != -1)
+		memory_failure_queue(pfn, 0, flags);
 #endif
 }