2011-06-22 11:20:06

by Stefan Assmann

[permalink] [raw]
Subject: [PATCH v2 0/3] support for broken memory modules (BadRAM)

Following the RFC for the BadRAM feature here's the updated version with
spelling fixes, thanks go to Randy Dunlap. Also the code is now less verbose,
as requested by Andi Kleen.
v2 with even more spelling fixes suggested by Randy.
Patches are against vanilla 2.6.39.
Repost with LKML in Cc as suggested by Andrew Morton.

The idea is to allow the user to specify RAM addresses that shouldn't be
touched by the OS, because they are broken in some way. Not all machines have
hardware support for hwpoison, ECC RAM, etc, so here's a solution that allows to
use bitmasks to mask address patterns with the new "badram" kernel command line
parameter.
Memtest86 has an option to generate these patterns since v2.3 so the only thing
for the user to do should be:
- run Memtest86
- note down the pattern
- add badram=<pattern> to the kernel command line

The concerning pages are then marked with the hwpoison flag and thus won't be
used by the memory managment system.

Link to Ricks original patches and docs:
http://rick.vanrein.org/linux/badram/

Stefan

Stefan Assmann (3):
Add string parsing function get_next_ulong
support for broken memory modules (BadRAM)
Add documentation and credits for BadRAM

CREDITS | 9 +
Documentation/BadRAM.txt | 370 +++++++++++++++++++++++++++++++++++
Documentation/kernel-parameters.txt | 6 +
include/linux/kernel.h | 1 +
lib/cmdline.c | 35 ++++
mm/memory-failure.c | 100 ++++++++++
6 files changed, 521 insertions(+), 0 deletions(-)
create mode 100644 Documentation/BadRAM.txt

--
1.7.4


2011-06-22 11:19:53

by Stefan Assmann

[permalink] [raw]
Subject: [PATCH v2 1/3] Add string parsing function get_next_ulong

Adding this function to allow easy parsing of unsigned long values from the
beginning of strings. Convenience function to parse pointers from the kernel
command line.

Signed-off-by: Stefan Assmann <[email protected]>
Acked-by: Tony Luck <[email protected]>
Acked-by: Andi Kleen <[email protected]>
---
include/linux/kernel.h | 1 +
lib/cmdline.c | 35 +++++++++++++++++++++++++++++++++++
2 files changed, 36 insertions(+), 0 deletions(-)

diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 00cec4d..98c1916 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -280,6 +280,7 @@ extern int vsscanf(const char *, const char *, va_list)

extern int get_option(char **str, int *pint);
extern char *get_options(const char *str, int nints, int *ints);
+extern int get_next_ulong(char **str, unsigned long *val, char sep, int base);
extern unsigned long long memparse(const char *ptr, char **retptr);

extern int core_kernel_text(unsigned long addr);
diff --git a/lib/cmdline.c b/lib/cmdline.c
index f5f3ad8..82a6616 100644
--- a/lib/cmdline.c
+++ b/lib/cmdline.c
@@ -114,6 +114,41 @@ char *get_options(const char *str, int nints, int *ints)
}

/**
+ * get_next_ulong - Parse unsigned long at the beginning of a string
+ * @strp: (output) String to be parsed
+ * @val: (output) unsigned long carrying the result
+ * @sep: character specifying the separator
+ * @base: number system of the parsed value
+ *
+ * This function parses an unsigned long value at the beginning of a
+ * string. The string may begin with a separator or an unsigned long
+ * value.
+ * After the function is run val will contain the parsed value and strp
+ * will point to the character *after* the parsed unsigned long.
+ *
+ * In the error case 0 is returned, val and *strp stay unaltered.
+ * Otherwise return 1.
+ */
+int get_next_ulong(char **strp, unsigned long *val, char sep, int base)
+{
+ char *tmp;
+
+ if (!strp || !(*strp))
+ return 0;
+
+ tmp = *strp;
+ if (*tmp == sep)
+ tmp++;
+
+ *val = simple_strtoul(tmp, strp, base);
+
+ if (tmp == *strp)
+ return 0; /* no new value parsed */
+ else
+ return 1;
+}
+
+/**
* memparse - parse a string with mem suffixes into a number
* @ptr: Where parse begins
* @retptr: (output) Optional pointer to next char after parse completes
--
1.7.4

2011-06-22 11:19:57

by Stefan Assmann

[permalink] [raw]
Subject: [PATCH v2 2/3] support for broken memory modules (BadRAM)

BadRAM is a mechanism to exclude memory addresses (pages) from being used by
the system. The addresses are given to the kernel via kernel command line.
This is useful for systems with defective RAM modules, especially if the RAM
modules cannot be replaced.

command line parameter: badram=<addr>,<mask>[,...]

Patterns for the command line parameter can be obtained by running Memtest86.
In Memtest86 press "c" for configuration, select "Error Report Mode" and
finally "BadRAM Patterns"

This has already been done by Rick van Rein a long time ago but it never found
it's way into the kernel.

Signed-off-by: Stefan Assmann <[email protected]>
Acked-by: Tony Luck <[email protected]>
Acked-by: Andi Kleen <[email protected]>
---
mm/memory-failure.c | 100 +++++++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 100 insertions(+), 0 deletions(-)

diff --git a/mm/memory-failure.c b/mm/memory-failure.c
index 2b9a5ee..97c7f7c 100644
--- a/mm/memory-failure.c
+++ b/mm/memory-failure.c
@@ -52,6 +52,8 @@
#include <linux/swapops.h>
#include <linux/hugetlb.h>
#include <linux/memory_hotplug.h>
+#include <linux/memblock.h>
+#include <linux/bootmem.h>
#include "internal.h"

int sysctl_memory_failure_early_kill __read_mostly = 0;
@@ -1487,3 +1489,101 @@ done:
/* keep elevated page count for bad page */
return ret;
}
+
+/*
+ * Return 0 if no address found else return 1, new address is stored in addrp.
+ **/
+static int __init next_masked_address(unsigned long *addrp, unsigned long mask)
+{
+ unsigned long total_mem = (max_pfn + 1) << PAGE_SHIFT;
+ unsigned long tmp_addr = *addrp;
+ unsigned long inc = 1;
+
+ while (inc & mask)
+ inc = inc << 1;
+
+ while (inc != 0) {
+ tmp_addr += inc;
+ tmp_addr &= ~mask;
+ tmp_addr |= ((*addrp) & mask);
+
+ /* address is bigger than phys memory */
+ if (tmp_addr >= total_mem)
+ return 0;
+
+ /* address found */
+ if (tmp_addr > *addrp) {
+ *addrp = tmp_addr;
+ return 1;
+ }
+
+ while (inc & ~mask)
+ inc = inc << 1;
+ inc = inc << 1;
+ }
+
+ return 0;
+}
+
+/*
+ * Set hwpoison pageflag on all pages specified by addr/mask.
+ */
+static int __init badram_mark_pages(unsigned long addr, unsigned long mask)
+{
+ unsigned long pagecount = 0, is_reserved = 0;
+
+ mask |= ~PAGE_MASK; /* smallest chunk is a page */
+ addr &= mask;
+
+ printk(KERN_INFO "BadRAM: mark 0x%lx with mask 0x%0lx\n", addr, mask);
+
+ do {
+ unsigned long pfn = addr >> PAGE_SHIFT;
+ struct page *page = pfn_to_page(pfn);
+
+ if (!pfn_valid(pfn))
+ continue;
+ if (memblock_is_reserved(addr)) {
+ pr_debug("BadRAM: page %lu reserved by kernel\n", pfn);
+ is_reserved++;
+ continue;
+ }
+
+ SetPageHWPoison(page);
+ atomic_long_add(1, &mce_bad_pages);
+ pagecount++;
+ pr_debug("BadRAM: page %lu (addr 0x%0lx) marked bad "
+ "[total %lu]\n", pfn, addr, pagecount);
+ } while (next_masked_address(&addr, mask));
+
+ if (is_reserved)
+ printk(KERN_WARNING "BadRAM: %lu page(s) already reserved and "
+ "could not be marked bad\n", is_reserved);
+
+ return pagecount;
+}
+
+static int __init badram_setup(char *str)
+{
+ printk(KERN_DEBUG "BadRAM: cmdline option is %s\n", str);
+
+ if (*str++ != '=')
+ return 0;
+
+ while (*str) {
+ unsigned long addr = 0, mask = 0, pagecount = 0;
+
+ if (!get_next_ulong(&str, &addr, ',', 16)) {
+ printk(KERN_WARNING "BadRAM: parsing error\n");
+ return 0;
+ }
+ if (!get_next_ulong(&str, &mask, ',', 16))
+ mask = ~(0UL);
+
+ pagecount = badram_mark_pages(addr, mask);
+ printk(KERN_INFO "BadRAM: %lu page(s) bad\n", pagecount);
+ }
+
+ return 0;
+}
+__setup("badram", badram_setup);
--
1.7.4

2011-06-22 11:19:58

by Stefan Assmann

[permalink] [raw]
Subject: [PATCH v2 3/3] Add documentation and credits for BadRAM

Add Documentation/BadRAM.txt for in-depth information and update
Documentation/kernel-parameters.txt.

Signed-off-by: Stefan Assmann <[email protected]>
Acked-by: Tony Luck <[email protected]>
Acked-by: Andi Kleen <[email protected]>
---
CREDITS | 9 +
Documentation/BadRAM.txt | 370 +++++++++++++++++++++++++++++++++++
Documentation/kernel-parameters.txt | 6 +
3 files changed, 385 insertions(+), 0 deletions(-)
create mode 100644 Documentation/BadRAM.txt

diff --git a/CREDITS b/CREDITS
index dca6abc..d57d4af 100644
--- a/CREDITS
+++ b/CREDITS
@@ -2899,6 +2899,15 @@ S: 6 Karen Drive
S: Malvern, Pennsylvania 19355
S: USA

