[PATCH 00/16] dynamic debug: ehancements and cleanups

[PATCH 00/16] dynamic debug: ehancements and cleanups

[Jason Baron]

Hi,

Mostly cleanups and minor feature ehancements for dynamic debug. I haven't
included the module parameter feature, since its implementation is still
being worked out, but I wanted to get some of Jim's patches included, since
I've got so many. These have already been through several review iterations.
Patches are against Linus's latest tree.

Thanks,

-Jason

Jim Cromie (16):
  dynamic_debug: fix whitespace complaints from scripts/cleanfile
  dynamic_debug: drop enabled field from struct _ddebug, use _DPRINTK_FLAGS_PRINT
  dynamic_debug: make dynamic-debug supersede DEBUG ccflag
  dynamic_debug: change verbosity at runtime
  dynamic_debug: replace strcpy with strlcpy, in ddebug_setup_query()
  dynamic_debug: pr_err() call should not depend upon verbosity
  dynamic_debug: drop explicit !=NULL checks
  dynamic_debug: describe_flags with '=[pmflt_]*'
  dynamic_debug: tighten up error checking on debug queries
  dynamic_debug: early return if _ddebug table is empty
  dynamic_debug: reduce lineno field to a saner 18 bits
  dynamic_debug: chop off comments in ddebug_tokenize
  dynamic_debug: enlarge command/query write buffer
  dynamic_debug: add trim_prefix() to provide source-root relative paths
  dynamic_debug: factor vpr_info_dq out of ddebug_parse_query
  dynamic_debug: process multiple debug-queries on a line

 Documentation/dynamic-debug-howto.txt |   30 ++---
 include/linux/device.h                |    8 +-
 include/linux/dynamic_debug.h         |   19 ++-
 include/linux/netdevice.h             |    8 +-
 include/linux/printk.h                |    8 +-
 lib/dynamic_debug.c                   |  270 +++++++++++++++++++++++----------
 6 files changed, 222 insertions(+), 121 deletions(-)

-- 
1.7.7.3

[PATCH 01/16] dynamic_debug: fix whitespace complaints from scripts/cleanfile

[Jason Baron]

From: Jim Cromie <jim.cromie@gmail.com>

Style cleanups.

Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
Signed-off-by: Jason Baron <jbaron@redhat.com>
---
 lib/dynamic_debug.c |   16 +++++++++-------
 1 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index dcdade3..e487d13 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -187,7 +187,7 @@ static int ddebug_tokenize(char *buf, char *words[], int maxwords)
 		if (!*buf)
 			break;	/* oh, it was trailing whitespace */
 
-		/* Run `end' over a word, either whitespace separated or quoted */
+		/* find `end' of word, whitespace separated or quoted */
 		if (*buf == '"' || *buf == '\'') {
 			int quote = *buf++;
 			for (end = buf ; *end && *end != quote ; end++)
@@ -199,8 +199,8 @@ static int ddebug_tokenize(char *buf, char *words[], int maxwords)
 				;
 			BUG_ON(end == buf);
 		}
-		/* Here `buf' is the start of the word, `end' is one past the end */
 
+		/* `buf' is start of word, `end' is one past its end */
 		if (nwords == maxwords)
 			return -EINVAL;	/* ran out of words[] before bytes */
 		if (*end)
@@ -452,7 +452,8 @@ static char *dynamic_emit_prefix(const struct _ddebug *desc, char *buf)
 		pos += snprintf(buf + pos, remaining(pos), "%s:",
 					desc->function);
 	if (desc->flags & _DPRINTK_FLAGS_INCL_LINENO)
-		pos += snprintf(buf + pos, remaining(pos), "%d:", desc->lineno);
+		pos += snprintf(buf + pos, remaining(pos), "%d:",
+					desc->lineno);
 	if (pos - pos_after_tid)
 		pos += snprintf(buf + pos, remaining(pos), " ");
 	if (pos >= PREFIX_SIZE)
