Skip to contentSkip to conversation

noisy edac

PATCH In review Dave Jones 7 participants 19 messages Jan 27, 2006 – Feb 2, 2006

Subsystems (3)
EDAC-core EDAC-E752X EDAC-E7XXX
lore
Share & tools
lore

Dave Jones

author
#0 lore raw

e752x_edac is very noisy on my PCIE system..
my dmesg is filled with these...

[91671.488379] Non-Fatal Error PCI Express B
[91671.492468] Non-Fatal Error PCI Express B
[91901.100576] Non-Fatal Error PCI Express B
[91901.104675] Non-Fatal Error PCI Express B

Something need whitelisting?

Dave

00:00.0 Host bridge: Intel Corporation E7525 Memory Controller Hub (rev 09)
00:00.1 Class ff00: Intel Corporation E7525/E7520 Error Reporting Registers (rev 09)
00:02.0 PCI bridge: Intel Corporation E7525/E7520/E7320 PCI Express Port A (rev 09)
00:03.0 PCI bridge: Intel Corporation E7525/E7520/E7320 PCI Express Port A1 (rev 09)
00:04.0 PCI bridge: Intel Corporation E7525/E7520 PCI Express Port B (rev 09)
00:1d.0 USB Controller: Intel Corporation 82801EB/ER (ICH5/ICH5R) USB UHCI Controller #1 (rev 02) 00:1d.1 USB Controller: Intel Corporation 82801EB/ER (ICH5/ICH5R) USB UHCI Controller #2 (rev 02) 00:1d.2 USB Controller: Intel Corporation 82801EB/ER (ICH5/ICH5R) USB UHCI Controller #3 (rev 02) 00:1d.3 USB Controller: Intel Corporation 82801EB/ER (ICH5/ICH5R) USB UHCI Controller #4 (rev 02) 00:1d.7 USB Controller: Intel Corporation 82801EB/ER (ICH5/ICH5R) USB2 EHCI Controller (rev 02) 00:1e.0 PCI bridge: Intel Corporation 82801 PCI Bridge (rev c2)
00:1f.0 ISA bridge: Intel Corporation 82801EB/ER (ICH5/ICH5R) LPC Interface Bridge (rev 02) 00:1f.1 IDE interface: Intel Corporation 82801EB/ER (ICH5/ICH5R) IDE Controller (rev 02) 00:1f.2 IDE interface: Intel Corporation 82801EB (ICH5) SATA Controller (rev 02)
00:1f.3 SMBus: Intel Corporation 82801EB/ER (ICH5/ICH5R) SMBus Controller (rev 02)
00:1f.5 Multimedia audio controller: Intel Corporation 82801EB/ER (ICH5/ICH5R) AC'97 Audio Controlle r (rev 02) 01:00.0 PCI bridge: Intel Corporation 6700PXH PCI Express-to-PCI Bridge A
01:00.2 PCI bridge: Intel Corporation 6700PXH PCI Express-to-PCI Bridge B
03:0e.0 Ethernet controller: Intel Corporation 82545GM Gigabit Ethernet Controller (rev 04) 05:00.0 VGA compatible controller: ATI Technologies Inc RV370 5B60 [Radeon X300 (PCIE)] 05:00.1 Display controller: ATI Technologies Inc RV370 [Radeon X300SE]
06:0d.0 Multimedia audio controller: Creative Labs SB Live! EMU10k1 (rev 04)
06:0d.1 Input device controller: Creative Labs SB Live! MIDI/Game Port (rev 01)

#1 lore raw
Quoted 7 lines · Dave Jones

e752x_edac is very noisy on my PCIE system.. my dmesg is filled with these...

[91671.488379] Non-Fatal Error PCI Express B
[91671.492468] Non-Fatal Error PCI Express B
[91901.100576] Non-Fatal Error PCI Express B
[91901.104675] Non-Fatal Error PCI Express B

Pre-production system or final release ?

Quoted 1 line

Something need whitelisting?

Its logging in a manner consistent with real errors so its hard to be sure at first glance.

author
#2 lore raw
Quoted 10 lines · Alan Cox
Quoted 7 lines · Dave Jones

e752x_edac is very noisy on my PCIE system.. my dmesg is filled with these...

[91671.488379] Non-Fatal Error PCI Express B
[91671.492468] Non-Fatal Error PCI Express B
[91901.100576] Non-Fatal Error PCI Express B
[91901.104675] Non-Fatal Error PCI Express B

Pre-production system or final release ?

Currently shipping Dell Precision 470.

Dave

#3 lore raw

that driver should be refactored to only output NON-FATALs with debug turned on.

Copying to edac/bluesmoke mailing list

doug t

Quoted 14 lines · --- Dave Jones
Quoted 10 lines · Alan Cox
Quoted 7 lines · Dave Jones

e752x_edac is very noisy on my PCIE system.. my dmesg is filled with these...

[91671.488379] Non-Fatal Error PCI Express B
[91671.492468] Non-Fatal Error PCI Express B
[91901.100576] Non-Fatal Error PCI Express B
[91901.104675] Non-Fatal Error PCI Express B

Pre-production system or final release ?

Currently shipping Dell Precision 470.

Quoted 8 lines

Dave

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

"If you think Education is expensive, just try Ignorance"

"Don't tell people HOW to do things, tell them WHAT you want and they will surprise you with their ingenuity."
Gen George Patton

#4 lore raw
Quoted 2 lines · Doug Thompson

that driver should be refactored to only output NON-FATALs with debug turned on.

I would prefer a sysfs option or something similar that allows the user to determine what action to take on these errors. I think the debug option should only pertain to messages whose purpose is for debugging the EDAC code itself, as opposed to hardware errors detected by EDAC.

Quoted 20 lines

Copying to edac/bluesmoke mailing list

doug t

Quoted 15 lines · --- Dave Jones
Quoted 10 lines · Alan Cox
Quoted 7 lines · Dave Jones

e752x_edac is very noisy on my PCIE system.. my dmesg is filled with these...

