Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1266955 > unrolled thread

[PATCH] Cleanup useless codes in CMCI handler

Back to article view | Back to linux.kernel

Contents

  [PATCH] Cleanup useless codes in CMCI handler "Chen, Gong" <gong.chen@linux.intel.com> - 2015-11-11 04:30 +0100
    Re: [PATCH] Cleanup useless codes in CMCI handler "Luck, Tony" <tony.luck@intel.com> - 2015-11-11 20:40 +0100
      [UNTESTED PATCH] x86, mce: Avoid double entry of deferred errors into the genpool. Tony Luck <tony.luck@intel.com> - 2015-11-11 23:30 +0100
        Re: [UNTESTED PATCH] x86, mce: Avoid double entry of deferred errors  into the genpool. "Chen, Gong" <gong.chen@linux.intel.com> - 2015-11-12 05:30 +0100

#1266955 — [PATCH] Cleanup useless codes in CMCI handler

UCNA errors share the same handler with CMCI. But it doesn't
need extra operation to save error record in genpool. Remove
these uselss codes.

Signed-off-by: Chen, Gong <gong.chen@linux.intel.com>
---
 arch/x86/kernel/cpu/mcheck/mce.c | 14 --------------
 1 file changed, 14 deletions(-)

diff --git a/arch/x86/kernel/cpu/mcheck/mce.c b/arch/x86/kernel/cpu/mcheck/mce.c
index c5b0d562dbf5..1ad3fb4f99b7 100644
--- a/arch/x86/kernel/cpu/mcheck/mce.c
+++ b/arch/x86/kernel/cpu/mcheck/mce.c
@@ -609,20 +609,6 @@ bool machine_check_poll(enum mcp_flags flags, mce_banks_t *b)
 		severity = mce_severity(&m, mca_cfg.tolerant, NULL, false);
 
 		/*
-		 * In the cases where we don't have a valid address after all,
-		 * do not add it into the ring buffer.
-		 */
-		if (severity == MCE_DEFERRED_SEVERITY && memory_error(&m)) {
-			if (m.status & MCI_STATUS_ADDRV) {
-				m.severity = severity;
-				m.usable_addr = mce_usable_address(&m);
-
-				if (!mce_gen_pool_add(&m))
-					mce_schedule_work();
-			}
-		}
-
-		/*
 		 * Don't get the IP here because it's unlikely to
 		 * have anything to do with the actual error location.
 		 */
-- 
2.3.2

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1267409

On Wed, Nov 11, 2015 at 10:16:45AM -0500, Chen, Gong wrote:
> UCNA errors share the same handler with CMCI. But it doesn't
> need extra operation to save error record in genpool. Remove
> these uselss codes.

I'd have emphasised that this same mce is being added to the genpool
*twice* (once here, and again when we call mce_log() just below).

Though there may be some corner cases depending on flags and
mca_cfg.dont_log_ce

-Tony

> 
> Signed-off-by: Chen, Gong <gong.chen@linux.intel.com>
> ---
>  arch/x86/kernel/cpu/mcheck/mce.c | 14 --------------
>  1 file changed, 14 deletions(-)
> 
> diff --git a/arch/x86/kernel/cpu/mcheck/mce.c b/arch/x86/kernel/cpu/mcheck/mce.c
> index c5b0d562dbf5..1ad3fb4f99b7 100644
> --- a/arch/x86/kernel/cpu/mcheck/mce.c
> +++ b/arch/x86/kernel/cpu/mcheck/mce.c
> @@ -609,20 +609,6 @@ bool machine_check_poll(enum mcp_flags flags, mce_banks_t *b)
>  		severity = mce_severity(&m, mca_cfg.tolerant, NULL, false);
>  
>  		/*
> -		 * In the cases where we don't have a valid address after all,
> -		 * do not add it into the ring buffer.
> -		 */
> -		if (severity == MCE_DEFERRED_SEVERITY && memory_error(&m)) {
> -			if (m.status & MCI_STATUS_ADDRV) {
> -				m.severity = severity;
> -				m.usable_addr = mce_usable_address(&m);
> -
> -				if (!mce_gen_pool_add(&m))
> -					mce_schedule_work();
> -			}
> -		}
> -
> -		/*
>  		 * Don't get the IP here because it's unlikely to
>  		 * have anything to do with the actual error location.
>  		 */
> -- 
> 2.3.2
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1267508 — [UNTESTED PATCH] x86, mce: Avoid double entry of deferred errors into the genpool.

We used to have a special ring buffer for deferred errors that
was used to mark problem pages. We replaced that with a genpool.
Then later converted mce_log() to also use the same genpool. As
a result we end up adding all deferred errors to the genpool twice.

Rearrange this code. Make sure to set the m.severity and m.usable_addr
fields for deferred errors. Then if flags and mca_cfg.dont_log_ce mean
we call mce_log() we are done, because that will add this entry to the
genpool.

If we skipped mce_log(), then we still want to take action for the
deferred error, so add to the genpool.

