Re: Constant output in syslog of EDAC message

[Jason Baron <jbaron@akamai.com>]

On 10/20/20 5:19 AM, Borislav Petkov wrote:
> On Mon, Oct 19, 2020 at 05:25:43PM -0400, Jason Baron wrote:
>> Yes, I likely was just following what was in other edac drivers at
>> the time - for example, i3200_check() has a similar debug. I guess
>> it could have a higher level. But if we remove this one, we may
>> want to audit some of the other edac drivers as well.
> 
> Sounds good. So one patch removing them all would make sense. If
> you feel like doing it, be my guest. Otherwise, it'll land on my
> ever-growing todo. :-\
> 
> Thx.
> 

Hi Boris,

I thought a bit about this - and was wondering if it made sense to
hook the edac debug messages up to 'dynamic debug'. The idea is that
different users may want different debug messages enabled, and this
way they can control each message as desired. Jim (added to 'cc)
recently exported 'ddebug_exec_queries()' to make that easier to do:
https://lore.kernel.org/lkml/20200620180643.887546-16-jim.cromie@gmail.com/

So, we would still continue to support edac_debug_level=N but the
user would have additional control. I tried it out (below) and
doesn't look too bad (at least to me).

Thanks,

-Jason

diff --git a/drivers/edac/Kconfig b/drivers/edac/Kconfig
index 7a47680d6f07..0c1c97e42a63 100644
--- a/drivers/edac/Kconfig
+++ b/drivers/edac/Kconfig
@@ -34,6 +34,7 @@ config EDAC_LEGACY_SYSFS
 config EDAC_DEBUG
 	bool "Debugging"
 	select DEBUG_FS
+	select DYNAMIC_DEBUG_CORE
 	help
 	  This turns on debugging information for the entire EDAC subsystem.
 	  You do so by inserting edac_module with "edac_debug_level=x." Valid
diff --git a/drivers/edac/Makefile b/drivers/edac/Makefile
index 3a849168780d..21ad452f1193 100644
--- a/drivers/edac/Makefile
+++ b/drivers/edac/Makefile
@@ -8,6 +8,10 @@

 obj-$(CONFIG_EDAC)			:= edac_core.o

+ifdef CONFIG_EDAC_DEBUG
+ccflags-y += -DDYNAMIC_DEBUG_MODULE
+endif
+
 edac_core-y	:= edac_mc.o edac_device.o edac_mc_sysfs.o
 edac_core-y	+= edac_module.o edac_device_sysfs.o wq.o

diff --git a/drivers/edac/edac_mc.h b/drivers/edac/edac_mc.h
index 881b00eadf7a..75dd4357d430 100644
--- a/drivers/edac/edac_mc.h
+++ b/drivers/edac/edac_mc.h
@@ -48,6 +48,9 @@
 #define edac_printk(level, prefix, fmt, arg...) \
 	printk(level "EDAC " prefix ": " fmt, ##arg)

+#define edac_dbg_printk(level, prefix, fmt, arg...) \
+	pr_debug("EDAC " prefix #level ": " fmt, ##arg)
+
 #define edac_mc_printk(mci, level, fmt, arg...) \
 	printk(level "EDAC MC%d: " fmt, mci->mc_idx, ##arg)

@@ -70,11 +73,9 @@ extern const char * const edac_mem_types[];
 #ifdef CONFIG_EDAC_DEBUG
 extern int edac_debug_level;

-#define edac_dbg(level, fmt, ...)					\
-do {									\
-	if (level <= edac_debug_level)					\
-		edac_printk(KERN_DEBUG, EDAC_DEBUG,			\
-			    "%s: " fmt, __func__, ##__VA_ARGS__);	\
+#define edac_dbg(level, fmt, ...)					    \
+do {									    \
+	edac_dbg_printk(level, EDAC_DEBUG, "%s: " fmt, __func__, ##__VA_ARGS__);\
 } while (0)

 #else				/* !CONFIG_EDAC_DEBUG */
diff --git a/drivers/edac/edac_module.c b/drivers/edac/edac_module.c
index 32a931d0cb71..6109589858c0 100644
--- a/drivers/edac/edac_module.c
+++ b/drivers/edac/edac_module.c
@@ -11,6 +11,7 @@
  *
  */
 #include <linux/edac.h>
+#include <linux/dynamic_debug.h>

 #include "edac_mc.h"
 #include "edac_module.h"
@@ -19,6 +20,29 @@

 #ifdef CONFIG_EDAC_DEBUG

+static int edac_debug_level_initialized;
+/* Values of 0 to 4 will generate output */
+int edac_debug_level = 2;
+EXPORT_SYMBOL_GPL(edac_debug_level);
+
+static void edac_set_dynamic_debug_level(int level)
+{
+	char buf[32];
+	int i;
+
+	edac_debug_level_initialized = 1;
+	for (i = 0; i <= 4; i++) {
+		snprintf(buf, 32, "format '^EDAC DEBUG%d' %cp", i, i <= level ? '+' : '-');
+		dynamic_debug_exec_queries(buf, NULL);
+	}
+}
+
+static void edac_set_debug_level_init(void)
+{
+	if (!edac_debug_level_initialized)
+		edac_set_dynamic_debug_level(edac_debug_level);
+}
+
 static int edac_set_debug_level(const char *buf,
 				const struct kernel_param *kp)
 {
@@ -32,13 +56,11 @@ static int edac_set_debug_level(const char *buf,
 	if (val > 4)
 		return -EINVAL;

+	edac_set_dynamic_debug_level(val);
+
 	return param_set_int(buf, kp);
 }

-/* Values of 0 to 4 will generate output */
-int edac_debug_level = 2;
-EXPORT_SYMBOL_GPL(edac_debug_level);
-
 module_param_call(edac_debug_level, edac_set_debug_level, param_get_int,
 		  &edac_debug_level, 0644);
 MODULE_PARM_DESC(edac_debug_level, "EDAC debug level: [0-4], default: 2");
@@ -130,6 +152,8 @@ static int __init edac_init(void)
 		goto err_wq;
 	}

+	edac_set_debug_level_init();
+
 	return 0;

 err_wq:
diff --git a/drivers/edac/edac_module.h b/drivers/edac/edac_module.h
index aa1f91688eb8..7f16428a266a 100644
--- a/drivers/edac/edac_module.h
+++ b/drivers/edac/edac_module.h
@@ -99,6 +99,7 @@ static inline void edac_debugfs_create_x16(const char *name, umode_t mode,
 					   struct dentry *parent, u16 *value)	{ }
 static inline void edac_debugfs_create_x32(const char *name, umode_t mode,
 		       struct dentry *parent, u32 *value)			{ }
+static inline void edac_set_debug_level_init(void)				{ }
 #endif

 /*