[91671.488379] Non-Fatal Error PCI Express B
[91671.492468] Non-Fatal Error PCI Express B
[91901.100576] Non-Fatal Error PCI Express B
[91901.104675] Non-Fatal Error PCI Express B

Pre-production system or final release ?

Currently shipping Dell Precision 470.

Dave

#5 lore raw
Quoted 8 lines · Dave Peterson
Quoted 2 lines · Doug Thompson

that driver should be refactored to only output NON-FATALs with debug turned on.

I would prefer a sysfs option or something similar that allows the user to determine what action to take on these errors. I think the debug option should only pertain to messages whose purpose is for debugging the EDAC code itself, as opposed to hardware errors detected by EDAC.

Something like an ERROR report verbose level? 0 to 7 like?

0 being quiet, 7 being very verbose? or the reverse.

/sys/drivers/system/edac/mc/error_report_verbosity ????

This tackles the immediate issue, but there is a systemic issue we have to face sometime.

One problem that this e752x_edac module exhibits, which is manifest on all of the drivers to one degree, is the output of driver specific error messages directly, since there is not an abstracted error interface (yet) in the EDAC core. The messages are or can be very specific to the MC being driven. In time, we can (should) add a better MC error interface to the core and then map errors from specific MC errors to the new CORE error interface. Similiar to how SCSI and SATA have higher level abstract errors which the transport drivers map errors to.

This e752x_edac module just plainly outputs to printk() with KERN_WARNING w/o any other output control.

Looks like the old "how do we report errors" pattern, with its first implementation now looking old.

doug t

Quoted 22 lines
Quoted 21 lines

Copying to edac/bluesmoke mailing list

doug t

Quoted 15 lines · --- Dave Jones
Quoted 10 lines · Alan Cox
Quoted 7 lines · Dave Jones

e752x_edac is very noisy on my PCIE system.. my dmesg is filled with these...

[91671.488379] Non-Fatal Error PCI Express B
[91671.492468] Non-Fatal Error PCI Express B
[91901.100576] Non-Fatal Error PCI Express B
[91901.104675] Non-Fatal Error PCI Express B

Pre-production system or final release ?

Currently shipping Dell Precision 470.

Dave

#6 lore raw
Quoted 18 lines · doug thompson

Something like an ERROR report verbose level? 0 to 7 like?

0 being quiet, 7 being very verbose? or the reverse.

/sys/drivers/system/edac/mc/error_report_verbosity ????

This tackles the immediate issue, but there is a systemic issue we have to face sometime.

One problem that this e752x_edac module exhibits, which is manifest on all of the drivers to one degree, is the output of driver specific error messages directly, since there is not an abstracted error interface (yet) in the EDAC core. The messages are or can be very specific to the MC being driven. In time, we can (should) add a better MC error interface to the core and then map errors from specific MC errors to the new CORE error interface. Similiar to how SCSI and SATA have higher level abstract errors which the transport drivers map errors to.

Quoted 5 lines

This e752x_edac module just plainly outputs to printk() with KERN_WARNING w/o any other output control.

Looks like the old "how do we report errors" pattern, with its first implementation now looking old.

For each individual type of error that is specific to a particular low-level chipset driver (e752x, amd76x, etc.) there could be an entry in the appropriate part of the sysfs hierarchy under the given chipset driver. This entry could have several settings that the user may choose from such as { ignore, syslog, panic }. For the implementation, there could be a generic piece of code in the core EDAC module that a chipset driver calls into. The generic code would do the dirty work of creating the sysfs entries (and destroying them when the chipset module is unloading). How does this sound?

#7 lore raw
Quoted 29 lines · Dave Peterson
Quoted 18 lines · doug thompson

Something like an ERROR report verbose level? 0 to 7 like?

0 being quiet, 7 being very verbose? or the reverse.

/sys/drivers/system/edac/mc/error_report_verbosity ????

This tackles the immediate issue, but there is a systemic issue we have to face sometime.

One problem that this e752x_edac module exhibits, which is manifest on all of the drivers to one degree, is the output of driver specific error messages directly, since there is not an abstracted error interface (yet) in the EDAC core. The messages are or can be very specific to the MC being driven. In time, we can (should) add a better MC error interface to the core and then map errors from specific MC errors to the new CORE error interface. Similiar to how SCSI and SATA have higher level abstract errors which the transport drivers map errors to.

Quoted 7 lines

This e752x_edac module just plainly outputs to printk() with KERN_WARNING w/o any other output control.

Looks like the old "how do we report errors" pattern, with its first implementation now looking old.

Quoted 12 lines

For each individual type of error that is specific to a particular low-level chipset driver (e752x, amd76x, etc.) there could be an entry in the appropriate part of the sysfs hierarchy under the given chipset driver. This entry could have several settings that the user may choose from such as { ignore, syslog, panic }. For the implementation, there could be a generic piece of code in the core EDAC module that a chipset driver calls into. The generic code would do the dirty work of creating the sysfs entries (and destroying them when the chipset module is unloading). How does this sound?
-

Over-Engineered.

#8 lore raw
Quoted 11 lines · Gunther Mayer
Quoted 9 lines

For each individual type of error that is specific to a particular low-level chipset driver (e752x, amd76x, etc.) there could be an entry in the appropriate part of the sysfs hierarchy under the given chipset driver. This entry could have several settings that the user may choose from such as { ignore, syslog, panic }. For the implementation, there could be a generic piece of code in the core EDAC module that a chipset driver calls into. The generic code would do the dirty work of creating the sysfs entries (and destroying them when the chipset module is unloading). How does this sound?

Over-Engineered.

Do you have an alternate suggestion?

#9 lore raw
Quoted 18 lines · Dave Peterson
Quoted 15 lines · Gunther Mayer
Quoted 15 lines

For each individual type of error that is specific to a particular low-level chipset driver (e752x, amd76x, etc.) there could be an entry in the appropriate part of the sysfs hierarchy under the given chipset driver. This entry could have several settings that the user may choose

from such as { ignore, syslog, panic }. For the implementation, there

