various fixes v3
Subsystems (5)
cover various fixes v3
Hi,
Dynamic debug fixes and cleanups. Only changes from v2 are a re-base against Linus's latest tree and the inclusion of the re-posted version for patch #11.
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 (7):
dynamic_debug: remove unused control variables
dynamic_debug: add Jason Baron 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
dynamic_debug: use a single printk() to emit msgs
MAINTAINERS | 6 ++ drivers/base/core.c | 5 +- include/linux/device.h | 5 ++ include/linux/dynamic_debug.h | 51 ++++++++----- include/linux/netdevice.h | 10 ++-- lib/dynamic_debug.c | 155 ++++++++++++++++++++++++++-------------- net/core/dev.c | 3 +- 7 files changed, 153 insertions(+), 82 deletions(-)
Signature · 1 line
1.7.5.4
PATCH 1/11 dynamic_debug: Add __dynamic_dev_dbg
From: Joe Perches <[email protected]>
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.
--- 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(-)
drivers/base/core.c +3 -2
@@ -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, ...)
include/linux/device.h +5 -0
@@ -785,6 +785,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 )));
@@ -805,6 +807,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 )));
include/linux/dynamic_debug.h +8 -2
@@ -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
lib/dynamic_debug.c +38 -0
@@ -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 (KERN_CONT , 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) {
Signature · 1 line
1.7.5.4
PATCH 2/11 dynamic_debug: Consolidate prefix output to single routine
In reply to Jason BaronFrom: Joe Perches <[email protected]>
Adding dynamic_dev_dbg duplicated prefix output.
Consolidate that output to a single routine.
--- lib/dynamic_debug.c | 38 ++++++++++++++++++-------------------- 1 files changed, 18 insertions(+), 20 deletions(-)
lib/dynamic_debug.c +18 -20
@@ -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 (KERN_CONT , dev, &vaf);va_end (args);
Signature · 1 line
1.7.5.4
PATCH 3/11 dynamic_debug: Remove uses of KERN_CONT in dynamic_emit_prefix
In reply to Jason BaronFrom: Joe Perches <[email protected]>
Multiple printks with KERN_CONT can be interleaved by other printks. Reduce the likelihood of that interleaving by consolidating multiple calls to printk.
--- lib/dynamic_debug.c | 34 +++++++++++++++++++++++----------- 1 files changed, 23 insertions(+), 11 deletions(-)
lib/dynamic_debug.c +23 -11
@@ -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, ...)
Signature · 1 line
1.7.5.4
PATCH 4/11 dynamic_debug: Convert printks to pr_<level>
In reply to Jason BaronFrom: Joe Perches <[email protected]>
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.
--- lib/dynamic_debug.c | 59 ++++++++++++++++++++++---------------------------- 1 files changed, 26 insertions(+), 33 deletions(-)
lib/dynamic_debug.c +26 -33
@@ -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);
Signature · 1 line
1.7.5.4
PATCH 5/11 dynamic_debug: remove unused control variables
In reply to Jason BaronFrom: Jason Baron <[email protected]>
Remove no longer used dynamic debug control variables. The definitions were removed a while ago, but we forgot to clean up the extern references.
--- include/linux/dynamic_debug.h | 7 ------- 1 files changed, 0 insertions(+), 7 deletions(-)
include/linux/dynamic_debug.h +0 -7
@@ -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,
Signature · 1 line
1.7.5.4
PATCH 6/11 dynamic_debug: add Jason Baron as maintainer
In reply to Jason BaronFrom: Jason Baron <[email protected]>
Add a maintainers entry for dynamic debug. Hopefully nobody will object to me as maintainer...
--- MAINTAINERS | 6 ++++++ 1 files changed, 6 insertions(+), 0 deletions(-)
MAINTAINERS +6 -0
@@ -2297,6 +2297,12 @@ L: [email protected] S: Maintained F: drivers/net/wan/dscc4.c +DYNAMIC DEBUG +M: Jason Baron <[email protected]> +S: Maintained +F: lib/dynamic_debug.c +F: include/linux/dynamic_debug.h + DZ DECSTATION DZ11 SERIAL DRIVER M: "Maciej W. Rozycki" <[email protected]> S: Maintained
Signature · 1 line
1.7.5.4
PATCH 7/11 dynamic_debug: make netdev_dbg() call __netdev_printk()
In reply to Jason BaronFrom: Jason Baron <[email protected]>
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().
--- 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(-)
include/linux/dynamic_debug.h +17 -0
@@ -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)
include/linux/netdevice.h +4 -2
@@ -2617,6 +2617,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 )));
@@ -2644,8 +2647,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...) \
lib/dynamic_debug.c +25 -0
@@ -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 (KERN_CONT , 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) {
net/core/dev.c +2 -1
@@ -6290,7 +6290,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;
@@ -6305,6 +6305,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, ...)
Signature · 1 line
1.7.5.4
PATCH 1/1 dynamic_debug: call __netdev_printk only for CONFIG_NET
__netdev_printk is only defined when CONFIG_NET is set. Since we only need __dynamic_netdev_dbg for network drivers, we can make it conditional on the same Kconfig symbol.
---
Quoted 10 lines · Jason Baron
From: Jason Baron <[email protected]>
Previously, if dynamic debug was enabled
netdev_dbg()was usingdynamic_dev_dbg()to print out the underlying msg. Fix this by making surenetdev_dbg()uses__netdev_printk().Cc David S. Miller <[email protected]>Signed-off-by Jason Baron <[email protected]>---
lib/dynamic_debug.c +2 -0
@@ -504,6 +504,7 @@ int __dynamic_dev_dbg(struct _ddebug *descriptor, }EXPORT_SYMBOL (__dynamic_dev_dbg); +#ifdef CONFIG_NET int __dynamic_netdev_dbg (struct _ddebug *descriptor,const struct net_device *dev,const char *fmt, ...) {
@@ -527,6 +528,7 @@ int __dynamic_netdev_dbg(struct _ddebug *descriptor, return res; }EXPORT_SYMBOL (__dynamic_netdev_dbg); +#endif static __initdata char ddebug_setup_string[1024 ];static __init int ddebug_setup_query (char *str)
Re: [PATCH ] dynamic_debug: call __netdev_printk only for CONFIG_NET
Quoted 4 lines · Arnd Bergmann
__netdev_printkis only defined whenCONFIG_NETis set. Since we only need__dynamic_netdev_dbgfor network drivers, we can make it conditional on the sameKconfigsymbol.
Hi,
Yes, I've posted a fix for this:
https://lkml.org/lkml/2011/8/30/297
Hopefully, it will be pulled in soon.
Thanks,
-Jason
Re: [PATCH ] dynamic_debug: call __netdev_printk only for CONFIG_NET
Quoted 6 lines · Jason Baron
Quoted 4 lines · Arnd Bergmann
__netdev_printkis only defined whenCONFIG_NETis set. Since we only need__dynamic_netdev_dbgfor network drivers, we can make it conditional on the sameKconfigsymbol.
Quoted 7 lines
Hi,
Yes, I've posted a fix for this:
https://lkml.org/lkml/2011/8/30/297
Hopefully, it will be pulled in soon.
As that thread again spun off into confusion, can you please resend the end result?
thanks,
greg k-h
Re: [PATCH ] dynamic_debug: call __netdev_printk only for CONFIG_NET
Quoted 9 lines · Greg KH
Quoted 8 lines · Jason Baron
Quoted 4 lines · Arnd Bergmann
__netdev_printkis only defined whenCONFIG_NETis set. Since we only need__dynamic_netdev_dbgfor network drivers, we can make it conditional on the sameKconfigsymbol.Hi,
Quoted 8 lines
Quoted 5 lines
Yes, I've posted a fix for this:
https://lkml.org/lkml/2011/8/30/297
Hopefully, it will be pulled in soon.
As that thread again spun off into confusion, can you please resend the end result?
That spinning confusion had nothing to do with the posted & correct patch which could have been applied several weeks ago.
I'm curious: Do you delete most of your email on a routine basis?
Signature · 2 lines
~Randy *** Remember to use Documentation/SubmitChecklist when testing your code ***
Re: [PATCH ] dynamic_debug: call __netdev_printk only for CONFIG_NET
Quoted 16 lines · Randy Dunlap
Quoted 15 lines · Greg KH
Quoted 13 lines · Jason Baron
Quoted 4 lines · Arnd Bergmann
__netdev_printkis only defined whenCONFIG_NETis set. Since we only need__dynamic_netdev_dbgfor network drivers, we can make it conditional on the sameKconfigsymbol.Hi,
Yes, I've posted a fix for this:
https://lkml.org/lkml/2011/8/30/297
Hopefully, it will be pulled in soon.
Quoted 7 lines
Quoted 2 lines
As that thread again spun off into confusion, can you please resend the end result?
That spinning confusion had nothing to do with the posted & correct patch which could have been applied several weeks ago.
I'm curious: Do you delete most of your email on a routine basis?
No, only after going through pending patches do I purge them. And when a series of patches generates a thread like this one, where people are arguing over the way the macros are named, and no one seems to agree, I will take it as the fact that this series was contentious and needs to be resent after taking into consideration the original complaints.
For me to keep all email threads, based on the amount of email I get[1], would be ludicrous.
greg k-h
Re: [PATCH ] dynamic_debug: call __netdev_printk only for CONFIG_NET
Quoted 23 lines · Greg KH
Quoted 22 lines · Randy Dunlap
Quoted 9 lines · Greg KH
Quoted 8 lines · Jason Baron
Quoted 4 lines · Arnd Bergmann
__netdev_printkis only defined whenCONFIG_NETis set. Since we only need__dynamic_netdev_dbgfor network drivers, we can make it conditional on the sameKconfigsymbol.Hi,
Quoted 8 lines
Quoted 5 lines
Yes, I've posted a fix for this:
https://lkml.org/lkml/2011/8/30/297
Hopefully, it will be pulled in soon.
As that thread again spun off into confusion, can you please resend the end result?
That spinning confusion had nothing to do with the posted & correct patch which could have been applied several weeks ago.
Quoted 7 lines
Quoted 1 line
I'm curious: Do you delete most of your email on a routine basis?
No, only after going through pending patches do I purge them. And when a series of patches generates a thread like this one, where people are arguing over the way the macros are named, and no one seems to agree, I will take it as the fact that this series was contentious and needs to be resent after taking into consideration the original complaints.
Sheesh. The naming of the macros has nothing to do with this build fix.
Quoted 6 lines
For me to keep all email threads, based on the amount of email I get[1], would be ludicrous.
greg k-h
Signature · 2 lines
~Randy *** Remember to use Documentation/SubmitChecklist when testing your code ***
Re: [PATCH ] dynamic_debug: call __netdev_printk only for CONFIG_NET
In reply to Greg KHQuoted 13 lines · Greg KH
Quoted 12 lines · Jason Baron
Quoted 4 lines · Arnd Bergmann
__netdev_printkis only defined whenCONFIG_NETis set. Since we only need__dynamic_netdev_dbgfor network drivers, we can make it conditional on the sameKconfigsymbol.Hi,
Yes, I've posted a fix for this:
Quoted 8 lines
Quoted 1 line
Hopefully, it will be pulled in soon.
As that thread again spun off into confusion, can you please resend the end result?
thanks,
greg k-h
Hi,
Andrew Morton has pulled these into his -mm tree...so I think the series should be all set.
Thanks,
-Jason
Re: [PATCH ] dynamic_debug: call __netdev_printk only for CONFIG_NET
Quoted 19 lines · Jason Baron
Quoted 18 lines · Greg KH
Quoted 6 lines · Jason Baron
Quoted 4 lines · Arnd Bergmann
__netdev_printkis only defined whenCONFIG_NETis set. Since we only need__dynamic_netdev_dbgfor network drivers, we can make it conditional on the sameKconfigsymbol.Quoted 7 lines
Hi,
Yes, I've posted a fix for this:
https://lkml.org/lkml/2011/8/30/297
Hopefully, it will be pulled in soon.
As that thread again spun off into confusion, can you please resend the end result?
Quoted 8 lines
Quoted 3 lines
thanks,
greg k-h
Hi,
Andrew Morton has pulled these into his -mm tree...so I think the series should be all set.
Good. The build fix is still needed in today's linux-next (20110919).
Signature · 2 lines
~Randy *** Remember to use Documentation/SubmitChecklist when testing your code ***
PATCH 8/11 dynamic_debug: make netif_dbg() call __netdev_printk()
In reply to Jason BaronFrom: Jason Baron <[email protected]>
Previously, netif_dbg() was using dynamic_dev_dbg() to perform the underlying printk. Fix it to use __netdev_printk(), instead.
--- include/linux/netdevice.h | 4 +--- 1 files changed, 1 insertions(+), 3 deletions(-)
include/linux/netdevice.h +1 -3
@@ -2714,9 +2714,7 @@ do { \ #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_netdev_dbg(netdev, format, ##args); \ } while (0)#else #define netif_dbg(priv, type, dev, format, args...) \
Signature · 1 line
1.7.5.4
PATCH 9/11 dynamic_debug: consolidate repetitive struct _ddebug descriptor definitions
In reply to Jason BaronFrom: Jason Baron <[email protected]>
Replace the repetitive struct _ddebug descriptor definitions with a new DYNAMIC_DEBUG_META_DATA(fmt) macro.
--- include/linux/dynamic_debug.h | 33 ++++++++++++++------------------- 1 files changed, 14 insertions(+), 19 deletions(-)
include/linux/dynamic_debug.h +14 -19
@@ -54,33 +54,28 @@ 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)
Signature · 1 line
1.7.5.4
Re: [PATCH 09/11] dynamic_debug: consolidate repetitive struct _ddebug descriptor definitions
Quoted 3 lines · Jason Baron
From: Jason Baron <[email protected]>
Replace the repetitive struct_ddebugdescriptor definitions with a newDYNAMIC_DEBUG_META_DATA(fmt)macro.
Hey Jason.
I think some improvements can be made to this one.
include/linux/dynamic_debug.h
Quoted 1 line
include/linux/dynamic_debug.h +0
-0
[]
include/linux/dynamic_debug.h extern int __dynamic_netdev_dbg(struct _ddebug *descriptor, · line 63
Quoted 11 lines
include/linux/dynamic_debug.h +7
-0@@ -54,33 +54,28 @@ 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 }; +
I think this is unclear and should be:
#define DECLARE_DYNAMIC_DEBUG_METADATA(name, fmt) \
static struct _ddebug name \
__used \
__attribute__((section("__verbose"), aligned(8))) = \
{ KBUILD_MODNAME, __func__, __FILE__, fmt, __LINE__, \
_DPRINTK_FLAGS_DEFAULT }(extra semicolon at end removed)
so the uses become:
include/linux/dynamic_debug.h extern int __dynamic_netdev_dbg(struct _ddebug *descriptor, · line 71
Quoted 7 lines
include/linux/dynamic_debug.h +1
-5#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); \
DECLARE_DYNAMIC_DEBUG_METADATA(descriptor, fmt);
include/linux/dynamic_debug.h extern int __dynamic_netdev_dbg(struct _ddebug *descriptor, · line 74
Quoted 4 lines
include/linux/dynamic_debug.h +1
-1if (unlikely (descriptor.enabled)) \ - __dynamic_dev_dbg(&descriptor, dev, fmt, ##__VA_ARGS__); \ + __dynamic_dev_dbg(&descriptor, dev, fmt, ##__VA_ARGS__);\ }while (0 )
so then there aren't any magic variable names.
cheers, Joe
Re: [PATCH 09/11 re-post] dynamic_debug: consolidate repetitive struct _ddebug descriptor definitions
Quoted 45 lines · Joe Perches
Quoted 3 lines
From: Jason Baron <[email protected]>
Replace the repetitive struct_ddebugdescriptor definitions with a newDYNAMIC_DEBUG_META_DATA(fmt)macro.Hey Jason.
I think some improvements can be made to this one.
include/linux/dynamic_debug.h
Quoted 1 line
include/linux/dynamic_debug.h +0
-0[]
include/linux/dynamic_debug.h extern int __dynamic_netdev_dbg(struct _ddebug *descriptor, · line 63
Quoted 11 lines
include/linux/dynamic_debug.h +7
-0@@ -54,33 +54,28 @@ 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 }; +I think this is unclear and should be:
#define DECLARE_DYNAMIC_DEBUG_METADATA(name, fmt) \ static struct _ddebug name \ __used \ __attribute__((section("__verbose"), aligned(8))) = \ { KBUILD_MODNAME, __func__, __FILE__, fmt, __LINE__, \ _DPRINTK_FLAGS_DEFAULT }(extra semicolon at end removed)
so the uses become:
include/linux/dynamic_debug.h extern int __dynamic_netdev_dbg(struct _ddebug *descriptor, · line 65
Quoted 7 lines
include/linux/dynamic_debug.h +1
-5#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); \DECLARE_DYNAMIC_DEBUG_METADATA(descriptor, fmt);
Quoted 9 lines
include/linux/dynamic_debug.h extern int __dynamic_netdev_dbg(struct _ddebug *descriptor, · line 68
Quoted 4 lines
include/linux/dynamic_debug.h +1
-1if (unlikely (descriptor.enabled)) \ - __dynamic_dev_dbg(&descriptor, dev, fmt, ##__VA_ARGS__); \ + __dynamic_dev_dbg(&descriptor, dev, fmt, ##__VA_ARGS__);\ }while (0 )so then there aren't any magic variable names.
cheers, Joe
Ok, Thanks for the review. Here's a re-post of it:
From: Jason Baron <[email protected]>
Replace the repetitive struct _ddebug descriptor definitions with a new DECLARE_DYNAMIC_DEBUG_METADATA(name, fmt) macro.
---
include/linux/dynamic_debug.h | 33 ++++++++++++++------------------- 1 files changed, 14 insertions(+), 19 deletions(-)
include/linux/dynamic_debug.h +14 -19
@@ -54,33 +54,28 @@ extern int __dynamic_netdev_dbg(struct _ddebug *descriptor, const char *fmt, ...) __attribute__ ((format (printf, 3, 4))); +#define DECLARE_DYNAMIC_DEBUG_METADATA (name, fmt) \ + static struct _ddebug name \ + __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 }; \ + DECLARE_DYNAMIC_DEBUG_METADATA(descriptor, 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 }; \ + DECLARE_DYNAMIC_DEBUG_METADATA(descriptor, 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 { \ + DECLARE_DYNAMIC_DEBUG_METADATA(descriptor, fmt); \ + if (unlikely(descriptor.enabled)) \ __dynamic_netdev_dbg(&descriptor, dev, fmt, ##__VA_ARGS__);\ } while (0)
Re: [PATCH 09/11 re-post] dynamic_debug: consolidate repetitive struct _ddebug descriptor definitions
Quoted 2 lines · Jason Baron
Replace the repetitive struct
_ddebugdescriptor definitions with a newDECLARE_DYNAMIC_DEBUG_METADATA(name, fmt)macro.
[]
include/linux/dynamic_debug.h
Quoted 1 line
include/linux/dynamic_debug.h +0
-0
[]
include/linux/dynamic_debug.h extern int __dynamic_netdev_dbg(struct _ddebug *descriptor, · line 62
Quoted 10 lines
include/linux/dynamic_debug.h +6
-0@@ -54,33 +54,28 @@ extern int __dynamic_netdev_dbg(struct _ddebug *descriptor, const char *fmt, ...) __attribute__ ((format (printf, 3, 4))); +#define DECLARE_DYNAMIC_DEBUG_METADATA (name, fmt) \ + static struct _ddebug name \ + __used \ + __attribute__((section("__verbose"), aligned(8))) = \ + { KBUILD_MODNAME, __func__, __FILE__, fmt, __LINE__, \ + _DPRINTK_FLAGS_DEFAULT }
Hey again Jason.
Quoted 1 line
From a kernel coding style consistency POV, shouldn't
this use c99 named initializers?
#define DECLARE_DYNAMIC_DEBUG_METADATA(name, fmt) \
static struct _ddebug name = { \
.modname = KBUILD_MODNAME, \
.function = __func__, \
.filename = __FILE__, \
.format = fmt, \
.lineno = __LINE__, \
.flags = _DPRINTK_FLAGS_DEFAULT, \
.enabled = false, \
} __used __aligned(8) __attribute__((section("__verbose")))If the pr_debug/dynamic_debug inversion is done,
the .enabled flag should be set on as well when
-DDEBUG is set, correct?
include/linux/dynamic_debug.h extern int __dynamic_netdev_dbg(struct _ddebug *descriptor, · line 68
Quoted 11 lines
#define
dynamic_pr_debug(fmt, ...)do { \include/linux/dynamic_debug.h +2
-6- static struct _ddebug descriptor \ -__used \ - __attribute__((section("__verbose"), aligned(8))) = \ - {KBUILD_MODNAME , __func__, __FILE__, fmt, __LINE__, \ - _DPRINTK_FLAGS_DEFAULT }; \ +DECLARE_DYNAMIC_DEBUG_METADATA (descriptor, 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 )
Perhaps
#define dynamic_pr_debug(fmt, ...) \
do { \
DECLARE_DYNAMIC_DEBUG_METADATA(descriptor, fmt); \
if (unlikely(descriptor.enabled)) \
__dynamic_pr_debug(&descriptor, pr_fmt(fmt), \
##__VA_ARGS__); \
} while (0)Lastly, are the unlikely()s really useful?
Re: [PATCH 09/11 re-post take #2] dynamic_debug: consolidate repetitive struct _ddebug descriptor definitions
Hi Joe,
Quoted 19 lines · Joe Perches
Quoted 2 lines · Jason Baron
Replace the repetitive struct
_ddebugdescriptor definitions with a newDECLARE_DYNAMIC_DEBUG_METADATA(name, fmt)macro.[]
include/linux/dynamic_debug.h
Quoted 1 line
include/linux/dynamic_debug.h +0
-0[]
include/linux/dynamic_debug.h extern int __dynamic_netdev_dbg(struct _ddebug *descriptor, · line 62
Quoted 10 lines
include/linux/dynamic_debug.h +6
-0@@ -54,33 +54,28 @@ extern int __dynamic_netdev_dbg(struct _ddebug *descriptor, const char *fmt, ...) __attribute__ ((format (printf, 3, 4))); +#define DECLARE_DYNAMIC_DEBUG_METADATA (name, fmt) \ + static struct _ddebug name \ + __used \ + __attribute__((section("__verbose"), aligned(8))) = \ + { KBUILD_MODNAME, __func__, __FILE__, fmt, __LINE__, \ + _DPRINTK_FLAGS_DEFAULT }Hey again Jason.
Quoted 2 lines
From a kernel coding style consistency POV, shouldn't this use c99 named initializers?
yes, that would be better than what we have now.
Quoted 12 lines
#define DECLARE_DYNAMIC_DEBUG_METADATA(name, fmt) \
static struct _ddebug name = { \
.modname = KBUILD_MODNAME, \
.function = __func__, \
.filename = __FILE__, \
.format = fmt, \
.lineno = __LINE__, \
.flags = _DPRINTK_FLAGS_DEFAULT, \
.enabled = false, \
} __used __aligned(8) __attribute__((section("__verbose")))Quoted 3 lines
If the pr_debug/dynamic_debug inversion is done,
the .enabled flag should be set on as well when
-DDEBUG is set, correct?
right, but I'm leaving this to a subsequent patch.
Quoted 15 lines
include/linux/dynamic_debug.h extern int __dynamic_netdev_dbg(struct _ddebug *descriptor, · line 67
Quoted 12 lines
include/linux/dynamic_debug.h +2
-6#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 }; \ + DECLARE_DYNAMIC_DEBUG_METADATA(descriptor, 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)Perhaps
Quoted 7 lines
#define
dynamic_pr_debug(fmt, ...)\do { \ DECLARE_DYNAMIC_DEBUG_METADATA(descriptor, fmt); \ if (unlikely(descriptor.enabled)) \ __dynamic_pr_debug(&descriptor, pr_fmt(fmt), \ ##__VA_ARGS__); \ } while (0)
ok.
Quoted 3 lines
Lastly, are the unlikely()s really useful?
I think the unlikely's make sense, since we don't expect these to be enabled by default.
So, I've re-spun the patch, with the above comments (below). Thanks!
-Jason
Replace the repetitive struct _ddebug descriptor definitions with a new DECLARE_DYNAMIC_DEBUG_META_DATA(name, fmt) macro.
--- include/linux/dynamic_debug.h | 64 ++++++++++++++++++++++------------------ 1 files changed, 35 insertions(+), 29 deletions(-)
include/linux/dynamic_debug.h +35 -29
@@ -54,35 +54,41 @@ extern int __dynamic_netdev_dbg(struct _ddebug *descriptor, const char *fmt, ...) __attribute__ ((format (printf, 3, 4))); -#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 }; \ - if (unlikely(descriptor.enabled)) \ - __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 }; \ - if (unlikely(descriptor.enabled)) \ - __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) +#define DECLARE_DYNAMIC_DEBUG_METADATA (name, fmt) \ + static struct _ddebug __used __aligned(8) \ + __attribute__((section("__verbose"))) name = { \ + .modname = KBUILD_MODNAME, \ + .function = __func__, \ + .filename = __FILE__, \ + .format = fmt, \ + .lineno = __LINE__, \ + .flags = _DPRINTK_FLAGS_DEFAULT, \ + .enabled = false, \ + } + +#define dynamic_pr_debug (fmt, ...) \ +do { \ + DECLARE_DYNAMIC_DEBUG_METADATA(descriptor, fmt); \ + if (unlikely(descriptor.enabled)) \ + __dynamic_pr_debug(&descriptor, pr_fmt(fmt), \ + ##__VA_ARGS__); \ +} while (0) + +#define dynamic_dev_dbg (dev, fmt, ...) \ +do { \ + DECLARE_DYNAMIC_DEBUG_METADATA(descriptor, fmt); \ + if (unlikely(descriptor.enabled)) \ + __dynamic_dev_dbg(&descriptor, dev, fmt, \ + ##__VA_ARGS__); \ +} while (0) + +#define dynamic_netdev_dbg (dev, fmt, ...) \ +do { \ + DECLARE_DYNAMIC_DEBUG_METADATA(descriptor, fmt); \ + if (unlikely(descriptor.enabled)) \ + __dynamic_netdev_dbg(&descriptor, dev, fmt, \ + ##__VA_ARGS__); \ +} while (0)#else
Signature · 1 line
1.7.5.4
Re: [PATCH 09/11 re-post take #2] dynamic_debug: consolidate repetitive struct _ddebug descriptor definitions
Quoted 2 lines · Jason Baron
Replace the repetitive struct
_ddebugdescriptor definitions with a newDECLARE_DYNAMIC_DEBUG_META_DATA(name, fmt)macro.
[]
include/linux/dynamic_debug.h
Quoted 1 line
include/linux/dynamic_debug.h +0
-0
[]
include/linux/dynamic_debug.h extern int __dynamic_netdev_dbg(struct _ddebug *descriptor, · line 59
Quoted 3 lines
include/linux/dynamic_debug.h +3
-0+#define DECLARE_DYNAMIC_DEBUG_METADATA (name, fmt) \ + static struct _ddebug __used __aligned(8) \ + __attribute__((section("__verbose"))) name = { \
[]
Jason, just one more thing...
Because the original struct _ddebug definition
above this uses __attribute__((aligned(8))),
struct _ddebug {
[...]
} __attribute__((aligned(8)));( and I suppose that should be __aligned(8) instead )
the __aligned(8) use in DECLARE_DYNAMIC_DEBUG_METADATA is not necessary.
cheers, Joe
Re: [PATCH 09/11 re-post take #2] dynamic_debug: consolidate repetitive struct _ddebug descriptor definitions
Quoted 20 lines · Joe Perches
Quoted 2 lines · Jason Baron
Replace the repetitive struct
_ddebugdescriptor definitions with a newDECLARE_DYNAMIC_DEBUG_META_DATA(name, fmt)macro.[]
include/linux/dynamic_debug.h
Quoted 1 line
include/linux/dynamic_debug.h +0
-0[]
include/linux/dynamic_debug.h extern int __dynamic_netdev_dbg(struct _ddebug *descriptor, · line 59
Quoted 3 lines
include/linux/dynamic_debug.h +3
-0+#define DECLARE_DYNAMIC_DEBUG_METADATA (name, fmt) \ + static struct _ddebug __used __aligned(8) \ + __attribute__((section("__verbose"))) name = { \[]
Jason, just one more thing...
Because the original struct
_ddebugdefinition
above this uses __attribute__((aligned(8))),struct _ddebug { [...] } __attribute__((aligned(8)));
Quoted 7 lines
( and I suppose that should be
__aligned(8)instead )the
__aligned(8)use inDECLARE_DYNAMIC_DEBUG_METADATAis not necessary.cheers, Joe
Hi Joe,
hmmm....what we probably need here, is an extra array of pointers into the dynamic debug structures. The reason being is that 'aligned' only specifies the minimum alignment, so the dynamic debug structures could be aligned to higher multiples, thus leaving extra padding. Since the dynamic debug code relies on a contiguous array this could lead to NULl pointer exceptions. This has been implemented for tracepoints, see:
https://lkml.org/lkml/2011/1/26/463. I haven't seen this issue in practice though...
So I'm going to leave the extra alignment specifications for now (they can't hurt), and at some point we probably need to look at the approach mentioned above.
Thanks,
-Jason
PATCH 10/11 dynamic_debug: remove num_enabled accounting
In reply to Jason BaronFrom: Jason Baron <[email protected]>
The num_enabled accouting aren't actually used anywhere - remove them.
--- lib/dynamic_debug.c | 7 ------- 1 files changed, 0 insertions(+), 7 deletions(-)
lib/dynamic_debug.c +0 -7
@@ -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);
Signature · 1 line
1.7.5.4
PATCH 11/11 dynamic_debug: use a single printk() to emit msgs
In reply to Jason BaronFrom: Jason Baron <[email protected]>
We were using KERN_CONT to combine msgs with their prefix. However, KERN_CONT is not smp safe, in the sense that it can interleave messages. This interleaving can result in printks coming out at the wrong loglevel. With the high frequency of printks, that dynamic debug can produce, this is not desirable.
Thus, make dynamic_emit_prefix(), fill a char buf[64], instead of doing a printk directly. If we enable printing out of function, module, line, or pid info, they are placed in this 64 byte buffer. In my testing 64 bytes was enough size to fulfill all requests. Even if its not, we can match up the printk itself to see where its from, so to me this is no big deal.
--- lib/dynamic_debug.c | 66 +++++++++++++++++++++------------------------------ 1 files changed, 27 insertions(+), 39 deletions(-)
lib/dynamic_debug.c +27 -39
@@ -422,52 +422,46 @@ static int ddebug_exec_query(char *query_string)return 0 ; } -static int dynamic_emit_prefix (const struct _ddebug *descriptor) +#define PREFIX_SIZE 64 +#define LEFT (wrote) ((PREFIX_SIZE - wrote) > 0) ? (PREFIX_SIZE - wrote) : 0 + +static char *dynamic_emit_prefix (const struct _ddebug *desc,char *buf) { -char tid[sizeof (int ) +sizeof (int )/2 +4 ]; -char lineno[sizeof (int ) +sizeof (int )/2 ]; +int pos =0 ; -if (descriptor->flags & _DPRINTK_FLAGS_INCL_TID) { + pos +=snprintf (buf + pos,LEFT (pos),"%s" ,KERN_DEBUG ); +if (desc->flags & _DPRINTK_FLAGS_INCL_TID) {if (in_interrupt ()) -snprintf (tid,sizeof (tid),"%s" ,"<intr> " ); + pos +=snprintf (buf + pos,LEFT (pos),"%s " , +"<intr>" );else -snprintf (tid,sizeof (tid),"[%d] " , -task_pid_vnr (current)); - }else { - tid[0 ] =0 ; + pos +=snprintf (buf + pos,LEFT (pos),"[%d] " , +task_pid_vnr (current)); } +if (desc->flags & _DPRINTK_FLAGS_INCL_MODNAME) + pos +=snprintf (buf + pos,LEFT (pos),"%s:" , desc->modname); +if (desc->flags & _DPRINTK_FLAGS_INCL_FUNCNAME) + pos +=snprintf (buf + pos,LEFT (pos),"%s:" , desc->function); +if (desc->flags & _DPRINTK_FLAGS_INCL_LINENO) + pos +=snprintf (buf + pos,LEFT (pos),"%d " , desc->lineno); -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); +return buf; }int __dynamic_pr_debug (struct _ddebug *descriptor,const char *fmt, ...) {va_list args;int res; +struct va_format vaf; +char buf[PREFIX_SIZE ];BUG_ON (!descriptor);BUG_ON (!fmt);va_start (args, fmt); - - res =dynamic_emit_prefix (descriptor); - res +=vprintk (fmt, args); - + vaf.fmt = fmt; + vaf.va = &args; + res =printk ("%s%pV" ,dynamic_emit_prefix (descriptor, buf), &vaf);va_end (args);return res;
@@ -480,18 +474,15 @@ int __dynamic_dev_dbg(struct _ddebug *descriptor,struct va_format vaf;va_list args;int res; +char buf[PREFIX_SIZE ];BUG_ON (!descriptor);BUG_ON (!fmt);va_start (args, fmt); - vaf.fmt = fmt; vaf.va = &args; - - res =dynamic_emit_prefix (descriptor); - res +=__dev_printk (KERN_CONT , dev, &vaf); - + res =__dev_printk (dynamic_emit_prefix (descriptor, buf), dev, &vaf);va_end (args);return res;
@@ -504,18 +495,15 @@ int __dynamic_netdev_dbg(struct _ddebug *descriptor,struct va_format vaf;va_list args;int res; +char buf[PREFIX_SIZE ];BUG_ON (!descriptor);BUG_ON (!fmt);va_start (args, fmt); - vaf.fmt = fmt; vaf.va = &args; - - res =dynamic_emit_prefix (descriptor); - res +=__netdev_printk (KERN_CONT , dev, &vaf); - + res =__netdev_printk (dynamic_emit_prefix (descriptor, buf), dev, &vaf);va_end (args);return res;
Signature · 1 line
1.7.5.4
Quoted 4 lines · Jason Baron
Hi,
Dynamic debug fixes and cleanups. Only changes from v2 are a re-base against Linus's latest tree and the inclusion of the re-posted version for patch #11.
I applied the first 8, as it seemed like there was lots of discussion about #9.
Care to send the rest when you all have those worked out?
thanks,
greg k-h
Quoted 11 lines · Greg KH
Quoted 4 lines · Jason Baron
Hi,
Dynamic debug fixes and cleanups. Only changes from v2 are a re-base against Linus's latest tree and the inclusion of the re-posted version for patch #11.
I applied the first 8, as it seemed like there was lots of discussion about #9.
Care to send the rest when you all have those worked out?
Hi Greg,
The comments on #9 were mostly around formatting. The 'take #2' version is fine: https://lkml.org/lkml/2011/8/15/395. Do you need me to re-post it? If not please also take 10, 11.
Thanks,
-Jason
Quoted 10 lines · Jason Baron
Quoted 9 lines · Greg KH
Quoted 4 lines · Jason Baron
Hi,
Dynamic debug fixes and cleanups. Only changes from v2 are a re-base against Linus's latest tree and the inclusion of the re-posted version for patch #11.
I applied the first 8, as it seemed like there was lots of discussion about #9.
Quoted 8 lines
Quoted 2 lines
Care to send the rest when you all have those worked out?
Hi Greg,
The comments on #9 were mostly around formatting. The 'take #2' version is fine: https://lkml.org/lkml/2011/8/15/395. Do you need me to re-post it? If not please also take 10, 11.
As stated above, pleae resend what you want me to apply.
thanks,
greg k-h