Skip to contentSkip to conversation

RAS: Add CEC collector and deprecate mcelog

PATCH In review Borislav Petkov 2 participants 15 messages Mar 9, 2017 – Mar 24, 2017

Subsystems (3)
lore
Share & tools
lore

Borislav Petkov

author maintainer
#0 lore raw

cover RAS: Add CEC collector and deprecate mcelog

From: Borislav Petkov <[email protected]>

Hi,

here's the latest incarnation of the CEC collector. I think I've taken care of all review comments but feel free to correct me here. The introductory comment in cec.c should explain the whole deal - I'm referring to there so that we have that text in the actual source and not spread it around commit messages. So pls have a look there for more info.

The thing has knobs in debugfs now which can control its operation, I hope I've chosen sane default values.

Borislav Petkov (3):
x86/MCE: Rename mce_log()'s argument
x86/MCE: Rename mce_log to mce_log_buffer
RAS: Add a Corrected Errors Collector

Tony Luck (1):
x86/mce: Deprecate /dev/mcelog

 Documentation/admin-guide/kernel-parameters.txt |   6 +
 arch/x86/Kconfig                                |  10 +-
 arch/x86/include/asm/mce.h                      |  12 +-
 arch/x86/kernel/cpu/mcheck/Makefile             |   2 +
 arch/x86/kernel/cpu/mcheck/dev-mcelog.c         | 397 ++++++++++++++++++
 arch/x86/kernel/cpu/mcheck/mce-internal.h       |   8 +
 arch/x86/kernel/cpu/mcheck/mce.c                | 493 ++++------------------
 arch/x86/ras/Kconfig                            |  14 +
 drivers/ras/Makefile                            |   3 +-
 drivers/ras/cec.c                               | 530 ++++++++++++++++++++++++
 drivers/ras/debugfs.c                           |   2 +-
 drivers/ras/debugfs.h                           |   8 +
 drivers/ras/ras.c                               |  11 +
 include/linux/ras.h                             |  13 +-
 14 files changed, 1097 insertions(+), 412 deletions(-)
 create mode 100644 arch/x86/kernel/cpu/mcheck/dev-mcelog.c
 create mode 100644 drivers/ras/cec.c
 create mode 100644 drivers/ras/debugfs.h
Signature · 1 line
2.11.0
author maintainer
#1 lore raw

PATCH 1/4 x86/MCE: Rename mce_log()'s argument

From: Borislav Petkov <[email protected]>

We call it everywhere "struct mce *m". Adjust that here too to avoid confusion.

No functionality change.

---
 arch/x86/kernel/cpu/mcheck/mce.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
arch/x86/kernel/cpu/mcheck/mce.c +4 -4
index 177472ace838..dee5db6065cb 100644
@@ -156,14 +156,14 @@ static struct mce_log mcelog = {
 	.recordlen	= sizeof(struct mce),
 };
 
-void mce_log(struct mce *mce)
+void mce_log(struct mce *m)
 {
 	unsigned next, entry;
 
 	/* Emit the trace record: */
-	trace_mce_record(mce);
+	trace_mce_record(m);
 
-	if (!mce_gen_pool_add(mce))
+	if (!mce_gen_pool_add(m))
 		irq_work_queue(&mce_irq_work);
 
 	wmb();
@@ -193,7 +193,7 @@ void mce_log(struct mce *mce)
 		if (cmpxchg(&mcelog.next, entry, next) == entry)
 			break;
 	}
-	memcpy(mcelog.entry + entry, mce, sizeof(struct mce));
+	memcpy(mcelog.entry + entry, m, sizeof(struct mce));
 	wmb();
 	mcelog.entry[entry].finished = 1;
 	wmb();
Signature · 1 line
2.11.0
author maintainer
#2 lore raw

PATCH 2/4 x86/MCE: Rename mce_log to mce_log_buffer

In reply to Borislav Petkov

From: Borislav Petkov <[email protected]>

It is confusing when staring at "struct mce_log mcelog" and then there's also a function called mce_log(). So call the buffer what it is.

No functionality change.

---
 arch/x86/include/asm/mce.h       |  2 +-
 arch/x86/kernel/cpu/mcheck/mce.c | 30 +++++++++++++++---------------
 2 files changed, 16 insertions(+), 16 deletions(-)