could be a generic piece of code in the core EDAC module that a chipset driver calls into. The generic code would do the dirty work of creating the sysfs entries (and destroying them when the chipset module is unloading). How does this sound?

Quoted 7 lines
Quoted 3 lines

Over-Engineered.

Do you have an alternate suggestion?

Just printk() the exact driver specific low-level error, even if non-fatal.

Single non-fatal errors just show your system recovers correctly.

Multiple (e.g. noisy) non-fatal are either an indication of a serious problem
(e.g. after how many corrected ECC errors on the same address in which
time interval will you replace your dimm? How many S-ATA CRC-errors
will indicate marginal bad cabling? )
or it shows the problem needs to be root analyzed. But don't disable the messages as this will only hide the real problem.

Concerning Non-Fatal PCI Express errors, the error cause registers need to be printed in case of error, too (see Intel Chipset Specifications)

-
Gunther

#10 lore raw
Quoted 39 lines · Gunther Mayer
Quoted 18 lines · Dave Peterson
Quoted 15 lines · Gunther Mayer
Quoted 15 lines

For each individual type of error that is specific to a particular low-level chipset driver (e752x, amd76x, etc.) there could be an entry in the appropriate part of the sysfs hierarchy under the given chipset driver. This entry could have several settings that the user may choose

from such as { ignore, syslog, panic }. For the implementation, there

could be a generic piece of code in the core EDAC module that a chipset driver calls into. The generic code would do the dirty work of creating the sysfs entries (and destroying them when the chipset module is unloading). How does this sound?

Quoted 7 lines
Quoted 3 lines

Over-Engineered.

Do you have an alternate suggestion?

Just printk() the exact driver specific low-level error, even if non-fatal.

Single non-fatal errors just show your system recovers correctly.

Multiple (e.g. noisy) non-fatal are either an indication of a serious problem
(e.g. after how many corrected ECC errors on the same address in which
time interval will you replace your dimm? How many S-ATA CRC-errors
will indicate marginal bad cabling? )
or it shows the problem needs to be root analyzed. But don't disable the messages as this will only hide the real problem.

Quoted 2 lines

Concerning Non-Fatal PCI Express errors, the error cause registers need to be printed in case of error, too (see Intel Chipset Specifications)

EDAC currently presents a common interface for harvesting the number of CEs (and other data) that occur AND maps to a DIMM label, which can be populated to a mobo silk screen label. This is via the sysfs interface.

Currently each of the MC drivers some of their output error messages in their own pattern of output, while also funnelling common info to the core module. We don't want to lose that device specific information, but it would be a better pattern to funnel that device specific output to the EDAC Core module for presentation in a more uniforum manner, through the new sysfs interface, such as:

/sys/drivers/system/edac/mc/mc_driver_error_report

or some such.

The EDAC stack is like a SCSI stack: A core module and a lower level instance driver.

We want to move some of the information that is currently being dumped via printk() to the sysfs interface. Some not all.

The message that is being printed now that caused this post, just prints some MC specific information, but not quite enough to be of real use. It is extra noise, which needs to be examined in depth, to form a clean error stack.

New MC drivers are in the works. New MC chipsets are coming down the line and I don't want to have each driver output its OWN output that proves more difficult to "harvest" over time, because each driver writer does it his way.

EDAC is not perfect enough yet, but with time and more refactoring it will approach there.

thanks

doug t

Quoted 15 lines

-
Gunther

-------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc. Do you grep through log files for problems? Stop! Download the new AJAX search engine that makes searching your log files as easy as surfing the web. DOWNLOAD SPLUNK!
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=103432&bid=230486&dat=121642
_______________________________________________
bluesmoke-devel mailing list [email protected]
https://lists.sourceforge.net/lists/listinfo/bluesmoke-devel

#11 lore raw
Quoted 6 lines · doug thompson

Currently each of the MC drivers some of their output error messages in their own pattern of output, while also funnelling common info to the core module. We don't want to lose that device specific information, but it would be a better pattern to funnel that device specific output to the EDAC Core module for presentation in a more uniforum manner, through the new sysfs interface, such as:

I think the kinds of error information that different chipsets report will be diverse enough that it will be hard or impossible to identify enough commonality to justify pushing chipset-specific error info to the core EDAC module.

Quoted 3 lines

/sys/drivers/system/edac/mc/mc_driver_error_report

or some such.

In general, I think having this info go to the console or the system log would fit better relative to the way other subsystems report their error information.

However, one thing I can think of that may be fairly common is a desire to keep track of the # of occurrences of a particular kind of error (as is currently done for correctible ECC memory errors). For errors for which this is desired, sysfs can provide an entry that gives the number of occurrences of a particular type of error. However, I would prefer to see that info provided under the part of the sysfs hierarchy for the particular chipset that detects a given type of error.

For errors for which we track the # of occurrences, it may be desirable to perform some action (perhaps emailing a sysadmin or halting the machine) when the error count reaches a certain threshold. I think it would be good to have the logic for determining when the threshold is reached (and the action to take when the threshold is reached) reside out in user space as much as possible.

For some chipsets (such as the Intel e7525), the hardware has a built-in "leaky bucket" mechanism for tracking counts of correctible ECC memory errors. For chipsets that support this, we will probably eventually want to provide a means for the user to enable this functionality. For chipsets that do not support it, the leaky bucket algorithm (or perhaps some other algorithm) can be done by user space code provided that there is some means of pushing the relevant info out to user space.

Dave

#12 lore raw
In reply to Gunther Mayer
Quoted 12 lines · Gunther Mayer

Just printk() the exact driver specific low-level error, even if non-fatal.

Single non-fatal errors just show your system recovers correctly.

Multiple (e.g. noisy) non-fatal are either an indication of a serious problem
(e.g. after how many corrected ECC errors on the same address in which
time interval will you replace your dimm? How many S-ATA CRC-errors
will indicate marginal bad cabling? )
or it shows the problem needs to be root analyzed. But don't disable the messages as this will only hide the real problem.

Quoted 2 lines

