Skip to contentSkip to conversation

Background memory scrubbing

PATCH Applied Robert Whitton 5 participants 11 messages Apr 20, 2011 – Apr 21, 2011

Subsystems (2)
EDAC-core EDAC-AMD64
lore
Share & tools
lore

Robert Whitton

author
#0 lore raw
Quoted 11 lines
Quoted 3 lines · Robert Whitton

I have a home grown module that performs background memory scrubbing to eliminate single bit memory errors before they become a problem. ... it is specifically targeted at the AMD64 PC architecture

Then why don't you use the memory controller's automatic background memory scrubbing support? Doesn't your BIOS have this option?

Regards,
Clemens

Hi,

Unfortunately in common with a large number of hardware platforms background scrubbing isn't supported in the hardware (even though ECC error correction is supported) and thus there is no BIOS option to enable it. The software solution has always been fine and the CPU load negligible as it's only necessary to complete one complete scrub every day or so. I just need to find a solution to making this work on newer Linux kernels.

Rob

#1 lore raw
Quoted 5 lines · Robert Whitton
Quoted 5 lines
Quoted 3 lines · Robert Whitton

I have a home grown module that performs background memory scrubbing to eliminate single bit memory errors before they become a problem. ... it is specifically targeted at the AMD64 PC architecture

Quoted 7 lines
Quoted 2 lines

Then why don't you use the memory controller's automatic background memory scrubbing support? Doesn't your BIOS have this option?

Unfortunately in common with a large number of hardware platforms background scrubbing isn't supported in the hardware (even though ECC error correction is supported) and thus there is no BIOS option to enable it.

