Skip to contentSkip to conversation

RAS/CEC: Fixes and cleanups

PATCH New Borislav Petkov 1 participant 12 messages May 9, 2019

Subsystems (3)
x86 EDAC-core RAS infrastructure
lore
Share & tools
lore

Borislav Petkov

author maintainer
#0 lore raw

cover RAS/CEC: Fixes and cleanups

From: Borislav Petkov <[email protected]>

Hi all,

here are a bunch of fixes and cleanups to the correctable errors collector which we've been working on in the recent weeks.

Thx.

Borislav Petkov (9):
RAS/CEC: Fix binary search function RAS/CEC: Fix pfn insertion
RAS/CEC: Check count_threshold unconditionally RAS/CEC: Do not set decay value on error RAS/CEC: Fix potential memory leak
RAS/CEC: Sanity-check array on every insertion RAS/CEC: Rename count_threshold to action_threshold RAS/CEC: Dump the different array element sections RAS/CEC: Add copyright

Cong Wang (1):
RAS/CEC: Convert the timer callback to a workqueue

Tony Luck (1):
RAS/CEC: Add CONFIG_RAS_CEC_DEBUG and move CEC debug features there

 arch/x86/ras/Kconfig |  10 ++
 drivers/ras/cec.c    | 216 +++++++++++++++++++++++++------------------
 2 files changed, 135 insertions(+), 91 deletions(-)
Signature · 1 line
2.21.0
author maintainer
#1 lore raw

PATCH 1/11 RAS/CEC: Fix binary search function

From: Borislav Petkov <[email protected]>

Switch to using Donald Knuth's binary search algorithm (The Art of Computer Programming, vol. 3, section 6.2.1). This should've been done from the very beginning but the author must've been smoking something very potent at the time.

The problem with the current one was that it would return the wrong element index in certain situations:

https://lkml.kernel.org/r/CAM_iQpVd02zkVJ846cj-Fg1yUNuz6tY5q1Vpj4LrXmE06dPYYg@mail.gmail.com

and the noodling code after the loop was fishy at best.

So switch to using Knuth's binary search. The final result is much cleaner and straightforward.

Fixes 011d82611172 ("RAS: Add a Corrected Errors Collector")
Reported-by Cong Wang <[email protected]>
Signed-off-by Borislav Petkov <[email protected]>
Cc Tony Luck <[email protected]>
Cc linux-edac <[email protected]>
Cc <[email protected]>
---
 drivers/ras/cec.c | 34 ++++++++++++++++++++--------------
 1 file changed, 20 insertions(+), 14 deletions(-)
