[PATCH 00/10] dynamic_debug: various fixes

[PATCH 00/10] dynamic_debug: various fixes

[Jason Baron]

Hi,

Various dynamic debug fixes and cleanups, and a patch to add myself as
maintainer. Hopefully, nobody will object too loudly :)

Thanks,

-Jason


Joe Perches (4):
  dynamic_debug: Add __dynamic_dev_dbg
  dynamic_debug: Consolidate prefix output to single routine
  dynamic_debug: Remove uses of KERN_CONT in dynamic_emit_prefix
  dynamic_debug: Convert printks to pr_<level>

Jason Baron (6):
  dynamic_debug: remove unused control variables
  dynamic_debug: add myslef as maintainer
  dynamic_debug: make netdev_dbg() call __netdev_printk()
  dynamic_debug: make netif_dbg() call __netdev_printk()
  dynamic_debug: consolidate repetitive struct _ddebug descriptor
    definitions
  dynamic_debug: remove num_enabled accounting

 MAINTAINERS                   |    6 ++
 drivers/base/core.c           |    5 +-
 include/linux/device.h        |    5 +
 include/linux/dynamic_debug.h |   58 ++++++++++-----
 include/linux/netdevice.h     |   12 ++--
 lib/dynamic_debug.c           |  165 ++++++++++++++++++++++++++++-------------
 net/core/dev.c                |    3 +-
 7 files changed, 172 insertions(+), 82 deletions(-)

-- 
1.7.5.4

[PATCH 01/10] dynamic_debug: Add __dynamic_dev_dbg

[Jason Baron]

From: Joe Perches <joe@perches.com>

Unlike dynamic_pr_debug, dynamic uses of dev_dbg can not
currently add task_pid/KBUILD_MODNAME/__func__/__LINE__
to selected debug output.

Add a new function similar to dynamic_pr_debug to
optionally emit these prefixes.

Cc: Aloisio Almeida <aloisio.almeida@openbossa.org>
Noticed-by: Aloisio Almeida <aloisio.almeida@openbossa.org>
Signed-off-by: Joe Perches <joe@perches.com>
Signed-off-by: Jason Baron <jbaron@redhat.com>
---
 drivers/base/core.c           |    5 +++--
 include/linux/device.h        |    5 +++++
 include/linux/dynamic_debug.h |   10 ++++++++--
 lib/dynamic_debug.c           |   38 ++++++++++++++++++++++++++++++++++++++
 4 files changed, 54 insertions(+), 4 deletions(-)

diff --git a/drivers/base/core.c b/drivers/base/core.c
index bc8729d..82c8654 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -1764,8 +1764,8 @@ void device_shutdown(void)
 
 #ifdef CONFIG_PRINTK
 