Which hardware platform is this? AFAICT all architectures with ECC (old AMD64, Family 0Fh, Family 10h) also have scrubbing support. If your BIOS is too dumb, just try enabling it directly (bits 0-4 of PCI configuration register 0x58 in function 3 of the CPU's northbridge device, see the BIOS and Kernel's Developer's Guide for details).

Regards,
Clemens

#2 lore raw
Quoted 10 lines · Clemens Ladisch
Quoted 4 lines

Unfortunately in common with a large number of hardware platforms background scrubbing isn't supported in the hardware (even though ECC error correction is supported) and thus there is no BIOS option to enable it.

Which hardware platform is this? AFAICT all architectures with ECC (old AMD64, Family 0Fh, Family 10h) also have scrubbing support. If your BIOS is too dumb, just try enabling it directly (bits 0-4 of PCI configuration register 0x58 in function 3 of the CPU's northbridge device, see the BIOS and Kernel's Developer's Guide for details).

Or even better, if on AMD, you can build the amd64_edac module (CONFIG_EDAC_AMD64) and do

echo <x> > /sys/devices/system/edac/mc/mc<y>/sdram_scrub_rate

where x is the scrubbing bandwidth in bytes/sec and y is the memory controller on the machine, i.e. node.

Signature · 8 lines
Regards/Gruss,
Boris.

Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
General Managers: Alberto Bozzo, Andrew Bowd
Registration: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632
#3 lore raw
Quoted 12 lines · Borislav Petkov
Quoted 10 lines · Clemens Ladisch
Quoted 4 lines

Unfortunately in common with a large number of hardware platforms background scrubbing isn't supported in the hardware (even though ECC error correction is supported) and thus there is no BIOS option to enable it.

Which hardware platform is this? AFAICT all architectures with ECC (old AMD64, Family 0Fh, Family 10h) also have scrubbing support. If your BIOS is too dumb, just try enabling it directly (bits 0-4 of PCI configuration register 0x58 in function 3 of the CPU's northbridge device, see the BIOS and Kernel's Developer's Guide for details).

Quoted 7 lines

Or even better, if on AMD, you can build the amd64_edac module (CONFIG_EDAC_AMD64) and do

echo <x> > /sys/devices/system/edac/mc/mc<y>/sdram_scrub_rate

where x is the scrubbing bandwidth in bytes/sec and y is the memory controller on the machine, i.e. node.

BTW is it really necessary to print the following to syslog:

EDAC amd64: pci-read, sdram scrub control value: 15 EDAC MC: Read scrub rate: 97650

everytime one runs:

 # cat /sys/devices/system/edac/mc/mc0/sdram_scrub_rate
97650

?

Signature · 1 line
Markus
#4 lore raw
Quoted 10 lines · Markus Trippelsdorf

BTW is it really necessary to print the following to syslog:

EDAC amd64: pci-read, sdram scrub control value: 15 EDAC MC: Read scrub rate: 97650

everytime one runs:

 # cat /sys/devices/system/edac/mc/mc0/sdram_scrub_rate
97650

?

This is KERN_DEBUG since .38 (commit 24f9a7fe3f19f3fd310f556364d01a22911724b3) and it shouldn't appear on the console if you don't change your default log level.

Signature · 8 lines
Regards/Gruss,
Boris.

Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
General Managers: Alberto Bozzo, Andrew Bowd
Registration: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632
#5 lore raw
Quoted 6 lines · Borislav Petkov
Quoted 5 lines · Markus Trippelsdorf

BTW is it really necessary to print the following to syslog:

EDAC amd64: pci-read, sdram scrub control value: 15 EDAC MC: Read scrub rate: 97650

Quoted 8 lines
Quoted 5 lines

everytime one runs:

 # cat /sys/devices/system/edac/mc/mc0/sdram_scrub_rate
97650

?

This is KERN_DEBUG since .38 (commit 24f9a7fe3f19f3fd310f556364d01a22911724b3) and it shouldn't appear on the console if you don't change your default log level.

Yes. Sorry, but I was referring to dmesg and not the console. What I mean is that maybe debugf1 or debugf2 is more appropriate than amd64_debug?

Signature · 1 line
Markus
#6 lore raw
Quoted 11 lines · Markus Trippelsdorf
Quoted 10 lines · Borislav Petkov
Quoted 9 lines · Markus Trippelsdorf

BTW is it really necessary to print the following to syslog:

EDAC amd64: pci-read, sdram scrub control value: 15 EDAC MC: Read scrub rate: 97650

everytime one runs:

 # cat /sys/devices/system/edac/mc/mc0/sdram_scrub_rate
97650
Quoted 8 lines
Quoted 4 lines
Quoted 1 line

?

This is KERN_DEBUG since .38 (commit 24f9a7fe3f19f3fd310f556364d01a22911724b3) and it shouldn't appear on the console if you don't change your default log level.

Yes. Sorry, but I was referring to dmesg and not the console. What I mean is that maybe debugf1 or debugf2 is more appropriate than amd64_debug?

In other words:

drivers/edac/amd64_edac.c +1 -1
index 31e71c4f..13b107e 100644
@@ -211,7 +211,7 @@ static int amd64_get_scrub_rate(struct mem_ctl_info *mci)
 
 	scrubval = scrubval & 0x001F;
 
-	amd64_debug("pci-read, sdram scrub control value: %d\n", scrubval);
+	debugf1 ("pci-read, sdram scrub control value: %d\n", scrubval);
 
 	for (i = 0; i < ARRAY_SIZE(scrubrates); i++) {
 		if (scrubrates[i].scrubval == scrubval) {
drivers/edac/edac_mc_sysfs.c +2 -2
index 26343fd..8a34aea 100644
@@ -459,7 +459,7 @@ static ssize_t mci_sdram_scrub_rate_store(struct mem_ctl_info *mci,
 
 	new_bw = mci->set_sdram_scrub_rate(mci, bandwidth);
 	if (new_bw >= 0) {
-		edac_printk(KERN_DEBUG, EDAC_MC, "Scrub rate set to %d\n", new_bw);
+		debugf1 ("Scrub rate set to %d\n", new_bw);
 		return count;
 	}
 

@@ -483,7 +483,7 @@ static ssize_t mci_sdram_scrub_rate_show(struct mem_ctl_info *mci, char *data)
 		return bandwidth;
 	}
 
-	edac_printk(KERN_DEBUG, EDAC_MC, "Read scrub rate: %d\n", bandwidth);
+	debugf1 ("Read scrub rate: %d\n", bandwidth);
 	return sprintf(data, "%d\n", bandwidth);
 }
Signature · 1 line
Markus
#7 lore raw
Quoted 8 lines · Markus Trippelsdorf
Quoted 6 lines
Quoted 2 lines

This is KERN_DEBUG since .38 (commit 24f9a7fe3f19f3fd310f556364d01a22911724b3) and it shouldn't appear on the console if you don't change your default log level.

Yes. Sorry, but I was referring to dmesg and not the console. What I mean is that maybe debugf1 or debugf2 is more appropriate than amd64_debug?

In other words:

Ok, that whole debugging output there is partly historical remains and we don't really need them if we're smart about it. But you'll have to change your patch and make a proper commit message :).

drivers/edac/amd64_edac.c static int amd64_get_scrub_rate(struct mem_ctl_info *mci) · line 214

Quoted 11 lines
drivers/edac/amd64_edac.c +1 -1
index 31e71c4f..13b107e 100644
@@ -211,7 +211,7 @@ static int amd64_get_scrub_rate(struct mem_ctl_info *mci)
 
 	scrubval = scrubval & 0x001F;
 
-	amd64_debug("pci-read, sdram scrub control value: %d\n", scrubval);
+	debugf1 ("pci-read, sdram scrub control value: %d\n", scrubval);

remove this one completely.

Quoted 8 lines
drivers/edac/amd64_edac.c +0 -0
 	for (i = 0; i < ARRAY_SIZE(scrubrates); i++) {
 		if (scrubrates[i].scrubval == scrubval) {
drivers/edac/edac_mc_sysfs.c +0 -0
index 26343fd..8a34aea 100644

drivers/edac/edac_mc_sysfs.c static ssize_t mci_sdram_scrub_rate_store(struct mem_ctl_info *mci, · line 464

Quoted 7 lines
drivers/edac/edac_mc_sysfs.c +1 -1
@@ -459,7 +459,7 @@ static ssize_t mci_sdram_scrub_rate_store(struct mem_ctl_info *mci,
 
 	new_bw = mci->set_sdram_scrub_rate(mci, bandwidth);
 	if (new_bw >= 0) {
-		edac_printk(KERN_DEBUG, EDAC_MC, "Scrub rate set to %d\n", new_bw);
+		debugf1 ("Scrub rate set to %d\n", new_bw);
 		return count;
 	}

Make here the success case implicit and issue a warning only if we fail setting the scrub rate:

	new_bw = mci->set_sdram_scrub_rate(mci, bandwidth);
	if (new_bw < 0) {
		edac_printk(KERN_WARNING, EDAC_MC,
			    "Error setting scrub rate to: %lu\n", bandwidth);
		return -EINVAL;
	}

and do the same thing with mci_sdram_scrub_rate_show() below.

drivers/edac/edac_mc_sysfs.c static ssize_t mci_sdram_scrub_rate_show(struct mem_ctl_info *mci, char *data) · line 486

Quoted 7 lines
drivers/edac/edac_mc_sysfs.c +1 -1
@@ -483,7 +483,7 @@ static ssize_t mci_sdram_scrub_rate_show(struct mem_ctl_info *mci, char *data)
 		return bandwidth;
 	}
 
-	edac_printk(KERN_DEBUG, EDAC_MC, "Read scrub rate: %d\n", bandwidth);
+	debugf1 ("Read scrub rate: %d\n", bandwidth);

This one can go too since the success-case is in sysfs anyway.

drivers/edac/edac_mc_sysfs.c static ssize_t mci_sdram_scrub_rate_show(struct mem_ctl_info *mci, char *data) · line 488

Quoted 2 lines
drivers/edac/edac_mc_sysfs.c +0 -0
 	return sprintf(data, "%d\n", bandwidth);
 }

Would you like to do that?

Thanks.

Signature · 8 lines
Regards/Gruss,
Boris.

Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
General Managers: Alberto Bozzo, Andrew Bowd
Registration: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632
#8 lore raw

PATCH 1/1 edac: Remove debugging output in scrub rate handling

Quoted 8 lines · Borislav Petkov
Quoted 7 lines · Markus Trippelsdorf
Quoted 6 lines
Quoted 2 lines

This is KERN_DEBUG since .38 (commit 24f9a7fe3f19f3fd310f556364d01a22911724b3) and it shouldn't appear on the console if you don't change your default log level.

Yes. Sorry, but I was referring to dmesg and not the console. What I mean is that maybe debugf1 or debugf2 is more appropriate than amd64_debug?

Quoted 5 lines
Quoted 1 line

In other words:

Ok, that whole debugging output there is partly historical remains and we don't really need them if we're smart about it. But you'll have to change your patch and make a proper commit message :).

This patch removes superfluous debugging output in the sysfs scrub rate handler. It also consolidates the error handling in mci_sdram_scrub_rate_store.

Signed-off-by Markus Trippelsdorf <[email protected]>
---
 drivers/edac/amd64_edac.c    |    2 --
 drivers/edac/edac_mc_sysfs.c |   11 +++++------
 2 files changed, 5 insertions(+), 8 deletions(-)
drivers/edac/amd64_edac.c +0 -2
index 31e71c4f..4b4071e 100644
@@ -211,8 +211,6 @@ static int amd64_get_scrub_rate(struct mem_ctl_info *mci)
 
 	scrubval = scrubval & 0x001F;
 
-	amd64_debug("pci-read, sdram scrub control value: %d\n", scrubval);
-
 	for (i = 0; i < ARRAY_SIZE(scrubrates); i++) {
 		if (scrubrates[i].scrubval == scrubval) {
 			retval = scrubrates[i].bandwidth;
drivers/edac/edac_mc_sysfs.c +5 -6
index 26343fd..6ffe438 100644
@@ -458,13 +458,13 @@ static ssize_t mci_sdram_scrub_rate_store(struct mem_ctl_info *mci,
 		return -EINVAL;
 
 	new_bw = mci->set_sdram_scrub_rate(mci, bandwidth);
-	if (new_bw >= 0) {
-		edac_printk(KERN_DEBUG, EDAC_MC, "Scrub rate set to %d\n", new_bw);
-		return count;
+	if (new_bw < 0) {
+		edac_printk(KERN_WARNING, EDAC_MC,
+			"Error setting scrub rate to: %lu\n", bandwidth);
+		return -EINVAL;
 	}
 
-	edac_printk(KERN_DEBUG, EDAC_MC, "Error setting scrub rate to: %lu\n", bandwidth);
-	return -EINVAL;
+	return count;
 }
 
 /*

@@ -483,7 +483,6 @@ static ssize_t mci_sdram_scrub_rate_show(struct mem_ctl_info *mci, char *data)
 		return bandwidth;
 	}
 
-	edac_printk(KERN_DEBUG, EDAC_MC, "Read scrub rate: %d\n", bandwidth);
 	return sprintf(data, "%d\n", bandwidth);
 }
Signature · 1 line
Markus
Applied
#9 lore raw

Re: [PATCH] edac: Remove debugging output in scrub rate handling

Quoted 6 lines · Markus Trippelsdorf

This patch removes superfluous debugging output in the sysfs scrub rate handler. It also consolidates the error handling in mci_sdram_scrub_rate_store.

Signed-off-by Markus Trippelsdorf <[email protected]>

Applied, thanks.

Signature · 8 lines
Regards/Gruss,
Boris.

Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
General Managers: Alberto Bozzo, Andrew Bowd
Registration: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632
#10 lore raw
In reply to Clemens Ladisch

Ladisch:

Quoted 5 lines · Clemens Ladisch

Which hardware platform is this?  AFAICT all architectures with ECC (old AMD64, Family 0Fh, Family 10h) also have scrubbing support. If your BIOS is too dumb, just try enabling it directly (bits 0-4 of PCI configuration register 0x58 in function 3 of the CPU's northbridge device, see the BIOS and Kernel's Developer's Guide for details).

That won't help non-AMD64 platforms that want to scrub.

Is there a way to make Robert's approach work? I'm aware of a few non-AMD64, somewhat-exotic platforms that require scrubbing the way that Robert is proposing...

b.g.

Signature · 2 lines
Bill Gatliff
[email protected]