+N: Rick van Rein
+E: [email protected]
+W: http://rick.vanrein.org/
+D: Memory, the BadRAM subsystem dealing with defective RAM modules.
+S: Haarlebrink 5
+S: 7544 WP Enschede
+S: The Netherlands
+P: 1024D/89754606 CD46 B5F2 E876 A5EE 9A85 1735 1411 A9C2 8975 4606
+
N: Stefan Reinauer
E: [email protected]
W: http://www.freiburg.linux.de/~stepan/
diff --git a/Documentation/BadRAM.txt b/Documentation/BadRAM.txt
new file mode 100644
index 0000000..a98dcd2
--- /dev/null
+++ b/Documentation/BadRAM.txt
@@ -0,0 +1,370 @@
+INFORMATION ON USING BAD RAM MODULES
+====================================
+
+The BadRAM feature enables Linux to run on broken memory. The
+resulting system will be stable and healthy, because the kernel
+simply never allocates the faulty pages for use. This is how
+to setup BadRAM if your memory is failing.
+
+
+Introduction
+------------
+
+As RAM memory grows smaller, it also becomes harder to manufacture
+chips that are perfect. Each single cell that is failing could cause
+an entire memory module to fail. Even though manufacturers put in
+extra cells to replace failed ones, it is still possible that the
+sensitive small structures get damaged by an electrical discharge on
+their pins. Such damage leads to problems in fixed locations of
+the address space of a memory module, which is what theory predicts
+and has been confirmed by years of experience with bad memory.
+
+It is not necessary for such a memory module to be discarded. All
+pages of memory behave the same, and if only we skip the failing
+pages we can continue to use the module for many more years. The
+operating system kernel simply has to avoid using the blocks that
+are damaged. This is easy to do in the part of the kernel where
+memory pages are allocated.
+
+
+Reasons for using BadRAM
+------------------------
+
+Chip manufacturing processes use lots of harsh chemicals, and the less
+of these used, the better. Being able to make good use of partially
+failed memory chips means that far less of those chemicals are needed
+to provide storage. This reduces expenses and it is lighter on the
+environment in which we live.
+
+This kernel feature clearly shows that Linux is "the flexible OS".
+If something does not work, fix it. Also, share it with all the
+others who could use it. After more than a decade of BadRAM,
+the response has been purely positive, because it has helped real
+people to solve real problems.
+
+One important use for this feature is with laptops that have their
+memory soldered in. Such laptops would have to be discarded as a
+whole, but with BadRAM in place they can continue to be used
+without further restrictions.
+
+Finally, running a system on broken memory is just plain cool ;-)
+
+
+Running example
+---------------
+
+To run this project, I was given two DIMMs, 32 MB each. One, that we
+shall use as a running example in this text, contained 512 faulty bits,
+spread over 1/4 of the address range in a regular pattern. This looks
+a lot like the fauly pattern that many others have reported; the only
+common other pattern is a single faulty spot. With such memory, a few
+tricks with a thorough RAM tester and some binary calculations suffice
+to write these fault patterns down in 2 longword numbers. The format
+of these is hexadecimal, which is a condensed way of writing down the
+binary patterns that make the hardware patterns recognisable.
+
+After being patched and invoked with the properly formatted description,
+the kernel held back only the memory pages with faults, and never handed
+them out for allocation. The allocation routines could therefore
+progress as normally, without any adaptation. This is important, since
+all the work is done at booting time. After booting, the kernel does
+not have to do spend any time to implement BadRAM.
+
+As a result of this initial exercise, I gained 30 MB out of the 32 MB
+DIMM that would otherwise have been thrown away. Of course, these
+numbers scale up with larger memory modules, but the principle is
+the same.
+
+
+The structure of memory failures
+--------------------------------
+
+Memory chips are usually laid out in a roughly equal number of rows
+and columns, making it a square of cells that each store one bit.
+When addressing a bit, the processor sends the row and column in
+separate phases, and then reads or writes its value. The rows and
+columns are therefore visible on the outside of a chip.
+
+The connections of row and column lines to the outside world is
+usually protected by a buffer. It can happen that a static
+discharge damages such a buffer, causing an entire row or an
+entire column to fail. This means that a series of bits become
+unusable in a single page or in a regular pattern of pages,
+depending on whether it was a row or column that got damaged.
+
+For this reason, BadRAM was designed to describe memory faults
+in a pattern of address/mask pairs. An address locates an
+error and a zero on the corresponding position in the mask
+defines which bits in the address may be replaced with any
+other value. This has shown to work as a tight description
+of error patterns: it is very compact, but does not waste pages
+that are good.
+
+
+BadRAM's notation for memory faults
+-----------------------------------
+
+Instead of manually providing all 512 errors in the running example
+to the kernel, it's easier to use a pattern notation. Since the
+regularity is based on address decoding software, which generally
+takes certain bits into account and ignores others, we shall
+provide a faulty address F, together with a bit mask M that
+specifies which bits must be equal to F. In C code, an address A
+is faulty if and only if
+
+ (F & M) == (A & M)
+
+or alternately (closer to a hardware implementation):
+
+ ~((F ^ A) & M)
+
+In the example 32 MB chip, I had the faulty addresses in 8MB-16MB:
+
+ xxx42f4 ....0100....
+ xxx62f4 ....0110....
+ xxxc2f4 ....1100....
+ xxxe2f4 ....1110....
+
+The second column represents the alternating hex digit in binary form.
+Apparently, the first and next to last binary digit can be anything,
+so the binary mask for that part is 0101. The mask for the part after
+this is 0xfff, and the part before should select anything in the range
+8MB-16MB, or 0x00800000-0x01000000; this is done with a bitmask
+0xff80xxxx. Combining these partial masks, we get:
+
+ F=0x008042f4 M=0xff805fff
+
+That covers every fault in this DIMM; for more complicated failing
+DIMMs, or for a combination of multiple failing DIMMs, it can be
+necessary to set up a number of such F/M pairs.
+
+
+Getting started
+---------------
+
+If you experience RAM trouble, first read Documentation/memory.txt
+and try out the mem=4M trick to see if at least some initial parts
+of your RAM work well. Note that 4 MB will not be able to hold a
+modern desktop, so if you rely on that you would have to set the
+limit higher (and accept that your sanity check is not as tight as
+possible).
+
+The BadRAM routines halt the kernel in panic if the reserved area
+of memory (containing kernel stuff) contains a faulty address. It
+will only do that when supplied with the patterns below; this
+initial check is merely to see if this is likely to happen.
+
+
+Running a memory checker
+------------------------
+
+There is no memory checker built into the kernel, to avoid delays
+at runtime or while booting. If you experience problems that may
+be caused by RAM, run a good outside RAM checker. The Memtest86
+checker is a popular, free, high-quality checker. Many Linux
+distributions include it as an alternate boot option, so you may
+simply find it in your boot loader's boot menu.
+
+
+The memory checker lists all addresses that have a fault. It will
+do this for a given configuration of the DIMMs in your motherboard;
+if you replace or move memory modules you may find other addresses.
+In the running example's 32 MB chip, with the DIMM in slot #0 on
+the motherboard, the errors were found in the 8MB-16MB range:
+
+ xxx42f4
+ xxx62f4
+ xxxc2f4
+ xxxe2f4
+
+The error reported was a "sticky 1 bit", a memory bit that always
+reads as "1" even if a "0" was just written to it. This is
+probably caused by a damaged buffer on one of the rows or columns
+in one of the memory chips.
+
+It would be a lot of work to collect the individual errors and
+condense them into a pattern. That is why I patched the
+Memtest86 (v2.3+) checker to directly print out the address/mask
+pairs that are used by this kernel feature. All you would do is
+select the BadRAM printout option at the start of the scan, and
+then leave it running for hours and hours, until it has made at
+least one pass. The patterns are printed each time a bit is
+added, but each line contains all faults found up to that point,
+so you would write down the last set of patterns printed, and
+supply that as a boot option in your next run of a
+BadRAM-capable Linux kernel.
+
+If you use this patch on an x86_64 architecture, your addresses are
+twice as long. Fill up with zeroes in the address and with f's in
+the mask. The latter example would thus become:
+
+ mem=24M badram=0x0000000000f00000,0xfffffffffff00000
+
+The patch applies the changes to both x86 and x86_64 code bases
+at the same time. Patching but not compiling maps the entire
+source tree at once, which makes more sense than splitting the
+patch into an x86 and x86_64 branch, because those two branches
+could not be applied at the same time because they would overlap.
+
+
+Rebooting Linux
+---------------
+
+Once the fault patterns are known we simply restart Linux with
+these F/M pairs as a parameter. If your normal boot options look
+like
+
+ root=/dev/sda1 ro
+
+you should now boot with options
+
+ root=/dev/sda1 ro badram=0x008042f4,0xff805fff
+
+or perhaps by mentioning more F/M pairs in an order F0,M0,F1,M1,...
+When you provide an odd number of arguments to badram, the default
+mask 0xffffffff (meaning that only one address is matched) is
+applied to the last address.
+
+If your bootloader is GRUB, you can supply this additional
+parameter interactively during boot. This way, you can try them
+before you edit /boot/grub/grub.conf to put them in forever.
+
+When the kernel now boots, it should not give any trouble with RAM.
+Mind you, this is under the assumption that the kernel and its data
+storage do not overlap an erroneous part. If they do, and the
+kernel does not choke on it right away, BadRAM itself will stop the
+system with a kernel panic. When the error is that low in memory,
+you will need additional bootloader magic, to load the kernel at an
+alternative address.
+
+Now look up your memory status with
+
+ cat /proc/meminfo |grep HardwareCorrupted
+
+which prints a single line with information like
+
+HardwareCorrupted: 2048 kB
+
+The entry HardwareCorrupted: 2048k represents the loss of 2MB
+of general purpose RAM due to the errors. Or, positively rephrased,
+instead of throwing out 32MB as useless, you only throw out 2MB.
+Note that 2048 kB equals 512 pages of 4kB. The size of a page is
+defined by the processor architecture.
+
+If the system is stable (which you can test by compiling a few
+kernels, and a few file finds in / or so) you can decide to add
+the boot parameter to /boot/grub/grub.conf, in addition to any
+other boot parameters that may already be there. For example,
+
+ kernel /boot/vmlinuz root=/dev/sda1 ro
+
+would become
+
+ kernel /boot/vmlinuz root=/dev/sda1 ro badram=0x008042f4,0xff805fff
+
+Depending on how helpful your Linux distribution is, you may
+have to add this feature again after upgrading your kernel. If
+your boot loader is GRUB, you can always do this manually if you
+rebooted before you remembered to make that adaptation.
+
+
+BadRAM classification
+---------------------
+
+This technique might start a lively market for "dead" RAM. It is
+important to realise that some RAMs are more dead than others. So,
+instead of just providing a RAM size, it is also important to know
+the BadRAM class, which is defined as follows:
+
+ A BadRAM class N means that at most 2^N bytes have a problem,
+ and that all problems with the RAMs are persistent: They
+ are predictable and always show up.
+
+The DIMM that serves as an example here was of class 9, since 512=2^9
+errors were found. Higher classes are worse, "correct" RAM is of class
+-1 (or even less, at your choice).
+Class N also means that the bitmask for your chip (if there's just one,
+that is) counts N bits "0" and it means that (if no faults fall in the
+same page) an amount of 2^N*PAGESIZE memory is lost, in the example on
+an x86 architecture that would be 2^9*4k=2MB, which accounts for the
+initial claim of 30MB RAM gained with this DIMM.
+
+Note that this scheme has deliberately been defined to be independent
+of memory technology and of computer architecture.
+
+
+Further Possibilities
+---------------------
+
+**Slab allocation support**
+
+It would be possible to use even more of the faulty RAMs by employing
+them for slabs. The smaller allocation granularity of slabs makes it
+possible to throw out just, say, 32 bytes surrounding an error. This
+would mean that the example DIMM only caused a loss of 16kB instead
+of 2MB, or scaled-up similar values for larger memory sizes. One
+specific area that could benefit from this is the growing market
+for embedded devices, which usually wants to meet tight budgets.
+
+It should be possible to make the slab allocator prefer pages with
+broken memory, and allocate the faulty places in memory before the
+other slabs are made available to the kernel. In the best possible
+situation, this could reduce the loss of good RAM cells to zero!
+
+**Support for low-memory errors**
+
+To the best of my knowledge, boot loaders like GRUB cannot load
+the Linux kernel in non-standard locations. This means that any
+errors at low memory locations cannot be overcome with BadRAM.
+
+Anything that physically alters the memory layout can be used
+to overcome such problems; this may be achieved through BIOS
+settings, or by adding or swapping memory modules.
+
+A general solution could be to use a boot loader that can load
+the Linux kernel (and its initial memory allocation) at other
+memory addresses than are standard.
+
+
+**Boot-time memory checking**
+
+Many suggestions have been made to insert a RAM checker at boot time;
+since this would leave the time to do only very meager checking, it
+is not a reasonable option; we already have a half-done BIOS check
+doing that!
+
+**ECC RAM integration**
+
+It would be interesting to integrate this functionality with the
+self-verifying nature of ECC RAM. These memories can even distinguish
+between recoverable and unrecoverable errors! Such memory has been
+handled in older operating systems by `testing' once-failed memory
+blocks for a while, by placing only (reloadable) program code in it.
+
+I possess no faulty ECC modules to work this out, and there is no
+general use for it either.
+
+
+Names and Places
+----------------
+
+The home page of this project is on
+ http://rick.vanrein.org/linux/badram
+This page also links to Nico Schmoigl's experimental extensions to
+this patch (with debugging and a few other fancy things).
+
+In case you have experiences with the BadRAM software which differ from
+the test reportings on that site, I hope you will mail me with that
+new information.
+
+The BadRAM project is an idea and implementation by
+ Rick van Rein
+ Haarlebrink 5
+ 7544 WP Enschede
+ The Netherlands
+ [email protected]
+If you like it, a postcard would be much appreciated ;-)
+
+
+ Enjoy,
+ -Rick.
diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index cc85a92..a12b27a 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -51,6 +51,7 @@ parameter is applicable:
FB The frame buffer device is enabled.
GCOV GCOV profiling is enabled.
HW Appropriate hardware is enabled.
+ HWPOISON HWPOISON support is enabled.
IA-64 IA-64 architecture is enabled.
IMA Integrity measurement architecture is enabled.
IOSCHED More than one I/O scheduler is enabled.
@@ -373,6 +374,11 @@ bytes respectively. Such letter suffixes can also be entirely omitted.

autotest [IA64]

+ badram= [HWPOISON]
+ Allows memory areas to be flagged as HWPOISON.
+ Format: <addr>,<mask>[,...]
+ See Documentation/BadRAM.txt
+
baycom_epp= [HW,AX25]
Format: <io>,<mode>

--
1.7.4.4

2011-06-22 18:01:04

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH v2 0/3] support for broken memory modules (BadRAM)

On Wed, 22 Jun 2011 13:18:51 +0200 Stefan Assmann <[email protected]> wrote:

> Following the RFC for the BadRAM feature here's the updated version with
> spelling fixes, thanks go to Randy Dunlap. Also the code is now less verbose,
> as requested by Andi Kleen.
> v2 with even more spelling fixes suggested by Randy.
> Patches are against vanilla 2.6.39.
>
> The idea is to allow the user to specify RAM addresses that shouldn't be
> touched by the OS, because they are broken in some way. Not all machines have
> hardware support for hwpoison, ECC RAM, etc, so here's a solution that allows to
> use bitmasks to mask address patterns with the new "badram" kernel command line
> parameter.
> Memtest86 has an option to generate these patterns since v2.3 so the only thing
> for the user to do should be:
> - run Memtest86
> - note down the pattern
> - add badram=<pattern> to the kernel command line
>
> The concerning pages are then marked with the hwpoison flag and thus won't be
> used by the memory managment system.

The google kernel has a similar capability. I asked Nancy to comment
on these patches and she said:

: One, the bad addresses are passed via the kernel command line, which
: has a limited length. It's okay if the addresses can be fit into a
: pattern, but that's not necessarily the case in the google kernel. And
: even with patterns, the limit on the command line length limits the
: number of patterns that user can specify. Instead we use lilo to pass
: a file containing the bad pages in e820 format to the kernel.
:
: Second, the BadRAM patch expands the address patterns from the command
: line into individual entries in the kernel's e820 table. The e820
: table is a fixed buffer that supports a very small, hard coded number
: of entries (128). We require a much larger number of entries (on
: the order of a few thousand), so much of the google kernel patch deals
: with expanding the e820 table. Also, with the BadRAM patch, entries
: that don't fit in the table are silently dropped and this isn't
: appropriate for us.
:
: Another caveat of mapping out too much bad memory in general. If too
: much memory is removed from low memory, a system may not boot. We
: solve this by generating good maps. Our userspace tools do not map out
: memory below a certain limit, and it verifies against a system's iomap
: that only addresses from memory is mapped out.

I have a couple of thoughts here:

- If this patchset is merged and a major user such as google is
unable to use it and has to continue to carry a separate patch then
that's a regrettable situation for the upstream kernel.

- Google's is, afaik, the largest use case we know of: zillions of
machines for a number of years. And this real-world experience tells
us that the badram patchset has shortcomings. Shortcomings which we
can expect other users to experience.

So. What are your thoughts on these issues?

Thanks

2011-06-22 18:06:37

by Josh Boyer

[permalink] [raw]
Subject: Re: [PATCH v2 0/3] support for broken memory modules (BadRAM)

On Wed, Jun 22, 2011 at 2:00 PM, Andrew Morton
<[email protected]> wrote:>
> I have a couple of thoughts here:
>
> - If this patchset is merged and a major user such as google is
> ?unable to use it and has to continue to carry a separate patch then
> ?that's a regrettable situation for the upstream kernel.
>
> - Google's is, afaik, the largest use case we know of: zillions of
> ?machines for a number of years. ?And this real-world experience tells
> ?us that the badram patchset has shortcomings. ?Shortcomings which we
> ?can expect other users to experience.
>
> So. ?What are your thoughts on these issues?

Has Google submitted patches for their implementation?

josh

2011-06-22 18:09:14

by Randy Dunlap

[permalink] [raw]
Subject: Re: [PATCH v2 0/3] support for broken memory modules (BadRAM)

On Wed, 22 Jun 2011 11:00:34 -0700 Andrew Morton wrote:

> On Wed, 22 Jun 2011 13:18:51 +0200 Stefan Assmann <[email protected]> wrote:
>
> > Following the RFC for the BadRAM feature here's the updated version with
> > spelling fixes, thanks go to Randy Dunlap. Also the code is now less verbose,
> > as requested by Andi Kleen.
> > v2 with even more spelling fixes suggested by Randy.
> > Patches are against vanilla 2.6.39.
> >
> > The idea is to allow the user to specify RAM addresses that shouldn't be
> > touched by the OS, because they are broken in some way. Not all machines have
> > hardware support for hwpoison, ECC RAM, etc, so here's a solution that allows to
> > use bitmasks to mask address patterns with the new "badram" kernel command line
> > parameter.
> > Memtest86 has an option to generate these patterns since v2.3 so the only thing
> > for the user to do should be:
> > - run Memtest86
> > - note down the pattern
> > - add badram=<pattern> to the kernel command line
> >
> > The concerning pages are then marked with the hwpoison flag and thus won't be
> > used by the memory managment system.
>
> The google kernel has a similar capability. I asked Nancy to comment
> on these patches and she said:
>
> : One, the bad addresses are passed via the kernel command line, which
> : has a limited length. It's okay if the addresses can be fit into a
> : pattern, but that's not necessarily the case in the google kernel. And
> : even with patterns, the limit on the command line length limits the
> : number of patterns that user can specify. Instead we use lilo to pass
> : a file containing the bad pages in e820 format to the kernel.
> :
> : Second, the BadRAM patch expands the address patterns from the command
> : line into individual entries in the kernel's e820 table. The e820
> : table is a fixed buffer that supports a very small, hard coded number
> : of entries (128). We require a much larger number of entries (on
> : the order of a few thousand), so much of the google kernel patch deals
> : with expanding the e820 table. Also, with the BadRAM patch, entries
> : that don't fit in the table are silently dropped and this isn't
> : appropriate for us.
> :
> : Another caveat of mapping out too much bad memory in general. If too
> : much memory is removed from low memory, a system may not boot. We
> : solve this by generating good maps. Our userspace tools do not map out
> : memory below a certain limit, and it verifies against a system's iomap
> : that only addresses from memory is mapped out.
>
> I have a couple of thoughts here:
>
> - If this patchset is merged and a major user such as google is
> unable to use it and has to continue to carry a separate patch then
> that's a regrettable situation for the upstream kernel.
>
> - Google's is, afaik, the largest use case we know of: zillions of
> machines for a number of years. And this real-world experience tells
> us that the badram patchset has shortcomings. Shortcomings which we
> can expect other users to experience.
>
> So. What are your thoughts on these issues?


Good comments, so where is google's patch submittal?

---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

2011-06-22 18:12:01

by Nancy Yuen

[permalink] [raw]
Subject: Re: [PATCH v2 0/3] support for broken memory modules (BadRAM)

I haven't had time to submit the patches, though it's on my todo list.

----------
Nancy



On Wed, Jun 22, 2011 at 11:09, Randy Dunlap <[email protected]> wrote:
> On Wed, 22 Jun 2011 11:00:34 -0700 Andrew Morton wrote:
>
>> On Wed, 22 Jun 2011 13:18:51 +0200 Stefan Assmann <[email protected]> wrote:
>>
>> > Following the RFC for the BadRAM feature here's the updated version with
>> > spelling fixes, thanks go to Randy Dunlap. Also the code is now less verbose,
>> > as requested by Andi Kleen.
>> > v2 with even more spelling fixes suggested by Randy.
>> > Patches are against vanilla 2.6.39.
>> >
>> > The idea is to allow the user to specify RAM addresses that shouldn't be
>> > touched by the OS, because they are broken in some way. Not all machines have
>> > hardware support for hwpoison, ECC RAM, etc, so here's a solution that allows to
>> > use bitmasks to mask address patterns with the new "badram" kernel command line
>> > parameter.
>> > Memtest86 has an option to generate these patterns since v2.3 so the only thing
>> > for the user to do should be:
>> > - run Memtest86
>> > - note down the pattern
>> > - add badram=<pattern> to the kernel command line
>> >
>> > The concerning pages are then marked with the hwpoison flag and thus won't be
>> > used by the memory managment system.
>>
>> The google kernel has a similar capability. ?I asked Nancy to comment
>> on these patches and she said:
>>
>> : One, the bad addresses are passed via the kernel command line, which
>> : has a limited length. ?It's okay if the addresses can be fit into a
>> : pattern, but that's not necessarily the case in the google kernel. ?And
>> : even with patterns, the limit on the command line length limits the
>> : number of patterns that user can specify. ?Instead we use lilo to pass
>> : a file containing the bad pages in e820 format to the kernel.
>> :
>> : Second, the BadRAM patch expands the address patterns from the command
>> : line into individual entries in the kernel's e820 table. ?The e820
>> : table is a fixed buffer that supports a very small, hard coded number
>> : of entries (128). ?We require a much larger number of entries (on
>> : the order of a few thousand), so much of the google kernel patch deals
>> : with expanding the e820 table. Also, with the BadRAM patch, entries
>> : that don't fit in the table are silently dropped and this isn't
>> : appropriate for us.
>> :
>> : Another caveat of mapping out too much bad memory in general. ?If too
>> : much memory is removed from low memory, a system may not boot. ?We
>> : solve this by generating good maps. ?Our userspace tools do not map out
>> : memory below a certain limit, and it verifies against a system's iomap
>> : that only addresses from memory is mapped out.
>>
>> I have a couple of thoughts here:
>>
>> - If this patchset is merged and a major user such as google is
>> ? unable to use it and has to continue to carry a separate patch then
>> ? that's a regrettable situation for the upstream kernel.
>>
>> - Google's is, afaik, the largest use case we know of: zillions of
>> ? machines for a number of years. ?And this real-world experience tells
>> ? us that the badram patchset has shortcomings. ?Shortcomings which we
>> ? can expect other users to experience.
>>
>> So. ?What are your thoughts on these issues?
>
>
> Good comments, so where is google's patch submittal?
>
> ---
> ~Randy
> *** Remember to use Documentation/SubmitChecklist when testing your code ***
>

2011-06-22 18:14:07

by H. Peter Anvin

[permalink] [raw]
Subject: Re: [PATCH v2 0/3] support for broken memory modules (BadRAM)

On 06/22/2011 11:00 AM, Andrew Morton wrote:
> :
> : Second, the BadRAM patch expands the address patterns from the command
> : line into individual entries in the kernel's e820 table. The e820
> : table is a fixed buffer that supports a very small, hard coded number
> : of entries (128). We require a much larger number of entries (on
> : the order of a few thousand), so much of the google kernel patch deals
> : with expanding the e820 table.

This has not been true for a long time.

> I have a couple of thoughts here:
>
> - If this patchset is merged and a major user such as google is
> unable to use it and has to continue to carry a separate patch then
> that's a regrettable situation for the upstream kernel.
>
> - Google's is, afaik, the largest use case we know of: zillions of
> machines for a number of years. And this real-world experience tells
> us that the badram patchset has shortcomings. Shortcomings which we
> can expect other users to experience.
>
> So. What are your thoughts on these issues?

I think a binary structure fed as a linked list data object makes a lot
more sense. We already support feeding e820 entries in this way,
bypassing the 128-entry limitation of the fixed table in the zeropage.

The main issue then is priority; in particular memory marked UNUSABLE
(type 5) in the fed-in e820 map will of course overlap entries with
normal RAM (type 1) information in the native map; we need to make sure
that the type 5 information takes priority.

-hpa

2011-06-22 18:15:49

by H. Peter Anvin

[permalink] [raw]
Subject: Re: [PATCH v2 0/3] support for broken memory modules (BadRAM)

On 06/22/2011 04:18 AM, Stefan Assmann wrote:
>
> The idea is to allow the user to specify RAM addresses that shouldn't be
> touched by the OS, because they are broken in some way. Not all machines have
> hardware support for hwpoison, ECC RAM, etc, so here's a solution that allows to
> use bitmasks to mask address patterns with the new "badram" kernel command line
> parameter.
> Memtest86 has an option to generate these patterns since v2.3 so the only thing
> for the user to do should be:
> - run Memtest86
> - note down the pattern
> - add badram=<pattern> to the kernel command line
>

We already support the equivalent functionality with
memmap=<address>$<length> for those with only a few ranges... this has
been supported for ages, literally. For those with a lot of ranges,
like Google, the command line is insufficient.

-hpa

2011-06-22 18:24:49

by Andi Kleen

[permalink] [raw]
Subject: Re: [PATCH v2 0/3] support for broken memory modules (BadRAM)

> So. What are your thoughts on these issues?

Sounds orthogonal to me. You have to crawl before you walk.

A better way to pass in the data would be nice, but can be always
added on top (e.g. some EFI environment variable)

For a first try a command line argument is quite
appropiate and simple enough.

A check for removing too much memory would be nice though,
although it's just a choice between panicing early or later.

-Andi

--
[email protected] -- Speaking for myself only.

2011-06-22 18:39:11

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH v2 0/3] support for broken memory modules (BadRAM)

On Wed, 22 Jun 2011 20:24:45 +0200
Andi Kleen <[email protected]> wrote:

> > So. What are your thoughts on these issues?
>
> Sounds orthogonal to me. You have to crawl before you walk.
>
> A better way to pass in the data would be nice, but can be always
> added on top (e.g. some EFI environment variable)
>
> For a first try a command line argument is quite
> appropiate and simple enough.
>
> A check for removing too much memory would be nice though,
> although it's just a choice between panicing early or later.
>

If something can be grafted on later then that's of course all good. I
do think we should have some sort of plan in which we work out how that
will be done. If we want to do it, that is.

However if we go this way then there's a risk that we'll end up with
two different ways of configuring the feature and we'll need to
maintain the old way for ever. That's a bad thing and we'd be better
off implementing the fancier scheme on day one.

2011-06-22 18:56:47

by Andi Kleen

[permalink] [raw]
Subject: Re: [PATCH v2 0/3] support for broken memory modules (BadRAM)

> If something can be grafted on later then that's of course all good. I
> do think we should have some sort of plan in which we work out how that
> will be done. If we want to do it, that is.
>
> However if we go this way then there's a risk that we'll end up with
> two different ways of configuring the feature and we'll need to

You'll always have multiple ways. Whatever magic you come up for
the google BIOS or for EFI won't help the majority of users with
old crufty legacy BIOS.

So you need a "everything included" way -- and the only straight forward
way to do that that I can see is the command line.

-Andi
--
[email protected] -- Speaking for myself only.

2011-06-22 19:01:55

by Nancy Yuen

[permalink] [raw]
Subject: Re: [PATCH v2 0/3] support for broken memory modules (BadRAM)

On Wed, Jun 22, 2011 at 11:13, H. Peter Anvin <[email protected]> wrote:
> On 06/22/2011 11:00 AM, Andrew Morton wrote:
>> :
>> : Second, the BadRAM patch expands the address patterns from the command
>> : line into individual entries in the kernel's e820 table. ?The e820
>> : table is a fixed buffer that supports a very small, hard coded number
>> : of entries (128). ?We require a much larger number of entries (on
>> : the order of a few thousand), so much of the google kernel patch deals
>> : with expanding the e820 table.
>
> This has not been true for a long time.

Good point. There's the MAX_NODES that expands it, though it's still
hard coded, and as I understand, intended for NUMA node entries. We
need anywhere from 8K to 64K 'bad' entries. This creates holes and
translates to twice as many entries in the e820. We only want to
allow this memory if it's needed, instead of hard coding it.

>
>> I have a couple of thoughts here:
>>
>> - If this patchset is merged and a major user such as google is
>> ? unable to use it and has to continue to carry a separate patch then
>> ? that's a regrettable situation for the upstream kernel.
>>
>> - Google's is, afaik, the largest use case we know of: zillions of
>> ? machines for a number of years. ?And this real-world experience tells
>> ? us that the badram patchset has shortcomings. ?Shortcomings which we
>> ? can expect other users to experience.
>>
>> So. ?What are your thoughts on these issues?
>
> I think a binary structure fed as a linked list data object makes a lot
> more sense. ?We already support feeding e820 entries in this way,
> bypassing the 128-entry limitation of the fixed table in the zeropage.
>
> The main issue then is priority; in particular memory marked UNUSABLE
> (type 5) in the fed-in e820 map will of course overlap entries with
> normal RAM (type 1) information in the native map; we need to make sure
> that the type 5 information takes priority.
>
> ? ? ? ?-hpa
>

2011-06-22 19:06:28

by H. Peter Anvin

[permalink] [raw]
Subject: Re: [PATCH v2 0/3] support for broken memory modules (BadRAM)

On 06/22/2011 11:56 AM, Andi Kleen wrote:
>
> You'll always have multiple ways. Whatever magic you come up for
> the google BIOS or for EFI won't help the majority of users with
> old crufty legacy BIOS.
>

I don't think this has anything to do with this.

-hpa

2011-06-22 19:07:26

by H. Peter Anvin

[permalink] [raw]
Subject: Re: [PATCH v2 0/3] support for broken memory modules (BadRAM)

On 06/22/2011 12:01 PM, Nancy Yuen wrote:
>
> Good point. There's the MAX_NODES that expands it, though it's still
> hard coded, and as I understand, intended for NUMA node entries. We
> need anywhere from 8K to 64K 'bad' entries. This creates holes and
> translates to twice as many entries in the e820. We only want to
> allow this memory if it's needed, instead of hard coding it.
>

It should be dynamic, probably. We can waste memory during early
reclaim, but the memblock stuff should be dynamic.

-hpa

2011-06-22 19:15:59

by Andi Kleen

[permalink] [raw]
Subject: Re: [PATCH v2 0/3] support for broken memory modules (BadRAM)

On Wed, Jun 22, 2011 at 12:05:07PM -0700, H. Peter Anvin wrote:
> On 06/22/2011 11:56 AM, Andi Kleen wrote:
> >
> > You'll always have multiple ways. Whatever magic you come up for
> > the google BIOS or for EFI won't help the majority of users with
> > old crufty legacy BIOS.
> >
>
> I don't think this has anything to do with this.

Please elaborate.

How would you pass the bad page information instead in a fully backwards
compatible way?

-Andi
--
[email protected] -- Speaking for myself only.

2011-06-22 19:46:13

by Mike Ditto

[permalink] [raw]
Subject: [PATCH] x86: e820: Eliminate bubble sort from sanitize_e820_map

Replace the bubble sort in sanitize_e820_map() with a call to the generic
kernel sort function to avoid pathological performance with large maps.

On large (thousands of entries) E820 maps, the previous code took minutes
to run; with this change it's now milliseconds.

Signed-off-by: Mike Ditto <[email protected]>
---
This is a small independent part of Google's BadRAM changes mentioned in
another thread.

arch/x86/kernel/e820.c | 59 +++++++++++++++++++----------------------------
1 files changed, 24 insertions(+), 35 deletions(-)

diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
index 3e2ef84..e2e212a 100644
--- a/arch/x86/kernel/e820.c
+++ b/arch/x86/kernel/e820.c
@@ -18,6 +18,7 @@
#include <linux/acpi.h>
#include <linux/firmware-map.h>
#include <linux/memblock.h>
+#include <linux/sort.h>

#include <asm/e820.h>
#include <asm/proto.h>
@@ -226,22 +227,38 @@ void __init e820_print_map(char *who)
* ____________________33__
* ______________________4_
*/
+struct change_member {
+ struct e820entry *pbios; /* pointer to original bios entry */
+ unsigned long long addr; /* address for this change point */
+};
+
+static int __init cpcompare(const void *a, const void *b)
+{
+ struct change_member * const *app = a, * const *bpp = b;
+ const struct change_member *ap = *app, *bp = *bpp;
+
+ /*
+ * Inputs are pointers to two elements of change_point[]. If their
+ * addresses are unequal, their difference dominates. If the addresses
+ * are equal, then consider one that represents the end of its region
+ * to be greater than one that does not.
+ */
+ if (ap->addr != bp->addr)
+ return ap->addr > bp->addr ? 1 : -1;
+
+ return (ap->addr != ap->pbios->addr) - (bp->addr != bp->pbios->addr);
+}

int __init sanitize_e820_map(struct e820entry *biosmap, int max_nr_map,
u32 *pnr_map)
{
- struct change_member {
- struct e820entry *pbios; /* pointer to original bios entry */
- unsigned long long addr; /* address for this change point */
- };
static struct change_member change_point_list[2*E820_X_MAX] __initdata;
static struct change_member *change_point[2*E820_X_MAX] __initdata;
static struct e820entry *overlap_list[E820_X_MAX] __initdata;
static struct e820entry new_bios[E820_X_MAX] __initdata;
- struct change_member *change_tmp;
unsigned long current_type, last_type;
unsigned long long last_addr;
- int chgidx, still_changing;
+ int chgidx;
int overlap_entries;
int new_bios_entry;
int old_nr, new_nr, chg_nr;
@@ -278,35 +295,7 @@ int __init sanitize_e820_map(struct e820entry *biosmap, int max_nr_map,
chg_nr = chgidx;

/* sort change-point list by memory addresses (low -> high) */
- still_changing = 1;
- while (still_changing) {
- still_changing = 0;
- for (i = 1; i < chg_nr; i++) {
- unsigned long long curaddr, lastaddr;
- unsigned long long curpbaddr, lastpbaddr;
-
- curaddr = change_point[i]->addr;
- lastaddr = change_point[i - 1]->addr;
- curpbaddr = change_point[i]->pbios->addr;
- lastpbaddr = change_point[i - 1]->pbios->addr;
-
- /*
- * swap entries, when:
- *
- * curaddr > lastaddr or
- * curaddr == lastaddr and curaddr == curpbaddr and
- * lastaddr != lastpbaddr
- */
- if (curaddr < lastaddr ||
- (curaddr == lastaddr && curaddr == curpbaddr &&
- lastaddr != lastpbaddr)) {
- change_tmp = change_point[i];
- change_point[i] = change_point[i-1];
- change_point[i-1] = change_tmp;
- still_changing = 1;
- }
- }
- }
+ sort(change_point, chg_nr, sizeof *change_point, cpcompare, 0);

/* create a new bios memory map, removing overlaps */
overlap_entries = 0; /* number of entries in the overlap table */
--
1.7.3.1

2011-06-22 20:25:24

by Stefan Assmann

[permalink] [raw]
Subject: Re: [PATCH v2 0/3] support for broken memory modules (BadRAM)

On 22.06.2011 20:00, Andrew Morton wrote:
> On Wed, 22 Jun 2011 13:18:51 +0200 Stefan Assmann <[email protected]> wrote:
>

[...]

>> The idea is to allow the user to specify RAM addresses that shouldn't be
>> touched by the OS, because they are broken in some way. Not all machines have
>> hardware support for hwpoison, ECC RAM, etc, so here's a solution that allows to
>> use bitmasks to mask address patterns with the new "badram" kernel command line
>> parameter.
>> Memtest86 has an option to generate these patterns since v2.3 so the only thing
>> for the user to do should be:
>> - run Memtest86
>> - note down the pattern
>> - add badram=<pattern> to the kernel command line
>>
>> The concerning pages are then marked with the hwpoison flag and thus won't be
>> used by the memory managment system.
>
> The google kernel has a similar capability. I asked Nancy to comment
> on these patches and she said:

This is the first time I hear about this feature from Google. If I had
known about it I sure would have talked to the person responsible.

>
> : One, the bad addresses are passed via the kernel command line, which
> : has a limited length. It's okay if the addresses can be fit into a
> : pattern, but that's not necessarily the case in the google kernel. And
> : even with patterns, the limit on the command line length limits the
> : number of patterns that user can specify. Instead we use lilo to pass
> : a file containing the bad pages in e820 format to the kernel.

I see no reason why there couldn't be multiple ways of specifying bad
addresses.

> :
> : Second, the BadRAM patch expands the address patterns from the command
> : line into individual entries in the kernel's e820 table. The e820
> : table is a fixed buffer that supports a very small, hard coded number
> : of entries (128). We require a much larger number of entries (on
> : the order of a few thousand), so much of the google kernel patch deals
> : with expanding the e820 table. Also, with the BadRAM patch, entries
> : that don't fit in the table are silently dropped and this isn't
> : appropriate for us.

So far the use case I had in mind wasn't "thousands of entries". However
expanding the e820 table is probably an issue that could be dealt with
separately ?

> :
> : Another caveat of mapping out too much bad memory in general. If too
> : much memory is removed from low memory, a system may not boot. We
> : solve this by generating good maps. Our userspace tools do not map out
> : memory below a certain limit, and it verifies against a system's iomap
> : that only addresses from memory is mapped out.

Well if too much low memory is bad, you're screwed anyway, not? :)

>
> I have a couple of thoughts here:
>
> - If this patchset is merged and a major user such as google is
> unable to use it and has to continue to carry a separate patch then
> that's a regrettable situation for the upstream kernel.

I'm all ears for making things work out for potential users, I just
didn't know.

>
> - Google's is, afaik, the largest use case we know of: zillions of
> machines for a number of years. And this real-world experience tells
> us that the badram patchset has shortcomings. Shortcomings which we
> can expect other users to experience.
>
> So. What are your thoughts on these issues?

I'm aware that the implementation I posted is not covering *everything*.
It's a start and I tried to keep it simple and make use of already
existing infrastructure.
At the moment I don't see any arguments why this patchset couldn't play
along nicely or get enhanced to support what Google needs, but I don't
know Googles patches yet.

Thanks!

Stefan

2011-06-22 20:26:11

by H. Peter Anvin

[permalink] [raw]
Subject: Re: [PATCH v2 0/3] support for broken memory modules (BadRAM)

On 06/22/2011 12:15 PM, Andi Kleen wrote:
> On Wed, Jun 22, 2011 at 12:05:07PM -0700, H. Peter Anvin wrote:
>> On 06/22/2011 11:56 AM, Andi Kleen wrote:
>>>
>>> You'll always have multiple ways. Whatever magic you come up for
>>> the google BIOS or for EFI won't help the majority of users with
>>> old crufty legacy BIOS.
>>>
>>
>> I don't think this has anything to do with this.
>
> Please elaborate.
>
> How would you pass the bad page information instead in a fully backwards
> compatible way?
>

Depends on what you mean with "fully backward compatible". In some ways
this is a nonsense statement since if we create anything new older
kernels will not run.

However, the other discussions in this thread have been about injecting
data in kernel-specific data structures and thus aren't dependent on the
firmware layer used.

The fully backward compatible way is "memmap=<address>$<length>".

-hpa

2011-06-22 20:28:16

by Andi Kleen

[permalink] [raw]
Subject: Re: [PATCH v2 0/3] support for broken memory modules (BadRAM)

> The fully backward compatible way is "memmap=<address>$<length>".

This doesn't really work for patterns. badmem is about making patterns/
strides/etc. work as far as I understand. Those are very common
with modern interleaving schemes.

Please read the original patchkit and its documentation.

-Andi
--
[email protected] -- Speaking for myself only.

2011-06-22 20:30:45

by Stefan Assmann

[permalink] [raw]
Subject: Re: [PATCH v2 0/3] support for broken memory modules (BadRAM)

On 22.06.2011 20:15, H. Peter Anvin wrote:
> On 06/22/2011 04:18 AM, Stefan Assmann wrote:
>>
>> The idea is to allow the user to specify RAM addresses that shouldn't be
>> touched by the OS, because they are broken in some way. Not all machines have
>> hardware support for hwpoison, ECC RAM, etc, so here's a solution that allows to
>> use bitmasks to mask address patterns with the new "badram" kernel command line
>> parameter.
>> Memtest86 has an option to generate these patterns since v2.3 so the only thing
>> for the user to do should be:
>> - run Memtest86
>> - note down the pattern
>> - add badram=<pattern> to the kernel command line
>>
>
> We already support the equivalent functionality with
> memmap=<address>$<length> for those with only a few ranges... this has
> been supported for ages, literally. For those with a lot of ranges,
> like Google, the command line is insufficient.

Right, I think this has been discussed a while ago. So the advantages I
see in this approach are. It allows to break down memory exclusion to
the page level with a pattern of non-consecutive pages. So if every
other page would be considered bad that's a bit tough to deal with using
memmap.
Secondly patterns can be easily generated by running Memtest86 and thus
easily be fed to the kernel by command line. Making it much more feasible
for the average user to take advantage of it.

Stefan

2011-06-22 20:34:04

by H. Peter Anvin

[permalink] [raw]
Subject: Re: [PATCH v2 0/3] support for broken memory modules (BadRAM)

On 06/22/2011 01:30 PM, Stefan Assmann wrote:
> On 22.06.2011 20:15, H. Peter Anvin wrote:
>> On 06/22/2011 04:18 AM, Stefan Assmann wrote:
>>>
>>> The idea is to allow the user to specify RAM addresses that shouldn't be
>>> touched by the OS, because they are broken in some way. Not all machines have
>>> hardware support for hwpoison, ECC RAM, etc, so here's a solution that allows to
>>> use bitmasks to mask address patterns with the new "badram" kernel command line
>>> parameter.
>>> Memtest86 has an option to generate these patterns since v2.3 so the only thing
>>> for the user to do should be:
>>> - run Memtest86
>>> - note down the pattern
>>> - add badram=<pattern> to the kernel command line
>>>
>>
>> We already support the equivalent functionality with
>> memmap=<address>$<length> for those with only a few ranges... this has
>> been supported for ages, literally. For those with a lot of ranges,
>> like Google, the command line is insufficient.
>
> Right, I think this has been discussed a while ago. So the advantages I
> see in this approach are. It allows to break down memory exclusion to
> the page level with a pattern of non-consecutive pages. So if every
> other page would be considered bad that's a bit tough to deal with using
> memmap.
> Secondly patterns can be easily generated by running Memtest86 and thus
> easily be fed to the kernel by command line. Making it much more feasible
> for the average user to take advantage of it.
>

How common are nontrivial patterns on real hardware? This would be
interesting to hear from Google or another large user.

If so, we should probably introduce this as another linked-list data
structure; we can allow it to be preprocessed from the command line if
need be.

I have to say I think Google's point that truncating the list is
unacceptable... that would mean running in a known-bad configuration,
and even a hard crash would be better.

-hpa

2011-06-23 10:42:19

by Rick van Rein

[permalink] [raw]
Subject: Re: [PATCH v2 0/3] support for broken memory modules (BadRAM)

Hello,

> > The concerning pages are then marked with the hwpoison flag and thus won't be
> > used by the memory managment system.
>
> The google kernel has a similar capability. I asked Nancy to comment
> on these patches and she said:
>
> : One, the bad addresses are passed via the kernel command line, which
> : has a limited length. It's okay if the addresses can be fit into a
> : pattern, but that's not necessarily the case in the google kernel.

They are guaranteed to fit in 5 patterns (and even that is a choice).
The BadRAM pattern printing option built into Memtest86 will never
create more than that. If your memory is really screwed, it will
simply make patterns so generic that at least all the faults are
covered.

The figure 5 is a bit arbitrary, but was chosen in a time that we all
used LILO and had to live with its limited cmdline length. GRUB is
more relaxed in that respect, but there has never been a need to go
beyond five. Most errors are regular patterns (because an entire row,
or an entire column is damaged if not just a single cell is affected)
that will fit into a limited number of patterns without a need for so
many.

> : And
> : even with patterns, the limit on the command line length limits the
> : number of patterns that user can specify. Instead we use lilo to pass
> : a file containing the bad pages in e820 format to the kernel.

I've looked into the aproach of e820 and actually turned away from it.
The e820 format does not permit to specify the regularity that comes
with real-life memory problems. Having made the BadRAM patch, I've seen
numerous examples, and they all came down to single-cell errors and
either one or more rows and/or one or more columns of cells. There has
never been a reporting of such erratic destruction that it could not
comfortably (that is, with minimal pages sacrificed) fit in the limit
of 5 patterns that Memtest86 (not BadRAM) imposes. I'm pretty sure I
would have heard about it if there had been any such problems, given
the interactivity of people who had gone through all the effort of
patching a kernel. Kernel patchers are not usually the silent kind
when it comes to an opportunity to improve Linux ;-)

> : Second, the BadRAM patch expands the address patterns from the command
> : line into individual entries in the kernel's e820 table. The e820
> : table is a fixed buffer [...]

This is not how BadRAM works -- it will set a page flag for defected
pages in Linux' page table. It does this before getting to the stage
where all pages are initially 'freed' into the memory pool, and can
thus avoid that damaged pages are ever released for allocation.

> : We require a much larger number of entries (on
> : the order of a few thousand), so much of the google kernel patch deals
> : with expanding the e820 table.

Interesting. I have made a deliberate choice not to go that way,
but that was because we were looking at e820 as a communications
mechanism between a BadRAM-supportive GRUB and the kernel. The
advantage of that would have been to do it before the kernel.

Indeed, if you take this route you will see a severe expansion of the
e820 table. A damaged row (or column) does indeed lead to 4096 or so
error spots, that is quite common.

I'd like to know -- are the pages with faults that you have not also
organised in a regular pattern, which is what BadRAM addresses? If
not, that would be a strongly countering argument for the
pattern-based approach of BadRAM, but I would be really surprised if
one or two patterns (or up to five) could not comfortably describe
the error patterns -- as they were designed to match how memory
hardware actually work.

Also, if you find 4093 error pages, you would not generalise it to
a 4096 page error, right? I would not feel comfortable in that
case.

> : Also, with the BadRAM patch, entries
> : that don't fit in the table are silently dropped and this isn't
> : appropriate for us.

The e820 page is not used, so nothing is silently dropped. BadRAM
would rather err at the expense of a few pages than miss an opportunity
to fix a problem. There's nothing Google-specific about that wish :-)

> : Another caveat of mapping out too much bad memory in general.

Never seen that, or heard complaints about it, in over 10 years. Do
you have examples on the contrary, or is this merely a concern?

> : If too
> : much memory is removed from low memory, a system may not boot. We
> : solve this by generating good maps. Our userspace tools do not map out
> : memory below a certain limit, and it verifies against a system's iomap
> : that only addresses from memory is mapped out.

I've seen rare occasions where a system could not be helped due to a
bug in the low parts of memory, indeed. Maybe 1 or 2 cases in >10 years.

> - If this patchset is merged and a major user such as google is
> unable to use it and has to continue to carry a separate patch then
> that's a regrettable situation for the upstream kernel.

First, I wonder if there is any conflict at all. If someone wanted
to use their own local approach, such as one based on e820 tables, I
don't think there would be any interference?

But I doubt that Google's requirements are that different from those of
other users. BadRAM adds a layer of abstraction, but this is not an
office worker's abstraction -- instead it reflects the structures of
hardware, leading to the BadRAM pattern abstraction. I really believe
that Google would be able to work easily with the BadRAM patch if it
was in conflict with their e820-based approach.

> - Google's is, afaik, the largest use case we know of: zillions of
> machines for a number of years. And this real-world experience tells
> us that the badram patchset has shortcomings. Shortcomings which we
> can expect other users to experience.

Please, do show examples and figures of how common they are if you have
anything concrete to counter the pattern-based approach. I am eager
to learn if my experience with a diverse set of individual cases for over
a decade has any shortcomings.


Best wishes,
-Rick

2011-06-23 10:33:24

by Rick van Rein

[permalink] [raw]
Subject: Re: [PATCH v2 0/3] support for broken memory modules (BadRAM)

Hello,

> We already support the equivalent functionality with
> memmap=<address>$<length> for those with only a few ranges...

This is not a realistic option for people whose memory failed.
Google is quite right when they say they hit thousands of erroneous
pages. If you have, say, a static discharge damaging the buffers
from the cell array to the outside world, then the entire row or
column behind that buffer will fail. I've seen many such examples.

> For those with a lot of ranges,
> like Google, the command line is insufficient.

Not if you recognise that there is a pattern :-)

Google does not seem to have realised that, and is simply listing
the pages that are defected. IMHO, but being the BadRAM author I
can hardly be called objective, this is the added value of BadRAM,
that it understands the nature of the problem and solves it with
an elegant concept at the right level of abstraction.

> So far the use case I had in mind wasn't "thousands of entries". However
> expanding the e820 table is probably an issue that could be dealt with
> separately ?

This could help with other approaches as well -- as mentioned,
there have been attempts to get BadRAM into GRUB, so that the
kernel needn't be aware of it. But adding BadRAM or expanding
the e820 table are both cases of changing the kernel, and in that
case I thought it'd be best to actually solve the problem and
not upgrade the messenger.

> Well if too much low memory is bad, you're screwed anyway, not? :)

If the kernel is always loaded in a fixed location, yes. That
is one assumption that the kernel makes (made?) that will only
work if all your memory is good.

> At the moment I don't see any arguments why this patchset couldn't play
> along nicely or get enhanced to support what Google needs, but I don't
> know Googles patches yet.

Changes to e820 should not interfere with setting flags (and
living by them) for failing memory pages. One property of BadRAM,
namely that it does not slow down your system (you have less
pages on hand, but that's all) may or may not apply to an e820-based
approach. I don't know if e820 is ever consulted after boot?

> How common are nontrivial patterns on real hardware? This would be
> interesting to hear from Google or another large user.

Yes. And "non-trivial" would mean that the patterns waste more space
than fair, *because of* the generalisation to patterns.

If you plug 10 DIMMs into your machine, and each has a faulty row
somewhere, then you will get into trouble if you stick to 5 patterns.
But if you happen to run into a faulty DIMM from time to time, the
patterns should be your way out.

> I have to say I think Google's point that truncating the list is
> unacceptable...

Of course, that is true. This is why memmap=... does not work.
It has nothing to do with BadRAM however, there will never be more
than 5 patterns.

> that would mean running in a known-bad configuration,
> and even a hard crash would be better.

...which is so sensible that it was of course taken into account in
the BadRAM design!


Cheers,
-Rick

2011-06-23 10:49:09

by Rick van Rein

[permalink] [raw]
Subject: Re: [PATCH v2 0/3] support for broken memory modules (BadRAM)

Hello,

My last email may have assumed that you knew all about BadRAM; this
is probably worth an expansion:

> If you plug 10 DIMMs into your machine, and each has a faulty row
> somewhere, then you will get into trouble if you stick to 5 patterns.

With "trouble" I mean that a 6th pattern would be merged with the
nearest of the already-found 5 patterns. It may be that this leads
to a pattern that covers more addresses than strictly needed. This
is how I can guarantee that there are never more than 5 patterns,
and so never more than the cmdline can take. No cut-offs are made.

> But if you happen to run into a faulty DIMM from time to time, the
> patterns should be your way out.

...without needing to be more general than really required. Of course,
if all your PCs ran on 10 DIMMs, you could expand the number of
patterns to a comfortably higher number, but what I've seen with the
various cases I've supported, this has never been necessary.

> > that would mean running in a known-bad configuration,
> > and even a hard crash would be better.
>
> ...which is so sensible that it was of course taken into account in
> the BadRAM design!

Meaning, that is why patterns are merged if the exceed the rather high
number of 5 patterns. Rather waste those extra pages than running
into a known fault.

This high number of patterns is not at all common, however, making it
safe to assume that the figure is high enough, in spite of leaving
space on even LILO's cmdline to support adding several other tweaks.


-Rick

2011-06-23 13:40:09

by Matthew Garrett

[permalink] [raw]
Subject: Re: [PATCH v2 0/3] support for broken memory modules (BadRAM)

On Wed, Jun 22, 2011 at 01:18:51PM +0200, Stefan Assmann wrote:
> Following the RFC for the BadRAM feature here's the updated version with
> spelling fixes, thanks go to Randy Dunlap. Also the code is now less verbose,
> as requested by Andi Kleen.
> v2 with even more spelling fixes suggested by Randy.
> Patches are against vanilla 2.6.39.
> Repost with LKML in Cc as suggested by Andrew Morton.

Would it be more reasonable to do this in the bootloader? You'd ideally
want this to be done as early as possible in order to avoid awkward
situations like your ramdisk ending up in the bad RAM area.

--
Matthew Garrett | [email protected]

2011-06-23 14:08:43

by Stefan Assmann

[permalink] [raw]
Subject: Re: [PATCH v2 0/3] support for broken memory modules (BadRAM)

On 23.06.2011 15:39, Matthew Garrett wrote:
> On Wed, Jun 22, 2011 at 01:18:51PM +0200, Stefan Assmann wrote:
>> Following the RFC for the BadRAM feature here's the updated version with
>> spelling fixes, thanks go to Randy Dunlap. Also the code is now less verbose,
>> as requested by Andi Kleen.
>> v2 with even more spelling fixes suggested by Randy.
>> Patches are against vanilla 2.6.39.
>> Repost with LKML in Cc as suggested by Andrew Morton.
>
> Would it be more reasonable to do this in the bootloader? You'd ideally
> want this to be done as early as possible in order to avoid awkward
> situations like your ramdisk ending up in the bad RAM area.

Not sure what exactly you are suggesting here. The kernel somehow needs
to know what memory areas to avoid so we supply this information via
kernel command line.
What the bootloader could do is to allow the kernel/initrd to be loaded
at an alternative address. That's briefly mentioned in the BadRAM
Documentation as well. Is that what you mean or am I missing something?

Stefan

2011-06-23 14:12:41

by Matthew Garrett

[permalink] [raw]
Subject: Re: [PATCH v2 0/3] support for broken memory modules (BadRAM)

On Thu, Jun 23, 2011 at 04:08:32PM +0200, Stefan Assmann wrote:
> On 23.06.2011 15:39, Matthew Garrett wrote:
> > Would it be more reasonable to do this in the bootloader? You'd ideally
> > want this to be done as early as possible in order to avoid awkward
> > situations like your ramdisk ending up in the bad RAM area.
>
> Not sure what exactly you are suggesting here. The kernel somehow needs
> to know what memory areas to avoid so we supply this information via
> kernel command line.
> What the bootloader could do is to allow the kernel/initrd to be loaded
> at an alternative address. That's briefly mentioned in the BadRAM
> Documentation as well. Is that what you mean or am I missing something?

For EFI booting we just hand an e820 map to the kernel. It ought to be
easy enough to add support for that to the 16-bit entry point as well.
Then the bootloader just needs to construct an e820 map of its own. I
think grub2 actually already has some support for this. The advantage of
this approach is that the knowledge of bad memory only has to exist in
one place (ie, the bootloader) - the kernel can remain blisfully
unaware.

--
Matthew Garrett | [email protected]

2011-06-23 15:38:00

by Stefan Assmann

[permalink] [raw]
Subject: Re: [PATCH v2 0/3] support for broken memory modules (BadRAM)

On 23.06.2011 16:12, Matthew Garrett wrote:
> On Thu, Jun 23, 2011 at 04:08:32PM +0200, Stefan Assmann wrote:
>> On 23.06.2011 15:39, Matthew Garrett wrote:
>>> Would it be more reasonable to do this in the bootloader? You'd ideally
>>> want this to be done as early as possible in order to avoid awkward
>>> situations like your ramdisk ending up in the bad RAM area.
>>
>> Not sure what exactly you are suggesting here. The kernel somehow needs
>> to know what memory areas to avoid so we supply this information via
>> kernel command line.
>> What the bootloader could do is to allow the kernel/initrd to be loaded
>> at an alternative address. That's briefly mentioned in the BadRAM
>> Documentation as well. Is that what you mean or am I missing something?
>
> For EFI booting we just hand an e820 map to the kernel. It ought to be
> easy enough to add support for that to the 16-bit entry point as well.
> Then the bootloader just needs to construct an e820 map of its own. I
> think grub2 actually already has some support for this. The advantage of
> this approach is that the knowledge of bad memory only has to exist in
> one place (ie, the bootloader) - the kernel can remain blisfully
> unaware.
>

According to Rick's reply in this thread a damaged row in a DIMM can
easily cause a few thousand entries in the e820 table because it doesn't
handle patterns. So the question I'm asking is, is it acceptable to
have an e820 table with thousands maybe ten-thousands of entries?
I really have no idea of the implications, maybe somebody else can
comment on that.

Stefan

2011-06-23 16:31:17

by H. Peter Anvin

[permalink] [raw]
Subject: Re: [PATCH v2 0/3] support for broken memory modules (BadRAM)

On 06/23/2011 08:37 AM, Stefan Assmann wrote:
>
> According to Rick's reply in this thread a damaged row in a DIMM can
> easily cause a few thousand entries in the e820 table because it doesn't
> handle patterns. So the question I'm asking is, is it acceptable to
> have an e820 table with thousands maybe ten-thousands of entries?
> I really have no idea of the implications, maybe somebody else can
> comment on that.
>

Given that that is what actually ends up happening in the kernel at some
point anyway, I don't see why it would matter.

The bubble sort has to go, but quite frankly stress-testing the range
handling isn't a bad thing.

-hpa

--
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel. I don't speak on their behalf.

2011-06-23 17:00:19

by Andi Kleen

[permalink] [raw]
Subject: Re: [PATCH v2 0/3] support for broken memory modules (BadRAM)

> According to Rick's reply in this thread a damaged row in a DIMM can
> easily cause a few thousand entries in the e820 table because it doesn't
> handle patterns. So the question I'm asking is, is it acceptable to
> have an e820 table with thousands maybe ten-thousands of entries?
> I really have no idea of the implications, maybe somebody else can
> comment on that.

I don't think it makes sense to handle something like that with a list.
The compact representation currently in badram is great for that.

-Andi

2011-06-23 17:12:59

by Luck, Tony

[permalink] [raw]
Subject: RE: [PATCH v2 0/3] support for broken memory modules (BadRAM)

> I don't think it makes sense to handle something like that with a list.
> The compact representation currently in badram is great for that.

I'd tend to agree here. Rick has made a convincing argument that there
are significant numbers of real world cases where a defective row/column
in a DIMM results in a predictable pattern of errors. The ball is now
in Google's court to take a look at their systems that have high numbers
of errors to see if they can actually be described by a small number
of BadRAM patterns as Rick has claimed.

-Tony

2011-06-24 00:59:13

by Andi Kleen

[permalink] [raw]
Subject: Re: [PATCH v2 0/3] support for broken memory modules (BadRAM)

On Thu, Jun 23, 2011 at 09:30:37AM -0700, H. Peter Anvin wrote:
> On 06/23/2011 08:37 AM, Stefan Assmann wrote:
> >
> > According to Rick's reply in this thread a damaged row in a DIMM can
> > easily cause a few thousand entries in the e820 table because it doesn't
> > handle patterns. So the question I'm asking is, is it acceptable to
> > have an e820 table with thousands maybe ten-thousands of entries?
> > I really have no idea of the implications, maybe somebody else can
> > comment on that.
> >
>
> Given that that is what actually ends up happening in the kernel at some
> point anyway,

hwpoison can poison most pages without any lists. Read Stefan's original patch.

The only thing that needs list really is conflict handling with
early allocations.

-Andi

2011-06-24 01:08:38

by Andi Kleen

[permalink] [raw]
Subject: Re: [PATCH v2 0/3] support for broken memory modules (BadRAM)

> We (Google) are working on a data-driven answer for this question. I know
> that there has been some analysis on this topic on the past, but I don't
> want to speculate until we've had some time to put all the pieces together.
> Stay tuned for specifics.

It would be also good if you posted your kernel patches.

It's highly unusual -- to say the least -- to let someone's openly
developed and posted patchkit compete with someone's else secret
internal solution for review purposes.

-Andi

2011-06-24 01:09:18

by Craig Bergstrom

[permalink] [raw]
Subject: Re: [PATCH v2 0/3] support for broken memory modules (BadRAM)

On Thu, Jun 23, 2011 at 10:12 AM, Luck, Tony <[email protected]> wrote:
>> I don't think it makes sense to handle something like that with a list.
>> The compact representation currently in badram is great for that.
>
> I'd tend to agree here. ?Rick has made a convincing argument that there
> are significant numbers of real world cases where a defective row/column
> in a DIMM results in a predictable pattern of errors. ?The ball is now
> in Google's court to take a look at their systems that have high numbers
> of errors to see if they can actually be described by a small number
> of BadRAM patterns as Rick has claimed.
>

Hi All,

We (Google) are working on a data-driven answer for this question. I
know that there has been some analysis on this topic on the past, but
I don't want to speculate until we've had some time to put all the
pieces together. Stay tuned for specifics.

Cheers,
CraigB


> -Tony
> --
> 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/
>

2011-06-24 01:22:23

by Craig Bergstrom

[permalink] [raw]
Subject: Re: [PATCH v2 0/3] support for broken memory modules (BadRAM)

On Thu, Jun 23, 2011 at 6:08 PM, Andi Kleen <[email protected]> wrote:
>> We (Google) are working on a data-driven answer for this question. ?I know
>> that there has been some analysis on this topic on the past, but I don't
>> want to speculate until we've had some time to put all the pieces together.
>> ?Stay tuned for specifics.
>
> It would be also good if you posted your kernel patches.
>
> It's highly unusual -- to say the least -- to let someone's openly
> developed and posted patchkit compete with someone's else secret
> internal solution for review purposes.

Hi Andi,

This is quite hard to argue with. Let me see what I can do.

Cheers,
CraigB

> -Andi
>

2011-06-24 08:05:44

by Rick van Rein

[permalink] [raw]
Subject: Re: [PATCH v2 0/3] support for broken memory modules (BadRAM)

Hi Craig,

> We (Google) are working on a data-driven answer for this question. I know
> that there has been some analysis on this topic on the past, but I don't
> want to speculate until we've had some time to put all the pieces together.

The easiest way to do this could be to take the algorithm from Memtest86
and apply it to your data, to see if it finds suitable patterns for the
cases tried.

By counting bits set to zero in the masks, you could then determine how
'tight' they are. A mask with all-ones covers one memory page; each
zero bit in the mask (outside of the CPU's page size) doubles the number
of pages covered.

You can ignore the address over which the mask is applied, although you
would then be assuming that all the pages covered by the mask are indeed
filled with RAM.

You would want to add the figures for the different masks.

I am very curious about your findings. Independently of those, I am in
favour of a patch that enables longer e820 tables if it has no further
impact on speed or space.


Cheers,
-Rick

2011-06-24 14:36:01

by Craig Bergstrom

[permalink] [raw]
Subject: Re: [PATCH v2 0/3] support for broken memory modules (BadRAM)

On Fri, Jun 24, 2011 at 1:05 AM, Rick van Rein <[email protected]> wrote:
>
> Hi Craig,
>
> > We (Google) are working on a data-driven answer for this question. ?I know
> > that there has been some analysis on this topic on the past, but I don't
> > want to speculate until we've had some time to put all the pieces together.
>
> The easiest way to do this could be to take the algorithm from Memtest86
> and apply it to your data, to see if it finds suitable patterns for the
> cases tried.
>
> By counting bits set to zero in the masks, you could then determine how
> 'tight' they are. ?A mask with all-ones covers one memory page; each
> zero bit in the mask (outside of the CPU's page size) doubles the number
> of pages covered.
>
> You can ignore the address over which the mask is applied, although you
> would then be assuming that all the pages covered by the mask are indeed
> filled with RAM.
>
> You would want to add the figures for the different masks.

This seems like a reasonable approach. I know there was some analysis
done, and I'm doing my best to get the folks who made the original
decision to weigh in.

>
> I am very curious about your findings. ?Independently of those, I am in
> favour of a patch that enables longer e820 tables if it has no further
> impact on speed or space.

I think that we'd all be satisfied with a mechanism that allows for
badram to be specified via both command line and an extended e820 map.

>
>
> Cheers,
> ?-Rick

2011-06-24 16:16:47

by H. Peter Anvin

[permalink] [raw]
Subject: Re: [PATCH v2 0/3] support for broken memory modules (BadRAM)

On 06/24/2011 01:05 AM, Rick van Rein wrote:
>
> I am very curious about your findings. Independently of those, I am in
> favour of a patch that enables longer e820 tables if it has no further
> impact on speed or space.
>

That is already in the mainline kernel, although only if fed from the
boot loader (it was developed in the context of mega-NUMA machines); the
stub fetching from INT 15h doesn't use this at the moment.

-hpa

2011-06-24 16:46:33

by Luck, Tony

[permalink] [raw]
Subject: RE: [PATCH v2 0/3] support for broken memory modules (BadRAM)

> > I am very curious about your findings. Independently of those, I am in
> > favour of a patch that enables longer e820 tables if it has no further
> > impact on speed or space.
> >
>
> That is already in the mainline kernel, although only if fed from the
> boot loader (it was developed in the context of mega-NUMA machines); the
> stub fetching from INT 15h doesn't use this at the moment.

Does it scale? Current X86 systems go up to about 2TB - presumably
in the form of 256 8GB DIMMs (or maybe 512 4GB ones). If a faulty
row or column on a DIMM can give rise to 4K bad pages, then these
large systems could conceivably have 1-2 million bad pages (while
still being quite usable - a loss of 4-8G from a 2TB system is down
in the noise). Can we handle a 2 million entry e820 table? Do we
want to?

Perhaps we may end up with a composite solution. Use e820 to map out
the bad pages below some limit (like 4GB). Preferably in the boot loader
so it can find a range of good memory to load the kernel. Then use
badRAM patterns for addresses over 4GB for Linux to avoid bad pages
by flagging their page structures.

-Tony

2011-06-24 16:56:17

by Rick van Rein

[permalink] [raw]
Subject: Re: [PATCH v2 0/3] support for broken memory modules (BadRAM)

Hello,

> Does it scale? [...] Perhaps we may end up with a composite solution.

If I had my way, there would be an extension to the e820 format to allow
the BadRAM patterns to be specified. Since the extension with bad page
information is specific to boot loader interaction, this would work in
exactly those cases that are covered by the current situation.

-Rick

2011-06-24 17:15:12

by H. Peter Anvin

[permalink] [raw]
Subject: Re: [PATCH v2 0/3] support for broken memory modules (BadRAM)

On 06/24/2011 09:56 AM, Rick van Rein wrote:
> Hello,
>
>> Does it scale? [...] Perhaps we may end up with a composite solution.
>
> If I had my way, there would be an extension to the e820 format to allow
> the BadRAM patterns to be specified. Since the extension with bad page
> information is specific to boot loader interaction, this would work in
> exactly those cases that are covered by the current situation.
>

Yes, a different table might be worthwhile.

Another question, however, is what does this look like at runtime. In
particular, if I'm not mistaken hwpoison will create struct pages for
these non-memory pages, which seems undesirable...

-hpa

2011-06-24 21:10:57

by Shane Nay

[permalink] [raw]
Subject: Re: [PATCH v2 0/3] support for broken memory modules (BadRAM)


> > For those with a lot of ranges,
> > like Google, the command line is insufficient.
>
> Not if you recognise that there is a pattern :-)
>
> Google does not seem to have realised that, and is simply listing
> the pages that are defected. IMHO, but being the BadRAM author I
> can hardly be called objective, this is the added value of BadRAM,
> that it understands the nature of the problem and solves it with
> an elegant concept at the right level of abstraction.

No, we have realized patterns when there is one. It depends on the specific defect that is at play. There are several different defect types, and incidence rate with respect to the defect being observed. We do observe "classic" failures of the type you are describing, where with the physical addressing information (bank, row, column), we can reproducibly cause errors to occur along that path.

One problem is that badram syntax doesn't cleanly mesh with all modern systems. For instance, not all chipsets have power-of-two bank interleave. Holes in addressing also create trouble on some systems.

Other defects look like white noise, these are typically indicative of manufacturing process defects.

When we find a crisp-pattern in the data, it's not always the entirety of that bit-maskable pattern which is effected. There can be interleaved subtractions from the underlying pattern orthogonal to interleave.

IMHO, badram is a good tool for it's intended purpose. They aren't really mutually exclusive anyway. We're cleaning up our existing patches to send out early next week. However, we had at one time had a way of inserting badram syntax generated e820's from command line along with passed in e820's, and extended versions. That bit isn't in our tree right now, but it's possible, and we're looking to see if we can make it work with the existing code.


> s (and
> living by them) for failing memory pages. One property of BadRAM,
> namely that it does not slow down your system (you have less
> pages on hand, but that's all) may or may not apply to an e820-based
> approach. I don't know if e820 is ever consulted after boot?
>
> > How common are nontrivial patterns on real hardware? This would be
> > interesting to hear from Google or another large user.
>
> Yes. And "non-trivial" would mean that the patterns waste more space
> than fair, *because of* the generalisation to patterns.
>
> If you plug 10 DIMMs into your machine, and each has a faulty row
> somewhere, then you will get into trouble if you stick to 5 patterns.
> But if you happen to run into a faulty DIMM from time to time, the
> patterns should be your way out.
>
> > I have to say I think Google's point that truncating the list is
> > unacceptable...
>
> Of course, that is true. This is why memmap=... does not work.
> It has nothing to do with BadRAM however, there will never be more
> than 5 patterns.
>
> > that would mean running in a known-bad configuration,
> > and even a hard crash would be better.
>
> ..which is so sensible that it was of course taken into account in
> the BadRAM design!
>
>
> Cheers,
> -Rick
> --
> 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/

2011-06-28 02:35:30

by Craig Bergstrom

[permalink] [raw]
Subject: Re: [PATCH v2 0/3] support for broken memory modules (BadRAM)

Hi All,

Just a quick update regarding the outstanding request for the
submission of Google's BadRAM patch.

I'm still making some final changes to Google's e820-based BadRAM
patch and plan to send it as an RFC patch to LKML soon (most likely
tomorrow).

Some folks had mentioned that they're interested in details about what
we've learned about bad ram from our fleet of machines. I suspect
that you need ACM portal access to read this, but for those folks an
interesting read can be found at the link shown below. My sincere
apologies that I cannot post a world-readable copy.

http://portal.acm.org/citation.cfm?id=1555372

Cheers,
CraigB

On Fri, Jun 24, 2011 at 2:10 PM, Shane Nay <[email protected]> wrote:
>
>> > For those with a lot of ranges,
>> > like Google, the command line is insufficient.
>>
>> Not if you recognise that there is a pattern :-)
>>
>> Google does not seem to have realised that, and is simply listing
>> the pages that are defected. ?IMHO, but being the BadRAM author I
>> can hardly be called objective, this is the added value of BadRAM,
>> that it understands the nature of the problem and solves it with
>> an elegant concept at the right level of abstraction.
>
> No, we have realized patterns when there is one. ?It depends on the specific defect that is at play. ?There are several different defect types, and incidence rate with respect to the defect being observed. ?We do observe "classic" failures of the type you are describing, where with the physical addressing information (bank, row, column), we can reproducibly cause errors to occur along that path.
>
> One problem is that badram syntax doesn't cleanly mesh with all modern systems. ?For instance, not all chipsets have power-of-two bank interleave. ?Holes in addressing also create trouble on some systems.
>
> Other defects look like white noise, these are typically indicative of manufacturing process defects.
>
> When we find a crisp-pattern in the data, it's not always the entirety of that bit-maskable pattern which is effected. ?There can be interleaved subtractions from the underlying pattern orthogonal to interleave.
>
> IMHO, badram is a good tool for it's intended purpose. ?They aren't really mutually exclusive anyway. ?We're cleaning up our existing patches to send out early next week. ?However, we had at one time had a way of inserting badram syntax generated e820's from command line along with passed in e820's, and extended versions. ?That bit isn't in our tree right now, but it's possible, and we're looking to see if we can make it work with the existing code.
>
>
>> s (and
>> living by them) for failing memory pages. ?One property of BadRAM,
>> namely that it does not slow down your system (you have less
>> pages on hand, but that's all) may or may not apply to an e820-based
>> approach. ?I don't know if e820 is ever consulted after boot?
>>
>> > How common are nontrivial patterns on real hardware? ?This would be
>> > interesting to hear from Google or another large user.
>>
>> Yes. ?And "non-trivial" would mean that the patterns waste more space
>> than fair, *because of* the generalisation to patterns.
>>
>> If you plug 10 DIMMs into your machine, and each has a faulty row
>> somewhere, then you will get into trouble if you stick to 5 patterns.
>> But if you happen to run into a faulty DIMM from time to time, the
>> patterns should be your way out.
>>
>> > I have to say I think Google's point that truncating the list is
>> > unacceptable...
>>
>> Of course, that is true. ?This is why memmap=... does not work.
>> It has nothing to do with BadRAM however, there will never be more
>> than 5 patterns.
>>
>> > that would mean running in a known-bad configuration,
>> > and even a hard crash would be better.
>>
>> ..which is so sensible that it was of course taken into account in
>> the BadRAM design!
>>
>>
>> Cheers,
>> ?-Rick
>> --
>> 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/
>
> --
> 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/
>

2011-06-29 08:08:36

by Rick van Rein

[permalink] [raw]
Subject: Re: [PATCH v2 0/3] support for broken memory modules (BadRAM)

Hello Craig,

> Some folks had mentioned that they're interested in details about what
> we've learned about bad ram from our fleet of machines. I suspect
> that you need ACM portal access to read this,

I'm happy that this didn't cause a flame, but clearly this is not the
right response in an open environment. ACM may have copyright on the
*form* in which you present your knowledge, but could you please poor
the knowledge in another form that bypasses their copyright so the
knowledge is made available to all?


Thanks,
-Rick

2011-06-29 15:29:57

by craig lkml

[permalink] [raw]
Subject: Re: [PATCH v2 0/3] support for broken memory modules (BadRAM)

Hi Rick,

Thanks for your response. My sincere apologies for not posting the
work directly.

My intention is to point interested parties to contributions that
Google has made to this space through known and respected channels.
The cited research is not my research but the research of my
colleagues. As a result, I hesitate to paraphrase the work as I will
likely get the details wrong. In any case, Shane's points are the
most relevant for the discussion here. Please refer to his post in
this thread.

In an attempt to contribute to the community as much as I can, I have
prepared and mailed our BadRAM patch as requested. In case it is not
otherwise clear, my belief is that the ideal solution for the upstream
kernel is a hybrid of our approaches.

Thank you,
CraigB

On Wed, Jun 29, 2011 at 1:08 AM, Rick van Rein <[email protected]> wrote:
> Hello Craig,
>
>> Some folks had mentioned that they're interested in details about what
>> we've learned about bad ram from our fleet of machines. ?I suspect
>> that you need ACM portal access to read this,
>
> I'm happy that this didn't cause a flame, but clearly this is not the
> right response in an open environment. ?ACM may have copyright on the
> *form* in which you present your knowledge, but could you please poor
> the knowledge in another form that bypasses their copyright so the
> knowledge is made available to all?
>
>
> Thanks,
> ?-Rick
> --
> 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/
>

2011-06-29 16:06:22

by Craig Bergstrom

[permalink] [raw]
Subject: Re: [PATCH v2 0/3] support for broken memory modules (BadRAM)

My apologies, I send this initial reply from the wrong address. Please
reply to this @google.com address.

Cheers,
CraigB

On Wed, Jun 29, 2011 at 8:28 AM, craig lkml <[email protected]> wrote:
> Hi Rick,
> Thanks for your response. ?My sincere apologies for not posting the work
> directly.
> My intention is to point interested parties to contributions that Google has
> made to this space through known and respected channels. ?The cited research
> is not my research but the research of my?colleagues. ?As a result, I
> hesitate to paraphrase the work as I will likely get the details wrong. ?In
> any case, Shane's points are the most relevant for the discussion here.
> ?Please refer to his post in this thread.
> In an attempt to contribute to the community as much as I can, I have
> prepared and mailed our BadRAM patch as requested. ?In case it is not
> otherwise clear, my belief is that the ideal solution for the upstream
> kernel is a hybrid of our approaches.
> Thank you,
> CraigB
>
> On Wed, Jun 29, 2011 at 1:08 AM, Rick van Rein <[email protected]> wrote:
>>
>> Hello Craig,
>>
>> > Some folks had mentioned that they're interested in details about what
>> > we've learned about bad ram from our fleet of machines. ?I suspect
>> > that you need ACM portal access to read this,
>>
>> I'm happy that this didn't cause a flame, but clearly this is not the
>> right response in an open environment. ?ACM may have copyright on the
>> *form* in which you present your knowledge, but could you please poor
>> the knowledge in another form that bypasses their copyright so the
>> knowledge is made available to all?
>>
>>
>> Thanks,
>> ?-Rick
>> --
>> 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/
>
>

2011-06-29 21:24:45

by Luck, Tony

[permalink] [raw]
Subject: Re: [PATCH v2 0/3] support for broken memory modules (BadRAM)

One extra consideration for this whole proposal ...

Is the "physical address" a stable enough representation of the location
of the faulty memory cells?

On high end systems I can see a number of ways where the mapping
from cells to physical address may change across reboot:

1) System support redundant memory (rank sparing or mirroring)
2) BIOS self test removes some memory from use
3) A multi-node system elects a different node to be boot-meister,
which results in reshuffling of the address map.

If any of these can happen: then it doesn't matter whether we have
a list of addresses, or a pattern that expands to a list of addresses.
We'll still mark some innocent memory as bad, and allow some known
bad memory to be used - because our "addresses" no longer correspond
to the bad memory cells.

-Tony

2011-06-30 14:33:03

by Jody Belka

[permalink] [raw]
Subject: Re: [PATCH v2 0/3] support for broken memory modules (BadRAM)

On 29 June 2011 09:08, Rick van Rein <[email protected]> wrote:
>
> Hello Craig,
>
> > Some folks had mentioned that they're interested in details about what
> > we've learned about bad ram from our fleet of machines.  I suspect
> > that you need ACM portal access to read this,
>
> I'm happy that this didn't cause a flame, but clearly this is not the
> right response in an open environment.  ACM may have copyright on the
> *form* in which you present your knowledge, but could you please poor
> the knowledge in another form that bypasses their copyright so the
> knowledge is made available to all?

Luckily one of the authors (Bianca Schroeder) has a copy on her
university web space, free for personal/classroom use. Can be found at
http://www.cs.toronto.edu/~bianca/, search for "DRAM errors in the
wild".