Concerning Non-Fatal PCI Express errors, the error cause registers need to be printed in case of error, too (see Intel Chipset Specifications)

I agree that in general, the kernel should not be silent when errors are detected. However, for a particular type of error, it may be that the user is aware of the error (because (s)he has seen the messages), the user has determined the root cause, and it turns out that the error is benign. Therefore the user may want to suppress further messages of this type to avoid cluttering the logs. If you don't provide that option to the user, then it can be viewed as hardcoding a certain aspect of sysadmin policy into the kernel.

Depending on the particular type of error, it may be appropriate to just offer the user two options: either printk() or be silent. For other types of errors, it may make sense to give the user more than two options (for instance ignore, printk(), or panic()). I think developers of chipset drivers can make this decision individually for each type of error.

#13 lore raw
Quoted 25 lines · Dave Peterson
Quoted 12 lines · Gunther Mayer

Just printk() the exact driver specific low-level error, even if non-fatal.

Single non-fatal errors just show your system recovers correctly.

Multiple (e.g. noisy) non-fatal are either an indication of a serious problem
(e.g. after how many corrected ECC errors on the same address in which
time interval will you replace your dimm? How many S-ATA CRC-errors
will indicate marginal bad cabling? )
or it shows the problem needs to be root analyzed. But don't disable the messages as this will only hide the real problem.

Quoted 2 lines

Concerning Non-Fatal PCI Express errors, the error cause registers need to be printed in case of error, too (see Intel Chipset Specifications)

I agree that in general, the kernel should not be silent when errors are detected. However, for a particular type of error, it may be that the user is aware of the error (because (s)he has seen the messages), the user has determined the root cause, and it turns out that the error is benign. Therefore the user may want to suppress further messages of this type to avoid cluttering the logs. If you don't provide that option to the user, then it can be viewed as hardcoding a certain aspect of sysadmin policy into the kernel.

Quoted 5 lines

Depending on the particular type of error, it may be appropriate to just offer the user two options: either printk() or be silent. For other types of errors, it may make sense to give the user more than two options (for instance ignore, printk(), or panic()). I think developers of chipset drivers can make this decision individually for each type of error.

One piece missing from this conversation is the issue that we need errors in a uniform format. That is why edac_mc has helper functions.

However there will always be errors that don't fit any particular model. Could we add a edac_printk(dev, ); That is similar to dev_printk but prints out an EDAC header and the device on which the error was found? Letting the rest of the string be user specified.

For actual control that interface may be to blunt, but at least for people looking in the logs it allows all of the errors to be detected and harvested.

Eric

#14 lore raw
Quoted 7 lines · Eric W. Biederman

One piece missing from this conversation is the issue that we need errors in a uniform format. That is why edac_mc has helper functions.

However there will always be errors that don't fit any particular model. Could we add a edac_printk(dev, ); That is similar to dev_printk but prints out an EDAC header and the device on which the error was found? Letting the rest of the string be user specified.

I like this idea. It would facilitate grepping in logfiles for EDAC-related errors.

#15 lore raw
Quoted 11 lines · Dave Peterson
Quoted 7 lines · Eric W. Biederman

One piece missing from this conversation is the issue that we need errors in a uniform format. That is why edac_mc has helper functions.

However there will always be errors that don't fit any particular model. Could we add a edac_printk(dev, ); That is similar to dev_printk but prints out an EDAC header and the device on which the error was found? Letting the rest of the string be user specified.

I like this idea. It would facilitate grepping in logfiles for EDAC-related errors.

Yes, eric, that puts into words what I was trying to express.

Refactor all the MC drivers to utilize such a 'edac_printk()' helper function, and add control mechanisms to that function for level of output, etc. Then the begining format of the output stream can be "tagged" as EDAC output, etc, for harvest patterns.

As to Dave Jones original, I remember that the "NON-FATAL" tag was still indicating that some error was occuring, but it wasn't a "take the machine down" type of event. But it was some problem. Time to break out the Intel Specs for that MC.

doug t

#16 lore raw

PATCH 1/1 EDAC printk() cleanup

In reply to Eric W. Biederman
Quoted 11 lines · Eric W. Biederman

One piece missing from this conversation is the issue that we need errors in a uniform format. That is why edac_mc has helper functions.

However there will always be errors that don't fit any particular model. Could we add a edac_printk(dev, ); That is similar to dev_printk but prints out an EDAC header and the device on which the error was found? Letting the rest of the string be user specified.

For actual control that interface may be to blunt, but at least for people looking in the logs it allows all of the errors to be detected and harvested.

Ok, the patch below (which applies to the 2.6.16-rc1-git4 kernel) is an initial attempt at implementing this sort of thing. Here is some sample output (produced by loading the edac_mc and e7xxx_edac moules):

EDAC MC: /tftpboot/dsp/printk/linux/drivers/edac/edac_mc.c version edac_mc Ver: 2.0.0 Feb 1 2006 EDAC DEBUG: /tftpboot/dsp/printk/linux/drivers/edac/edac_mc.c: edac_sysfs_memctrl_setup() EDAC DEBUG: Registered '.../edac/mc' kobject
EDAC DEBUG: /tftpboot/dsp/printk/linux/drivers/edac/edac_mc.c: edac_sysfs_pci_setup() EDAC DEBUG: Registered '.../edac/pci' kobject
EDAC DEBUG: /tftpboot/dsp/printk/linux/drivers/edac/e7xxx_edac.c: e7xxx_init_one() EDAC DEBUG: /tftpboot/dsp/printk/linux/drivers/edac/e7xxx_edac.c: e7xxx_probe1(): mci EDAC e7xxx: tolm = 40000, remapbase = ffc000, remaplimit = 0
EDAC DEBUG: /tftpboot/dsp/printk/linux/drivers/edac/edac_mc.c: edac_mc_add_mc() EDAC DEBUG: /tftpboot/dsp/printk/linux/drivers/edac/edac_mc.c: edac_create_sysfs_mci_device() idx=0 EDAC DEBUG: /tftpboot/dsp/printk/linux/drivers/edac/edac_mc.c: edac_create_csrow_object() EDAC DEBUG: Registered CSROW0
EDAC MC0: Giving out device to e7xxx_edac E7500: PCI 0000:00:00.0