arch/x86/include/asm/mce.h +1 -1
index e63873683d4a..0512dcc11750 100644
@@ -128,7 +128,7 @@
  * debugging tools.  Each entry is only valid when its finished flag
  * is set.
  */
-struct mce_log {
+struct mce_log_buffer {
 	char signature[12]; /* "MACHINECHECK" */
 	unsigned len;	    /* = MCE_LOG_LEN */
 	unsigned next;
arch/x86/kernel/cpu/mcheck/mce.c +15 -15
index dee5db6065cb..8a347f98eda2 100644
@@ -150,7 +150,7 @@ EXPORT_PER_CPU_SYMBOL_GPL(injectm);
  * separate MCEs from kernel messages to avoid bogus bug reports.
  */
 
-static struct mce_log mcelog = {
+static struct mce_log_buffer mcelog_buf = {
 	.signature	= MCE_LOG_SIGNATURE,
 	.len		= MCE_LOG_LEN,
 	.recordlen	= sizeof(struct mce),
@@ -168,7 +168,7 @@ void mce_log(struct mce *m)
 
 	wmb();
 	for (;;) {
-		entry = mce_log_get_idx_check(mcelog.next);
+		entry = mce_log_get_idx_check(mcelog_buf.next);
 		for (;;) {
 
 			/*
@@ -178,11 +178,11 @@ void mce_log(struct mce *m)
 			 */
 			if (entry >= MCE_LOG_LEN) {
 				set_bit(MCE_OVERFLOW,
-					(unsigned long *)&mcelog.flags);
+					(unsigned long *)&mcelog_buf.flags);
 				return;
 			}
 			/* Old left over entry. Skip: */
-			if (mcelog.entry[entry].finished) {
+			if (mcelog_buf.entry[entry].finished) {
 				entry++;
 				continue;
 			}
@@ -190,12 +190,12 @@ void mce_log(struct mce *m)
 		}
 		smp_rmb();
 		next = entry + 1;
-		if (cmpxchg(&mcelog.next, entry, next) == entry)
+		if (cmpxchg(&mcelog_buf.next, entry, next) == entry)
 			break;
 	}
-	memcpy(mcelog.entry + entry, m, sizeof(struct mce));
+	memcpy(mcelog_buf.entry + entry, m, sizeof(struct mce));
 	wmb();
-	mcelog.entry[entry].finished = 1;
+	mcelog_buf.entry[entry].finished = 1;
 	wmb();
 
 	set_bit(0, &mce_need_notify);
@@ -1947,7 +1947,7 @@ static ssize_t mce_chrdev_read(struct file *filp, char __user *ubuf,
 			goto out;
 	}
 
-	next = mce_log_get_idx_check(mcelog.next);
+	next = mce_log_get_idx_check(mcelog_buf.next);
 
 	/* Only supports full reads right now */
 	err = -EINVAL;
@@ -1959,7 +1959,7 @@ static ssize_t mce_chrdev_read(struct file *filp, char __user *ubuf,
 	do {
 		for (i = prev; i < next; i++) {
 			unsigned long start = jiffies;
-			struct mce *m = &mcelog.entry[i];
+			struct mce *m = &mcelog_buf.entry[i];
 
 			while (!m->finished) {
 				if (time_after_eq(jiffies, start + 2)) {
@@ -1975,10 +1975,10 @@ static ssize_t mce_chrdev_read(struct file *filp, char __user *ubuf,
 			;
 		}
 
-		memset(mcelog.entry + prev, 0,
+		memset(mcelog_buf.entry + prev, 0,
 		       (next - prev) * sizeof(struct mce));
 		prev = next;
-		next = cmpxchg(&mcelog.next, prev, 0);
+		next = cmpxchg(&mcelog_buf.next, prev, 0);
 	} while (next != prev);
 
 	synchronize_sched();
@@ -1990,7 +1990,7 @@ static ssize_t mce_chrdev_read(struct file *filp, char __user *ubuf,
 	on_each_cpu(collect_tscs, cpu_tsc, 1);
 
 	for (i = next; i < MCE_LOG_LEN; i++) {
-		struct mce *m = &mcelog.entry[i];
+		struct mce *m = &mcelog_buf.entry[i];
 
 		if (m->finished && m->tsc < cpu_tsc[m->cpu]) {
 			err |= copy_to_user(buf, m, sizeof(*m));
@@ -2013,7 +2013,7 @@ static ssize_t mce_chrdev_read(struct file *filp, char __user *ubuf,
 static unsigned int mce_chrdev_poll(struct file *file, poll_table *wait)
 {
 	poll_wait(file, &mce_chrdev_wait, wait);
-	if (READ_ONCE(mcelog.next))
+	if (READ_ONCE(mcelog_buf.next))
 		return POLLIN | POLLRDNORM;
 	if (!mce_apei_read_done && apei_check_mce())
 		return POLLIN | POLLRDNORM;
@@ -2037,8 +2037,8 @@ static long mce_chrdev_ioctl(struct file *f, unsigned int cmd,
 		unsigned flags;
 
 		do {
-			flags = mcelog.flags;
-		} while (cmpxchg(&mcelog.flags, flags, 0) != flags);
+			flags = mcelog_buf.flags;
+		} while (cmpxchg(&mcelog_buf.flags, flags, 0) != flags);
 
 		return put_user(flags, p);
 	}
Signature · 1 line
2.11.0
author maintainer
#3 lore raw

PATCH 3/4 RAS: Add a Corrected Errors Collector

In reply to Borislav Petkov

From: Borislav Petkov <[email protected]>

A simple data structure for collecting correctable errors along with accessors. More detailed description in the code itself.

The error decoding is done with the decoding chain now and mce_first_notifier() gets to see the error first and the CEC decides whether to log it and then the rest of the chain doesn't hear about it - basically the main reason for the CE collector - or to continue running the notifiers.

When the CEC hits the action threshold, it will try to soft-offine the page containing the ECC and then the whole decoding chain gets to see the error.

---
 Documentation/admin-guide/kernel-parameters.txt |   6 +
 arch/x86/include/asm/mce.h                      |   9 +-
 arch/x86/kernel/cpu/mcheck/mce.c                | 188 +++++----
 arch/x86/ras/Kconfig                            |  14 +
 drivers/ras/Makefile                            |   3 +-
 drivers/ras/cec.c                               | 530 ++++++++++++++++++++++++
 drivers/ras/debugfs.c                           |   2 +-
 drivers/ras/debugfs.h                           |   8 +
 drivers/ras/ras.c                               |  11 +
 include/linux/ras.h                             |  13 +-
 10 files changed, 701 insertions(+), 83 deletions(-)
 create mode 100644 drivers/ras/cec.c
 create mode 100644 drivers/ras/debugfs.h
Documentation/admin-guide/kernel-parameters.txt +6 -0
index 986e44387dad..7a0844cff130 100644
@@ -3163,6 +3163,12 @@
 	ramdisk_size=	[RAM] Sizes of RAM disks in kilobytes
 			See Documentation/blockdev/ramdisk.txt.
 
+	ras=option[,option,...]	[KNL] RAS-specific options
+
+		cec_disable	[X86]
+				Disable the Correctable Errors Collector,
+				see CONFIG_RAS_CEC help text.
+
 	rcu_nocbs=	[KNL]
 			The argument is a cpu list, as described above.
 
arch/x86/include/asm/mce.h +5 -4
index 0512dcc11750..c5ae545d27d8 100644
@@ -191,10 +191,11 @@ extern struct mca_config mca_cfg;
 extern struct mca_msr_regs msr_ops;
 
 enum mce_notifier_prios {
-	MCE_PRIO_SRAO		= INT_MAX,
-	MCE_PRIO_EXTLOG		= INT_MAX - 1,
-	MCE_PRIO_NFIT		= INT_MAX - 2,
-	MCE_PRIO_EDAC		= INT_MAX - 3,
+	MCE_PRIO_FIRST		= INT_MAX,
+	MCE_PRIO_SRAO		= INT_MAX - 1,
+	MCE_PRIO_EXTLOG		= INT_MAX - 2,
+	MCE_PRIO_NFIT		= INT_MAX - 3,
+	MCE_PRIO_EDAC		= INT_MAX - 4,
 	MCE_PRIO_LOWEST		= 0,
 };
 
arch/x86/kernel/cpu/mcheck/mce.c +112 -76

244 lines not on this page · raw message

arch/x86/ras/Kconfig +14 -0
index 0bc60a308730..2a2d89d39af6 100644
@@ -7,3 +7,17 @@ config MCE_AMD_INJ
 	  aspects of the MCE handling code.
 
 	  WARNING: Do not even assume this interface is staying stable!
+
+config RAS_CEC
+	bool "Correctable Errors Collector"
+	depends on X86_MCE && MEMORY_FAILURE && DEBUG_FS
+	---help---
+	  This is a small cache which collects correctable memory errors per 4K
+	  page PFN and counts their repeated occurrence. Once the counter for a
+	  PFN overflows, we try to soft-offline that page as we take it to mean
+	  that it has reached a relatively high error count and would probably
+	  be best if we don't use it anymore.
+
+	  Bear in mind that this is absolutely useless if your platform doesn't
+	  have ECC DIMMs and doesn't have DRAM ECC checking enabled in the BIOS.
+
drivers/ras/Makefile +2 -1
index d7f73341ced3..7b26dd3aa5d0 100644
@@ -1 +1,2 @@
-obj-$(CONFIG_RAS) += ras.o debugfs.o
+obj-$(CONFIG_RAS)	+= ras.o debugfs.o
+obj-$(CONFIG_RAS_CEC)	+= cec.o
drivers/ras/cec.c new file +530 -0

530 lines not on this page · raw message

drivers/ras/debugfs.c +1 -1
index 0322acf67ea5..501603057dff 100644
@@ -1,6 +1,6 @@
 #include <linux/debugfs.h>
 
-static struct dentry *ras_debugfs_dir;
+struct dentry *ras_debugfs_dir;
 
 static atomic_t trace_count = ATOMIC_INIT(0);
 
drivers/ras/debugfs.h new file +8 -0
new file mode 100644
index 000000000000..db72e4513191
@@ -0,0 +1,8 @@
+#ifndef __RAS_DEBUGFS_H__
+#define __RAS_DEBUGFS_H__
+
+#include <linux/debugfs.h>
+
+extern struct dentry *ras_debugfs_dir;
+
+#endif /* __RAS_DEBUGFS_H__ */
drivers/ras/ras.c +11 -0
index b67dd362b7b6..94f8038864b4 100644
@@ -27,3 +27,14 @@ subsys_initcall(ras_init);
 EXPORT_TRACEPOINT_SYMBOL_GPL(extlog_mem_event);
 #endif
 EXPORT_TRACEPOINT_SYMBOL_GPL(mc_event);
+
+
+int __init parse_ras_param(char *str)
+{
+#ifdef CONFIG_RAS_CEC
+	parse_cec_param(str);
+#endif
+
+	return 1;
+}
+__setup("ras", parse_ras_param);
include/linux/ras.h +12 -1
index 2aceeafd6fe5..ffb147185e8d 100644
@@ -1,14 +1,25 @@
 #ifndef __RAS_H__
 #define __RAS_H__
 
+#include <asm/errno.h>
+
 #ifdef CONFIG_DEBUG_FS
 int ras_userspace_consumers(void);
 void ras_debugfs_init(void);
 int ras_add_daemon_trace(void);
 #else
 static inline int ras_userspace_consumers(void) { return 0; }
-static inline void ras_debugfs_init(void) { return; }
+static inline void ras_debugfs_init(void) { }
 static inline int ras_add_daemon_trace(void) { return 0; }
 #endif
 
+#ifdef CONFIG_RAS_CEC
+void __init cec_init(void);
+int __init parse_cec_param(char *str);
+int cec_add_elem(u64 pfn);
+#else
+static inline void __init cec_init(void)	{ }
+static inline int cec_add_elem(u64 pfn)		{ return -ENODEV; }
 #endif
+
+#endif /* __RAS_H__ */
Signature · 1 line
2.11.0
author maintainer
#4 lore raw

Re: [PATCH 3/4] RAS: Add a Corrected Errors Collector

Quoted 1 line · Borislav Petkov

From: Borislav Petkov <[email protected]>

...

arch/x86/ras/Kconfig config MCE_AMD_INJ · line 14

Quoted 13 lines
arch/x86/ras/Kconfig +5 -0
index 0bc60a308730..2a2d89d39af6 100644
@@ -7,3 +7,17 @@ config MCE_AMD_INJ
 	  aspects of the MCE handling code.
 
 	  WARNING: Do not even assume this interface is staying stable!
+
+config RAS_CEC
+	bool "Correctable Errors Collector"
+	depends on X86_MCE && MEMORY_FAILURE && DEBUG_FS
+	---help---

Btw, we should run it a couple releases like this and once we have the warm fuzzy feeling, make it default y.

Signature · 1 line
Sent from a small device: formatting sux and brevity is inevitable. 
maintainer
#5 lore raw

Re: [PATCH 3/4] RAS: Add a Corrected Errors Collector

In reply to Borislav Petkov

arch/x86/ras/Kconfig config MCE_AMD_INJ · line 19

Quoted 9 lines · Borislav Petkov
arch/x86/ras/Kconfig +9 -0
+config RAS_CEC
+	bool "Correctable Errors Collector"
+	depends on X86_MCE && MEMORY_FAILURE && DEBUG_FS
+	---help---
+	  This is a small cache which collects correctable memory errors per 4K
+	  page PFN and counts their repeated occurrence. Once the counter for a
+	  PFN overflows, we try to soft-offline that page as we take it to mean
+	  that it has reached a relatively high error count and would probably
+	  be best if we don't use it anymore.

You added "count_threshold" for me ... so the condition isn't quite "overflows" like it was in the early versions.

We may need to give some thought on what to do if the attempt to offline the page fails (e.g. because the page belongs to the kernel). Right now you delete it from the list, but we will see more errors as the page is still in use. Eventually the counter will hit count_threshold and we will try to offline again. Rinse, repeat.

Someone also recently sent me a log from a machine with corrected errors in over 9000 unique addresses. Need a parameter to allocate more than one page for the collector, or a way to grow the space.

-Tony

author maintainer
#6 lore raw

Re: [PATCH 3/4] RAS: Add a Corrected Errors Collector

Quoted 2 lines · Tony

You added "count_threshold" for me ... so the condition isn't quite "overflows" like it was in the early versions.

It is a max count which, when reached, causes the soft offline attempt. What did you mean with "overflows" exactly then?

Quoted 5 lines

We may need to give some thought on what to do if the attempt to offline the page fails (e.g. because the page belongs to the kernel). Right now you delete it from the list, but we will see more errors as the page is still in use. Eventually the counter will hit count_threshold and we will try to offline again. Rinse, repeat.

Well, what *is* there we can do? If the offlining code can't offline it, there's not a whole lot we *can* do. The error would keep repeating as a corrected error, rinse, repeat and we will keep trying to offline containing page.

That is, until it degrades to an uncorrectable error and then we're dead.

Either way, the collector can't really do anything about it. This would be beyond its functionality anyway.

IMO.

Quoted 3 lines

Someone also recently sent me a log from a machine with corrected errors in over 9000 unique addresses. Need a parameter to allocate more than one page for the collector, or a way to grow the space.

Well, so even with the amount of unique addresses higher than the CEC slots, we should be able to deal with them ok: the moment we enter more than CLEAN_ELEMS pfns, we will trigger a spring cleaning which will degrade the already logged errors. Once the array is filled up, we will replace the LRU pfn with the new one.

And so on.

And this way it would fulfill its purpose of *not* generating error records into the decoding chain after it. If one of those 9000 errors overflows, we will try to offline the page.

Either way we work as advertized.

Lemme try to write a small script exercising exactly that scenario to see whether I'm actually not talking crap here :-)

Signature · 5 lines
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--
author maintainer
#7 lore raw

Re: [PATCH 3/4] RAS: Add a Corrected Errors Collector

Quoted 2 lines · Borislav Petkov

Lemme try to write a small script exercising exactly that scenario to see whether I'm actually not talking crap here :-)

Ok, here's a snapshot from the CEC after letting it run for a couple of hours in a guest with a script running twice in parallel and injecting random PFNs. We have 0 offlined pages because a PFN number doesn't repeat frequently enough to cause an overflow.

When I force the occurrence of a single PFN for 1023 and more times and do that more than once, this happens:

[ 6629.091239] RAS: Soft-offlining pfn: 0x7fff
[ 6629.093036] __get_any_page: 0x7fff free buddy page
[ 6653.259476] RAS: Soft-offlining pfn: 0x7fff
[ 6653.260100] soft offline: 0x7fff page already poisoned

...

Stats:
CEs: 32614
offlined pages: 2
^^^^^^^^^^^^^^^^^

Flags: 0x0
Timer interval: 86400 seconds
Decays: 254
Action threshold: 1023

The "already poisoned" thing shouldn't happen in real life because once the page frame is poisoned, it shouldn't generate MCEs.

Every 2.0s: head -n 40 array; tail -n 40 array Thu Mar 23 17:15:15 2017

{ n: 512

 000: [0000000000000056|c01]
 001: [000000000000011f|801]
 002: [0000000000000171|401]
 003: [00000000000001ce|401]
 004: [000000000000024a|401]
 005: [000000000000026e|401]
 006: [000000000000034d|c01]
 007: [0000000000000395|c01]
 008: [00000000000003b9|801]
 009: [0000000000000458|003]
 010: [000000000000045c|401]
 011: [00000000000004f9|401]
 012: [00000000000005d1|c01]
 013: [0000000000000677|801]
 014: [000000000000069d|401]
 015: [00000000000006b3|401]
 016: [00000000000006f5|c01]
 017: [00000000000006fc|401]
 018: [000000000000074d|401]
 019: [0000000000000764|c01]
 020: [00000000000008a8|801]
 021: [0000000000000951|401]
 022: [0000000000000994|401]
 023: [0000000000000aa8|401]
 024: [0000000000000ac7|801]
 025: [0000000000000af2|801]
 026: [0000000000000bb5|801]
 027: [0000000000000bd5|401]
 028: [0000000000000be0|c01]
 029: [0000000000000c30|c01]
 030: [0000000000000c61|801]
 031: [0000000000000c8a|401]
 032: [0000000000000d0d|801]
 033: [0000000000000d2a|003]
 034: [0000000000000d4d|401]
 035: [0000000000000d87|c01]
 036: [0000000000000da4|c01]
 037: [0000000000000e06|401]
 038: [0000000000000e23|c01]

...

 480: [0000000000007d22|005]
 481: [0000000000007d5f|002]
 482: [0000000000007d9f|004]
 483: [0000000000007db1|c01]
 484: [0000000000007dbf|002]
 485: [0000000000007dcf|002]
 486: [0000000000007dd8|401]
 487: [0000000000007df0|001]
 488: [0000000000007df4|002]
 489: [0000000000007e1f|003]
 490: [0000000000007e35|801]
 491: [0000000000007e73|003]
 492: [0000000000007e77|401]
 493: [0000000000007e80|002]
 494: [0000000000007e9c|002]
 495: [0000000000007eac|002]
 496: [0000000000007ecb|002]
 497: [0000000000007ed8|801]
 498: [0000000000007edc|003]
 499: [0000000000007ee3|801]
 500: [0000000000007f05|004]
 501: [0000000000007f15|002]
 502: [0000000000007f51|004]
 503: [0000000000007f5e|003]
 504: [0000000000007f80|801]
 505: [0000000000007f92|003]
 506: [0000000000007fb2|002]
 507: [0000000000007fd9|002]
 508: [0000000000007fdf|002]
 509: [0000000000007fe5|004]
 510: [0000000000007ff4|801]
 511: [0000000000007ffa|001]

}
Stats:
CEs: 30074
offlined pages: 0
Flags: 0x0
Timer interval: 86400 seconds
Decays: 234
Action threshold: 1023

Signature · 4 lines
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.
maintainer
#8 lore raw

Re: [PATCH 3/4] RAS: Add a Corrected Errors Collector

Quoted 24 lines · Borislav Petkov
Quoted 2 lines · Borislav Petkov

Lemme try to write a small script exercising exactly that scenario to see whether I'm actually not talking crap here :-)

Ok, here's a snapshot from the CEC after letting it run for a couple of hours in a guest with a script running twice in parallel and injecting random PFNs. We have 0 offlined pages because a PFN number doesn't repeat frequently enough to cause an overflow.

When I force the occurrence of a single PFN for 1023 and more times and do that more than once, this happens:

[ 6629.091239] RAS: Soft-offlining pfn: 0x7fff
[ 6629.093036] __get_any_page: 0x7fff free buddy page
[ 6653.259476] RAS: Soft-offlining pfn: 0x7fff
[ 6653.260100] soft offline: 0x7fff page already poisoned

...

Stats:
CEs: 32614
offlined pages: 2
^^^^^^^^^^^^^^^^^

Quoted 7 lines

Flags: 0x0
Timer interval: 86400 seconds
Decays: 254
Action threshold: 1023

The "already poisoned" thing shouldn't happen in real life because once the page frame is poisoned, it shouldn't generate MCEs.

It can happen if Linux didn't actually take the page offline (because it was a kernel page). The CEC code only knows that it queued this page to be taken offline ... and has no way to know if that succeeded or not.

Some people have grumbled about mcelog(8) doing the same thing.

So is it worth keeping track of the page numbers that we tried to offline? If they show up again we shouldn't add them back into the array.

-Tony

author maintainer
#9 lore raw

Re: [PATCH 3/4] RAS: Add a Corrected Errors Collector

Quoted 4 lines · Tony

It can happen if Linux didn't actually take the page offline (because it was a kernel page). The CEC code only knows that it queued this page to be taken offline ... and has no way to know if that succeeded or not.

Right, that's the case when the offlining fails for whatever reason.

Quoted 3 lines

So is it worth keeping track of the page numbers that we tried to offline? If they show up again we shouldn't add them back into the array.

Meh, I don't like the idea of keeping an evergrowing list of PFNs we can't do anything about anyway.

And actually, you want the kernel to keep complaining about not being able to offline those because then admins should consider speeding up the arrival of the maintenance window - the kernel memory itself is going sick so that not even RAS actions help here.

I'm wondering if we should make the offlining code dump a more comprehensible message with hints what to do...

Signature · 4 lines
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.
maintainer
#10 lore raw

Re: [PATCH 3/4] RAS: Add a Corrected Errors Collector

Quoted 2 lines · Borislav Petkov

Meh, I don't like the idea of keeping an evergrowing list of PFNs we can't do anything about anyway.

Keeping every PFN would be overkill (most of them should be taken offline with no issues). A fixed array of a few of them with timestamps to drop the oldest would likely be a good enough(TM) solution.

Quoted 4 lines

And actually, you want the kernel to keep complaining about not being able to offline those because then admins should consider speeding up the arrival of the maintenance window - the kernel memory itself is going sick so that not even RAS actions help here.

Worst case is pretty ugly. A frequently used kernel page with a stuck bit could be added to the CEC array, overflow, and generate a message at a pretty high rate.

Quoted 2 lines

I'm wondering if we should make the offlining code dump a more comprehensible message with hints what to do...

Maybe ... but it gets into opinion rather than science. Some folks think that very low numbers of corrected errors warrant DIMM replacement. Others think that you can keep running almost forever with a several stuck bits per DIMM.

Some of the best decisions would be made by correlating error logs from multiple reboots ... which the kernel can't do.

-Tony

author maintainer
#11 lore raw

Re: [PATCH 3/4] RAS: Add a Corrected Errors Collector

Quoted 3 lines · Tony

Keeping every PFN would be overkill (most of them should be taken offline with no issues). A fixed array of a few of them with timestamps to drop the oldest would likely be a good enough(TM) solution.

The reason being? Prevent the CEC from adding it and trying to unsuccessfully offline it again?

If so, that means, we will query that list on every element insertion so it needs to be something we can search pretty quickly.

Quoted 3 lines

Worst case is pretty ugly. A frequently used kernel page with a stuck bit could be added to the CEC array, overflow, and generate a message at a pretty high rate.

Oh sure, but it would still be lower rate than generating a message for *each* correctable error. And I really think that these messages should *not* be supressed as they're important. The CEC kinda ratelimits them a bit though...

Quoted 7 lines

Maybe ... but it gets into opinion rather than science. Some folks think that very low numbers of corrected errors warrant DIMM replacement. Others think that you can keep running almost forever with a several stuck bits per DIMM.

Some of the best decisions would be made by correlating error logs from multiple reboots ... which the kernel can't do.

... and maybe even then it doesn't fit everybody's use case.

Signature · 4 lines
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.
maintainer
#12 lore raw

Re: [PATCH 3/4] RAS: Add a Corrected Errors Collector

In reply to Borislav Petkov

arch/x86/kernel/cpu/mcheck/mce.c static int mce_usable_address(struct mce *m) · line 574

Quoted 11 lines · Borislav Petkov
arch/x86/kernel/cpu/mcheck/mce.c +11 -0
+static bool cec_add_mce(struct mce *m)
+{
+	if (!m)
+		return false;
+
+	if (memory_error(m) && mce_usable_address(m))
+		if (!cec_add_elem(m->addr >> PAGE_SHIFT))
+			return true;
+
+	return false;
+}

You also need to check that bit 61 of m->status is zero here. The collector is hiding uncorrected errors too.

-Tony

author maintainer
#13 lore raw

Re: [PATCH 3/4] RAS: Add a Corrected Errors Collector

Quoted 2 lines · Tony

You also need to check that bit 61 of m->status is zero here. The collector is hiding uncorrected errors too.

Good catch.

I think I wanna do something like this:

	if (memory_error(m) && !(m->status & MCI_STATUS_UC) ...

as we want to make sure we're looking at a memory error first and then decide on severity.

Alternatively I could stick that logic in another helper called correctable_memory_error() or so but I don't have a strong preference.

Thanks.

Signature · 5 lines
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--
author maintainer
#14 lore raw

PATCH 4/4 x86/mce: Deprecate /dev/mcelog

In reply to Borislav Petkov

From: Tony Luck <[email protected]>

Move all code relating to /dev/mcelog to a separate source file. /dev/mcelog driver can now operate from the machine check notifier with lowest prio.

Boris:
* Move the mce_helper and trigger functionality behind CONFIG_X86_MCELOG.

---
 arch/x86/Kconfig                          |  10 +-
 arch/x86/include/asm/mce.h                |   1 +
 arch/x86/kernel/cpu/mcheck/Makefile       |   2 +
 arch/x86/kernel/cpu/mcheck/dev-mcelog.c   | 397 ++++++++++++++++++++++++++++++
 arch/x86/kernel/cpu/mcheck/mce-internal.h |   8 +
 arch/x86/kernel/cpu/mcheck/mce.c          | 367 +--------------------------
 6 files changed, 426 insertions(+), 359 deletions(-)
 create mode 100644 arch/x86/kernel/cpu/mcheck/dev-mcelog.c
arch/x86/Kconfig +9 -1
index cc98d5a294ee..f1216d0114b0 100644
@@ -1043,6 +1043,14 @@ config X86_MCE
 	  The action the kernel takes depends on the severity of the problem,
 	  ranging from warning messages to halting the machine.
 
+config X86_MCELOG
+	bool "Support for deprecated /dev/mcelog character device"
+	depends on X86_MCE
+	---help---
+	  Enable support for /dev/mcelog which is needed by the old mcelog
+	  userspace logging daemon. Consider switching to the new generation
+	  rasdaemon solution.
+
 config X86_MCE_INTEL
 	def_bool y
 	prompt "Intel MCE features"
@@ -1072,7 +1080,7 @@ config X86_MCE_THRESHOLD
 	def_bool y
 
 config X86_MCE_INJECT
-	depends on X86_MCE && X86_LOCAL_APIC
+	depends on X86_MCE && X86_LOCAL_APIC && X86_MCELOG
 	tristate "Machine check injector support"
 	---help---
 	  Provide support for injecting machine checks for testing purposes.
arch/x86/include/asm/mce.h +1 -0
index c5ae545d27d8..4fd5195deed0 100644
@@ -196,6 +196,7 @@ enum mce_notifier_prios {
 	MCE_PRIO_EXTLOG		= INT_MAX - 2,
 	MCE_PRIO_NFIT		= INT_MAX - 3,
 	MCE_PRIO_EDAC		= INT_MAX - 4,
+	MCE_PRIO_MCELOG		= 1,
 	MCE_PRIO_LOWEST		= 0,
 };
 
arch/x86/kernel/cpu/mcheck/Makefile +2 -0
index a3311c886194..950e9ff602ea 100644
@@ -9,3 +9,5 @@ obj-$(CONFIG_X86_MCE_INJECT)	+= mce-inject.o
 obj-$(CONFIG_X86_THERMAL_VECTOR) += therm_throt.o
 
 obj-$(CONFIG_ACPI_APEI)		+= mce-apei.o
+
+obj-$(CONFIG_X86_MCELOG)	+= dev-mcelog.o
arch/x86/kernel/cpu/mcheck/dev-mcelog.c new file +397 -0

397 lines not on this page · raw message

arch/x86/kernel/cpu/mcheck/mce-internal.h +8 -0
index 903043e6a62b..f096d64485d0 100644
@@ -96,3 +96,11 @@ static inline bool mce_cmp(struct mce *m1, struct mce *m2)
 		m1->addr != m2->addr ||
 		m1->misc != m2->misc;
 }
+
+extern struct device_attribute dev_attr_trigger;
+
+#ifdef CONFIG_X86_MCELOG
+extern void mce_work_trigger(void);
+#else
+static inline void mce_work_trigger(void)	{ }
+#endif
arch/x86/kernel/cpu/mcheck/mce.c +9 -358

465 lines not on this page · raw message

Signature · 1 line
2.11.0