drivers/ras/cec.c +20 -14
index 88e4f3ff0cb8..dbfe3e61d2c2 100644
@@ -183,32 +183,38 @@ static void cec_timer_fn(struct timer_list *unused)
  */
 static int __find_elem(struct ce_array *ca, u64 pfn, unsigned int *to)
 {
+	int min = 0, max = ca->n - 1;
 	u64 this_pfn;
-	int min = 0, max = ca->n;
 
-	while (min < max) {
-		int tmp = (max + min) >> 1;
+	while (min <= max) {
+		int i = (min + max) >> 1;
 
-		this_pfn = PFN(ca->array[tmp]);
+		this_pfn = PFN(ca->array[i]);
 
 		if (this_pfn < pfn)
-			min = tmp + 1;
+			min = i + 1;
 		else if (this_pfn > pfn)
-			max = tmp;
-		else {
-			min = tmp;
-			break;
+			max = i - 1;
+		else if (this_pfn == pfn) {
+			if (to)
+				*to = i;
+
+			return i;
 		}
 	}
 
+	/*
+	 * When the loop terminates without finding @pfn, min has the index of
+	 * the element slot where the new @pfn should be inserted. The loop
+	 * terminates when min > max, which means the min index points to the
+	 * bigger element while the max index to the smaller element, in-between
+	 * which the new @pfn belongs to.
+	 *
+	 * For more details, see exercise 1, Section 6.2.1 in TAOCP, vol. 3.
+	 */
 	if (to)
 		*to = min;
 
-	this_pfn = PFN(ca->array[min]);
-
-	if (this_pfn == pfn)
-		return min;
-
 	return -ENOKEY;
 }
Signature · 1 line
2.21.0
author maintainer
#2 lore raw

PATCH 2/11 RAS/CEC: Convert the timer callback to a workqueue

In reply to Borislav Petkov

From: Cong Wang <[email protected]>

cec_timer_fn() is a timer callback which reads ce_arr.array[] and updates its decay values. However, it runs in interrupt context and the mutex protection the CEC uses for that array, is inadequate. Convert the used timer to a workqueue to keep the tasks the CEC performs preemptible and thus low-prio.

[ bp: Rewrite commit message.
s/timer/decay/gi to make it agnostic as to what facility is used. ]

Fixes 011d82611172 ("RAS: Add a Corrected Errors Collector")
Signed-off-by Cong Wang <[email protected]>
Signed-off-by Borislav Petkov <[email protected]>
Cc Thomas Gleixner <[email protected]>
Cc Tony Luck <[email protected]>
Cc linux-edac <[email protected]>
Cc <[email protected]>
Link https://lkml.kernel.org/r/[email protected] open here
---
 drivers/ras/cec.c | 46 ++++++++++++++++++++++------------------------
 1 file changed, 22 insertions(+), 24 deletions(-)
drivers/ras/cec.c +22 -24
index dbfe3e61d2c2..673f8a128397 100644
@@ -2,6 +2,7 @@
 #include <linux/mm.h>
 #include <linux/gfp.h>
 #include <linux/kernel.h>
+#include <linux/workqueue.h>
 
 #include <asm/mce.h>
 

@@ -123,16 +124,12 @@ static u64 dfs_pfn;
 /* Amount of errors after which we offline */
 static unsigned int count_threshold = COUNT_MASK;
 
-/*
- * The timer "decays" element count each timer_interval which is 24hrs by
- * default.
- */
-
-#define CEC_TIMER_DEFAULT_INTERVAL	24 * 60 * 60	/* 24 hrs */
-#define CEC_TIMER_MIN_INTERVAL		 1 * 60 * 60	/* 1h */
-#define CEC_TIMER_MAX_INTERVAL	   30 *	24 * 60 * 60	/* one month */
-static struct timer_list cec_timer;
-static u64 timer_interval = CEC_TIMER_DEFAULT_INTERVAL;
+/* Each element "decays" each decay_interval which is 24hrs by default. */
+#define CEC_DECAY_DEFAULT_INTERVAL	24 * 60 * 60	/* 24 hrs */
+#define CEC_DECAY_MIN_INTERVAL		 1 * 60 * 60	/* 1h */
+#define CEC_DECAY_MAX_INTERVAL	   30 *	24 * 60 * 60	/* one month */
+static struct delayed_work cec_work;
+static u64 decay_interval = CEC_DECAY_DEFAULT_INTERVAL;
 
 /*
  * Decrement decay value. We're using DECAY_BITS bits to denote decay of an

@@ -160,20 +157,21 @@ static void do_spring_cleaning(struct ce_array *ca)
 /*
  * @interval in seconds
  */
-static void cec_mod_timer(struct timer_list *t, unsigned long interval)
+static void cec_mod_work(unsigned long interval)
 {
 	unsigned long iv;
 
-	iv = interval * HZ + jiffies;
-
-	mod_timer(t, round_jiffies(iv));
+	iv = interval * HZ;
+	mod_delayed_work(system_wq, &cec_work, round_jiffies(iv));
 }
 
-static void cec_timer_fn(struct timer_list *unused)
+static void cec_work_fn(struct work_struct *work)
 {
+	mutex_lock(&ce_mutex);
 	do_spring_cleaning(&ce_arr);
+	mutex_unlock(&ce_mutex);
 
-	cec_mod_timer(&cec_timer, timer_interval);
+	cec_mod_work(decay_interval);
 }
 
 /*

@@ -380,15 +378,15 @@ static int decay_interval_set(void *data, u64 val)
 {
 	*(u64 *)data = val;
 
-	if (val < CEC_TIMER_MIN_INTERVAL)
+	if (val < CEC_DECAY_MIN_INTERVAL)
 		return -EINVAL;
 
-	if (val > CEC_TIMER_MAX_INTERVAL)
+	if (val > CEC_DECAY_MAX_INTERVAL)
 		return -EINVAL;
 
-	timer_interval = val;
+	decay_interval = val;
 
-	cec_mod_timer(&cec_timer, timer_interval);
+	cec_mod_work(decay_interval);
 	return 0;
 }
 DEFINE_DEBUGFS_ATTRIBUTE(decay_interval_ops, u64_get, decay_interval_set, "%lld\n");

@@ -432,7 +430,7 @@ static int array_dump(struct seq_file *m, void *v)
 
 	seq_printf(m, "Flags: 0x%x\n", ca->flags);
 
-	seq_printf(m, "Timer interval: %lld seconds\n", timer_interval);
+	seq_printf(m, "Decay interval: %lld seconds\n", decay_interval);
 	seq_printf(m, "Decays: %lld\n", ca->decays_done);
 
 	seq_printf(m, "Action threshold: %d\n", count_threshold);

@@ -478,7 +476,7 @@ static int __init create_debugfs_nodes(void)
 	}
 
 	decay = debugfs_create_file("decay_interval", S_IRUSR | S_IWUSR, d,
-				    &timer_interval, &decay_interval_ops);
+				    &decay_interval, &decay_interval_ops);
 	if (!decay) {
 		pr_warn("Error creating decay_interval debugfs node!\n");
 		goto err;

@@ -514,8 +512,8 @@ void __init cec_init(void)
 	if (create_debugfs_nodes())
 		return;
 
-	timer_setup(&cec_timer, cec_timer_fn, 0);
-	cec_mod_timer(&cec_timer, CEC_TIMER_DEFAULT_INTERVAL);
+	INIT_DELAYED_WORK(&cec_work, cec_work_fn);
+	schedule_delayed_work(&cec_work, CEC_DECAY_DEFAULT_INTERVAL);
 
 	pr_info("Correctable Errors collector initialized.\n");
 }
Signature · 1 line
2.21.0
author maintainer
#3 lore raw

PATCH 3/11 RAS/CEC: Fix pfn insertion

In reply to Borislav Petkov

From: Borislav Petkov <[email protected]>

When inserting random PFNs for debugging the CEC through (debugfs)/ras/cec/pfn, depending on the return value of pfn_set(), multiple values get inserted per a single write.

That is because simple_attr_write() interprets a retval of 0 as success and claims the whole input. However, pfn_set() returns the cec_add_elem() value, which, if > 0 and smaller than the whole input length, makes glibc continue issuing the write syscall until there's input left:

pfn_set
simple_attr_write
debugfs_attr_write
full_proxy_write
vfs_write
ksys_write
do_syscall_64
entry_SYSCALL_64_after_hwframe

leading to those repeated calls.

Return 0 to fix that.

Rename u64_get() to pfn_get() while at it.

Signed-off-by Borislav Petkov <[email protected]>
Cc Tony Luck <[email protected]>
Cc linux-edac <[email protected]>
---
 drivers/ras/cec.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)
drivers/ras/cec.c +7 -5
index 673f8a128397..1275907ff21c 100644
@@ -358,7 +358,7 @@ int cec_add_elem(u64 pfn)
 	return ret;
 }
 
-static int u64_get(void *data, u64 *val)
+static int pfn_get(void *data, u64 *val)
 {
 	*val = *(u64 *)data;
 

@@ -369,10 +369,12 @@ static int pfn_set(void *data, u64 val)
 {
 	*(u64 *)data = val;
 
-	return cec_add_elem(val);
+	cec_add_elem(val);
+
+	return 0;
 }
 
-DEFINE_DEBUGFS_ATTRIBUTE(pfn_ops, u64_get, pfn_set, "0x%llx\n");
+DEFINE_DEBUGFS_ATTRIBUTE(pfn_ops, pfn_get, pfn_set, "0x%llx\n");
 
 static int decay_interval_set(void *data, u64 val)
 {

@@ -389,7 +391,7 @@ static int decay_interval_set(void *data, u64 val)
 	cec_mod_work(decay_interval);
 	return 0;
 }
-DEFINE_DEBUGFS_ATTRIBUTE(decay_interval_ops, u64_get, decay_interval_set, "%lld\n");
+DEFINE_DEBUGFS_ATTRIBUTE(decay_interval_ops, pfn_get, decay_interval_set, "%lld\n");
 
 static int count_threshold_set(void *data, u64 val)
 {

@@ -402,7 +404,7 @@ static int count_threshold_set(void *data, u64 val)
 
 	return 0;
 }
-DEFINE_DEBUGFS_ATTRIBUTE(count_threshold_ops, u64_get, count_threshold_set, "%lld\n");
+DEFINE_DEBUGFS_ATTRIBUTE(count_threshold_ops, pfn_get, count_threshold_set, "%lld\n");
 
 static int array_dump(struct seq_file *m, void *v)
 {
Signature · 1 line
2.21.0
author maintainer
#4 lore raw

PATCH 4/11 RAS/CEC: Check count_threshold unconditionally

In reply to Borislav Petkov

From: Borislav Petkov <[email protected]>

The count_threshold should be checked unconditionally, after insertion too, so that a count_threshold value of 1 can cause an immediate offlining. I.e., offline the page on the *first* error encountered.

Add comments to make it clear what cec_add_elem() does, while at it.

Reported-by WANG Chao <[email protected]>
Signed-off-by Borislav Petkov <[email protected]>
Cc Tony Luck <[email protected]>
Cc [email protected]
Link https://lkml.kernel.org/r/[email protected] open here
---
 drivers/ras/cec.c | 27 ++++++++++-----------------
 1 file changed, 10 insertions(+), 17 deletions(-)
drivers/ras/cec.c +10 -17
index 1275907ff21c..73216b7f67be 100644
@@ -294,6 +294,7 @@ int cec_add_elem(u64 pfn)
 
 	ca->ces_entered++;
 
+	/* Array full, free the LRU slot. */
 	if (ca->n == MAX_ELEMS)
 		WARN_ON(!del_lru_elem_unlocked(ca));
 

@@ -306,24 +307,17 @@ int cec_add_elem(u64 pfn)
 			(void *)&ca->array[to],
 			(ca->n - to) * sizeof(u64));
 
-		ca->array[to] = (pfn << PAGE_SHIFT) |
-				(DECAY_MASK << COUNT_BITS) | 1;
-
+		ca->array[to] = pfn << PAGE_SHIFT;
 		ca->n++;
-
-		ret = 0;
-
-		goto decay;
 	}
 