Changed the name of the boolean "error_logged" to "error_seen", we
should set it whether of not we logged an error because the return
value from machine_check_poll() is used to decide whether storms
have subsided or not.

Reported-by: Chen, Gong <gong.chen.linux.intel.com>
Signed-off-by: Tony Luck <tony.luck@intel.com>
---
 arch/x86/kernel/cpu/mcheck/mce.c | 24 +++++++++++++-----------
 1 file changed, 13 insertions(+), 11 deletions(-)

diff --git a/arch/x86/kernel/cpu/mcheck/mce.c b/arch/x86/kernel/cpu/mcheck/mce.c
index c5b0d562dbf5..6531cb46803c 100644
--- a/arch/x86/kernel/cpu/mcheck/mce.c
+++ b/arch/x86/kernel/cpu/mcheck/mce.c
@@ -567,7 +567,7 @@ DEFINE_PER_CPU(unsigned, mce_poll_count);
  */
 bool machine_check_poll(enum mcp_flags flags, mce_banks_t *b)
 {
-	bool error_logged = false;
+	bool error_seen = false;
 	struct mce m;
 	int severity;
 	int i;
@@ -601,6 +601,8 @@ bool machine_check_poll(enum mcp_flags flags, mce_banks_t *b)
 		    (m.status & (mca_cfg.ser ? MCI_STATUS_S : MCI_STATUS_UC)))
 			continue;
 
+		error_seen = true;
+
 		mce_read_aux(&m, i);
 
 		if (!(flags & MCP_TIMESTAMP))
@@ -608,17 +610,10 @@ bool machine_check_poll(enum mcp_flags flags, mce_banks_t *b)
 
 		severity = mce_severity(&m, mca_cfg.tolerant, NULL, false);
 
-		/*
-		 * In the cases where we don't have a valid address after all,
-		 * do not add it into the ring buffer.
-		 */
 		if (severity == MCE_DEFERRED_SEVERITY && memory_error(&m)) {
 			if (m.status & MCI_STATUS_ADDRV) {
 				m.severity = severity;
 				m.usable_addr = mce_usable_address(&m);
-
-				if (!mce_gen_pool_add(&m))
-					mce_schedule_work();
 			}
 		}
 
@@ -626,9 +621,16 @@ bool machine_check_poll(enum mcp_flags flags, mce_banks_t *b)
 		 * Don't get the IP here because it's unlikely to
 		 * have anything to do with the actual error location.
 		 */
-		if (!(flags & MCP_DONTLOG) && !mca_cfg.dont_log_ce) {
-			error_logged = true;
+		if (!(flags & MCP_DONTLOG) && !mca_cfg.dont_log_ce)
 			mce_log(&m);
+		else if (m.usable_addr) {
+			/*
+			 * Although we skipped logging this, we still want
+			 * to take action. Add to the pool so the registered
+			 * notifiers will see it.
+			 */
+			if (!mce_gen_pool_add(&m))
+				mce_schedule_work();
 		}
 
 		/*
@@ -644,7 +646,7 @@ bool machine_check_poll(enum mcp_flags flags, mce_banks_t *b)
 
 	sync_core();
 
-	return error_logged;
+	return error_seen;
 }
 EXPORT_SYMBOL_GPL(machine_check_poll);
 
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1267608 — Re: [UNTESTED PATCH] x86, mce: Avoid double entry of deferred errors into the genpool.

[Multipart message — attachments visible in raw view] — view raw

On Wed, Nov 11, 2015 at 02:01:51PM -0800, Luck, Tony wrote:
> Date: Wed, 11 Nov 2015 14:01:51 -0800
> From: Tony Luck <tony.luck@intel.com>
> To: "Chen, Gong" <gong.chen@linux.intel.com>
> Cc: bp@alien8.de, linux-edac@vger.kernel.org, linux-kernel@vger.kernel.org
> Subject: [UNTESTED PATCH] x86, mce: Avoid double entry of deferred errors
>  into the genpool.
> 
> We used to have a special ring buffer for deferred errors that
> was used to mark problem pages. We replaced that with a genpool.
> Then later converted mce_log() to also use the same genpool. As
> a result we end up adding all deferred errors to the genpool twice.
> 
> Rearrange this code. Make sure to set the m.severity and m.usable_addr
> fields for deferred errors. Then if flags and mca_cfg.dont_log_ce mean
> we call mce_log() we are done, because that will add this entry to the
> genpool.
> 
> If we skipped mce_log(), then we still want to take action for the
> deferred error, so add to the genpool.
> 
> Changed the name of the boolean "error_logged" to "error_seen", we
> should set it whether of not we logged an error because the return
> value from machine_check_poll() is used to decide whether storms
> have subsided or not.
> 
> Reported-by: Chen, Gong <gong.chen.linux.intel.com>
> Signed-off-by: Tony Luck <tony.luck@intel.com>
> ---

It's much better than my original version.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web