@@ -708,10 +709,11 @@ static const struct seq_operations ddebug_proc_seqops = {
 };
 
 /*
- * File_ops->open method for <debugfs>/dynamic_debug/control.  Does the seq_file
- * setup dance, and also creates an iterator to walk the _ddebugs.
- * Note that we create a seq_file always, even for O_WRONLY files
- * where it's not needed, as doing so simplifies the ->release method.
+ * File_ops->open method for <debugfs>/dynamic_debug/control.  Does
+ * the seq_file setup dance, and also creates an iterator to walk the
+ * _ddebugs.  Note that we create a seq_file always, even for O_WRONLY
+ * files where it's not needed, as doing so simplifies the ->release
+ * method.
  */
 static int ddebug_proc_open(struct inode *inode, struct file *file)
 {
-- 
1.7.7.3

[PATCH 02/16] dynamic_debug: drop enabled field from struct _ddebug, use _DPRINTK_FLAGS_PRINT

[Jason Baron]

From: Jim Cromie <jim.cromie@gmail.com>

Currently any enabled dynamic-debug flag on a pr_debug callsite will
enable printing, even if _DPRINTK_FLAGS_PRINT is off.  Checking print
flag directly allows "-p" to disable callsites without fussing with
other flags, so the following disables everything, without altering
flags user may have set:

	echo -p > $DBGFS/dynamic_debug/control

Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
Signed-off-by: Jason Baron <jbaron@redhat.com>
---
 include/linux/dynamic_debug.h |   10 ++++------
 lib/dynamic_debug.c           |    4 ----
 2 files changed, 4 insertions(+), 10 deletions(-)

diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index 0564e3c..f71a6b04 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -28,7 +28,6 @@ struct _ddebug {
 #define _DPRINTK_FLAGS_INCL_TID		(1<<4)
 #define _DPRINTK_FLAGS_DEFAULT 0
 	unsigned int flags:8;
-	char enabled;
 } __attribute__((aligned(8)));
 
 
@@ -62,21 +61,20 @@ int __dynamic_netdev_dbg(struct _ddebug *descriptor,
 		.format = (fmt),				\
 		.lineno = __LINE__,				\
 		.flags =  _DPRINTK_FLAGS_DEFAULT,		\
-		.enabled = false,				\
 	}
 
 #define dynamic_pr_debug(fmt, ...)				\
 do {								\
 	DEFINE_DYNAMIC_DEBUG_METADATA(descriptor, fmt);		\
-	if (unlikely(descriptor.enabled))			\
+	if (unlikely(descriptor.flags & _DPRINTK_FLAGS_PRINT))	\
 		__dynamic_pr_debug(&descriptor, pr_fmt(fmt),	\
 				   ##__VA_ARGS__);		\
 } while (0)
 
 #define dynamic_dev_dbg(dev, fmt, ...)				\
 do {								\
-	DEFINE_DYNAMIC_DEBUG_METADATA(descriptor, fmt);	\
-	if (unlikely(descriptor.enabled))			\
+	DEFINE_DYNAMIC_DEBUG_METADATA(descriptor, fmt);		\
+	if (unlikely(descriptor.flags & _DPRINTK_FLAGS_PRINT))	\
 		__dynamic_dev_dbg(&descriptor, dev, fmt,	\
 				  ##__VA_ARGS__);		\
 } while (0)
@@ -84,7 +82,7 @@ do {								\
 #define dynamic_netdev_dbg(dev, fmt, ...)			\
 do {								\
 	DEFINE_DYNAMIC_DEBUG_METADATA(descriptor, fmt);		\
-	if (unlikely(descriptor.enabled))			\
+	if (unlikely(descriptor.flags & _DPRINTK_FLAGS_PRINT))	\
 		__dynamic_netdev_dbg(&descriptor, dev, fmt,	\
 				     ##__VA_ARGS__);		\
 } while (0)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index e487d13..416c079 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -151,10 +151,6 @@ static void ddebug_change(const struct ddebug_query *query,
 			if (newflags == dp->flags)
 				continue;
 			dp->flags = newflags;
-			if (newflags)
-				dp->enabled = 1;
-			else
-				dp->enabled = 0;
 			if (verbose)
 				pr_info("changed %s:%d [%s]%s %s\n",
 					dp->filename, dp->lineno,
-- 
1.7.7.3

[PATCH 03/16] dynamic_debug: make dynamic-debug supersede DEBUG ccflag

[Jason Baron]

From: Jim Cromie <jim.cromie@gmail.com>

If CONFIG_DYNAMIC_DEBUG is defined, honor it over DEBUG, so that
pr_debug()s are controllable, instead of always-on.  When DEBUG is
also defined, change _DPRINTK_FLAGS_DEFAULT to enable printing by
default.

Also adding _DPRINTK_FLAGS_INCL_MODNAME would be nice, but there are
numerous cases of pr_debug(NAME ": ...), which would result in double
printing of module-name.  So defer this until things settle.

Cc: David Miller <davem@davemloft.net>
Cc: Joe Perches <joe@perches.com>
Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
Signed-off-by: Jason Baron <jbaron@redhat.com>
---
 include/linux/device.h        |    8 ++++----
 include/linux/dynamic_debug.h |    4 ++++
 include/linux/netdevice.h     |    8 ++++----
 include/linux/printk.h        |    8 ++++----
 4 files changed, 16 insertions(+), 12 deletions(-)

diff --git a/include/linux/device.h b/include/linux/device.h
index 3136ede..c9468c1 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -872,14 +872,14 @@ int _dev_info(const struct device *dev, const char *fmt, ...)
 
 #define dev_info(dev, fmt, arg...) _dev_info(dev, fmt, ##arg)
 
-#if defined(DEBUG)
-#define dev_dbg(dev, format, arg...)		\
-	dev_printk(KERN_DEBUG, dev, format, ##arg)
-#elif defined(CONFIG_DYNAMIC_DEBUG)
+#if defined(CONFIG_DYNAMIC_DEBUG)
 #define dev_dbg(dev, format, ...)		     \
 do {						     \
 	dynamic_dev_dbg(dev, format, ##__VA_ARGS__); \
 } while (0)
+#elif defined(DEBUG)
+#define dev_dbg(dev, format, arg...)		\
+	dev_printk(KERN_DEBUG, dev, format, ##arg)
 #else
 #define dev_dbg(dev, format, arg...)				\
 ({								\
diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index f71a6b04..29ea09a 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -26,7 +26,11 @@ struct _ddebug {
 #define _DPRINTK_FLAGS_INCL_FUNCNAME	(1<<2)
 #define _DPRINTK_FLAGS_INCL_LINENO	(1<<3)
 #define _DPRINTK_FLAGS_INCL_TID		(1<<4)
+#if defined DEBUG
+#define _DPRINTK_FLAGS_DEFAULT _DPRINTK_FLAGS_PRINT
+#else
 #define _DPRINTK_FLAGS_DEFAULT 0
+#endif
 	unsigned int flags:8;
 } __attribute__((aligned(8)));
 
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index a82ad4d..9a222e7 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -2645,14 +2645,14 @@ int netdev_info(const struct net_device *dev, const char *format, ...);
 #define MODULE_ALIAS_NETDEV(device) \
 	MODULE_ALIAS("netdev-" device)
 
-#if defined(DEBUG)
-#define netdev_dbg(__dev, format, args...)			\
-	netdev_printk(KERN_DEBUG, __dev, format, ##args)
-#elif defined(CONFIG_DYNAMIC_DEBUG)
+#if defined(CONFIG_DYNAMIC_DEBUG)
 #define netdev_dbg(__dev, format, args...)			\
 do {								\
 	dynamic_netdev_dbg(__dev, format, ##args);		\
 } while (0)
+#elif defined(DEBUG)
+#define netdev_dbg(__dev, format, args...)			\
+	netdev_printk(KERN_DEBUG, __dev, format, ##args)
 #else
 #define netdev_dbg(__dev, format, args...)			\
 ({								\
diff --git a/include/linux/printk.h b/include/linux/printk.h
index f0e22f7..f9abd93 100644
--- a/include/linux/printk.h
+++ b/include/linux/printk.h
@@ -180,13 +180,13 @@ extern void dump_stack(void) __cold;
 #endif
 
 /* If you are writing a driver, please use dev_dbg instead */
-#if defined(DEBUG)
-#define pr_debug(fmt, ...) \
-	printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
-#elif defined(CONFIG_DYNAMIC_DEBUG)
+#if defined(CONFIG_DYNAMIC_DEBUG)
 /* dynamic_pr_debug() uses pr_fmt() internally so we don't need it here */
 #define pr_debug(fmt, ...) \
 	dynamic_pr_debug(fmt, ##__VA_ARGS__)
+#elif defined(DEBUG)
+#define pr_debug(fmt, ...) \
+	printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
 #else
 #define pr_debug(fmt, ...) \
 	no_printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
-- 
1.7.7.3

[PATCH 04/16] dynamic_debug: change verbosity at runtime

[Jason Baron]

From: Jim Cromie <jim.cromie@gmail.com>

Allow changing dynamic_debug verbosity at run-time, to ease debugging
of ddebug queries as you add them, improving usability.

at boot time: dynamic_debug.verbose=1
at runtime:
root@voyage:~# echo 1 > /sys/module/dynamic_debug/parameters/verbose

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

diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 416c079..8c88b89 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -60,6 +60,7 @@ struct ddebug_iter {
 static DEFINE_MUTEX(ddebug_lock);
 static LIST_HEAD(ddebug_tables);
 static int verbose = 0;
+module_param(verbose, int, 0644);
 
 /* Return the last part of a pathname */
 static inline const char *basename(const char *path)
-- 
1.7.7.3

[PATCH 05/16] dynamic_debug: replace strcpy with strlcpy, in ddebug_setup_query()

[Jason Baron]

From: Jim Cromie <jim.cromie@gmail.com>

Replace strcpy with strlcpy, and add define for the size constant.

[jbaron@redhat.com: Use DDEBUG_STRING_SIZE for overflow check]
Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
Signed-off-by: Jason Baron <jbaron@redhat.com>
---
 lib/dynamic_debug.c |    8 +++++---
 1 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 8c88b89..6fc8622 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -525,14 +525,16 @@ EXPORT_SYMBOL(__dynamic_netdev_dbg);
 
 #endif
 
-static __initdata char ddebug_setup_string[1024];
+#define DDEBUG_STRING_SIZE 1024
+static __initdata char ddebug_setup_string[DDEBUG_STRING_SIZE];
+
 static __init int ddebug_setup_query(char *str)
 {
-	if (strlen(str) >= 1024) {
+	if (strlen(str) >= DDEBUG_STRING_SIZE) {
 		pr_warn("ddebug boot param string too large\n");
 		return 0;
 	}
-	strcpy(ddebug_setup_string, str);
+	strlcpy(ddebug_setup_string, str, DDEBUG_STRING_SIZE);
 	return 1;
 }
 
-- 
1.7.7.3

Re: [PATCH 05/16] dynamic_debug: replace strcpy with strlcpy, in ddebug_setup_query()

[Bart Van Assche]

On Mon, Dec 19, 2011 at 10:12 PM, Jason Baron <jbaron@redhat.com> wrote:
>
> From: Jim Cromie <jim.cromie@gmail.com>
>
> Replace strcpy with strlcpy, and add define for the size constant.
>
> [jbaron@redhat.com: Use DDEBUG_STRING_SIZE for overflow check]
> Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
> Signed-off-by: Jason Baron <jbaron@redhat.com>
> ---
>  lib/dynamic_debug.c |    8 +++++---
>  1 files changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
> index 8c88b89..6fc8622 100644
> --- a/lib/dynamic_debug.c
> +++ b/lib/dynamic_debug.c
> @@ -525,14 +525,16 @@ EXPORT_SYMBOL(__dynamic_netdev_dbg);
>
>  #endif
>
> -static __initdata char ddebug_setup_string[1024];
> +#define DDEBUG_STRING_SIZE 1024
> +static __initdata char ddebug_setup_string[DDEBUG_STRING_SIZE];
> +
>  static __init int ddebug_setup_query(char *str)
>  {
> -       if (strlen(str) >= 1024) {
> +       if (strlen(str) >= DDEBUG_STRING_SIZE) {
>                pr_warn("ddebug boot param string too large\n");
>                return 0;
>        }
> -       strcpy(ddebug_setup_string, str);
> +       strlcpy(ddebug_setup_string, str, DDEBUG_STRING_SIZE);
>        return 1;
>  }

Sorry, but it's not clear to me why it make sense to introduce the
DDEBUG_STRING_SIZE macro instead of using ARRAY_SIZE().

Bart.

Re: [PATCH 05/16] dynamic_debug: replace strcpy with strlcpy, in ddebug_setup_query()

[Jason Baron]

On Tue, Dec 20, 2011 at 08:08:41AM +0000, Bart Van Assche wrote:
> On Mon, Dec 19, 2011 at 10:12 PM, Jason Baron <jbaron@redhat.com> wrote:
> >
> > From: Jim Cromie <jim.cromie@gmail.com>
> >
> > Replace strcpy with strlcpy, and add define for the size constant.
> >
> > [jbaron@redhat.com: Use DDEBUG_STRING_SIZE for overflow check]
> > Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
> > Signed-off-by: Jason Baron <jbaron@redhat.com>
> > ---
> >  lib/dynamic_debug.c |    8 +++++---
> >  1 files changed, 5 insertions(+), 3 deletions(-)
> >
> > diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
> > index 8c88b89..6fc8622 100644
> > --- a/lib/dynamic_debug.c
> > +++ b/lib/dynamic_debug.c
> > @@ -525,14 +525,16 @@ EXPORT_SYMBOL(__dynamic_netdev_dbg);
> >
> >  #endif
> >
> > -static __initdata char ddebug_setup_string[1024];
> > +#define DDEBUG_STRING_SIZE 1024
> > +static __initdata char ddebug_setup_string[DDEBUG_STRING_SIZE];
> > +
> >  static __init int ddebug_setup_query(char *str)
> >  {
> > -       if (strlen(str) >= 1024) {
> > +       if (strlen(str) >= DDEBUG_STRING_SIZE) {
> >                pr_warn("ddebug boot param string too large\n");
> >                return 0;
> >        }
> > -       strcpy(ddebug_setup_string, str);
> > +       strlcpy(ddebug_setup_string, str, DDEBUG_STRING_SIZE);
> >        return 1;
> >  }
> 
> Sorry, but it's not clear to me why it make sense to introduce the
> DDEBUG_STRING_SIZE macro instead of using ARRAY_SIZE().
> 
> Bart.

Greg,

The change suggested here by Bart isn't a functional change. If you
think it warrants a re-post, let me know, but I am fine with it as-is.

Thanks,

-Jason

[PATCH 06/16] dynamic_debug: pr_err() call should not depend upon verbosity

[Jason Baron]

From: Jim Cromie <jim.cromie@gmail.com>

Issue keyword/parsing errors even w/o verbose set;
uncover otherwize mysterious non-functionality.

Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
Signed-off-by: Jason Baron <jbaron@redhat.com>
---
 lib/dynamic_debug.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 6fc8622..d232025 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -322,8 +322,7 @@ static int ddebug_parse_query(char *words[], int nwords,
 				query->last_lineno = query->first_lineno;
 			}
 		} else {
-			if (verbose)
-				pr_err("unknown keyword \"%s\"\n", words[i]);
+			pr_err("unknown keyword \"%s\"\n", words[i]);
 			return -EINVAL;
 		}
 	}
-- 
1.7.7.3

[PATCH 07/16] dynamic_debug: drop explicit !=NULL checks

[Jason Baron]

From: Jim Cromie <jim.cromie@gmail.com>

Convert 'if (x !=NULL)' checks into 'if (x)'.

Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
Signed-off-by: Jason Baron <jbaron@redhat.com>
---
 lib/dynamic_debug.c |   11 +++++------
 1 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index d232025..b199e09 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -115,27 +115,26 @@ static void ddebug_change(const struct ddebug_query *query,
 	list_for_each_entry(dt, &ddebug_tables, link) {
 
 		/* match against the module name */
-		if (query->module != NULL &&
-		    strcmp(query->module, dt->mod_name))
+		if (query->module && strcmp(query->module, dt->mod_name))
 			continue;
 
 		for (i = 0 ; i < dt->num_ddebugs ; i++) {
 			struct _ddebug *dp = &dt->ddebugs[i];
 
 			/* match against the source filename */
-			if (query->filename != NULL &&
+			if (query->filename &&
 			    strcmp(query->filename, dp->filename) &&
 			    strcmp(query->filename, basename(dp->filename)))
 				continue;
 
 			/* match against the function */
-			if (query->function != NULL &&
+			if (query->function &&
 			    strcmp(query->function, dp->function))
 				continue;
 
 			/* match against the format */
-			if (query->format != NULL &&
-			    strstr(dp->format, query->format) == NULL)
+			if (query->format &&
+			    !strstr(dp->format, query->format))
 				continue;
 
 			/* match against the line number range */
-- 
1.7.7.3

[PATCH 08/16] dynamic_debug: describe_flags with '=[pmflt_]*'

[Jason Baron]

From: Jim Cromie <jim.cromie@gmail.com>

Change describe_flags() to emit '=[pmflt_]+' for current callsite
flags, or just '=_' when they're disabled.  Having '=' in output
allows a more selective grep expression; in contrast '-' may appear
in filenames, line-ranges, and format-strings.  '=' also has better
mnemonics, saying; "the current setting is equal to <flags>".

This allows grep "=_" <dbgfs>/dynamic_debug/control to see disabled
callsites while avoiding the many occurrences of " = " seen in format
strings.

Enlarge flagsbufs to handle additional flag char, and alter
ddebug_parse_flags() to allow flags=0, so that user can turn off all
debug flags via:

  ~# echo =_ > <dbgfs>/dynamic_debug/control

Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
Signed-off-by: Jason Baron <jbaron@redhat.com>
---
 include/linux/dynamic_debug.h |    3 ++-
 lib/dynamic_debug.c           |   21 ++++++++++-----------
 2 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index 29ea09a..fc39640 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -21,7 +21,8 @@ struct _ddebug {
  	 * The bits here are changed dynamically when the user
 	 * writes commands to <debugfs>/dynamic_debug/control
 	 */
-#define _DPRINTK_FLAGS_PRINT   (1<<0)  /* printk() a message using the format */
+#define _DPRINTK_FLAGS_NONE	0
+#define _DPRINTK_FLAGS_PRINT	(1<<0) /* printk() a message using the format */
 #define _DPRINTK_FLAGS_INCL_MODNAME	(1<<1)
 #define _DPRINTK_FLAGS_INCL_FUNCNAME	(1<<2)
 #define _DPRINTK_FLAGS_INCL_LINENO	(1<<3)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index b199e09..cde4dfe 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -75,6 +75,7 @@ static struct { unsigned flag:8; char opt_char; } opt_array[] = {
 	{ _DPRINTK_FLAGS_INCL_FUNCNAME, 'f' },
 	{ _DPRINTK_FLAGS_INCL_LINENO, 'l' },
 	{ _DPRINTK_FLAGS_INCL_TID, 't' },
+	{ _DPRINTK_FLAGS_NONE, '_' },
 };
 
 /* format a string into buf[] which describes the _ddebug's flags */
@@ -84,12 +85,12 @@ static char *ddebug_describe_flags(struct _ddebug *dp, char *buf,
 	char *p = buf;
 	int i;
 
-	BUG_ON(maxlen < 4);
+	BUG_ON(maxlen < 6);
 	for (i = 0; i < ARRAY_SIZE(opt_array); ++i)
 		if (dp->flags & opt_array[i].flag)
 			*p++ = opt_array[i].opt_char;
 	if (p == buf)
-		*p++ = '-';
+		*p++ = '_';
 	*p = '\0';
 
 	return buf;
@@ -108,7 +109,7 @@ static void ddebug_change(const struct ddebug_query *query,
 	struct ddebug_table *dt;
 	unsigned int newflags;
 	unsigned int nfound = 0;
-	char flagbuf[8];
+	char flagbuf[10];
 
 	/* search for matching ddebugs */
 	mutex_lock(&ddebug_lock);
@@ -152,7 +153,7 @@ static void ddebug_change(const struct ddebug_query *query,
 				continue;
 			dp->flags = newflags;
 			if (verbose)
-				pr_info("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,
@@ -370,8 +371,6 @@ static int ddebug_parse_flags(const char *str, unsigned int *flagsp,
 		if (i < 0)
 			return -EINVAL;
 	}
-	if (flags == 0)
-		return -EINVAL;
 	if (verbose)
 		pr_info("flags=0x%x\n", flags);
 
@@ -666,7 +665,7 @@ static int ddebug_proc_show(struct seq_file *m, void *p)
 {
 	struct ddebug_iter *iter = m->private;
 	struct _ddebug *dp = p;
-	char flagsbuf[8];
+	char flagsbuf[10];
 
 	if (verbose)
 		pr_info("called m=%p p=%p\n", m, p);
@@ -677,10 +676,10 @@ static int ddebug_proc_show(struct seq_file *m, void *p)
 		return 0;
 	}
 
-	seq_printf(m, "%s:%u [%s]%s %s \"",
-		   dp->filename, dp->lineno,
-		   iter->table->mod_name, dp->function,
-		   ddebug_describe_flags(dp, flagsbuf, sizeof(flagsbuf)));
+	seq_printf(m, "%s:%u [%s]%s =%s \"",
+		dp->filename, dp->lineno,
+		iter->table->mod_name, dp->function,
+		ddebug_describe_flags(dp, flagsbuf, sizeof(flagsbuf)));
 	seq_escape(m, dp->format, "\t\r\n\"");
 	seq_puts(m, "\"\n");
 
-- 
1.7.7.3

[PATCH 09/16] dynamic_debug: tighten up error checking on debug queries

[Jason Baron]

From: Jim Cromie <jim.cromie@gmail.com>

Issue error when a match-spec is given multiple times in a rule.
Previous code kept last one, but was silent about it.  Docs imply only
one is allowed by saying match-specs are ANDed together, given that
module M cannot match both A and B.  Also error when last_line < 1st_line.

Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
Signed-off-by: Jason Baron <jbaron@redhat.com>
---
 lib/dynamic_debug.c |   39 +++++++++++++++++++++++++++++++++------
 1 files changed, 33 insertions(+), 6 deletions(-)

diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index cde4dfe..5a7bacc 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -276,6 +276,19 @@ static char *unescape(char *str)
 	return str;
 }
 
+static int check_set(const char **dest, char *src, char *name)
+{
+	int rc = 0;
+
+	if (*dest) {
+		rc = -EINVAL;
+		pr_err("match-spec:%s val:%s overridden by %s",
+			name, *dest, src);
+	}
+	*dest = src;
+	return rc;
+}
+
 /*
  * Parse words[] as a ddebug query specification, which is a series
  * of (keyword, value) pairs chosen from these possibilities:
@@ -287,11 +300,15 @@ static char *unescape(char *str)
  * format <escaped-string-to-find-in-format>
  * line <lineno>
  * line <first-lineno>-<last-lineno> // where either may be empty
+ *
+ * Only 1 of each type is allowed.
+ * Returns 0 on success, <0 on error.
  */
 static int ddebug_parse_query(char *words[], int nwords,
 			       struct ddebug_query *query)
 {
 	unsigned int i;
+	int rc;
 
 	/* check we have an even number of words */
 	if (nwords % 2 != 0)
@@ -300,24 +317,32 @@ static int ddebug_parse_query(char *words[], int nwords,
 
 	for (i = 0 ; i < nwords ; i += 2) {
 		if (!strcmp(words[i], "func"))
-			query->function = words[i+1];
+			rc = check_set(&query->function, words[i+1], "func");
 		else if (!strcmp(words[i], "file"))
-			query->filename = words[i+1];
+			rc = check_set(&query->filename, words[i+1], "file");
 		else if (!strcmp(words[i], "module"))
-			query->module = words[i+1];
+			rc = check_set(&query->module, words[i+1], "module");
 		else if (!strcmp(words[i], "format"))
-			query->format = unescape(words[i+1]);
+			rc = check_set(&query->format, unescape(words[i+1]),
+				"format");
 		else if (!strcmp(words[i], "line")) {
 			char *first = words[i+1];
 			char *last = strchr(first, '-');
+			if (query->first_lineno || query->last_lineno) {
+				pr_err("match-spec:line given 2 times\n");
+				return -EINVAL;
+			}
 			if (last)
 				*last++ = '\0';
 			if (parse_lineno(first, &query->first_lineno) < 0)
 				return -EINVAL;
-			if (last != NULL) {
+			if (last) {
 				/* range <first>-<last> */
-				if (parse_lineno(last, &query->last_lineno) < 0)
+				if (parse_lineno(last, &query->last_lineno)
+				    < query->first_lineno) {
+					pr_err("last-line < 1st-line\n");
 					return -EINVAL;
+				}
 			} else {
 				query->last_lineno = query->first_lineno;
 			}
@@ -325,6 +350,8 @@ static int ddebug_parse_query(char *words[], int nwords,
 			pr_err("unknown keyword \"%s\"\n", words[i]);
 			return -EINVAL;
 		}
+		if (rc)
+			return rc;
 	}
 
 	if (verbose)
-- 
1.7.7.3

[PATCH 10/16] dynamic_debug: early return if _ddebug table is empty

[Jason Baron]

From: Jim Cromie <jim.cromie@gmail.com>

If _ddebug table is empty (in a CONFIG_DYNAMIC_DEBUG build this
shouldn't happen), then warn (error?) and return early.  This skips
empty table scan and parsing of setup-string, including the pr_info
call noting the parse.  By inspection, copy return-code handling from
1st ddebug_add_module() callsite to 2nd.

Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
Signed-off-by: Jason Baron <jbaron@redhat.com>
---
 lib/dynamic_debug.c |   35 ++++++++++++++++++++---------------
 1 files changed, 20 insertions(+), 15 deletions(-)

diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 5a7bacc..4be55d8 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -871,23 +871,28 @@ static int __init dynamic_debug_init(void)
 	int ret = 0;
 	int n = 0;
 
-	if (__start___verbose != __stop___verbose) {
-		iter = __start___verbose;
-		modname = iter->modname;
-		iter_start = iter;
-		for (; iter < __stop___verbose; iter++) {
-			if (strcmp(modname, iter->modname)) {
-				ret = ddebug_add_module(iter_start, n, modname);
-				if (ret)
-					goto out_free;
-				n = 0;
-				modname = iter->modname;
-				iter_start = iter;
-			}
-			n++;
+	if (__start___verbose == __stop___verbose) {
+		pr_warn("_ddebug table is empty in a "
+			"CONFIG_DYNAMIC_DEBUG build");
+		return 1;
+	}
+	iter = __start___verbose;
+	modname = iter->modname;
+	iter_start = iter;
+	for (; iter < __stop___verbose; iter++) {
+		if (strcmp(modname, iter->modname)) {
+			ret = ddebug_add_module(iter_start, n, modname);
+			if (ret)
+				goto out_free;
+			n = 0;
+			modname = iter->modname;
+			iter_start = iter;
 		}
-		ret = ddebug_add_module(iter_start, n, modname);
+		n++;
 	}
+	ret = ddebug_add_module(iter_start, n, modname);
+	if (ret)
+		goto out_free;
 
 	/* ddebug_query boot param got passed -> set it up */
 	if (ddebug_setup_string[0] != '\0') {
-- 
1.7.7.3

[PATCH 11/16] dynamic_debug: reduce lineno field to a saner 18 bits

[Jason Baron]

From: Jim Cromie <jim.cromie@gmail.com>

lineno:24 allows files with 4 million lines, an insane file-size, even
for never-to-get-in-tree machine generated code.  Reduce this to 18
bits, which still allows 256k lines.  This is still insanely big, but
its not raving mad.

Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
Signed-off-by: Jason Baron <jbaron@redhat.com>
---
 include/linux/dynamic_debug.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index fc39640..7e3c53a 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -15,7 +15,7 @@ struct _ddebug {
 	const char *function;
 	const char *filename;
 	const char *format;
-	unsigned int lineno:24;
+	unsigned int lineno:18;
 	/*
  	 * The flags field controls the behaviour at the callsite.
  	 * The bits here are changed dynamically when the user
-- 
1.7.7.3

[PATCH 12/16] dynamic_debug: chop off comments in ddebug_tokenize

[Jason Baron]

From: Jim Cromie <jim.cromie@gmail.com>

If a token begins with #, the remainder of query string is a comment,
so drop it.  Doing it here avoids '#' in quoted strings.

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

diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 4be55d8..86a9abb 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -183,6 +183,8 @@ static int ddebug_tokenize(char *buf, char *words[], int maxwords)
 		buf = skip_spaces(buf);
 		if (!*buf)
 			break;	/* oh, it was trailing whitespace */
+		if (*buf == '#')
+			break;	/* token starts comment, skip rest of line */
 
 		/* find `end' of word, whitespace separated or quoted */
 		if (*buf == '"' || *buf == '\'') {
-- 
1.7.7.3

[PATCH 13/16] dynamic_debug: enlarge command/query write buffer

[Jason Baron]

From: Jim Cromie <jim.cromie@gmail.com>

Current query write buffer is 256 bytes, on stack.  In comparison, the
ddebug_query boot-arg is 1024.  Allocate the buffer off heap, and
enlarge it to 4096 bytes, big enough for ~100 queries (at 40 bytes
each), and error out if not.  This makes it play nicely with large
query sets (to be added later).  The buffer should be enough for most
uses, and others should probably be split into subsets.

[jbaron@redhat.com: changed USER_BUF_PAGE from 4095 -> 4096 ]
Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
Signed-off-by: Jason Baron <jbaron@redhat.com>
---
 lib/dynamic_debug.c |   16 ++++++++++++----
 1 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 86a9abb..d8773dc 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -570,24 +570,32 @@ __setup("ddebug_query=", ddebug_setup_query);
  * File_ops->write method for <debugfs>/dynamic_debug/conrol.  Gathers the
  * command text from userspace, parses and executes it.
  */
+#define USER_BUF_PAGE 4096
 static ssize_t ddebug_proc_write(struct file *file, const char __user *ubuf,
 				  size_t len, loff_t *offp)
 {
-	char tmpbuf[256];
+	char *tmpbuf;
 	int ret;
 
 	if (len == 0)
 		return 0;
-	/* we don't check *offp -- multiple writes() are allowed */
-	if (len > sizeof(tmpbuf)-1)
+	if (len > USER_BUF_PAGE - 1) {
+		pr_warn("expected <%d bytes into control\n", USER_BUF_PAGE);
 		return -E2BIG;
-	if (copy_from_user(tmpbuf, ubuf, len))
+	}
+	tmpbuf = kmalloc(len + 1, GFP_KERNEL);
+	if (!tmpbuf)
+		return -ENOMEM;
+	if (copy_from_user(tmpbuf, ubuf, len)) {
+		kfree(tmpbuf);
 		return -EFAULT;
+	}
 	tmpbuf[len] = '\0';
 	if (verbose)
 		pr_info("read %d bytes from userspace\n", (int)len);
 
 	ret = ddebug_exec_query(tmpbuf);
+	kfree(tmpbuf);
 	if (ret)
 		return ret;
 
-- 
1.7.7.3

[PATCH 14/16] dynamic_debug: add trim_prefix() to provide source-root relative paths

[Jason Baron]

From: Jim Cromie <jim.cromie@gmail.com>

trim_prefix(path) skips past the absolute source path root, and
returns the pointer to the relative path from there.  It is used to
shorten the displayed path in $DBGMT/dynamic_debug/control via
ddebug_proc_show(), and in ddebug_change() to allow relative filenames
to be used in applied queries.  For example:

  ~# echo file kernel/freezer.c +p > $DBGMT/dynamic_debug/control

  kernel/freezer.c:128 [freezer]cancel_freezing p "  clean up: %s\012"

trim_prefix(path) insures common prefix before trimming it, so
out-of-tree module paths are shown as full absolute paths.

Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
Signed-off-by: Jason Baron <jbaron@redhat.com>
---
 Documentation/dynamic-debug-howto.txt |    7 ++++---
 lib/dynamic_debug.c                   |   18 +++++++++++++++---
 2 files changed, 19 insertions(+), 6 deletions(-)

diff --git a/Documentation/dynamic-debug-howto.txt b/Documentation/dynamic-debug-howto.txt
index f959909..378b5d1 100644
--- a/Documentation/dynamic-debug-howto.txt
+++ b/Documentation/dynamic-debug-howto.txt
@@ -144,11 +144,12 @@ func
     func svc_tcp_accept
 
 file
-    The given string is compared against either the full
-    pathname or the basename of the source file of each
-    callsite.  Examples:
+    The given string is compared against either the full pathname, the
+    src-root relative pathname, or the basename of the source file of
+    each callsite.  Examples:
 
     file svcsock.c
+    file kernel/freezer.c
     file /usr/src/packages/BUILD/sgi-enhancednfs-1.4/default/net/sunrpc/svcsock.c
 
 module
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index d8773dc..a5508a1 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -69,6 +69,17 @@ static inline const char *basename(const char *path)
 	return tail ? tail+1 : path;
 }
 
+/* Return the path relative to source root */
+static inline const char *trim_prefix(const char *path)
+{
+	int skip = strlen(__FILE__) - strlen("lib/dynamic_debug.c");
+
+	if (strncmp(path, __FILE__, skip))
+		skip = 0; /* prefix mismatch, don't skip */
+
+	return path + skip;
+}
+
 static struct { unsigned flag:8; char opt_char; } opt_array[] = {
 	{ _DPRINTK_FLAGS_PRINT, 'p' },
 	{ _DPRINTK_FLAGS_INCL_MODNAME, 'm' },
@@ -125,7 +136,8 @@ static void ddebug_change(const struct ddebug_query *query,
 			/* match against the source filename */
 			if (query->filename &&
 			    strcmp(query->filename, dp->filename) &&
-			    strcmp(query->filename, basename(dp->filename)))
+			    strcmp(query->filename, basename(dp->filename)) &&
+			    strcmp(query->filename, trim_prefix(dp->filename)))
 				continue;
 
 			/* match against the function */
@@ -154,7 +166,7 @@ static void ddebug_change(const struct ddebug_query *query,
 			dp->flags = newflags;
 			if (verbose)
 				pr_info("changed %s:%d [%s]%s =%s\n",
-					dp->filename, dp->lineno,
+					trim_prefix(dp->filename), dp->lineno,
 					dt->mod_name, dp->function,
 					ddebug_describe_flags(dp, flagbuf,
 							sizeof(flagbuf)));
@@ -714,7 +726,7 @@ static int ddebug_proc_show(struct seq_file *m, void *p)
 	}
 
 	seq_printf(m, "%s:%u [%s]%s =%s \"",
-		dp->filename, dp->lineno,
+		trim_prefix(dp->filename), dp->lineno,
 		iter->table->mod_name, dp->function,
 		ddebug_describe_flags(dp, flagsbuf, sizeof(flagsbuf)));
 	seq_escape(m, dp->format, "\t\r\n\"");
-- 
1.7.7.3

Re: [PATCH 14/16] dynamic_debug: add trim_prefix() to provide source-root relative paths

[Bart Van Assche]

On Mon, Dec 19, 2011 at 10:13 PM, Jason Baron <jbaron@redhat.com> wrote:
> +/* Return the path relative to source root */
> +static inline const char *trim_prefix(const char *path)
> +{
> +       int skip = strlen(__FILE__) - strlen("lib/dynamic_debug.c");
> +
> +       if (strncmp(path, __FILE__, skip))
> +               skip = 0; /* prefix mismatch, don't skip */
> +
> +       return path + skip;
> +}

Isn't the preferred style to write if (strncmp(...) != 0) ..., i.e. to
mention "!= 0" explicitly instead of leaving it out when comparing
strings ?

Bart.

Re: [PATCH 14/16] dynamic_debug: add trim_prefix() to provide source-root relative paths

[Jason Baron]

On Tue, Dec 20, 2011 at 08:15:55AM +0000, Bart Van Assche wrote:
> On Mon, Dec 19, 2011 at 10:13 PM, Jason Baron <jbaron@redhat.com> wrote:
> > +/* Return the path relative to source root */
> > +static inline const char *trim_prefix(const char *path)
> > +{
> > +       int skip = strlen(__FILE__) - strlen("lib/dynamic_debug.c");
> > +
> > +       if (strncmp(path, __FILE__, skip))
> > +               skip = 0; /* prefix mismatch, don't skip */
> > +
> > +       return path + skip;
> > +}
> 
> Isn't the preferred style to write if (strncmp(...) != 0) ..., i.e. to
> mention "!= 0" explicitly instead of leaving it out when comparing
> strings ?
> 
> Bart.

Greg,

Again, no functional change here, just a style thing. Let me know if you
want a re-post. I am fine with it as-is.

Thanks,

-Jason

[PATCH 15/16] dynamic_debug: factor vpr_info_dq out of ddebug_parse_query

[Jason Baron]

From: Jim Cromie <jim.cromie@gmail.com>

Factor pr_info(query) out of ddebug_parse_query, into pr_info_dq(),
for reuse later.  Also change the printed labels: file, func to agree
with the query-spec keywords accepted in the control file.  Pass ""
when string is null, to avoid "(null)" output from sprintf.  For
format print, use precision to skip last char, assuming its '\n', no
great harm if not, its a debug msg.

Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
Signed-off-by: Jason Baron <jbaron@redhat.com>
---
 lib/dynamic_debug.c |   25 +++++++++++++++++--------
 1 files changed, 17 insertions(+), 8 deletions(-)

diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index a5508a1..93fc5d5 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -107,6 +107,22 @@ static char *ddebug_describe_flags(struct _ddebug *dp, char *buf,
 	return buf;
 }
 
+#define vpr_info_dq(q, msg)						\
+do {									\
+	if (verbose)							\
+		/* trim last char off format print */			\
+		pr_info("%s: func=\"%s\" file=\"%s\" "			\
+			"module=\"%s\" format=\"%.*s\" "		\
+			"lineno=%u-%u",					\
+			msg,						\
+			q->function ? q->function : "",			\
+			q->filename ? q->filename : "",			\
+			q->module ? q->module : "",			\
+			(int)(q->format ? strlen(q->format) - 1 : 0),	\
+			q->format ? q->format : "",			\
+			q->first_lineno, q->last_lineno);		\
+} while (0)
+
 /*
  * Search the tables for _ddebug's which match the given
  * `query' and apply the `flags' and `mask' to them.  Tells
@@ -367,14 +383,7 @@ static int ddebug_parse_query(char *words[], int nwords,
 		if (rc)
 			return rc;
 	}
-
-	if (verbose)
-		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);
-
+	vpr_info_dq(query, "parsed");
 	return 0;
 }
 
-- 
1.7.7.3

Re: [PATCH 15/16] dynamic_debug: factor vpr_info_dq out of ddebug_parse_query

[Bart Van Assche]

On Mon, Dec 19, 2011 at 10:13 PM, Jason Baron <jbaron@redhat.com> wrote:
> From: Jim Cromie <jim.cromie@gmail.com>
>
> Factor pr_info(query) out of ddebug_parse_query, into pr_info_dq(),
> for reuse later.  Also change the printed labels: file, func to agree
> with the query-spec keywords accepted in the control file.  Pass ""
> when string is null, to avoid "(null)" output from sprintf.  For
> format print, use precision to skip last char, assuming its '\n', no
> great harm if not, its a debug msg.

pr_info_dq() or vpr_info_dq() ?

Bart.

Re: [PATCH 15/16] dynamic_debug: factor vpr_info_dq out of ddebug_parse_query

[Jim Cromie]

On Tue, Dec 20, 2011 at 1:17 AM, Bart Van Assche <bvanassche@acm.org> wrote:
> On Mon, Dec 19, 2011 at 10:13 PM, Jason Baron <jbaron@redhat.com> wrote:
>> From: Jim Cromie <jim.cromie@gmail.com>
>>
>> Factor pr_info(query) out of ddebug_parse_query, into pr_info_dq(),
>> for reuse later.  Also change the printed labels: file, func to agree
>> with the query-spec keywords accepted in the control file.  Pass ""
>> when string is null, to avoid "(null)" output from sprintf.  For
>> format print, use precision to skip last char, assuming its '\n', no
>> great harm if not, its a debug msg.
>
> pr_info_dq() or vpr_info_dq() ?
>
> Bart.

I chose vpr_info_dq, with leading v to hint at the "if (verbose)" part

Re: [PATCH 15/16] dynamic_debug: factor vpr_info_dq out of ddebug_parse_query

[Bart Van Assche]

On Tue, Dec 20, 2011 at 3:47 PM, Jim Cromie <jim.cromie@gmail.com> wrote:
> On Tue, Dec 20, 2011 at 1:17 AM, Bart Van Assche <bvanassche@acm.org> wrote:
> > On Mon, Dec 19, 2011 at 10:13 PM, Jason Baron <jbaron@redhat.com> wrote:
> >> From: Jim Cromie <jim.cromie@gmail.com>
> >>
> >> Factor pr_info(query) out of ddebug_parse_query, into pr_info_dq(),
> >> for reuse later.  Also change the printed labels: file, func to agree
> >> with the query-spec keywords accepted in the control file.  Pass ""
> >> when string is null, to avoid "(null)" output from sprintf.  For
> >> format print, use precision to skip last char, assuming its '\n', no
> >> great harm if not, its a debug msg.
> >
> > pr_info_dq() or vpr_info_dq() ?
>
> I chose vpr_info_dq, with leading v to hint at the "if (verbose)" part

In case my reply was a little too terse: isn't there a letter "v"
missing in the commit message ?

Bart.

Re: [PATCH 15/16] dynamic_debug: factor vpr_info_dq out of ddebug_parse_query

[Jason Baron]

On Tue, Dec 20, 2011 at 03:49:54PM +0000, Bart Van Assche wrote:
> On Tue, Dec 20, 2011 at 3:47 PM, Jim Cromie <jim.cromie@gmail.com> wrote:
> > On Tue, Dec 20, 2011 at 1:17 AM, Bart Van Assche <bvanassche@acm.org> wrote:
> > > On Mon, Dec 19, 2011 at 10:13 PM, Jason Baron <jbaron@redhat.com> wrote:
> > >> From: Jim Cromie <jim.cromie@gmail.com>
> > >>
> > >> Factor pr_info(query) out of ddebug_parse_query, into pr_info_dq(),
> > >> for reuse later.  Also change the printed labels: file, func to agree
> > >> with the query-spec keywords accepted in the control file.  Pass ""
> > >> when string is null, to avoid "(null)" output from sprintf.  For
> > >> format print, use precision to skip last char, assuming its '\n', no
> > >> great harm if not, its a debug msg.
> > >
> > > pr_info_dq() or vpr_info_dq() ?
> >
> > I chose vpr_info_dq, with leading v to hint at the "if (verbose)" part
> 
> In case my reply was a little too terse: isn't there a letter "v"
> missing in the commit message ?
> 
> Bart.

Thanks for catching this.

Greg, if you want to s/pr_info_dq/vpr_info_dq, in the commit msg, this
one is all set.

Thanks,

-Jason

Re: [PATCH 15/16] dynamic_debug: factor vpr_info_dq out of ddebug_parse_query

[Greg KH]

On Wed, Dec 21, 2011 at 04:33:42PM -0500, Jason Baron wrote:
> On Tue, Dec 20, 2011 at 03:49:54PM +0000, Bart Van Assche wrote:
> > On Tue, Dec 20, 2011 at 3:47 PM, Jim Cromie <jim.cromie@gmail.com> wrote:
> > > On Tue, Dec 20, 2011 at 1:17 AM, Bart Van Assche <bvanassche@acm.org> wrote:
> > > > On Mon, Dec 19, 2011 at 10:13 PM, Jason Baron <jbaron@redhat.com> wrote:
> > > >> From: Jim Cromie <jim.cromie@gmail.com>
> > > >>
> > > >> Factor pr_info(query) out of ddebug_parse_query, into pr_info_dq(),
> > > >> for reuse later.  Also change the printed labels: file, func to agree
> > > >> with the query-spec keywords accepted in the control file.  Pass ""
> > > >> when string is null, to avoid "(null)" output from sprintf.  For
> > > >> format print, use precision to skip last char, assuming its '\n', no
> > > >> great harm if not, its a debug msg.
> > > >
> > > > pr_info_dq() or vpr_info_dq() ?
> > >
> > > I chose vpr_info_dq, with leading v to hint at the "if (verbose)" part
> > 
> > In case my reply was a little too terse: isn't there a letter "v"
> > missing in the commit message ?
> > 
> > Bart.
> 
> Thanks for catching this.
> 
> Greg, if you want to s/pr_info_dq/vpr_info_dq, in the commit msg, this
> one is all set.

Now done, thanks.

greg k-h

[PATCH 16/16] dynamic_debug: process multiple debug-queries on a line

[Jason Baron]

From: Jim Cromie <jim.cromie@gmail.com>

Insert ddebug_exec_queries() in place of ddebug_exec_query().  It
splits the query string on [;\n], and calls ddebug_exec_query() on
each.  All queries are processed independent of errors, allowing a
query to fail, for example when a module is not installed.  Empty
lines and comments are skipped.  Errors are counted, and the last
error seen (negative) or the number of callsites found (0 or positive)
is returned.  Return code checks are altered accordingly.

With this, multiple queries can be given in ddebug_query, allowing
more selective enabling of callsites.  As a side effect, a set of
commands can be batched in:

	cat cmd-file > $DBGMT/dynamic_debug/control

We dont want a ddebug_query syntax error to kill the dynamic debug
facility, so dynamic_debug_init() zeros ddebug_exec_queries()'s return
code after logging the appropriate message, so that ddebug tables are
preserved and $DBGMT/dynamic_debug/control file is created.  This
would be appropriate even without accepting multiple queries.

This patch also alters ddebug_change() to return number of callsites
matched (which typically is the same as number of callsites changed).
ddebug_exec_query() also returns the number found, or a negative value
if theres a parse error on the query.

Splitting on [;\n] prevents their use in format-specs, but selecting
callsites on punctuation is brittle anyway, meaningful and selective
substrings are more typical.

Note: splitting queries on ';' before handling trailing #comments
means that a ';' also terminates a comment, and text after the ';' is
treated as another query.  This trailing query will almost certainly
result in a parse error and thus have no effect other than the error
message.  The double corner case with unexpected results is:

     ddebug_query="func foo +p # enable foo ; +p"

Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
Signed-off-by: Jason Baron <jbaron@redhat.com>
---
 Documentation/dynamic-debug-howto.txt |   23 ++++-------
 lib/dynamic_debug.c                   |   73 ++++++++++++++++++++++++++-------
 2 files changed, 66 insertions(+), 30 deletions(-)

diff --git a/Documentation/dynamic-debug-howto.txt b/Documentation/dynamic-debug-howto.txt
index 378b5d1..74e6c77 100644
--- a/Documentation/dynamic-debug-howto.txt
+++ b/Documentation/dynamic-debug-howto.txt
@@ -12,7 +12,7 @@ dynamically enabled per-callsite.
 Dynamic debug has even more useful features:
 
  * Simple query language allows turning on and off debugging statements by
-   matching any combination of:
+   matching any combination of 0 or 1 of:
 
    - source filename
    - function name
@@ -79,31 +79,24 @@ Command Language Reference
 ==========================
 
 At the lexical level, a command comprises a sequence of words separated
-by whitespace characters.  Note that newlines are treated as word
-separators and do *not* end a command or allow multiple commands to
-be done together.  So these are all equivalent:
+by spaces or tabs.  So these are all equivalent:
 
 nullarbor:~ # echo -c 'file svcsock.c line 1603 +p' >
 				<debugfs>/dynamic_debug/control
 nullarbor:~ # echo -c '  file   svcsock.c     line  1603 +p  ' >
 				<debugfs>/dynamic_debug/control
-nullarbor:~ # echo -c 'file svcsock.c\nline 1603 +p' >
-				<debugfs>/dynamic_debug/control
 nullarbor:~ # echo -n 'file svcsock.c line 1603 +p' >
 				<debugfs>/dynamic_debug/control
 
-Commands are bounded by a write() system call.  If you want to do
-multiple commands you need to do a separate "echo" for each, like:
+Command submissions are bounded by a write() system call.
+Multiple commands can be written together, separated by ';' or '\n'.
 
-nullarbor:~ # echo 'file svcsock.c line 1603 +p' > /proc/dprintk ;\
-> echo 'file svcsock.c line 1563 +p' > /proc/dprintk
+  ~# echo "func pnpacpi_get_resources +p; func pnp_assign_mem +p" \
+     > <debugfs>/dynamic_debug/control
 
-or even like:
+If your query set is big, you can batch them too:
 
-nullarbor:~ # (
-> echo 'file svcsock.c line 1603 +p' ;\
-> echo 'file svcsock.c line 1563 +p' ;\
-> ) > /proc/dprintk
+  ~# cat query-batch-file > <debugfs>/dynamic_debug/control
 
 At the syntactical level, a command comprises a sequence of match
 specifications, followed by a flags change specification.
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 93fc5d5..310c753 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -124,13 +124,13 @@ do {									\
 } while (0)
 
 /*
- * Search the tables for _ddebug's which match the given
- * `query' and apply the `flags' and `mask' to them.  Tells
- * the user which ddebug's were changed, or whether none
- * were matched.
+ * Search the tables for _ddebug's which match the given `query' and
+ * apply the `flags' and `mask' to them.  Returns number of matching
+ * callsites, normally the same as number of changes.  If verbose,
+ * logs the changes.  Takes ddebug_lock.
  */
-static void ddebug_change(const struct ddebug_query *query,
-			   unsigned int flags, unsigned int mask)
+static int ddebug_change(const struct ddebug_query *query,
+			unsigned int flags, unsigned int mask)
 {
 	int i;
 	struct ddebug_table *dt;
@@ -192,6 +192,8 @@ static void ddebug_change(const struct ddebug_query *query,
 
 	if (!nfound && verbose)
 		pr_info("no matches for query\n");
+
+	return nfound;
 }
 
 /*
@@ -449,7 +451,7 @@ static int ddebug_exec_query(char *query_string)
 	unsigned int flags = 0, mask = 0;
 	struct ddebug_query query;
 #define MAXWORDS 9
-	int nwords;
+	int nwords, nfound;
 	char *words[MAXWORDS];
 
 	nwords = ddebug_tokenize(query_string, words, MAXWORDS);
@@ -461,8 +463,47 @@ static int ddebug_exec_query(char *query_string)
 		return -EINVAL;
 
 	/* actually go and implement the change */
-	ddebug_change(&query, flags, mask);
-	return 0;
+	nfound = ddebug_change(&query, flags, mask);
+	vpr_info_dq((&query), (nfound) ? "applied" : "no-match");
+
+	return nfound;
+}
+
+/* handle multiple queries in query string, continue on error, return
+   last error or number of matching callsites.  Module name is either
+   in param (for boot arg) or perhaps in query string.
+*/
+static int ddebug_exec_queries(char *query)
+{
+	char *split;
+	int i, errs = 0, exitcode = 0, rc, nfound = 0;
+
+	for (i = 0; query; query = split) {
+		split = strpbrk(query, ";\n");
+		if (split)
+			*split++ = '\0';
+
+		query = skip_spaces(query);
+		if (!query || !*query || *query == '#')
+			continue;
+
+		if (verbose)
+			pr_info("query %d: \"%s\"\n", i, query);
+
+		rc = ddebug_exec_query(query);
+		if (rc < 0) {
+			errs++;
+			exitcode = rc;
+		} else
+			nfound += rc;
+		i++;
+	}
+	pr_info("processed %d queries, with %d matches, %d errs\n",
+		 i, nfound, errs);
+
+	if (exitcode)
+		return exitcode;
+	return nfound;
 }
 
 #define PREFIX_SIZE 64
@@ -615,9 +656,9 @@ static ssize_t ddebug_proc_write(struct file *file, const char __user *ubuf,
 	if (verbose)
 		pr_info("read %d bytes from userspace\n", (int)len);
 
-	ret = ddebug_exec_query(tmpbuf);
+	ret = ddebug_exec_queries(tmpbuf);
 	kfree(tmpbuf);
-	if (ret)
+	if (ret < 0)
 		return ret;
 
 	*offp += len;
@@ -927,13 +968,15 @@ static int __init dynamic_debug_init(void)
 
 	/* ddebug_query boot param got passed -> set it up */
 	if (ddebug_setup_string[0] != '\0') {
-		ret = ddebug_exec_query(ddebug_setup_string);
-		if (ret)
+		ret = ddebug_exec_queries(ddebug_setup_string);
+		if (ret < 0)
 			pr_warn("Invalid ddebug boot param %s",
 				ddebug_setup_string);
 		else
-			pr_info("ddebug initialized with string %s",
-				ddebug_setup_string);
+			pr_info("%d changes by ddebug_query\n", ret);
+
+		/* keep tables even on ddebug_query parse error */
+		ret = 0;
 	}
 
 out_free:
-- 
1.7.7.3