-static int __dev_printk(const char *level, const struct device *dev,
-			struct va_format *vaf)
+int __dev_printk(const char *level, const struct device *dev,
+		 struct va_format *vaf)
 {
 	if (!dev)
 		return printk("%s(NULL device *): %pV", level, vaf);
@@ -1773,6 +1773,7 @@ static int __dev_printk(const char *level, const struct device *dev,
 	return printk("%s%s %s: %pV",
 		      level, dev_driver_string(dev), dev_name(dev), vaf);
 }
+EXPORT_SYMBOL(__dev_printk);
 
 int dev_printk(const char *level, const struct device *dev,
 	       const char *fmt, ...)
diff --git a/include/linux/device.h b/include/linux/device.h
index 553fd37..5a9c8b5 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -786,6 +786,8 @@ extern const char *dev_driver_string(const struct device *dev);
 
 #ifdef CONFIG_PRINTK
 
+extern int __dev_printk(const char *level, const struct device *dev,
+			struct va_format *vaf);
 extern int dev_printk(const char *level, const struct device *dev,
 		      const char *fmt, ...)
 	__attribute__ ((format (printf, 3, 4)));
@@ -806,6 +808,9 @@ extern int _dev_info(const struct device *dev, const char *fmt, ...)
 
 #else
 
+static inline int __dev_printk(const char *level, const struct device *dev,
+			       struct va_format *vaf)
+	 { return 0; }
 static inline int dev_printk(const char *level, const struct device *dev,
 		      const char *fmt, ...)
 	__attribute__ ((format (printf, 3, 4)));
diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index e747ecd..bdf1531 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -47,6 +47,13 @@ extern int ddebug_remove_module(const char *mod_name);
 extern int __dynamic_pr_debug(struct _ddebug *descriptor, const char *fmt, ...)
 	__attribute__ ((format (printf, 2, 3)));
 
+struct device;
+
+extern int __dynamic_dev_dbg(struct _ddebug *descriptor,
+			     const struct device *dev,
+			     const char *fmt, ...)
+	__attribute__ ((format (printf, 3, 4)));
+
 #define dynamic_pr_debug(fmt, ...) do {					\
 	static struct _ddebug descriptor				\
 	__used								\
@@ -57,7 +64,6 @@ extern int __dynamic_pr_debug(struct _ddebug *descriptor, const char *fmt, ...)
 		__dynamic_pr_debug(&descriptor, pr_fmt(fmt), ##__VA_ARGS__); \
 	} while (0)
 
-
 #define dynamic_dev_dbg(dev, fmt, ...) do {				\
 	static struct _ddebug descriptor				\
 	__used								\
@@ -65,7 +71,7 @@ extern int __dynamic_pr_debug(struct _ddebug *descriptor, const char *fmt, ...)
 	{ KBUILD_MODNAME, __func__, __FILE__, fmt, __LINE__,		\
 		_DPRINTK_FLAGS_DEFAULT };				\
 	if (unlikely(descriptor.enabled))				\
-		dev_printk(KERN_DEBUG, dev, fmt, ##__VA_ARGS__);	\
+		__dynamic_dev_dbg(&descriptor, dev, fmt, ##__VA_ARGS__);	\
 	} while (0)
 
 #else
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 75ca78f..5c5f8f9 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -30,6 +30,7 @@
 #include <linux/jump_label.h>
 #include <linux/hardirq.h>
 #include <linux/sched.h>
+#include <linux/device.h>
 
 extern struct _ddebug __start___verbose[];
 extern struct _ddebug __stop___verbose[];
@@ -456,6 +457,43 @@ int __dynamic_pr_debug(struct _ddebug *descriptor, const char *fmt, ...)
 }
 EXPORT_SYMBOL(__dynamic_pr_debug);
 
+int __dynamic_dev_dbg(struct _ddebug *descriptor,
+		      const struct device *dev, const char *fmt, ...)
+{
+	struct va_format vaf;
+	va_list args;
+	int res;
+
+	BUG_ON(!descriptor);
+	BUG_ON(!fmt);
+
+	va_start(args, fmt);
+
+	vaf.fmt = fmt;
+	vaf.va = &args;
+
+	res = printk(KERN_DEBUG);
+	if (descriptor->flags & _DPRINTK_FLAGS_INCL_TID) {
+		if (in_interrupt())
+			res += printk(KERN_CONT "<intr> ");
+		else
+			res += printk(KERN_CONT "[%d] ", task_pid_vnr(current));
+	}
+	if (descriptor->flags & _DPRINTK_FLAGS_INCL_MODNAME)
+		res += printk(KERN_CONT "%s:", descriptor->modname);
+	if (descriptor->flags & _DPRINTK_FLAGS_INCL_FUNCNAME)
+		res += printk(KERN_CONT "%s:", descriptor->function);
+	if (descriptor->flags & _DPRINTK_FLAGS_INCL_LINENO)
+		res += printk(KERN_CONT "%d ", descriptor->lineno);
+
+	res += __dev_printk("", dev, &vaf);
+
+	va_end(args);
+
+	return res;
+}
+EXPORT_SYMBOL(__dynamic_dev_dbg);
+
 static __initdata char ddebug_setup_string[1024];
 static __init int ddebug_setup_query(char *str)
 {
-- 
1.7.5.4

[PATCH 02/10] dynamic_debug: Consolidate prefix output to single routine

[Jason Baron]

From: Joe Perches <joe@perches.com>

Adding dynamic_dev_dbg duplicated prefix output.
Consolidate that output to a single routine.

Signed-off-by: Joe Perches <joe@perches.com>
Signed-off-by: Jason Baron <jbaron@redhat.com>
---
 lib/dynamic_debug.c |   38 ++++++++++++++++++--------------------
 1 files changed, 18 insertions(+), 20 deletions(-)

diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 5c5f8f9..758e922 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -428,15 +428,10 @@ static int ddebug_exec_query(char *query_string)
 	return 0;
 }
 
-int __dynamic_pr_debug(struct _ddebug *descriptor, const char *fmt, ...)
+static int dynamic_emit_prefix(const struct _ddebug *descriptor)
 {
-	va_list args;
 	int res;
 
-	BUG_ON(!descriptor);
-	BUG_ON(!fmt);
-
-	va_start(args, fmt);
 	res = printk(KERN_DEBUG);
 	if (descriptor->flags & _DPRINTK_FLAGS_INCL_TID) {
 		if (in_interrupt())
@@ -450,7 +445,23 @@ int __dynamic_pr_debug(struct _ddebug *descriptor, const char *fmt, ...)
 		res += printk(KERN_CONT "%s:", descriptor->function);
 	if (descriptor->flags & _DPRINTK_FLAGS_INCL_LINENO)
 		res += printk(KERN_CONT "%d ", descriptor->lineno);
+
+	return res;
+}
+
+int __dynamic_pr_debug(struct _ddebug *descriptor, const char *fmt, ...)
+{
+	va_list args;
+	int res;
+
+	BUG_ON(!descriptor);
+	BUG_ON(!fmt);
+
+	va_start(args, fmt);
+
+	res = dynamic_emit_prefix(descriptor);
 	res += vprintk(fmt, args);
+
 	va_end(args);
 
 	return res;
@@ -472,20 +483,7 @@ int __dynamic_dev_dbg(struct _ddebug *descriptor,
 	vaf.fmt = fmt;
 	vaf.va = &args;
 
-	res = printk(KERN_DEBUG);
-	if (descriptor->flags & _DPRINTK_FLAGS_INCL_TID) {
-		if (in_interrupt())
-			res += printk(KERN_CONT "<intr> ");
-		else
-			res += printk(KERN_CONT "[%d] ", task_pid_vnr(current));
-	}
-	if (descriptor->flags & _DPRINTK_FLAGS_INCL_MODNAME)
-		res += printk(KERN_CONT "%s:", descriptor->modname);
-	if (descriptor->flags & _DPRINTK_FLAGS_INCL_FUNCNAME)
-		res += printk(KERN_CONT "%s:", descriptor->function);
-	if (descriptor->flags & _DPRINTK_FLAGS_INCL_LINENO)
-		res += printk(KERN_CONT "%d ", descriptor->lineno);
-
+	res = dynamic_emit_prefix(descriptor);
 	res += __dev_printk("", dev, &vaf);
 
 	va_end(args);
-- 
1.7.5.4

[PATCH 03/10] dynamic_debug: Remove uses of KERN_CONT in dynamic_emit_prefix

[Jason Baron]

From: Joe Perches <joe@perches.com>

Multiple printks with KERN_CONT can be interleaved by
other printks.  Reduce the likelihood of that interleaving
by consolidating multiple calls to printk.

Signed-off-by: Joe Perches <joe@perches.com>
Signed-off-by: Jason Baron <jbaron@redhat.com>
---
 lib/dynamic_debug.c |   34 +++++++++++++++++++++++-----------
 1 files changed, 23 insertions(+), 11 deletions(-)

diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 758e922..6669078 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -430,23 +430,35 @@ static int ddebug_exec_query(char *query_string)
 
 static int dynamic_emit_prefix(const struct _ddebug *descriptor)
 {
-	int res;
+	char tid[sizeof(int) + sizeof(int)/2 + 4];
+	char lineno[sizeof(int) + sizeof(int)/2];
 
-	res = printk(KERN_DEBUG);
 	if (descriptor->flags & _DPRINTK_FLAGS_INCL_TID) {
 		if (in_interrupt())
-			res += printk(KERN_CONT "<intr> ");
+			snprintf(tid, sizeof(tid), "%s", "<intr> ");
 		else
-			res += printk(KERN_CONT "[%d] ", task_pid_vnr(current));
+			snprintf(tid, sizeof(tid), "[%d] ",
+				 task_pid_vnr(current));
+	} else {
+		tid[0] = 0;
 	}
-	if (descriptor->flags & _DPRINTK_FLAGS_INCL_MODNAME)
-		res += printk(KERN_CONT "%s:", descriptor->modname);
-	if (descriptor->flags & _DPRINTK_FLAGS_INCL_FUNCNAME)
-		res += printk(KERN_CONT "%s:", descriptor->function);
-	if (descriptor->flags & _DPRINTK_FLAGS_INCL_LINENO)
-		res += printk(KERN_CONT "%d ", descriptor->lineno);
 
-	return res;
+	if (descriptor->flags & _DPRINTK_FLAGS_INCL_LINENO)
+		snprintf(lineno, sizeof(lineno), "%d", descriptor->lineno);
+	else
+		lineno[0] = 0;
+
+	return printk(KERN_DEBUG "%s%s%s%s%s%s",
+		      tid,
+		      (descriptor->flags & _DPRINTK_FLAGS_INCL_MODNAME) ?
+		      descriptor->modname : "",
+		      (descriptor->flags & _DPRINTK_FLAGS_INCL_MODNAME) ?
+		      ":" : "",
+		      (descriptor->flags & _DPRINTK_FLAGS_INCL_FUNCNAME) ?
+		      descriptor->function : "",
+		      (descriptor->flags & _DPRINTK_FLAGS_INCL_FUNCNAME) ?
+		      ":" : "",
+		      lineno);
 }
 
 int __dynamic_pr_debug(struct _ddebug *descriptor, const char *fmt, ...)
-- 
1.7.5.4

[PATCH 04/10] dynamic_debug: Convert printks to pr_<level>

[Jason Baron]

From: Joe Perches <joe@perches.com>

Add pr_fmt(fmt) with __func__.
Converts "ddebug:" prefix to "dynamic_debug:".

Most likely the if (verbose) outputs could
also be converted from pr_info to pr_debug.

Signed-off-by: Joe Perches <joe@perches.com>
Signed-off-by: Jason Baron <jbaron@redhat.com>
---
 lib/dynamic_debug.c |   59 ++++++++++++++++++++++----------------------------
 1 files changed, 26 insertions(+), 33 deletions(-)

diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 6669078..e627874 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -10,6 +10,8 @@
  * Copyright (C) 2011 Bart Van Assche.  All Rights Reserved.
  */
 
+#define pr_fmt(fmt) KBUILD_MODNAME ":%s: " fmt, __func__
+
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/moduleparam.h>
@@ -160,8 +162,7 @@ static void ddebug_change(const struct ddebug_query *query,
 			else
 				dp->enabled = 0;
 			if (verbose)
-				printk(KERN_INFO
-					"ddebug: changed %s:%d [%s]%s %s\n",
+				pr_info("changed %s:%d [%s]%s %s\n",
 					dp->filename, dp->lineno,
 					dt->mod_name, dp->function,
 					ddebug_describe_flags(dp, flagbuf,
@@ -171,7 +172,7 @@ static void ddebug_change(const struct ddebug_query *query,
 	mutex_unlock(&ddebug_lock);
 
 	if (!nfound && verbose)
-		printk(KERN_INFO "ddebug: no matches for query\n");
+		pr_info("no matches for query\n");
 }
 
 /*
@@ -216,10 +217,10 @@ static int ddebug_tokenize(char *buf, char *words[], int maxwords)
 
 	if (verbose) {
 		int i;
-		printk(KERN_INFO "%s: split into words:", __func__);
+		pr_info("split into words:");
 		for (i = 0 ; i < nwords ; i++)
-			printk(" \"%s\"", words[i]);
-		printk("\n");
+			pr_cont(" \"%s\"", words[i]);
+		pr_cont("\n");
 	}
 
 	return nwords;
@@ -331,16 +332,15 @@ static int ddebug_parse_query(char *words[], int nwords,
 			}
 		} else {
 			if (verbose)
-				printk(KERN_ERR "%s: unknown keyword \"%s\"\n",
-					__func__, words[i]);
+				pr_err("unknown keyword \"%s\"\n", words[i]);
 			return -EINVAL;
 		}
 	}
 
 	if (verbose)
-		printk(KERN_INFO "%s: q->function=\"%s\" q->filename=\"%s\" "
-		       "q->module=\"%s\" q->format=\"%s\" q->lineno=%u-%u\n",
-			__func__, query->function, query->filename,
+		pr_info("q->function=\"%s\" q->filename=\"%s\" "
+			"q->module=\"%s\" q->format=\"%s\" q->lineno=%u-%u\n",
+			query->function, query->filename,
 			query->module, query->format, query->first_lineno,
 			query->last_lineno);
 
@@ -369,7 +369,7 @@ static int ddebug_parse_flags(const char *str, unsigned int *flagsp,
 		return -EINVAL;
 	}
 	if (verbose)
-		printk(KERN_INFO "%s: op='%c'\n", __func__, op);
+		pr_info("op='%c'\n", op);
 
 	for ( ; *str ; ++str) {
 		for (i = ARRAY_SIZE(opt_array) - 1; i >= 0; i--) {
@@ -384,7 +384,7 @@ static int ddebug_parse_flags(const char *str, unsigned int *flagsp,
 	if (flags == 0)
 		return -EINVAL;
 	if (verbose)
-		printk(KERN_INFO "%s: flags=0x%x\n", __func__, flags);
+		pr_info("flags=0x%x\n", flags);
 
 	/* calculate final *flagsp, *maskp according to mask and op */
 	switch (op) {
@@ -402,8 +402,7 @@ static int ddebug_parse_flags(const char *str, unsigned int *flagsp,
 		break;
 	}
 	if (verbose)
-		printk(KERN_INFO "%s: *flagsp=0x%x *maskp=0x%x\n",
-			__func__, *flagsp, *maskp);
+		pr_info("*flagsp=0x%x *maskp=0x%x\n", *flagsp, *maskp);
 	return 0;
 }
 
@@ -508,7 +507,7 @@ static __initdata char ddebug_setup_string[1024];
 static __init int ddebug_setup_query(char *str)
 {
 	if (strlen(str) >= 1024) {
-		pr_warning("ddebug boot param string too large\n");
+		pr_warn("ddebug boot param string too large\n");
 		return 0;
 	}
 	strcpy(ddebug_setup_string, str);
@@ -536,8 +535,7 @@ static ssize_t ddebug_proc_write(struct file *file, const char __user *ubuf,
 		return -EFAULT;
 	tmpbuf[len] = '\0';
 	if (verbose)
-		printk(KERN_INFO "%s: read %d bytes from userspace\n",
-			__func__, (int)len);
+		pr_info("read %d bytes from userspace\n", (int)len);
 
 	ret = ddebug_exec_query(tmpbuf);
 	if (ret)
@@ -600,8 +598,7 @@ static void *ddebug_proc_start(struct seq_file *m, loff_t *pos)
 	int n = *pos;
 
 	if (verbose)
-		printk(KERN_INFO "%s: called m=%p *pos=%lld\n",
-			__func__, m, (unsigned long long)*pos);
+		pr_info("called m=%p *pos=%lld\n", m, (unsigned long long)*pos);
 
 	mutex_lock(&ddebug_lock);
 
@@ -626,8 +623,8 @@ static void *ddebug_proc_next(struct seq_file *m, void *p, loff_t *pos)
 	struct _ddebug *dp;
 
 	if (verbose)
-		printk(KERN_INFO "%s: called m=%p p=%p *pos=%lld\n",
-			__func__, m, p, (unsigned long long)*pos);
+		pr_info("called m=%p p=%p *pos=%lld\n",
+			m, p, (unsigned long long)*pos);
 
 	if (p == SEQ_START_TOKEN)
 		dp = ddebug_iter_first(iter);
@@ -650,8 +647,7 @@ static int ddebug_proc_show(struct seq_file *m, void *p)
 	char flagsbuf[8];
 
 	if (verbose)
-		printk(KERN_INFO "%s: called m=%p p=%p\n",
-			__func__, m, p);
+		pr_info("called m=%p p=%p\n", m, p);
 
 	if (p == SEQ_START_TOKEN) {
 		seq_puts(m,
@@ -676,8 +672,7 @@ static int ddebug_proc_show(struct seq_file *m, void *p)
 static void ddebug_proc_stop(struct seq_file *m, void *p)
 {
 	if (verbose)
-		printk(KERN_INFO "%s: called m=%p p=%p\n",
-			__func__, m, p);
+		pr_info("called m=%p p=%p\n", m, p);
 	mutex_unlock(&ddebug_lock);
 }
 
@@ -700,7 +695,7 @@ static int ddebug_proc_open(struct inode *inode, struct file *file)
 	int err;
 
 	if (verbose)
-		printk(KERN_INFO "%s: called\n", __func__);
+		pr_info("called\n");
 
 	iter = kzalloc(sizeof(*iter), GFP_KERNEL);
 	if (iter == NULL)
@@ -752,8 +747,7 @@ int ddebug_add_module(struct _ddebug *tab, unsigned int n,
 	mutex_unlock(&ddebug_lock);
 
 	if (verbose)
-		printk(KERN_INFO "%u debug prints in module %s\n",
-				 n, dt->mod_name);
+		pr_info("%u debug prints in module %s\n", n, dt->mod_name);
 	return 0;
 }
 EXPORT_SYMBOL_GPL(ddebug_add_module);
@@ -775,8 +769,7 @@ int ddebug_remove_module(const char *mod_name)
 	int ret = -ENOENT;
 
 	if (verbose)
-		printk(KERN_INFO "%s: removing module \"%s\"\n",
-				__func__, mod_name);
+		pr_info("removing module \"%s\"\n", mod_name);
 
 	mutex_lock(&ddebug_lock);
 	list_for_each_entry_safe(dt, nextdt, &ddebug_tables, link) {
@@ -852,8 +845,8 @@ static int __init dynamic_debug_init(void)
 	if (ddebug_setup_string[0] != '\0') {
 		ret = ddebug_exec_query(ddebug_setup_string);
 		if (ret)
-			pr_warning("Invalid ddebug boot param %s",
-				   ddebug_setup_string);
+			pr_warn("Invalid ddebug boot param %s",
+				ddebug_setup_string);
 		else
 			pr_info("ddebug initialized with string %s",
 				ddebug_setup_string);
-- 
1.7.5.4

[PATCH 05/10] dynamic_debug: remove unused control variables

[Jason Baron]

From: Jason Baron <jbaron@redhat.com>

Remove no longer used dynamic debug control variables. The
definitions were removed a while ago, but we forgot to clean
up the extern references.

Signed-off-by: Jason Baron <jbaron@redhat.com>
---
 include/linux/dynamic_debug.h |    7 -------
 1 files changed, 0 insertions(+), 7 deletions(-)

diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index bdf1531..843cb9e 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -1,13 +1,6 @@
 #ifndef _DYNAMIC_DEBUG_H
 #define _DYNAMIC_DEBUG_H
 
-/* dynamic_printk_enabled, and dynamic_printk_enabled2 are bitmasks in which
- * bit n is set to 1 if any modname hashes into the bucket n, 0 otherwise. They
- * use independent hash functions, to reduce the chance of false positives.
- */
-extern long long dynamic_debug_enabled;
-extern long long dynamic_debug_enabled2;
-
 /*
  * An instance of this structure is created in a special
  * ELF section at every dynamic debug callsite.  At runtime,
-- 
1.7.5.4

[PATCH 06/10] dynamic_debug: add myslef as maintainer

[Jason Baron]

From: Jason Baron <jbaron@redhat.com>

Add a maintainers entry for dynamic debug. Hopefully nobody
will object to me as maintainer...

Signed-off-by: Jason Baron <jbaron@redhat.com>
---
 MAINTAINERS |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index f0358cd..b3d3eb1 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2264,6 +2264,12 @@ L:	netdev@vger.kernel.org
 S:	Maintained
 F:	drivers/net/wan/dscc4.c
 
+DYNAMIC DEBUG
+M:	Jason Baron <jbaron@redhat.com>
+S:	Maintained
+F:	lib/dynamic_debug.c
+F:	include/linux/dynamic_debug.h
+
 DZ DECSTATION DZ11 SERIAL DRIVER
 M:	"Maciej W. Rozycki" <macro@linux-mips.org>
 S:	Maintained
-- 
1.7.5.4

[PATCH 07/10] dynamic_debug: make netdev_dbg() call __netdev_printk()

[Jason Baron]

From: Jason Baron <jbaron@redhat.com>

Previously, if dynamic debug was enabled netdev_dbg() was using
dynamic_dev_dbg() to print out the underlying msg. Fix this by making
sure netdev_dbg() uses __netdev_printk().

Cc: David S. Miller <davem@davemloft.net>
Signed-off-by: Jason Baron <jbaron@redhat.com>
---
 include/linux/dynamic_debug.h |   17 +++++++++++++++++
 include/linux/netdevice.h     |    6 ++++--
 lib/dynamic_debug.c           |   25 +++++++++++++++++++++++++
 net/core/dev.c                |    3 ++-
 4 files changed, 48 insertions(+), 3 deletions(-)

diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index 843cb9e..feaac1e 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -47,6 +47,13 @@ extern int __dynamic_dev_dbg(struct _ddebug *descriptor,
 			     const char *fmt, ...)
 	__attribute__ ((format (printf, 3, 4)));
 
+struct net_device;
+
+extern int __dynamic_netdev_dbg(struct _ddebug *descriptor,
+			     const struct net_device *dev,
+			     const char *fmt, ...)
+	__attribute__ ((format (printf, 3, 4)));
+
 #define dynamic_pr_debug(fmt, ...) do {					\
 	static struct _ddebug descriptor				\
 	__used								\
@@ -67,6 +74,16 @@ extern int __dynamic_dev_dbg(struct _ddebug *descriptor,
 		__dynamic_dev_dbg(&descriptor, dev, fmt, ##__VA_ARGS__);	\
 	} while (0)
 
+#define dynamic_netdev_dbg(dev, fmt, ...) do {				\
+	static struct _ddebug descriptor				\
+	__used								\
+	__attribute__((section("__verbose"), aligned(8))) =		\
+	{ KBUILD_MODNAME, __func__, __FILE__, fmt, __LINE__,		\
+		_DPRINTK_FLAGS_DEFAULT };				\
+	if (unlikely(descriptor.enabled))				\
+		__dynamic_netdev_dbg(&descriptor, dev, fmt, ##__VA_ARGS__);\
+	} while (0)
+
 #else
 
 static inline int ddebug_remove_module(const char *mod)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 54b8b4d..9b132ef 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -2635,6 +2635,9 @@ static inline const char *netdev_name(const struct net_device *dev)
 	return dev->name;
 }
 
+extern int __netdev_printk(const char *level, const struct net_device *dev,
+			struct va_format *vaf);
+
 extern int netdev_printk(const char *level, const struct net_device *dev,
 			 const char *format, ...)
 	__attribute__ ((format (printf, 3, 4)));
@@ -2662,8 +2665,7 @@ extern int netdev_info(const struct net_device *dev, const char *format, ...)
 #elif defined(CONFIG_DYNAMIC_DEBUG)
 #define netdev_dbg(__dev, format, args...)			\
 do {								\
-	dynamic_dev_dbg((__dev)->dev.parent, "%s: " format,	\
-			netdev_name(__dev), ##args);		\
+	dynamic_netdev_dbg(__dev, format, ##args);		\
 } while (0)
 #else
 #define netdev_dbg(__dev, format, args...)			\
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index e627874..db66a48 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -33,6 +33,7 @@
 #include <linux/hardirq.h>
 #include <linux/sched.h>
 #include <linux/device.h>
+#include <linux/netdevice.h>
 
 extern struct _ddebug __start___verbose[];
 extern struct _ddebug __stop___verbose[];
@@ -503,6 +504,30 @@ int __dynamic_dev_dbg(struct _ddebug *descriptor,
 }
 EXPORT_SYMBOL(__dynamic_dev_dbg);
 
+int __dynamic_netdev_dbg(struct _ddebug *descriptor,
+		      const struct net_device *dev, const char *fmt, ...)
+{
+	struct va_format vaf;
+	va_list args;
+	int res;
+
+	BUG_ON(!descriptor);
+	BUG_ON(!fmt);
+
+	va_start(args, fmt);
+
+	vaf.fmt = fmt;
+	vaf.va = &args;
+
+	res = dynamic_emit_prefix(descriptor);
+	res += __netdev_printk("", dev, &vaf);
+
+	va_end(args);
+
+	return res;
+}
+EXPORT_SYMBOL(__dynamic_netdev_dbg);
+
 static __initdata char ddebug_setup_string[1024];
 static __init int ddebug_setup_query(char *str)
 {
diff --git a/net/core/dev.c b/net/core/dev.c
index 9c58c1e..d6d48b2 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -6288,7 +6288,7 @@ const char *netdev_drivername(const struct net_device *dev)
 	return empty;
 }
 
-static int __netdev_printk(const char *level, const struct net_device *dev,
+int __netdev_printk(const char *level, const struct net_device *dev,
 			   struct va_format *vaf)
 {
 	int r;
@@ -6303,6 +6303,7 @@ static int __netdev_printk(const char *level, const struct net_device *dev,
 
 	return r;
 }
+EXPORT_SYMBOL(__netdev_printk);
 
 int netdev_printk(const char *level, const struct net_device *dev,
 		  const char *format, ...)
-- 
1.7.5.4

[PATCH 08/10] dynamic_debug: make netif_dbg() call __netdev_printk()

[Jason Baron]

From: Jason Baron <jbaron@redhat.com>

Previously, netif_dbg() was using dynamic_dev_dbg() to perform
the underlying printk. Fix it to use __netdev_printk(), instead.

Cc: David S. Miller <davem@davemloft.net>
Signed-off-by: Jason Baron <jbaron@redhat.com>
---
 include/linux/dynamic_debug.h |   12 ++++++++++++
 include/linux/netdevice.h     |    6 ++----
 2 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index feaac1e..7048e64 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -84,6 +84,18 @@ extern int __dynamic_netdev_dbg(struct _ddebug *descriptor,
 		__dynamic_netdev_dbg(&descriptor, dev, fmt, ##__VA_ARGS__);\
 	} while (0)
 
+#define dynamic_netif_dbg(dev, cond, fmt, ...) do {			\
+	static struct _ddebug descriptor				\
+	__used								\
+	__attribute__((section("__verbose"), aligned(8))) =		\
+	{ KBUILD_MODNAME, __func__, __FILE__, fmt, __LINE__,		\
+		_DPRINTK_FLAGS_DEFAULT };				\
+	if (unlikely(descriptor.enabled)) {				\
+		if (cond)						\
+			__dynamic_netdev_dbg(&descriptor, dev, fmt, ##__VA_ARGS__);\
+	}								\
+	} while (0)
+
 #else
 
 static inline int ddebug_remove_module(const char *mod)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 9b132ef..99c358f 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -2731,10 +2731,8 @@ do {								\
 #elif defined(CONFIG_DYNAMIC_DEBUG)
 #define netif_dbg(priv, type, netdev, format, args...)		\
 do {								\
-	if (netif_msg_##type(priv))				\
-		dynamic_dev_dbg((netdev)->dev.parent,		\
-				"%s: " format,			\
-				netdev_name(netdev), ##args);	\
+	dynamic_netif_dbg(netdev, (netif_msg_##type(priv)),	\
+				  format, ##args);		\
 } while (0)
 #else
 #define netif_dbg(priv, type, dev, format, args...)			\
-- 
1.7.5.4

[PATCH 09/10] dynamic_debug: consolidate repetitive struct _ddebug descriptor definitions

[Jason Baron]

From: Jason Baron <jbaron@redhat.com>

Replace the repetitive  struct _ddebug descriptor definitions with
a new DYNAMIC_DEBUG_META_DATA(fmt) macro.

Signed-off-by: Jason Baron <jbaron@redhat.com>
---
 include/linux/dynamic_debug.h |   48 ++++++++++++++++------------------------
 1 files changed, 19 insertions(+), 29 deletions(-)

diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index 7048e64..fdd3c0e 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -54,46 +54,36 @@ extern int __dynamic_netdev_dbg(struct _ddebug *descriptor,
 			     const char *fmt, ...)
 	__attribute__ ((format (printf, 3, 4)));
 
+#define DYNAMIC_DEBUG_METADATA(fmt)				\
+	static struct _ddebug descriptor			\
+	__used							\
+	__attribute__((section("__verbose"), aligned(8))) =	\
+	{ KBUILD_MODNAME, __func__, __FILE__, fmt, __LINE__,	\
+		_DPRINTK_FLAGS_DEFAULT };
+
 #define dynamic_pr_debug(fmt, ...) do {					\
-	static struct _ddebug descriptor				\
-	__used								\
-	__attribute__((section("__verbose"), aligned(8))) =		\
-	{ KBUILD_MODNAME, __func__, __FILE__, fmt, __LINE__,		\
-		_DPRINTK_FLAGS_DEFAULT };				\
+	DYNAMIC_DEBUG_METADATA(fmt);					\
 	if (unlikely(descriptor.enabled))				\
-		__dynamic_pr_debug(&descriptor, pr_fmt(fmt), ##__VA_ARGS__); \
+		__dynamic_pr_debug(&descriptor, pr_fmt(fmt), ##__VA_ARGS__);\
 	} while (0)
 
 #define dynamic_dev_dbg(dev, fmt, ...) do {				\
-	static struct _ddebug descriptor				\
-	__used								\
-	__attribute__((section("__verbose"), aligned(8))) =		\
-	{ KBUILD_MODNAME, __func__, __FILE__, fmt, __LINE__,		\
-		_DPRINTK_FLAGS_DEFAULT };				\
+	DYNAMIC_DEBUG_METADATA(fmt);					\
 	if (unlikely(descriptor.enabled))				\
-		__dynamic_dev_dbg(&descriptor, dev, fmt, ##__VA_ARGS__);	\
+		__dynamic_dev_dbg(&descriptor, dev, fmt, ##__VA_ARGS__);\
 	} while (0)
 
-#define dynamic_netdev_dbg(dev, fmt, ...) do {				\
-	static struct _ddebug descriptor				\
-	__used								\
-	__attribute__((section("__verbose"), aligned(8))) =		\
-	{ KBUILD_MODNAME, __func__, __FILE__, fmt, __LINE__,		\
-		_DPRINTK_FLAGS_DEFAULT };				\
-	if (unlikely(descriptor.enabled))				\
+#define dynamic_netdev_dbg(dev, fmt, ...) do {				   \
+	DYNAMIC_DEBUG_METADATA(fmt);					   \
+	if (unlikely(descriptor.enabled))				   \
 		__dynamic_netdev_dbg(&descriptor, dev, fmt, ##__VA_ARGS__);\
 	} while (0)
 
-#define dynamic_netif_dbg(dev, cond, fmt, ...) do {			\
-	static struct _ddebug descriptor				\
-	__used								\
-	__attribute__((section("__verbose"), aligned(8))) =		\
-	{ KBUILD_MODNAME, __func__, __FILE__, fmt, __LINE__,		\
-		_DPRINTK_FLAGS_DEFAULT };				\
-	if (unlikely(descriptor.enabled)) {				\
-		if (cond)						\
-			__dynamic_netdev_dbg(&descriptor, dev, fmt, ##__VA_ARGS__);\
-	}								\
+#define dynamic_netif_dbg(dev, cond, fmt, ...) do {			   \
+	DYNAMIC_DEBUG_METADATA(fmt);					   \
+	if (unlikely(descriptor.enabled) && (cond)) {			   \
+		__dynamic_netdev_dbg(&descriptor, dev, fmt, ##__VA_ARGS__);\
+	}								   \
 	} while (0)
 
 #else
-- 
1.7.5.4

[PATCH 10/10] dynamic_debug: remove num_enabled accounting

[Jason Baron]

From: Jason Baron <jbaron@redhat.com>

The num_enabled accouting aren't actually used anywhere - remove them.

Signed-off-by: Jason Baron <jbaron@redhat.com>
---
 lib/dynamic_debug.c |    7 -------
 1 files changed, 0 insertions(+), 7 deletions(-)

diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index db66a48..fbb4547 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -42,7 +42,6 @@ struct ddebug_table {
 	struct list_head link;
 	char *mod_name;
 	unsigned int num_ddebugs;
-	unsigned int num_enabled;
 	struct _ddebug *ddebugs;
 };
 
@@ -152,11 +151,6 @@ static void ddebug_change(const struct ddebug_query *query,
 			newflags = (dp->flags & mask) | flags;
 			if (newflags == dp->flags)
 				continue;
-
-			if (!newflags)
-				dt->num_enabled--;
-			else if (!dp->flags)
-				dt->num_enabled++;
 			dp->flags = newflags;
 			if (newflags)
 				dp->enabled = 1;
@@ -764,7 +758,6 @@ int ddebug_add_module(struct _ddebug *tab, unsigned int n,
 	}
 	dt->mod_name = new_name;
 	dt->num_ddebugs = n;
-	dt->num_enabled = 0;
 	dt->ddebugs = tab;
 
 	mutex_lock(&ddebug_lock);
-- 
1.7.5.4

Re: [PATCH 06/10] dynamic_debug: add myslef as maintainer

[Bart Van Assche]

On Wed, Jul 6, 2011 at 7:24 PM, Jason Baron <jbaron@redhat.com> wrote:
>
> From: Jason Baron <jbaron@redhat.com>
>
> Add a maintainers entry for dynamic debug.

Shouldn't that be "myself" in the subject ?

Bart.

Re: [PATCH 06/10] dynamic_debug: add myslef as maintainer

[Jason Baron]

On Wed, Jul 06, 2011 at 07:29:54PM +0200, Bart Van Assche wrote:
> On Wed, Jul 6, 2011 at 7:24 PM, Jason Baron <jbaron@redhat.com> wrote:
> >
> > From: Jason Baron <jbaron@redhat.com>
> >
> > Add a maintainers entry for dynamic debug.
> 
> Shouldn't that be "myself" in the subject ?
> 
> Bart.

yes, bad typo. thanks.

Re: [PATCH 00/10] dynamic_debug: various fixes

[Jim Cromie]

On Wed, Jul 6, 2011 at 11:24 AM, Jason Baron <jbaron@redhat.com> wrote:
> Hi,
>
> Various dynamic debug fixes and cleanups, and a patch to add myself as
> maintainer. Hopefully, nobody will object too loudly :)
>

do you have this in a git-tree somewhere I can pull ?

> Thanks,
>
> -Jason
>
>
> Joe Perches (4):
>  dynamic_debug: Add __dynamic_dev_dbg
>  dynamic_debug: Consolidate prefix output to single routine
>  dynamic_debug: Remove uses of KERN_CONT in dynamic_emit_prefix
>  dynamic_debug: Convert printks to pr_<level>
>
> Jason Baron (6):
>  dynamic_debug: remove unused control variables
>  dynamic_debug: add myslef as maintainer
>  dynamic_debug: make netdev_dbg() call __netdev_printk()
>  dynamic_debug: make netif_dbg() call __netdev_printk()
>  dynamic_debug: consolidate repetitive struct _ddebug descriptor
>    definitions
>  dynamic_debug: remove num_enabled accounting
>
>  MAINTAINERS                   |    6 ++
>  drivers/base/core.c           |    5 +-
>  include/linux/device.h        |    5 +
>  include/linux/dynamic_debug.h |   58 ++++++++++-----
>  include/linux/netdevice.h     |   12 ++--
>  lib/dynamic_debug.c           |  165 ++++++++++++++++++++++++++++-------------
>  net/core/dev.c                |    3 +-
>  7 files changed, 172 insertions(+), 82 deletions(-)
>
> --
> 1.7.5.4
>
>

Re: [PATCH 00/10] dynamic_debug: various fixes

[Jason Baron]

On Wed, Jul 06, 2011 at 11:57:21AM -0600, Jim Cromie wrote:
> >
> > Various dynamic debug fixes and cleanups, and a patch to add myself as
> > maintainer. Hopefully, nobody will object too loudly :)
> >
> 
> do you have this in a git-tree somewhere I can pull ?
> 

its only in a local tree...if you really need it, I can set something
up, but these are all the patches I have pending.

thanks,

-Jason

Re: [PATCH 01/10] dynamic_debug: Add __dynamic_dev_dbg

[Joe Perches]

On Wed, 2011-07-06 at 13:24 -0400, Jason Baron wrote:
> Unlike dynamic_pr_debug, dynamic uses of dev_dbg can not
> currently add task_pid/KBUILD_MODNAME/__func__/__LINE__
> to selected debug output.
> Add a new function similar to dynamic_pr_debug to
> optionally emit these prefixes.
[]
> diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
[]
> @@ -456,6 +457,43 @@ int __dynamic_pr_debug(struct _ddebug *descriptor, const char *fmt, ...)
[]
> +int __dynamic_dev_dbg(struct _ddebug *descriptor,
> +		      const struct device *dev, const char *fmt, ...)
[]
> +	res += __dev_printk("", dev, &vaf);

I suppose that should more properly be written as:

	res += __dev_printk(KERN_CONT, dev, &vaf);

Re: [PATCH 07/10] dynamic_debug: make netdev_dbg() call __netdev_printk()

[Joe Perches]

On Wed, 2011-07-06 at 13:25 -0400, Jason Baron wrote:
> diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
[]
> @@ -503,6 +504,30 @@ int __dynamic_dev_dbg(struct _ddebug *descriptor,
[]
> +int __dynamic_netdev_dbg(struct _ddebug *descriptor,
> +		      const struct net_device *dev, const char *fmt, ...)
[]
> +	res += __netdev_printk("", dev, &vaf);

KERN_CONT here too.

Re: [PATCH 08/10] dynamic_debug: make netif_dbg() call __netdev_printk()

[Joe Perches]

On Wed, 2011-07-06 at 13:25 -0400, Jason Baron wrote:
> From: Jason Baron <jbaron@redhat.com>
> 
> Previously, netif_dbg() was using dynamic_dev_dbg() to perform
> the underlying printk. Fix it to use __netdev_printk(), instead.
> 
> Cc: David S. Miller <davem@davemloft.net>
> Signed-off-by: Jason Baron <jbaron@redhat.com>
> ---
>  include/linux/dynamic_debug.h |   12 ++++++++++++
>  include/linux/netdevice.h     |    6 ++----
>  2 files changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
[]
> +#define dynamic_netif_dbg(dev, cond, fmt, ...) do {			\
> +	static struct _ddebug descriptor				\
> +	__used								\
> +	__attribute__((section("__verbose"), aligned(8))) =		\
> +	{ KBUILD_MODNAME, __func__, __FILE__, fmt, __LINE__,		\
> +		_DPRINTK_FLAGS_DEFAULT };				\
> +	if (unlikely(descriptor.enabled)) {				\
> +		if (cond)						\
> +			__dynamic_netdev_dbg(&descriptor, dev, fmt, ##__VA_ARGS__);\
> +	}								\
> +	} while (0)
> +

Just nits:

I think it'd be better to use
#define dynamic_netif_dbg(etc)						\
do {									\
	etc...
} while (0)

so that there aren't 2 consecutive close braces at the same indent level.

and maybe just use one test

	if (unlikely(descriptor.enabled) && cond)
		__dynamic_netdev_dbg(&descriptor, dev, fmt, ##__VA_ARGS__);


> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index 9b132ef..99c358f 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -2731,10 +2731,8 @@ do {								\
>  #elif defined(CONFIG_DYNAMIC_DEBUG)
>  #define netif_dbg(priv, type, netdev, format, args...)		\
>  do {								\
> -	if (netif_msg_##type(priv))				\
> -		dynamic_dev_dbg((netdev)->dev.parent,		\
> -				"%s: " format,			\
> -				netdev_name(netdev), ##args);	\
> +	dynamic_netif_dbg(netdev, (netif_msg_##type(priv)),	\
> +				  format, ##args);		\

Because you've already added dynamic_netdev_dbg,
maybe this should be:

#define netif_dbg(priv, type, netdev, format, args...)		\
do {								\
	if (netif_msg_##type(priv))				\
		dynamic_netdev_dbg(netdev, format, ##args);	\
} while (0)

Re: [PATCH 08/10] dynamic_debug: make netif_dbg() call __netdev_printk()

[Jason Baron]

On Wed, Jul 06, 2011 at 02:59:03PM -0700, Joe Perches wrote:
> On Wed, 2011-07-06 at 13:25 -0400, Jason Baron wrote:
> > From: Jason Baron <jbaron@redhat.com>
> > 
> > Previously, netif_dbg() was using dynamic_dev_dbg() to perform
> > the underlying printk. Fix it to use __netdev_printk(), instead.
> > 
> > Cc: David S. Miller <davem@davemloft.net>
> > Signed-off-by: Jason Baron <jbaron@redhat.com>
> > ---
> >  include/linux/dynamic_debug.h |   12 ++++++++++++
> >  include/linux/netdevice.h     |    6 ++----
> >  2 files changed, 14 insertions(+), 4 deletions(-)
> > 
> > diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
> []
> > +#define dynamic_netif_dbg(dev, cond, fmt, ...) do {			\
> > +	static struct _ddebug descriptor				\
> > +	__used								\
> > +	__attribute__((section("__verbose"), aligned(8))) =		\
> > +	{ KBUILD_MODNAME, __func__, __FILE__, fmt, __LINE__,		\
> > +		_DPRINTK_FLAGS_DEFAULT };				\
> > +	if (unlikely(descriptor.enabled)) {				\
> > +		if (cond)						\
> > +			__dynamic_netdev_dbg(&descriptor, dev, fmt, ##__VA_ARGS__);\
> > +	}								\
> > +	} while (0)
> > +
> 
> Just nits:
> 
> I think it'd be better to use
> #define dynamic_netif_dbg(etc)						\
> do {									\
> 	etc...
> } while (0)
> 
> so that there aren't 2 consecutive close braces at the same indent level.
> 
> and maybe just use one test
> 
> 	if (unlikely(descriptor.enabled) && cond)
> 		__dynamic_netdev_dbg(&descriptor, dev, fmt, ##__VA_ARGS__);
> 

If you look at the next patch, 9/10, I've combined the tests there
just as you've described. I agree, that it would be better if that were
folded into this patch. will fix.

> 
> > diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> > index 9b132ef..99c358f 100644
> > --- a/include/linux/netdevice.h
> > +++ b/include/linux/netdevice.h
> > @@ -2731,10 +2731,8 @@ do {								\
> >  #elif defined(CONFIG_DYNAMIC_DEBUG)
> >  #define netif_dbg(priv, type, netdev, format, args...)		\
> >  do {								\
> > -	if (netif_msg_##type(priv))				\
> > -		dynamic_dev_dbg((netdev)->dev.parent,		\
> > -				"%s: " format,			\
> > -				netdev_name(netdev), ##args);	\
> > +	dynamic_netif_dbg(netdev, (netif_msg_##type(priv)),	\
> > +				  format, ##args);		\
> 
> Because you've already added dynamic_netdev_dbg,
> maybe this should be:
> 
> #define netif_dbg(priv, type, netdev, format, args...)		\
> do {								\
> 	if (netif_msg_##type(priv))				\
> 		dynamic_netdev_dbg(netdev, format, ##args);	\
> } while (0)
> 
> 

The reason I didn't add it this way is b/c I plan on converting the
outer 'ifs' to the jump label infrastructure - which makes the disabled
case just a no-op and moves the printk and tests out of line.

Until that is done, i could see coding it as you've suggested, but I'd
prefer to leave it as is (and leave future churn to within the dynamic
debug code as opposed to the netdevice.h header).

Thanks,

-Jason

Re: [PATCH 08/10] dynamic_debug: make netif_dbg() call __netdev_printk()

[Joe Perches]

On Thu, 2011-07-07 at 10:13 -0400, Jason Baron wrote:
> On Wed, Jul 06, 2011 at 02:59:03PM -0700, Joe Perches wrote:
> > > diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> > > @@ -2731,10 +2731,8 @@ do {								\
> > >  #elif defined(CONFIG_DYNAMIC_DEBUG)
> > >  #define netif_dbg(priv, type, netdev, format, args...)		\
> > >  do {								\
> > > -	if (netif_msg_##type(priv))				\
> > > -		dynamic_dev_dbg((netdev)->dev.parent,		\
> > > -				"%s: " format,			\
> > > -				netdev_name(netdev), ##args);	\
> > > +	dynamic_netif_dbg(netdev, (netif_msg_##type(priv)),	\
> > > +				  format, ##args);		\
> > Because you've already added dynamic_netdev_dbg,
> > maybe this should be:
> > #define netif_dbg(priv, type, netdev, format, args...)		\
> > do {								\
> > 	if (netif_msg_##type(priv))				\
> > 		dynamic_netdev_dbg(netdev, format, ##args);	\
> > } while (0)
> The reason I didn't add it this way is b/c I plan on converting the
> outer 'ifs' to the jump label infrastructure - which makes the disabled
> case just a no-op and moves the printk and tests out of line.

Perhaps you needn't do that.

I think there's little to be gained to move the test
outwards and not perform the netif_msg##type(priv)

> Until that is done, i could see coding it as you've suggested, but I'd
> prefer to leave it as is (and leave future churn to within the dynamic
> debug code as opposed to the netdevice.h header).

Shrug.  I think that dynamic_debug will have continuing
impacts on various subsystems unless there's some generic
__dynamic_dbg() and _prefix() mechanism introduced into
more generic <foo>_dbg style.

Anything logging message that uses <foo>_dbg or <foo>_vdbg
is a candidate for dynamic_debug uses, but there's no
current generic mechanism to avoid subsystem specific needs.

Any of these could need some dynamic_debug consideration:

$ grep -rPoh --include=*.[ch] "[a-z_]+_[v]?dbg\(" * | sort | uniq
acpi_ut_allocate_object_desc_dbg(
acpi_ut_create_internal_object_dbg(
adc_dbg(
add_dyn_dbg(
airo_print_dbg(
ata_dev_dbg(
ata_link_dbg(
ata_port_dbg(
ath_dbg(
atm_dbg(
bat_dbg(
bit_dbg(
cafe_dev_dbg(
cam_dbg(
c_freq_dbg(
chan_dbg(
chan_reg_rule_print_dbg(
cmm_dbg(
c_pm_dbg(
ctrl_dbg(
__dbg(
desc_dbg(
dev_dbg(
dev_vdbg(
dma_request_channel_dbg(
__dump_desc_dbg(
dump_desc_dbg(
dump_pq_desc_dbg(
dynamic_dev_dbg(
e_dbg(
ehca_dbg(
ehca_gen_dbg(
ehci_dbg(
ehci_vdbg(
en_dbg(
ep_dbg(
ep_vdbg(
fhci_dbg(
fhci_vdbg(
fit_dbg(
gig_dbg(
gpio_dbg(
gru_dbg(
hgpk_dbg(
hid_dbg(
hw_dbg(
ibmvfc_dbg(
ipath_dbg(
ipoib_dbg(
ipr_dbg(
iser_dbg(
isp_isr_dbg(
itd_dbg(
ite_dbg(
l_dbg(
lg_dbg(
mce_dbg(
memblock_dbg(
mhwmp_dbg(
mpeg_dbg(
mpl_dbg(
msg_dbg(
mthca_dbg(
netdev_dbg(
netdev_vdbg(
netif_dbg(
netif_vdbg(
nfc_dbg(
nfc_dev_dbg(
nsp_dbg(
nvt_dbg(
ohci_dbg(
ohci_vdbg(
oxu_dbg(
oxu_vdbg(
pch_dbg(
pch_pci_dbg(
pm_dev_dbg(
pnp_dbg(
pop_dbg(
prep_dma_pq_dbg(
prep_dma_pqzero_sum_dbg(
prep_dma_xor_dbg(
_print_dbg(
print_dbg(
pwm_dbg(
rdev_dbg(
reg_dbg(
sh_keysc_map_dbg(
slice_dbg(
smsc_dbg(
start_dbg(
stop_dbg(
sysrq_handle_dbg(
tda_dbg(
tgt_dbg(
tipc_msg_dbg(
__tuner_dbg(
tuner_dbg(
tveeprom_dbg(
tx_dbg(
uea_dbg(
uea_vdbg(
ugeth_dbg(
ugeth_vdbg(
urb_dbg(
usb_dbg(
vpif_dbg(
wiphy_dbg(
wiphy_vdbg(
xhci_dbg(
x_show_dbg(

Re: [PATCH 08/10] dynamic_debug: make netif_dbg() call __netdev_printk()

[Jason Baron]

On Thu, Jul 07, 2011 at 09:29:21AM -0700, Joe Perches wrote:
> On Thu, 2011-07-07 at 10:13 -0400, Jason Baron wrote:
> > On Wed, Jul 06, 2011 at 02:59:03PM -0700, Joe Perches wrote:
> > > > diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> > > > @@ -2731,10 +2731,8 @@ do {								\
> > > >  #elif defined(CONFIG_DYNAMIC_DEBUG)
> > > >  #define netif_dbg(priv, type, netdev, format, args...)		\
> > > >  do {								\
> > > > -	if (netif_msg_##type(priv))				\
> > > > -		dynamic_dev_dbg((netdev)->dev.parent,		\
> > > > -				"%s: " format,			\
> > > > -				netdev_name(netdev), ##args);	\
> > > > +	dynamic_netif_dbg(netdev, (netif_msg_##type(priv)),	\
> > > > +				  format, ##args);		\
> > > Because you've already added dynamic_netdev_dbg,
> > > maybe this should be:
> > > #define netif_dbg(priv, type, netdev, format, args...)		\
> > > do {								\
> > > 	if (netif_msg_##type(priv))				\
> > > 		dynamic_netdev_dbg(netdev, format, ##args);	\
> > > } while (0)
> > The reason I didn't add it this way is b/c I plan on converting the
> > outer 'ifs' to the jump label infrastructure - which makes the disabled
> > case just a no-op and moves the printk and tests out of line.
> 
> Perhaps you needn't do that.
> 
> I think there's little to be gained to move the test
> outwards and not perform the netif_msg##type(priv)

In this particualr case, there might not be a large gain, but when I've
converted all of the dynamic debug infrastructure to jump labels I can
consistently see througput gains of 1% on tbench testing.

> 
> > Until that is done, i could see coding it as you've suggested, but I'd
> > prefer to leave it as is (and leave future churn to within the dynamic
> > debug code as opposed to the netdevice.h header).
> 
> Shrug.  I think that dynamic_debug will have continuing
> impacts on various subsystems unless there's some generic
> __dynamic_dbg() and _prefix() mechanism introduced into
> more generic <foo>_dbg style.
> 
> Anything logging message that uses <foo>_dbg or <foo>_vdbg
> is a candidate for dynamic_debug uses, but there's no
> current generic mechanism to avoid subsystem specific needs.
> 
> Any of these could need some dynamic_debug consideration:
> 

right. looking quickly over this list there seem to be a few different
categories:

-some just alias to dev_dbg(), so they are already picked up
-some use level logging, this could be easily added to dyanmic debug -
  we store level info in the descriptor and then check it against
  a currently set level, which can be per-debug statement
-any ones that can't fit the current model could probably be easily
 converted using a callback, That is we have some dynamic debug
 function take an optional function, which if the debugging is enabled
 is called. The format string, could optionally be blank in this case,
 I guess.

So I think it could be converted while being minimaly invasive in terms
of run-time checking. In fact, that was one of my original goals was to
try and convert all the disparate debugging calls, to a more generic
infrastructure. I know some subsystem converted to use pr_debug(), to
tie into dynamic debug, but it would take a bit of work to convert the
rest...thoughts?

thanks,

-Jason


> $ grep -rPoh --include=*.[ch] "[a-z_]+_[v]?dbg\(" * | sort | uniq
> acpi_ut_allocate_object_desc_dbg(
> acpi_ut_create_internal_object_dbg(
> adc_dbg(
> add_dyn_dbg(
> airo_print_dbg(
> ata_dev_dbg(
> ata_link_dbg(
> ata_port_dbg(
> ath_dbg(
> atm_dbg(
> bat_dbg(
> bit_dbg(
> cafe_dev_dbg(
> cam_dbg(
> c_freq_dbg(
> chan_dbg(
> chan_reg_rule_print_dbg(
> cmm_dbg(
> c_pm_dbg(
> ctrl_dbg(
> __dbg(
> desc_dbg(
> dev_dbg(
> dev_vdbg(
> dma_request_channel_dbg(
> __dump_desc_dbg(
> dump_desc_dbg(
> dump_pq_desc_dbg(
> dynamic_dev_dbg(
> e_dbg(
> ehca_dbg(
> ehca_gen_dbg(
> ehci_dbg(
> ehci_vdbg(
> en_dbg(
> ep_dbg(
> ep_vdbg(
> fhci_dbg(
> fhci_vdbg(
> fit_dbg(
> gig_dbg(
> gpio_dbg(
> gru_dbg(
> hgpk_dbg(
> hid_dbg(
> hw_dbg(
> ibmvfc_dbg(
> ipath_dbg(
> ipoib_dbg(
> ipr_dbg(
> iser_dbg(
> isp_isr_dbg(
> itd_dbg(
> ite_dbg(
> l_dbg(
> lg_dbg(
> mce_dbg(
> memblock_dbg(
> mhwmp_dbg(
> mpeg_dbg(
> mpl_dbg(
> msg_dbg(
> mthca_dbg(
> netdev_dbg(
> netdev_vdbg(
> netif_dbg(
> netif_vdbg(
> nfc_dbg(
> nfc_dev_dbg(
> nsp_dbg(
> nvt_dbg(
> ohci_dbg(
> ohci_vdbg(
> oxu_dbg(
> oxu_vdbg(
> pch_dbg(
> pch_pci_dbg(
> pm_dev_dbg(
> pnp_dbg(
> pop_dbg(
> prep_dma_pq_dbg(
> prep_dma_pqzero_sum_dbg(
> prep_dma_xor_dbg(
> _print_dbg(
> print_dbg(
> pwm_dbg(
> rdev_dbg(
> reg_dbg(
> sh_keysc_map_dbg(
> slice_dbg(
> smsc_dbg(
> start_dbg(
> stop_dbg(
> sysrq_handle_dbg(
> tda_dbg(
> tgt_dbg(
> tipc_msg_dbg(
> __tuner_dbg(
> tuner_dbg(
> tveeprom_dbg(
> tx_dbg(
> uea_dbg(
> uea_vdbg(
> ugeth_dbg(
> ugeth_vdbg(
> urb_dbg(
> usb_dbg(
> vpif_dbg(
> wiphy_dbg(
> wiphy_vdbg(
> xhci_dbg(
> x_show_dbg(
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 02/10] dynamic_debug: Consolidate prefix output to single routine

[Bart Van Assche]

On Wed, Jul 6, 2011 at 7:24 PM, Jason Baron <jbaron@redhat.com> wrote:
>
> From: Joe Perches <joe@perches.com>
>
> Adding dynamic_dev_dbg duplicated prefix output.
> Consolidate that output to a single routine.
>
> Signed-off-by: Joe Perches <joe@perches.com>
> Signed-off-by: Jason Baron <jbaron@redhat.com>
> ---
>  lib/dynamic_debug.c |   38 ++++++++++++++++++--------------------
>  1 files changed, 18 insertions(+), 20 deletions(-)
>
> diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
> index 5c5f8f9..758e922 100644
> --- a/lib/dynamic_debug.c
> +++ b/lib/dynamic_debug.c
> @@ -428,15 +428,10 @@ static int ddebug_exec_query(char *query_string)
>        return 0;
>  }
>
> -int __dynamic_pr_debug(struct _ddebug *descriptor, const char *fmt, ...)
> +static int dynamic_emit_prefix(const struct _ddebug *descriptor)
>  {

How about renaming dynamic_emit_prefix() into ddebug_emit_prefix() ?
Personally I prefer the latter name over the former.

Bart.

Re: [PATCH 02/10] dynamic_debug: Consolidate prefix output to single routine

[Jim Cromie]

On Thu, Jul 7, 2011 at 12:14 PM, Bart Van Assche <bvanassche@acm.org> wrote:
> On Wed, Jul 6, 2011 at 7:24 PM, Jason Baron <jbaron@redhat.com> wrote:
>>
>> From: Joe Perches <joe@perches.com>
>>
>> Adding dynamic_dev_dbg duplicated prefix output.
>> Consolidate that output to a single routine.
>>
>> Signed-off-by: Joe Perches <joe@perches.com>
>> Signed-off-by: Jason Baron <jbaron@redhat.com>
>> ---
>>  lib/dynamic_debug.c |   38 ++++++++++++++++++--------------------
>>  1 files changed, 18 insertions(+), 20 deletions(-)
>>
>> diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
>> index 5c5f8f9..758e922 100644
>> --- a/lib/dynamic_debug.c
>> +++ b/lib/dynamic_debug.c
>> @@ -428,15 +428,10 @@ static int ddebug_exec_query(char *query_string)
>>        return 0;
>>  }
>>
>> -int __dynamic_pr_debug(struct _ddebug *descriptor, const char *fmt, ...)
>> +static int dynamic_emit_prefix(const struct _ddebug *descriptor)
>>  {
>
> How about renaming dynamic_emit_prefix() into ddebug_emit_prefix() ?
> Personally I prefer the latter name over the former.
>
> Bart.
>

WRT naming, Id like dyndbg, esp as exposed on kernel cmdline, parameters..

dyndbg.query=....
dyndbg.verbose=1
/sys/modules/dyndbg/parameters/verbose

does this run up against ABI issues ?

Re: [PATCH 08/10] dynamic_debug: make netif_dbg() call __netdev_printk()

[Joe Perches]

On Thu, 2011-07-07 at 14:09 -0400, Jason Baron wrote:
> On Thu, Jul 07, 2011 at 09:29:21AM -0700, Joe Perches wrote:
> > I think there's little to be gained to move the test
> > outwards and not perform the netif_msg##type(priv)
> In this particualr case, there might not be a large gain, but when I've
> converted all of the dynamic debug infrastructure to jump labels I can
> consistently see througput gains of 1% on tbench testing.

And that's not this case is it.
I don't see any value here.

[]

> I think that dynamic_debug will have continuing
> > impacts on various subsystems unless there's some generic
> > __dynamic_dbg() and _prefix() mechanism introduced into
> > more generic <foo>_dbg style.
> > Anything logging message that uses <foo>_dbg or <foo>_vdbg
> > is a candidate for dynamic_debug uses, but there's no
> > current generic mechanism to avoid subsystem specific needs.
> > Any of these could need some dynamic_debug consideration:
> right. looking quickly over this list there seem to be a few different
> categories:
> -some just alias to dev_dbg(), so they are already picked up
> -some use level logging, this could be easily added to dyanmic debug -
>   we store level info in the descriptor and then check it against
>   a currently set level, which can be per-debug statement

Fine by me.

That might also make all other netif_<type>()
and <foo>_<level>(bitmap or level test, fmt, ...)
possible to combine in this mechanism as well.

There are a lot of those.

> -any ones that can't fit the current model could probably be easily
>  converted using a callback, That is we have some dynamic debug
>  function take an optional function, which if the debugging is enabled
>  is called.

I believe that would require some registration mechanism
for modules.

> In fact, that was one of my original goals was to
> try and convert all the disparate debugging calls, to a more generic
> infrastructure. I know some subsystem converted to use pr_debug(), to
> tie into dynamic debug, but it would take a bit of work to convert the
> rest...thoughts?

Go for it.
You're the ddebug maintainer.
I'm gladly review though.

cheers, Joe