[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

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);

[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

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.

[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

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

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