[PATCH] powerpc/eeh: Disable EEH stack dump by default

Subsystems: linux for powerpc (32-bit and 64-bit), pci enhanced error handling (eeh) for powerpc, the rest

STALE3273d

4 messages, 3 authors, 2017-09-20 · open the first message on its own page

[PATCH] powerpc/eeh: Disable EEH stack dump by default

From: Jose Ricardo Ziviani <hidden>
Date: 2017-09-19 14:25:50

Today, each EEH causes a stack dump to be printed in the logs. In
production environment it's not quite necessary. Thus, this patch
adds a new command line argument in order to enable the stack
dump for debugging purposes.

For example, instead of the following:

[  131.778661] EEH: Frozen PHB#2-PE#fd detected
[  131.778672] EEH: PE location: N/A, PHB location: N/A
[  131.778677] CPU: 21 PID: 10098 Comm: lspci Not tainted ...
[  131.778680] Call Trace:
[  131.778686] [c0000003a140bab0] [c000000000beb58c] dump_stack+...
<snip ~10 lines>
[  131.778770] EEH: Detected PCI bus error on PHB#2-PE#fd
[  131.778775] EEH: This PCI device has failed 1 times in the last hour
...

we will have this by default:

[12777.175880] EEH: Frozen PHB#2-PE#fd detected
[12777.175893] EEH: PE location: N/A, PHB location: N/A
[12777.175922] EEH: Detected PCI bus error on PHB#2-PE#fd
[12777.175931] EEH: This PCI device has failed 2 times in the last hour
...

Signed-off-by: Jose Ricardo Ziviani <redacted>
---
 arch/powerpc/kernel/eeh.c | 26 +++++++++++++++++++++++---
 1 file changed, 23 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
index 9e81678..4336c3b1 100644
--- a/arch/powerpc/kernel/eeh.c
+++ b/arch/powerpc/kernel/eeh.c