-	count = COUNT(ca->array[to]);
-
-	if (count < count_threshold) {
-		ca->array[to] |= (DECAY_MASK << COUNT_BITS);
-		ca->array[to]++;
+	/* Add/refresh element generation and increment count */
+	ca->array[to] |= DECAY_MASK << COUNT_BITS;
+	ca->array[to]++;
 
-		ret = 0;
-	} else {
+	/* Check action threshold and soft-offline, if reached. */
+	count = COUNT(ca->array[to]);
+	if (count >= count_threshold) {
 		u64 pfn = ca->array[to] >> PAGE_SHIFT;
 
 		if (!pfn_valid(pfn)) {

@@ -338,15 +332,14 @@ int cec_add_elem(u64 pfn)
 		del_elem(ca, to);
 
 		/*
-		 * Return a >0 value to denote that we've reached the offlining
-		 * threshold.
+		 * Return a >0 value to callers, to denote that we've reached
+		 * the offlining threshold.
 		 */
 		ret = 1;
 
 		goto unlock;
 	}
 
-decay:
 	ca->decay_count++;
 
 	if (ca->decay_count >= CLEAN_ELEMS)
Signature · 1 line
2.21.0
author maintainer
#5 lore raw

PATCH 5/11 RAS/CEC: Do not set decay value on error

In reply to Borislav Petkov

From: Borislav Petkov <[email protected]>

When the value requested doesn't match the allowed (min,max) range, the @data buffer should not be modified with the invalid value because reading "decay_interval" shows it otherwise.

Move the data write after the check.

Signed-off-by Borislav Petkov <[email protected]>
Cc Tony Luck <[email protected]>
Cc linux-edac <[email protected]>
---
 drivers/ras/cec.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/ras/cec.c +2 -2
index 73216b7f67be..667a126f39f1 100644
@@ -371,17 +371,17 @@ DEFINE_DEBUGFS_ATTRIBUTE(pfn_ops, pfn_get, pfn_set, "0x%llx\n");
 
 static int decay_interval_set(void *data, u64 val)
 {
-	*(u64 *)data = val;
-
 	if (val < CEC_DECAY_MIN_INTERVAL)
 		return -EINVAL;
 
 	if (val > CEC_DECAY_MAX_INTERVAL)
 		return -EINVAL;
 
+	*(u64 *)data   = val;
 	decay_interval = val;
 
 	cec_mod_work(decay_interval);
+
 	return 0;
 }
 DEFINE_DEBUGFS_ATTRIBUTE(decay_interval_ops, pfn_get, decay_interval_set, "%lld\n");
Signature · 1 line
2.21.0
author maintainer
#6 lore raw

PATCH 6/11 RAS/CEC: Fix potential memory leak

In reply to Borislav Petkov

From: Borislav Petkov <[email protected]>

Free the array page if a failure is encountered while creating the debugfs nodes.

Signed-off-by Borislav Petkov <[email protected]>
Cc Tony Luck <[email protected]>
Cc linux-edac <[email protected]>
---
 drivers/ras/cec.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
drivers/ras/cec.c +3 -1
index 667a126f39f1..8a04b9864192 100644
@@ -504,8 +504,10 @@ void __init cec_init(void)
 		return;
 	}
 
-	if (create_debugfs_nodes())
+	if (create_debugfs_nodes()) {
+		free_page((unsigned long)ce_arr.array);
 		return;
+	}
 
 	INIT_DELAYED_WORK(&cec_work, cec_work_fn);
 	schedule_delayed_work(&cec_work, CEC_DECAY_DEFAULT_INTERVAL);
Signature · 1 line
2.21.0
author maintainer
#7 lore raw

PATCH 7/11 RAS/CEC: Sanity-check array on every insertion

In reply to Borislav Petkov

From: Borislav Petkov <[email protected]>

Check the elements order in the array after every insertion.

Signed-off-by Borislav Petkov <[email protected]>
Cc Tony Luck <[email protected]>
Cc linux-edac <[email protected]>
---
 drivers/ras/cec.c | 37 +++++++++++++++++++++++++++++++------
 1 file changed, 31 insertions(+), 6 deletions(-)
drivers/ras/cec.c +31 -6
index 8a04b9864192..2c92a70fa2f0 100644
@@ -276,11 +276,39 @@ static u64 __maybe_unused del_lru_elem(void)
 	return pfn;
 }
 