Anyone who is interested, please give the patch a try and/or provide feedback.

Thanks,
Dave

Signed-off-by Dave Peterson <[email protected]> <[email protected]>
drivers/edac/amd76x_edac.c +14 -6
diff -urNp -X dontdiff linux-2.6.16-rc1-git4/drivers/edac/amd76x_edac.c linux-2.6.16-rc1-git4-printk/drivers/edac/amd76x_edac.c
@@ -25,6 +25,14 @@
 #include "edac_mc.h"
 
 
+#define amd76x_printk(level, fmt, arg...) \
+    edac_printk(level, "amd76x", fmt, ##arg)
+
+
+#define amd76x_mc_printk(mci, level, fmt, arg...) \
+    edac_mc_chipset_printk(mci, level, "amd76x", fmt, ##arg)
+
+
 #define AMD76X_NR_CSROWS 8
 #define AMD76X_NR_CHANS  1
 #define AMD76X_NR_DIMMS  4

@@ -174,7 +182,7 @@ static int amd76x_process_error_info (st
 static void amd76x_check(struct mem_ctl_info *mci)
 {
 	struct amd76x_error_info info;
-	debugf3("MC: " __FILE__ ": %s()\n", __func__);
+	debugf3(__FILE__ ": %s()\n", __func__);
 	amd76x_get_error_info(mci, &info);
 	amd76x_process_error_info(mci, &info, 1);
 }

@@ -204,7 +212,7 @@ static int amd76x_probe1(struct pci_dev 
 	u32 ems;
 	u32 ems_mode;
 
-	debugf0("MC: " __FILE__ ": %s()\n", __func__);
+	debugf0(__FILE__ ": %s()\n", __func__);
 
 	pci_read_config_dword(pdev, AMD76X_ECC_MODE_STATUS, &ems);
 	ems_mode = (ems >> 10) & 0x3;

@@ -216,7 +224,7 @@ static int amd76x_probe1(struct pci_dev 
 		goto fail;
 	}
 
-	debugf0("MC: " __FILE__ ": %s(): mci = %p\n", __func__, mci);
+	debugf0(__FILE__ ": %s(): mci = %p\n", __func__, mci);
 
 	mci->pdev = pci_dev_get(pdev);
 	mci->mtype_cap = MEM_FLAG_RDDR;

@@ -267,13 +275,13 @@ static int amd76x_probe1(struct pci_dev 
 			 (u32) (0x3 << 8));
 
 	if (edac_mc_add_mc(mci)) {
-		debugf3("MC: " __FILE__
+		debugf3(__FILE__
 			": %s(): failed edac_mc_add_mc()\n", __func__);
 		goto fail;
 	}
 
 	/* get this far and it's successful */
-	debugf3("MC: " __FILE__ ": %s(): success\n", __func__);
+	debugf3(__FILE__ ": %s(): success\n", __func__);
 	return 0;
 
 fail:

@@ -289,7 +297,7 @@ fail:
 static int __devinit amd76x_init_one(struct pci_dev *pdev,
 				     const struct pci_device_id *ent)
 {
-	debugf0("MC: " __FILE__ ": %s()\n", __func__);
+	debugf0(__FILE__ ": %s()\n", __func__);
 
 	/* don't need to call pci_device_enable() */
 	return amd76x_probe1(pdev, ent->driver_data);
drivers/edac/e752x_edac.c +41 -35

243 lines not on this page · raw message

drivers/edac/e7xxx_edac.c +30 -22

170 lines not on this page · raw message

drivers/edac/edac_mc.c +67 -71

373 lines not on this page · raw message

drivers/edac/edac_mc.h +21 -2
diff -urNp -X dontdiff linux-2.6.16-rc1-git4/drivers/edac/edac_mc.h linux-2.6.16-rc1-git4-printk/drivers/edac/edac_mc.h
@@ -43,10 +43,29 @@
 #define PAGES_TO_MiB( pages )	( ( pages ) << ( PAGE_SHIFT - 20 ) )
 #endif
 
+#define edac_printk(level, prefix, fmt, arg...) \
+    printk(level "EDAC " prefix ": " fmt, ##arg)
+
+#define edac_mc_printk(mci, level, fmt, arg...) \
+    printk(level "EDAC MC%d: " fmt, mci->mc_idx, ##arg)
+
+#define edac_mc_chipset_printk(mci, level, prefix, fmt, arg...) \
+    printk(level "EDAC " prefix " MC%d: " fmt, mci->mc_idx, ##arg)
+
+/* prefixes for edac_printk() and edac_mc_printk() */
+#define EDAC_MC "MC"
+#define EDAC_PCI "PCI"
+#define EDAC_DEBUG "DEBUG"
+
 #ifdef CONFIG_EDAC_DEBUG
 extern int edac_debug_level;
-#define edac_debug_printk(level, fmt, args...) \
-do { if (level <= edac_debug_level) printk(KERN_DEBUG fmt, ##args); } while(0)
+
+#define edac_debug_printk(level, fmt, arg...)                            \
+	do {                                                             \
+		if (level <= edac_debug_level)                           \
+			edac_printk(KERN_DEBUG, EDAC_DEBUG, fmt, ##arg); \
+	} while(0)
+
 #define debugf0( ... ) edac_debug_printk(0, __VA_ARGS__ )
 #define debugf1( ... ) edac_debug_printk(1, __VA_ARGS__ )
 #define debugf2( ... ) edac_debug_printk(2, __VA_ARGS__ )
drivers/edac/i82860_edac.c +16 -9
diff -urNp -X dontdiff linux-2.6.16-rc1-git4/drivers/edac/i82860_edac.c linux-2.6.16-rc1-git4-printk/drivers/edac/i82860_edac.c
@@ -19,6 +19,14 @@
 #include "edac_mc.h"
 
 
+#define i82860_printk(level, fmt, arg...) \
+    edac_printk(level, "i82860", fmt, ##arg)
+
+
+#define i82860_mc_printk(mci, level, fmt, arg...) \
+    edac_mc_chipset_printk(mci, level, "i82860", fmt, ##arg)
+
+
 #ifndef PCI_DEVICE_ID_INTEL_82860_0
 #define PCI_DEVICE_ID_INTEL_82860_0	0x2531
 #endif				/* PCI_DEVICE_ID_INTEL_82860_0 */

@@ -143,7 +151,7 @@ static int i82860_probe1(struct pci_dev 
 	if (!mci)
 		return -ENOMEM;
 
-	debugf3("MC: " __FILE__ ": %s(): init mci\n", __func__);
+	debugf3(__FILE__ ": %s(): init mci\n", __func__);
 
 	mci->pdev = pdev;
 	mci->mtype_cap = MEM_FLAG_DDR;

@@ -179,7 +187,7 @@ static int i82860_probe1(struct pci_dev 
 
 		cumul_size = (value & I82860_GBA_MASK) <<
 		    (I82860_GBA_SHIFT - PAGE_SHIFT);
-		debugf3("MC: " __FILE__ ": %s(): (%d) cumul_size 0x%x\n",
+		debugf3(__FILE__ ": %s(): (%d) cumul_size 0x%x\n",
 			__func__, index, cumul_size);
 		if (cumul_size == last_cumul_size)
 			continue;	/* not populated */

@@ -198,13 +206,12 @@ static int i82860_probe1(struct pci_dev 
 	pci_write_bits16(mci->pdev, I82860_ERRSTS, 0x0003, 0x0003);
 
 	if (edac_mc_add_mc(mci)) {
-		debugf3("MC: " __FILE__
-			": %s(): failed edac_mc_add_mc()\n",
+		debugf3(__FILE__ ": %s(): failed edac_mc_add_mc()\n",
 			__func__);
 		edac_mc_free(mci);
 	} else {
 		/* get this far and it's successful */
-		debugf3("MC: " __FILE__ ": %s(): success\n", __func__);
+		debugf3(__FILE__ ": %s(): success\n", __func__);
 		rc = 0;
 	}
 	return rc;

@@ -216,9 +223,9 @@ static int __devinit i82860_init_one(str
 {
 	int rc;
 
-	debugf0("MC: " __FILE__ ": %s()\n", __func__);
+	debugf0(__FILE__ ": %s()\n", __func__);
 
-	printk(KERN_INFO "i82860 init one\n");
+	i82860_printk(KERN_INFO, "i82860 init one\n");
 	if(pci_enable_device(pdev) < 0)
 		return -EIO;
 	rc = i82860_probe1(pdev, ent->driver_data);

@@ -257,7 +264,7 @@ static int __init i82860_init(void)
 {
 	int pci_rc;
 
-	debugf3("MC: " __FILE__ ": %s()\n", __func__);
+	debugf3(__FILE__ ": %s()\n", __func__);
 	if ((pci_rc = pci_register_driver(&i82860_driver)) < 0)
 		return pci_rc;
 

@@ -281,7 +288,7 @@ static int __init i82860_init(void)
 
 static void __exit i82860_exit(void)
 {
-	debugf3("MC: " __FILE__ ": %s()\n", __func__);
+	debugf3(__FILE__ ": %s()\n", __func__);
 
 	pci_unregister_driver(&i82860_driver);
 	if (!i82860_registered) {
drivers/edac/i82875p_edac.c +22 -14
diff -urNp -X dontdiff linux-2.6.16-rc1-git4/drivers/edac/i82875p_edac.c linux-2.6.16-rc1-git4-printk/drivers/edac/i82875p_edac.c
@@ -26,6 +26,14 @@
 #include "edac_mc.h"
 
 
+#define i82875p_printk(level, fmt, arg...) \
+    edac_printk(level, "i82875p", fmt, ##arg)
+
+
+#define i82875p_mc_printk(mci, level, fmt, arg...) \
+    edac_mc_chipset_printk(mci, level, "i82875p", fmt, ##arg)
+
+
 #ifndef PCI_DEVICE_ID_INTEL_82875_0
 #define PCI_DEVICE_ID_INTEL_82875_0	0x2578
 #endif				/* PCI_DEVICE_ID_INTEL_82875_0 */

@@ -279,7 +287,7 @@ static int i82875p_probe1(struct pci_dev
 	u32 nr_chans;
 	u32 drc_ddim;		/* DRAM Data Integrity Mode 0=none,2=edac */
 
-	debugf0("MC: " __FILE__ ": %s()\n", __func__);
+	debugf0(__FILE__ ": %s()\n", __func__);
 
 	ovrfl_pdev = pci_find_device(PCI_VEND_DEV(INTEL, 82875_6), NULL);
 

@@ -298,14 +306,14 @@ static int i82875p_probe1(struct pci_dev
 	}
 #ifdef CONFIG_PROC_FS
 	if (!ovrfl_pdev->procent && pci_proc_attach_device(ovrfl_pdev)) {
-		printk(KERN_ERR "MC: " __FILE__
+		i82875p_printk(KERN_ERR, __FILE__
 		       ": %s(): Failed to attach overflow device\n",
 		       __func__);
 		goto fail;
 	}
 #endif				/* CONFIG_PROC_FS */
 	if (pci_enable_device(ovrfl_pdev)) {
-		printk(KERN_ERR "MC: " __FILE__
+		i82875p_printk(KERN_ERR, __FILE__
 		       ": %s(): Failed to enable overflow device\n",
 		       __func__);
 		goto fail;

@@ -321,7 +329,7 @@ static int i82875p_probe1(struct pci_dev
 				       pci_resource_len(ovrfl_pdev, 0));
 
 	if (!ovrfl_window) {
-		printk(KERN_ERR "MC: " __FILE__
+		i82875p_printk(KERN_ERR, __FILE__
 		       ": %s(): Failed to ioremap bar6\n", __func__);
 		goto fail;
 	}

@@ -340,7 +348,7 @@ static int i82875p_probe1(struct pci_dev
 		goto fail;
 	}
 
-	debugf3("MC: " __FILE__ ": %s(): init mci\n", __func__);
+	debugf3(__FILE__ ": %s(): init mci\n", __func__);
 
 	mci->pdev = pdev;
 	mci->mtype_cap = MEM_FLAG_DDR;

@@ -355,7 +363,7 @@ static int i82875p_probe1(struct pci_dev
 	mci->edac_check = i82875p_check;
 	mci->ctl_page_to_phys = NULL;
 
-	debugf3("MC: " __FILE__ ": %s(): init pvt\n", __func__);
+	debugf3(__FILE__ ": %s(): init pvt\n", __func__);
 
 	pvt = (struct i82875p_pvt *) mci->pvt_info;
 	pvt->ovrfl_pdev = ovrfl_pdev;

@@ -374,7 +382,7 @@ static int i82875p_probe1(struct pci_dev
 
 		value = readb(ovrfl_window + I82875P_DRB + index);
 		cumul_size = value << (I82875P_DRB_SHIFT - PAGE_SHIFT);
-		debugf3("MC: " __FILE__ ": %s(): (%d) cumul_size 0x%x\n",
+		debugf3(__FILE__ ": %s(): (%d) cumul_size 0x%x\n",
 			__func__, index, cumul_size);
 		if (cumul_size == last_cumul_size)
 			continue;	/* not populated */

@@ -393,13 +401,13 @@ static int i82875p_probe1(struct pci_dev
 	pci_write_bits16(mci->pdev, I82875P_ERRSTS, 0x0081, 0x0081);
 
 	if (edac_mc_add_mc(mci)) {
-		debugf3("MC: " __FILE__
-			": %s(): failed edac_mc_add_mc()\n", __func__);
+		debugf3(__FILE__ ": %s(): failed edac_mc_add_mc()\n",
+			__func__);
 		goto fail;
 	}
 
 	/* get this far and it's successful */
-	debugf3("MC: " __FILE__ ": %s(): success\n", __func__);
+	debugf3(__FILE__ ": %s(): success\n", __func__);
 	return 0;
 
       fail:

@@ -425,9 +433,9 @@ static int __devinit i82875p_init_one(st
 {
 	int rc;
 
-	debugf0("MC: " __FILE__ ": %s()\n", __func__);
+	debugf0(__FILE__ ": %s()\n", __func__);
 
-	printk(KERN_INFO "i82875p init one\n");
+	i82875p_printk(KERN_INFO, "i82875p init one\n");
 	if(pci_enable_device(pdev) < 0)
 		return -EIO;
 	rc = i82875p_probe1(pdev, ent->driver_data);

@@ -487,7 +495,7 @@ static int __init i82875p_init(void)
 {
 	int pci_rc;
 
-	debugf3("MC: " __FILE__ ": %s()\n", __func__);
+	debugf3(__FILE__ ": %s()\n", __func__);
 	pci_rc = pci_register_driver(&i82875p_driver);
 	if (pci_rc < 0)
 		return pci_rc;

@@ -513,7 +521,7 @@ static int __init i82875p_init(void)
 
 static void __exit i82875p_exit(void)
 {
-	debugf3("MC: " __FILE__ ": %s()\n", __func__);
+	debugf3(__FILE__ ": %s()\n", __func__);
 
 	pci_unregister_driver(&i82875p_driver);
 	if (!i82875p_registered) {
drivers/edac/r82600_edac.c +16 -10
diff -urNp -X dontdiff linux-2.6.16-rc1-git4/drivers/edac/r82600_edac.c linux-2.6.16-rc1-git4-printk/drivers/edac/r82600_edac.c
@@ -26,6 +26,12 @@
 
 #include "edac_mc.h"
 
+#define r82600_printk(level, fmt, arg...) \
+    edac_printk(level, "r82600", fmt, ##arg)
+
+#define r82600_mc_printk(mci, level, fmt, arg...) \
+    edac_mc_chipset_printk(mci, level, "r82600", fmt, ##arg)
+
 /* Radisys say "The 82600 integrates a main memory SDRAM controller that
  * supports up to four banks of memory. The four banks can support a mix of
  * sizes of 64 bit wide (72 bits with ECC) Synchronous DRAM (SDRAM) DIMMs,

@@ -215,7 +221,7 @@ static int r82600_probe1(struct pci_dev 
 	u32 row_high_limit_last = 0;
 	u32 eap_init_bits;
 
-	debugf0("MC: " __FILE__ ": %s()\n", __func__);
+	debugf0(__FILE__ ": %s()\n", __func__);
 
 
 	pci_read_config_byte(pdev, R82600_DRAMC, &dramcr);

@@ -226,10 +232,10 @@ static int r82600_probe1(struct pci_dev 
 	scrub_disabled = eapr & BIT(31);
 	sdram_refresh_rate = dramcr & (BIT(0) | BIT(1));
 
-	debugf2("MC: " __FILE__ ": %s(): sdram refresh rate = %#0x\n",
+	debugf2(__FILE__ ": %s(): sdram refresh rate = %#0x\n",
 		__func__, sdram_refresh_rate);
 
-	debugf2("MC: " __FILE__ ": %s(): DRAMC register = %#0x\n", __func__,
+	debugf2(__FILE__ ": %s(): DRAMC register = %#0x\n", __func__,
 		dramcr);
 
 	mci = edac_mc_alloc(0, R82600_NR_CSROWS, R82600_NR_CHANS);

@@ -239,7 +245,7 @@ static int r82600_probe1(struct pci_dev 
 		goto fail;
 	}
 
-	debugf0("MC: " __FILE__ ": %s(): mci = %p\n", __func__, mci);
+	debugf0(__FILE__ ": %s(): mci = %p\n", __func__, mci);
 
 	mci->pdev = pdev;
 	mci->mtype_cap = MEM_FLAG_RDDR | MEM_FLAG_DDR;

@@ -255,7 +261,7 @@ static int r82600_probe1(struct pci_dev 
 	mci->edac_cap = EDAC_FLAG_NONE | EDAC_FLAG_EC | EDAC_FLAG_SECDED;
 	if (ecc_on) {
 		if (scrub_disabled)
-			debugf3("MC: " __FILE__ ": %s(): mci = %p - "
+			debugf3(__FILE__ ": %s(): mci = %p - "
 				"Scrubbing disabled! EAP: %#0x\n", __func__,
 				mci, eapr);
 	} else

@@ -312,8 +318,8 @@ static int r82600_probe1(struct pci_dev 
 	/* FIXME should we? */
 
 	if (edac_mc_add_mc(mci)) {
-		debugf3("MC: " __FILE__
-			": %s(): failed edac_mc_add_mc()\n", __func__);
+		debugf3(__FILE__ ": %s(): failed edac_mc_add_mc()\n",
+			__func__);
 		goto fail;
 	}
 

@@ -325,14 +331,14 @@ static int r82600_probe1(struct pci_dev 
 	eap_init_bits = BIT(0) & BIT(1);
 	if (disable_hardware_scrub) {
 		eap_init_bits |= BIT(31);
-		debugf3("MC: " __FILE__ ": %s(): Disabling Hardware Scrub "
+		debugf3(__FILE__ ": %s(): Disabling Hardware Scrub "
 			"(scrub on error)\n", __func__);
 	}
 
 	pci_write_bits32(mci->pdev, R82600_EAP, eap_init_bits,
 			 eap_init_bits);
 
-	debugf3("MC: " __FILE__ ": %s(): success\n", __func__);
+	debugf3(__FILE__ ": %s(): success\n", __func__);
 	return 0;
 
 fail:

@@ -346,7 +352,7 @@ fail:
 static int __devinit r82600_init_one(struct pci_dev *pdev,
 				     const struct pci_device_id *ent)
 {
-	debugf0("MC: " __FILE__ ": %s()\n", __func__);
+	debugf0(__FILE__ ": %s()\n", __func__);
 
 	/* don't need to call pci_device_enable() */
 	return r82600_probe1(pdev, ent->driver_data);
#17 lore raw

Re: [PATCH] EDAC printk() cleanup

Quoted 18 lines · Dave Peterson
Quoted 11 lines · Eric W. Biederman

One piece missing from this conversation is the issue that we need errors in a uniform format. That is why edac_mc has helper functions.

However there will always be errors that don't fit any particular model. Could we add a edac_printk(dev, ); That is similar to dev_printk but prints out an EDAC header and the device on which the error was found? Letting the rest of the string be user specified.

For actual control that interface may be to blunt, but at least for people looking in the logs it allows all of the errors to be detected and harvested.

Ok, the patch below (which applies to the 2.6.16-rc1-git4 kernel) is an initial attempt at implementing this sort of thing. Here is some sample output (produced by loading the edac_mc and e7xxx_edac moules):

Quoted 14 lines

EDAC MC: /tftpboot/dsp/printk/linux/drivers/edac/edac_mc.c version edac_mc Ver: 2.0.0 Feb 1 2006 EDAC DEBUG: /tftpboot/dsp/printk/linux/drivers/edac/edac_mc.c: edac_sysfs_memctrl_setup() EDAC DEBUG: Registered '.../edac/mc' kobject
EDAC DEBUG: /tftpboot/dsp/printk/linux/drivers/edac/edac_mc.c: edac_sysfs_pci_setup() EDAC DEBUG: Registered '.../edac/pci' kobject
EDAC DEBUG: /tftpboot/dsp/printk/linux/drivers/edac/e7xxx_edac.c: e7xxx_init_one() EDAC DEBUG: /tftpboot/dsp/printk/linux/drivers/edac/e7xxx_edac.c: e7xxx_probe1(): mci EDAC e7xxx: tolm = 40000, remapbase = ffc000, remaplimit = 0
EDAC DEBUG: /tftpboot/dsp/printk/linux/drivers/edac/edac_mc.c: edac_mc_add_mc() EDAC DEBUG: /tftpboot/dsp/printk/linux/drivers/edac/edac_mc.c: edac_create_sysfs_mci_device() idx=0 EDAC DEBUG: /tftpboot/dsp/printk/linux/drivers/edac/edac_mc.c: edac_create_csrow_object() EDAC DEBUG: Registered CSROW0
EDAC MC0: Giving out device to e7xxx_edac E7500: PCI 0000:00:00.0

Yes, yes, yes

that is exactly what I had in mind. The old stuff bothered me something bad, but I couldn't put my finger on exactly how to fix it yet - didn't have the time.

It gives subsystem info (EDAC), modules therein, a bit of context then event descriptions.

thanks dave and eric and good work

doug t

#18 lore raw
In reply to Doug Thompson
Quoted 1 line · Doug Thompson

that driver should be refactored to only output NON-FATALs with debug turned on.

We need to root cause this before that decision is made.

Quoted 12 lines
Quoted 10 lines · --- Dave Jones
Quoted 9 lines · Alan Cox
Quoted 7 lines · Dave Jones

e752x_edac is very noisy on my PCIE system.. my dmesg is filled with these...

[91671.488379] Non-Fatal Error PCI Express B
[91671.492468] Non-Fatal Error PCI Express B
[91901.100576] Non-Fatal Error PCI Express B
[91901.104675] Non-Fatal Error PCI Express B
Quoted 4 lines
Quoted 4 lines
Quoted 1 line

Pre-production system or final release ?

Currently shipping Dell Precision 470.

Actually I remember something very weird with this error. I think the Non-Fatal was some weird designation by Intel, that came right out of the data sheet but didn't have a useful interpretation.

I suspect that you have an actual hardware problem there. Quite possibly that a riser or card is not plugged in all of the way.

As I recall most of the checks clear the error bit when they detect the error so the fact that it is reoccurring suggests that the error is reoccurring this frequently.

So before we go off and give up and interpreting this error can we please root cause at least what the driver is doing?

It has been about a year since I saw something like this and someone else was tracking the error but I seem to remember an error message like this resulting from an actual fixable problem.

Eric