@@ -157,6 +157,19 @@ static int __init eeh_setup(char *str)
 __setup("eeh=", eeh_setup);
 
 /*
+ * It's not necessary to dump the stack trace when an EEH occours
+ * in the production environment. For debugging, the command line
+ * option "enable_eeh_stacktrace" brings the stack dump back
+ */
+static bool eeh_show_stacktrace;
+static int __init enable_eeh_stacktrace(char *p)
+{
+	eeh_show_stacktrace = true;
+	return 0;
+}
+early_param("enable_eeh_stacktrace", enable_eeh_stacktrace);
+
+/*
  * This routine captures assorted PCI configuration space data
  * for the indicated PCI device, and puts them into a buffer
  * for RTAS error logging.

@@ -407,7 +420,10 @@ static int eeh_phb_check_failure(struct eeh_pe *pe)
 
 	pr_err("EEH: PHB#%x failure detected, location: %s\n",
 		phb_pe->phb->global_number, eeh_pe_loc_get(phb_pe));
-	dump_stack();
+
+	if (eeh_show_stacktrace)
+		dump_stack();
+
 	eeh_send_failure_event(phb_pe);
 
 	return 1;

@@ -504,7 +520,9 @@ int eeh_dev_check_failure(struct eeh_dev *edev)
 				eeh_driver_name(dev), eeh_pci_name(dev));
 			printk(KERN_ERR "EEH: Might be infinite loop in %s driver\n",
 				eeh_driver_name(dev));
-			dump_stack();
+
+			if (eeh_show_stacktrace)
+				dump_stack();
 		}
 		goto dn_unlock;
 	}

@@ -572,7 +590,9 @@ int eeh_dev_check_failure(struct eeh_dev *edev)
 	       pe->phb->global_number, pe->addr);
 	pr_err("EEH: PE location: %s, PHB location: %s\n",
 	       eeh_pe_loc_get(pe), eeh_pe_loc_get(phb_pe));
-	dump_stack();
+
+	if (eeh_show_stacktrace)
+		dump_stack();
 
 	eeh_send_failure_event(pe);

-- 
2.7.4

Re: [PATCH] powerpc/eeh: Disable EEH stack dump by default

From: Michael Ellerman <mpe@ellerman.id.au>
Date: 2017-09-20 04:47:09

Jose Ricardo Ziviani [off-list ref] writes:

Today, each EEH causes a stack dump to be printed in the logs. In
production environment it's not quite necessary. Thus, this patch

I'm unconvinced. A production environment is exactly where you don't
want to be getting an EEH, and so if you *do* then every bit of
information is helpful.

For example, instead of the following:

[  131.778661] EEH: Frozen PHB#2-PE#fd detected
[  131.778672] EEH: PE location: N/A, PHB location: N/A
[  131.778677] CPU: 21 PID: 10098 Comm: lspci Not tainted ...
[  131.778680] Call Trace:
[  131.778686] [c0000003a140bab0] [c000000000beb58c] dump_stack+...
<snip ~10 lines>
[  131.778770] EEH: Detected PCI bus error on PHB#2-PE#fd
[  131.778775] EEH: This PCI device has failed 1 times in the last hour
...

we will have this by default:

[12777.175880] EEH: Frozen PHB#2-PE#fd detected
[12777.175893] EEH: PE location: N/A, PHB location: N/A
[12777.175922] EEH: Detected PCI bus error on PHB#2-PE#fd
[12777.175931] EEH: This PCI device has failed 2 times in the last hour

*What* PCI device?

How am I supposed to know what device/driver just failed? If I had the
stack trace I could probably at least work it out based on the driver
involved.

cheers

Re: [PATCH] powerpc/eeh: Disable EEH stack dump by default

From: Andrew Donnellan <hidden>
Date: 2017-09-20 05:54:54

On 20/09/17 00:25, Jose Ricardo Ziviani wrote:

Today, each EEH causes a stack dump to be printed in the logs. In
production environment it's not quite necessary. Thus, this patch
adds a new command line argument in order to enable the stack
dump for debugging purposes.

For example, instead of the following:

[  131.778661] EEH: Frozen PHB#2-PE#fd detected
[  131.778672] EEH: PE location: N/A, PHB location: N/A
[  131.778677] CPU: 21 PID: 10098 Comm: lspci Not tainted ...
[  131.778680] Call Trace:
[  131.778686] [c0000003a140bab0] [c000000000beb58c] dump_stack+...
<snip ~10 lines>
[  131.778770] EEH: Detected PCI bus error on PHB#2-PE#fd
[  131.778775] EEH: This PCI device has failed 1 times in the last hour
...

we will have this by default:

[12777.175880] EEH: Frozen PHB#2-PE#fd detected
[12777.175893] EEH: PE location: N/A, PHB location: N/A
[12777.175922] EEH: Detected PCI bus error on PHB#2-PE#fd
[12777.175931] EEH: This PCI device has failed 2 times in the last hour
...

Signed-off-by: Jose Ricardo Ziviani <redacted>

As someone who's had to debug far too many EEH-related bugs, I'd really 
prefer if this remained as is.


Andrew

quoted hunk

---
  arch/powerpc/kernel/eeh.c | 26 +++++++++++++++++++++++---
  1 file changed, 23 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
index 9e81678..4336c3b1 100644
--- a/arch/powerpc/kernel/eeh.c
+++ b/arch/powerpc/kernel/eeh.c

@@ -157,6 +157,19 @@ static int __init eeh_setup(char *str)
  __setup("eeh=", eeh_setup);
  
  /*
+ * It's not necessary to dump the stack trace when an EEH occours
+ * in the production environment. For debugging, the command line
+ * option "enable_eeh_stacktrace" brings the stack dump back
+ */
+static bool eeh_show_stacktrace;
+static int __init enable_eeh_stacktrace(char *p)
+{
+	eeh_show_stacktrace = true;
+	return 0;
+}
+early_param("enable_eeh_stacktrace", enable_eeh_stacktrace);
+
+/*
   * This routine captures assorted PCI configuration space data
   * for the indicated PCI device, and puts them into a buffer
   * for RTAS error logging.

@@ -407,7 +420,10 @@ static int eeh_phb_check_failure(struct eeh_pe *pe)
  
  	pr_err("EEH: PHB#%x failure detected, location: %s\n",
  		phb_pe->phb->global_number, eeh_pe_loc_get(phb_pe));
-	dump_stack();
+
+	if (eeh_show_stacktrace)
+		dump_stack();
+
  	eeh_send_failure_event(phb_pe);
  
  	return 1;

@@ -504,7 +520,9 @@ int eeh_dev_check_failure(struct eeh_dev *edev)
  				eeh_driver_name(dev), eeh_pci_name(dev));
  			printk(KERN_ERR "EEH: Might be infinite loop in %s driver\n",
  				eeh_driver_name(dev));
-			dump_stack();
+
+			if (eeh_show_stacktrace)
+				dump_stack();
  		}
  		goto dn_unlock;
  	}

@@ -572,7 +590,9 @@ int eeh_dev_check_failure(struct eeh_dev *edev)
  	       pe->phb->global_number, pe->addr);
  	pr_err("EEH: PE location: %s, PHB location: %s\n",
  	       eeh_pe_loc_get(pe), eeh_pe_loc_get(phb_pe));
-	dump_stack();
+
+	if (eeh_show_stacktrace)
+		dump_stack();
  
  	eeh_send_failure_event(pe);

-- 
Andrew Donnellan              OzLabs, ADL Canberra
andrew.donnellan@au1.ibm.com  IBM Australia Limited

Re: [PATCH] powerpc/eeh: Disable EEH stack dump by default

From: <hidden>
Date: 2017-09-20 17:55:31

On Wed, Sep 20, 2017 at 02:47:08PM +1000, Michael Ellerman wrote:

Jose Ricardo Ziviani [off-list ref] writes:

quoted

Today, each EEH causes a stack dump to be printed in the logs. In
production environment it's not quite necessary. Thus, this patch

I'm unconvinced. A production environment is exactly where you don't
want to be getting an EEH, and so if you *do* then every bit of
information is helpful.

quoted

For example, instead of the following:

[  131.778661] EEH: Frozen PHB#2-PE#fd detected
[  131.778672] EEH: PE location: N/A, PHB location: N/A
[  131.778677] CPU: 21 PID: 10098 Comm: lspci Not tainted ...
[  131.778680] Call Trace:
[  131.778686] [c0000003a140bab0] [c000000000beb58c] dump_stack+...
<snip ~10 lines>
[  131.778770] EEH: Detected PCI bus error on PHB#2-PE#fd
[  131.778775] EEH: This PCI device has failed 1 times in the last hour
...

we will have this by default:

[12777.175880] EEH: Frozen PHB#2-PE#fd detected
[12777.175893] EEH: PE location: N/A, PHB location: N/A
[12777.175922] EEH: Detected PCI bus error on PHB#2-PE#fd
[12777.175931] EEH: This PCI device has failed 2 times in the last hour

*What* PCI device?

How am I supposed to know what device/driver just failed? If I had the
stack trace I could probably at least work it out based on the driver
involved.

cheers

Thank you guys! More people told me it's important to keep it as is.
Please, disregard this patch.

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help