+static bool sanity_check(struct ce_array *ca)
+{
+	bool ret = false;
+	u64 prev = 0;
+	int i;
+
+	for (i = 0; i < ca->n; i++) {
+		u64 this = PFN(ca->array[i]);
+
+		if (WARN(prev > this, "prev: 0x%016llx <-> this: 0x%016llx\n", prev, this))
+			ret = true;
+
+		prev = this;
+	}
+
+	if (!ret)
+		return ret;
+
+	pr_info("Sanity check dump:\n{ n: %d\n", ca->n);
+	for (i = 0; i < ca->n; i++) {
+		u64 this = PFN(ca->array[i]);
+
+		pr_info(" %03d: [%016llx|%03llx]\n", i, this, FULL_COUNT(ca->array[i]));
+	}
+	pr_info("}\n");
+
+	return ret;
+}
 
 int cec_add_elem(u64 pfn)
 {
 	struct ce_array *ca = &ce_arr;
-	unsigned int to;
+	unsigned int to = 0;
 	int count, ret = 0;
 
 	/*

@@ -345,6 +373,8 @@ int cec_add_elem(u64 pfn)
 	if (ca->decay_count >= CLEAN_ELEMS)
 		do_spring_cleaning(ca);
 
+	WARN_ON_ONCE(sanity_check(ca));
+
 unlock:
 	mutex_unlock(&ce_mutex);
 

@@ -402,7 +432,6 @@ DEFINE_DEBUGFS_ATTRIBUTE(count_threshold_ops, pfn_get, count_threshold_set, "%ll
 static int array_dump(struct seq_file *m, void *v)
 {
 	struct ce_array *ca = &ce_arr;
-	u64 prev = 0;
 	int i;
 
 	mutex_lock(&ce_mutex);

@@ -412,10 +441,6 @@ static int array_dump(struct seq_file *m, void *v)
 		u64 this = PFN(ca->array[i]);
 
 		seq_printf(m, " %03d: [%016llx|%03llx]\n", i, this, FULL_COUNT(ca->array[i]));
-
-		WARN_ON(prev > this);
-
-		prev = this;
 	}
 
 	seq_printf(m, "}\n");
Signature · 1 line
2.21.0
author maintainer
#8 lore raw

PATCH 8/11 RAS/CEC: Rename count_threshold to action_threshold

In reply to Borislav Petkov

From: Borislav Petkov <[email protected]>

... which is the better, more-fitting name anyway.

Tony:
- make action_threshold u64 due to debugfs accessors expecting u64.
- rename the remaining: s/count_threshold/action_threshold/g

Co-developed-by Tony Luck <[email protected]>
Signed-off-by Tony Luck <[email protected]>
Signed-off-by Borislav Petkov <[email protected]>
Cc linux-edac <[email protected]>
---
 drivers/ras/cec.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)
drivers/ras/cec.c +12 -12
index 2c92a70fa2f0..1cbc1ed82afe 100644
@@ -37,9 +37,9 @@
  * thus emulate an an LRU-like behavior when deleting elements to free up space
  * in the page.
  *
- * When an element reaches it's max count of count_threshold, we try to poison
- * it by assuming that errors triggered count_threshold times in a single page
- * are excessive and that page shouldn't be used anymore. count_threshold is
+ * When an element reaches it's max count of action_threshold, we try to poison
+ * it by assuming that errors triggered action_threshold times in a single page
+ * are excessive and that page shouldn't be used anymore. action_threshold is
  * initialized to COUNT_MASK which is the maximum.
  *
  * That error event entry causes cec_add_elem() to return !0 value and thus

@@ -122,7 +122,7 @@ static DEFINE_MUTEX(ce_mutex);
 static u64 dfs_pfn;
 
 /* Amount of errors after which we offline */
-static unsigned int count_threshold = COUNT_MASK;
+static u64 action_threshold = COUNT_MASK;
 
 /* Each element "decays" each decay_interval which is 24hrs by default. */
 #define CEC_DECAY_DEFAULT_INTERVAL	24 * 60 * 60	/* 24 hrs */

@@ -345,7 +345,7 @@ int cec_add_elem(u64 pfn)
 
 	/* Check action threshold and soft-offline, if reached. */
 	count = COUNT(ca->array[to]);
-	if (count >= count_threshold) {
+	if (count >= action_threshold) {
 		u64 pfn = ca->array[to] >> PAGE_SHIFT;
 
 		if (!pfn_valid(pfn)) {

@@ -416,18 +416,18 @@ static int decay_interval_set(void *data, u64 val)
 }
 DEFINE_DEBUGFS_ATTRIBUTE(decay_interval_ops, pfn_get, decay_interval_set, "%lld\n");
 
-static int count_threshold_set(void *data, u64 val)
+static int action_threshold_set(void *data, u64 val)
 {
 	*(u64 *)data = val;
 
 	if (val > COUNT_MASK)
 		val = COUNT_MASK;
 
-	count_threshold = val;
+	action_threshold = val;
 
 	return 0;
 }
-DEFINE_DEBUGFS_ATTRIBUTE(count_threshold_ops, pfn_get, count_threshold_set, "%lld\n");
+DEFINE_DEBUGFS_ATTRIBUTE(action_threshold_ops, pfn_get, action_threshold_set, "%lld\n");
 
 static int array_dump(struct seq_file *m, void *v)
 {

@@ -453,7 +453,7 @@ static int array_dump(struct seq_file *m, void *v)
 	seq_printf(m, "Decay interval: %lld seconds\n", decay_interval);
 	seq_printf(m, "Decays: %lld\n", ca->decays_done);
 
-	seq_printf(m, "Action threshold: %d\n", count_threshold);
+	seq_printf(m, "Action threshold: %lld\n", action_threshold);
 
 	mutex_unlock(&ce_mutex);
 

@@ -502,10 +502,10 @@ static int __init create_debugfs_nodes(void)
 		goto err;
 	}
 
-	count = debugfs_create_file("count_threshold", S_IRUSR | S_IWUSR, d,
-				    &count_threshold, &count_threshold_ops);
+	count = debugfs_create_file("action_threshold", S_IRUSR | S_IWUSR, d,
+				    &action_threshold, &action_threshold_ops);
 	if (!count) {
-		pr_warn("Error creating count_threshold debugfs node!\n");
+		pr_warn("Error creating action_threshold debugfs node!\n");
 		goto err;
 	}
Signature · 1 line
2.21.0
author maintainer
#9 lore raw

PATCH 9/11 RAS/CEC: Dump the different array element sections

In reply to Borislav Petkov

From: Borislav Petkov <[email protected]>

When dumping the array elements, print them in the following format:

[ PFN | generation in binary | count ]

to be perfectly clear what all those sections are.

Signed-off-by Borislav Petkov <[email protected]>
Cc Tony Luck <[email protected]>
Cc linux-edac <[email protected]>
---
 drivers/ras/cec.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
drivers/ras/cec.c +4 -1
index 1cbc1ed82afe..71f3e14c35d8 100644
@@ -429,6 +429,8 @@ static int action_threshold_set(void *data, u64 val)
 }
 DEFINE_DEBUGFS_ATTRIBUTE(action_threshold_ops, pfn_get, action_threshold_set, "%lld\n");
 
+static const char * const bins[] = { "00", "01", "10", "11" };
+
 static int array_dump(struct seq_file *m, void *v)
 {
 	struct ce_array *ca = &ce_arr;

@@ -440,7 +442,8 @@ static int array_dump(struct seq_file *m, void *v)
 	for (i = 0; i < ca->n; i++) {
 		u64 this = PFN(ca->array[i]);
 
-		seq_printf(m, " %03d: [%016llx|%03llx]\n", i, this, FULL_COUNT(ca->array[i]));
+		seq_printf(m, " %3d: [%016llx|%s|%03llx]\n",
+			   i, this, bins[DECAY(ca->array[i])], COUNT(ca->array[i]));
 	}
 
 	seq_printf(m, "}\n");
Signature · 1 line
2.21.0
author maintainer
#10 lore raw

PATCH 10/11 RAS/CEC: Add CONFIG_RAS_CEC_DEBUG and move CEC debug features there

In reply to Borislav Petkov

From: Tony Luck <[email protected]>

The pfn and array files in (debugfs)/ras/cec are intended for debugging the CEC code itself. They are not needed on production systems, so the default setting for this CONFIG option is "n".

[ bp: Have it with less ifdeffery by using IS_ENABLED(). ]

Signed-off-by Tony Luck <[email protected]>
Signed-off-by Borislav Petkov <[email protected]>
---
 arch/x86/ras/Kconfig | 10 ++++++++++
 drivers/ras/cec.c    | 26 ++++++++++++++------------
 2 files changed, 24 insertions(+), 12 deletions(-)
arch/x86/ras/Kconfig +10 -0
index a9c3db125222..9ad6842de4b4 100644
@@ -11,3 +11,13 @@ config RAS_CEC
 
 	  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.
+
+config RAS_CEC_DEBUG
+	bool "CEC debugging machinery"
+	default n
+	depends on RAS_CEC
+	help
+	  Add extra files to (debugfs)/ras/cec to test the correctable error
+	  collector feature. "pfn" is a writable file that allows user to
+	  simulate an error in a particular page frame. "array" is a read-only
+	  file that dumps out the current state of all pages logged so far.
drivers/ras/cec.c +14 -12
index 71f3e14c35d8..c4505c0893a7 100644
@@ -486,18 +486,6 @@ static int __init create_debugfs_nodes(void)
 		return -1;
 	}
 
-	pfn = debugfs_create_file("pfn", S_IRUSR | S_IWUSR, d, &dfs_pfn, &pfn_ops);
-	if (!pfn) {
-		pr_warn("Error creating pfn debugfs node!\n");
-		goto err;
-	}
-
-	array = debugfs_create_file("array", S_IRUSR, d, NULL, &array_ops);
-	if (!array) {
-		pr_warn("Error creating array debugfs node!\n");
-		goto err;
-	}
-
 	decay = debugfs_create_file("decay_interval", S_IRUSR | S_IWUSR, d,
 				    &decay_interval, &decay_interval_ops);
 	if (!decay) {

@@ -512,6 +500,20 @@ static int __init create_debugfs_nodes(void)
 		goto err;
 	}
 
+	if (!IS_ENABLED(CONFIG_RAS_CEC_DEBUG))
+		return 0;
+
+	pfn = debugfs_create_file("pfn", S_IRUSR | S_IWUSR, d, &dfs_pfn, &pfn_ops);
+	if (!pfn) {
+		pr_warn("Error creating pfn debugfs node!\n");
+		goto err;
+	}
+
+	array = debugfs_create_file("array", S_IRUSR, d, NULL, &array_ops);
+	if (!array) {
+		pr_warn("Error creating array debugfs node!\n");
+		goto err;
+	}
 
 	return 0;
Signature · 1 line
2.21.0
author maintainer
#11 lore raw

PATCH 11/11 RAS/CEC: Add copyright

In reply to Borislav Petkov

From: Borislav Petkov <[email protected]>

Signed-off-by Borislav Petkov <[email protected]>
---
 drivers/ras/cec.c | 3 +++
 1 file changed, 3 insertions(+)
drivers/ras/cec.c +3 -0
index c4505c0893a7..5402e907ae7a 100644
@@ -1,4 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2017- Borislav Petkov, SUSE Labs.
+ */
 #include <linux/mm.h>
 #include <linux/gfp.h>
 #include <linux/kernel.h>
Signature · 1 line
2.21.0