[patch 00/25] dynamic-debug during module initialization

[patch 00/25] dynamic-debug during module initialization

[jim.cromie]

This patchset adds
- dynamic-debug during module initialization
- multiple queries

Unlike previous versions, this drops pending-query approach in
favor of "fake module parameter" approach proposed by Thomas Renninger.
    https://lkml.org/lkml/2010/9/15/397

Its based upon v3.2-rc3, cuz it includes a few adjustments to
dynamic_debug.h which are not in driver-core-next atm.


1	bug-fix for kernel/module.c under DEBUGP
2	whitespace cleanup
3-12	dynamic-debug cleanups, should be relatively uncontroversial
13-17	multiple queries in ddebug_query="..."

18-25	fake module parameter
20	maybe fold into 18 (kept separate since 18 is Thomas's work)
23	*.dyndbg=...
25	BUILD_BUG_DECL (likely discussion point ;-)

[jimc@groucho linux-2.6]$ git diff --stat v3.2-rc3..HEAD
 Documentation/dynamic-debug-howto.txt |   76 ++++++--
 drivers/pnp/base.h                    |    8 +-
 drivers/pnp/core.c                    |   13 ++
 include/linux/device.h                |    8 +-
 include/linux/dynamic_debug.h         |   31 +++-
 include/linux/kernel.h                |   10 +
 include/linux/moduleparam.h           |    4 +
 include/linux/netdevice.h             |    8 +-
 include/linux/printk.h                |    8 +-
 kernel/module.c                       |   47 ++---
 kernel/params.c                       |   27 ++-
 lib/dynamic_debug.c                   |  349 +++++++++++++++++++++++++--------
 12 files changed, 430 insertions(+), 159 deletions(-)

[PATCH 01/25] kernel/module.c: fix compile err, warnings under ifdef DEBUGP

[jim.cromie]

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

resubmit of https://lkml.org/lkml/2010/9/15/399 4/4 patch
with tiny mod.

Fixes these warnings, err if DEBUGP is defined in kernel/module.c:

kernel/module.c: In function ‘layout_sections’:
kernel/module.c:1776: error: ‘name’ undeclared (first use in this function)
kernel/module.c:1776: error: (Each undeclared identifier is reported only once
kernel/module.c:1776: error: for each function it appears in.)
kernel/module.c: In function ‘move_module’:
kernel/module.c:2394: warning: format ‘%lx’ expects type ‘long unsigned int’,
but argument 2 has type ‘Elf64_Addr’

Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
 kernel/module.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/kernel/module.c b/kernel/module.c
index 178333c..30ffed4 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -1978,7 +1978,7 @@ static void layout_sections(struct module *mod, struct load_info *info)
 			    || strstarts(sname, ".init"))
 				continue;
 			s->sh_entsize = get_offset(mod, &mod->core_size, s, i);
-			DEBUGP("\t%s\n", name);
+			DEBUGP("\t%s\n", sname);
 		}
 		switch (m) {
 		case 0: /* executable */
@@ -2639,8 +2639,8 @@ static int move_module(struct module *mod, struct load_info *info)
 			memcpy(dest, (void *)shdr->sh_addr, shdr->sh_size);
 		/* Update sh_addr to point to copy in image. */
 		shdr->sh_addr = (unsigned long)dest;
-		DEBUGP("\t0x%lx %s\n",
-		       shdr->sh_addr, info->secstrings + shdr->sh_name);
+		DEBUGP("\t0x%p %s\n",
+		       (void *)shdr->sh_addr, info->secstrings + shdr->sh_name);
 	}
 
 	return 0;
-- 
1.7.7.3

[PATCH 02/25] dynamic_debug: fix whitespace complaints from scripts/cleanfile

[jim.cromie]

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


Signed-off-by: Jim Cromie <jim.cromie@gmail.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 03/25] dynamic_debug: drop enabled field from struct _ddebug, use _DPRINTK_FLAGS_PRINT

[jim.cromie]

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>
---
 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 04/25] dynamic_debug: make dynamic-debug supersede DEBUG ccflag

[jim.cromie]

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: Joe Perches <joe@perches.com>
Signed-off-by: Jim Cromie <jim.cromie@gmail.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 cbeb586..f8b71a3c 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -2643,14 +2643,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 05/25] dynamic_debug: change verbosity at runtime

[jim.cromie]

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>
---
 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 06/25] dynamic_debug: replace strcpy with strlcpy, in ddebug_setup_query()

[jim.cromie]

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

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

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

diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 8c88b89..101e2e5 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) {
 		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

[PATCH 07/25] dynamic_debug: pr_err() call should not depend upon verbosity

[jim.cromie]

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>
---
 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 101e2e5..4c6d0b7 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 08/25] dynamic_debug: drop explicit !=NULL checks

[jim.cromie]

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


Signed-off-by: Jim Cromie <jim.cromie@gmail.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 4c6d0b7..6904fdc 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 09/25] dynamic_debug: describe_flags with '=[pmflt_]*'

[jim.cromie]

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>
---
 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 6904fdc..6a170c3 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 10/25] dynamic_debug: tighten up error checking on debug queries

[jim.cromie]

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>
---
 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 6a170c3..f133e03 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 11/25] dynamic_debug: early return if _ddebug table is empty

[jim.cromie]

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

Iif _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>
---
 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 f133e03..aaa29dc 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 12/25] dynamic_debug: reduce lineno field to a saner 18 bits

[jim.cromie]

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>
---
 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 13/25] dynamic_debug: chop off comments in ddebug_tokenize

[jim.cromie]

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>
---
 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 aaa29dc..d285bb5 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 14/25] dynamic_debug: enlarge command/query write buffer

[jim.cromie]

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.

Signed-off-by: Jim Cromie <jim.cromie@gmail.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 d285bb5..6e5b9a3 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 4095
 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 15/25] dynamic_debug: add trim_prefix() to provide source-root relative paths

[jim.cromie]

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>
---
 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 6e5b9a3..cb078d4 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

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

[jim.cromie]

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>
---
 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 cb078d4..1a0e9bd 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

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

[jim.cromie]

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>
---
 Documentation/dynamic-debug-howto.txt |   17 +++-----
 lib/dynamic_debug.c                   |   73 ++++++++++++++++++++++++++-------
 2 files changed, 65 insertions(+), 25 deletions(-)

diff --git a/Documentation/dynamic-debug-howto.txt b/Documentation/dynamic-debug-howto.txt
index 378b5d1..989a892 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
@@ -92,18 +92,15 @@ nullarbor:~ # echo -c 'file svcsock.c\nline 1603 +p' >
 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 1a0e9bd..516ad4e 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

[PATCH 18/25] dynamic_debug: Introduce global fake module param module.ddebug

[jim.cromie]

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

Rework Thomas Renninger's $module.ddebug boot-time debugging feature,
from https://lkml.org/lkml/2010/9/15/397

Dynamic Debug allows enabling of pr_debug and dev_dbg messages at
runtime.  This is controlled via $DBGFS/dynamic_debug/control.  One
major drawback is that the whole initialization of a module cannot be
tracked, because ddebug is only aware of debug strings of loaded
modules.  But this is the most interesting part...

This patch introduces a fake module parameter module.ddebug (not shown
in /sys/module/*/parameters, thus it does not use any resources).  If
a module passes ddebug as a module parameter (e.g. via module.ddebug
kernel boot param or via "modprobe module ddebug"), all debug strings
of this module get activated by issuing "module module_name +p"
internally (not via sysfs) when the module gets loaded.
A later patch extends this with an arg: module.ddebug="+mfp"

Modules must not use "ddebug" as module parameter or it will get
ignored.  If it's tried, a warning will show up at module load time
that it will get ignored (only works for not built-in modules).
TBD: how to use BUILD_BUG_ON() to detect this earlier ...

Originally tested (by Thomas) with (additional added pr_debug messages):
options hp-wmi ddebug
in modprobe.conf
-> works and pr_debug messages issued at module initialization time show
up. Also "p" flag gets set for the whole hp-wmi module:
grep hp-wmi /sys/../dynamic_debug/control
also tested with compiled-in modules, e.g. pnp.ddebug and an additional
patch later in the patch series which instruments pnp code to work with ddebug.

Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
CC: Thomas Renninger <trenn@suse.de>
CC: Rusty Russell <rusty@rustcorp.com.au>
---
 Documentation/dynamic-debug-howto.txt |   28 +++++++++++-
 include/linux/dynamic_debug.h         |   17 +++++++
 include/linux/moduleparam.h           |    3 +
 kernel/module.c                       |    1 +
 kernel/params.c                       |   13 +++++-
 lib/dynamic_debug.c                   |   82 ++++++++++++++++++++++++++++++++-
 6 files changed, 140 insertions(+), 4 deletions(-)

diff --git a/Documentation/dynamic-debug-howto.txt b/Documentation/dynamic-debug-howto.txt
index 989a892..269065d 100644
--- a/Documentation/dynamic-debug-howto.txt
+++ b/Documentation/dynamic-debug-howto.txt
@@ -219,7 +219,7 @@ Note also that there is no convenient syntax to remove all
 the flags at once, you need to use "-flmpt".
 
 
-Debug messages during boot process
+Debug messages during Boot Process
 ==================================
 
 To be able to activate debug messages during the boot process,
@@ -238,6 +238,32 @@ PCI (or other devices) initialization also is a hot candidate for using
 this boot parameter for debugging purposes.
 
 
+Debug Messages at Module Initialization Time
+============================================
+
+Enabling debug messages inside a module is only possible if the module itself
+is loaded already. If you unload a module, the dynamic debug flags associated
+to its debug messages are lost.
+Therefore, enabling debug messages that get processed at module initialization
+time through the <debugfs>/dynamic_debug/control interface is not possible.
+Instead, a "ddebug" module paramter can be passed:
+
+	- via kernel boot parameter:
+	  module.ddebug
+
+	- as an ordinary module parameter via modprobe
+	  modprobe module ddebug
+
+	- or the parameter can be used permanently via modprobe.conf(.local)
+	  options module ddebug
+
+The ddebug option is not implemented as an ordinary module parameter and thus
+will not show up in /sys/module/module_name/parameters/ddebug
+The settings can get reverted through the sysfs interface again when the
+module got loaded as soon as debug messages are not needed anymore:
+echo "module module_name -p" > <debugfs>/dynamic_debug/control
+as described in the "Command Language Reference" chapter above.
+
 Examples
 ========
 
diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index 7e3c53a..d6f5b5c 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -39,11 +39,18 @@ struct _ddebug {
 int ddebug_add_module(struct _ddebug *tab, unsigned int n,
 				const char *modname);
 
+struct kernel_param;
+
 #if defined(CONFIG_DYNAMIC_DEBUG)
 extern int ddebug_remove_module(const char *mod_name);
+
 extern __printf(2, 3)
 int __dynamic_pr_debug(struct _ddebug *descriptor, const char *fmt, ...);
 
+extern int ddebug_exec_query(char *query_string);
+extern void ddebug_module_parse_args(const char *name, char* args,
+				struct kernel_param *params, unsigned num);
+
 struct device;
 
 extern __printf(3, 4)
@@ -98,6 +105,16 @@ static inline int ddebug_remove_module(const char *mod)
 {
 	return 0;
 }
+static inline int ddebug_exec_query(char *query_string)
+{
+	return 0;
+}
+static inline void ddebug_module_parse_args(const char *name, char* args,
+					struct kernel_param *params,
+					unsigned num)
+{
+	return 0;
+}
 
 #define dynamic_pr_debug(fmt, ...)					\
 	do { if (0) printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__); } while (0)
diff --git a/include/linux/moduleparam.h b/include/linux/moduleparam.h
index 7939f63..e62e5ce 100644
--- a/include/linux/moduleparam.h
+++ b/include/linux/moduleparam.h
@@ -433,4 +433,7 @@ static inline void module_param_sysfs_remove(struct module *mod)
 { }
 #endif
 
+/* For being able to parse parameters the same way params.c does */
+extern char *next_arg(char *args, char **param, char **val);
+
 #endif /* _LINUX_MODULE_PARAMS_H */
diff --git a/kernel/module.c b/kernel/module.c
index 30ffed4..3806d7e 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -2892,6 +2892,7 @@ static struct module *load_module(void __user *umod,
 	list_add_rcu(&mod->list, &modules);
 	mutex_unlock(&module_mutex);
 
+	ddebug_module_parse_args(mod->name, mod->args, mod->kp, mod->num_kp);
 	/* Module is ready to execute: parsing args may do that. */
 	err = parse_args(mod->name, mod->args, mod->kp, mod->num_kp, NULL);
 	if (err < 0)
diff --git a/kernel/params.c b/kernel/params.c
index 65aae11..313ea84 100644
--- a/kernel/params.c
+++ b/kernel/params.c
@@ -98,6 +98,7 @@ static int parse_one(char *param,
 {
 	unsigned int i;
 	int err;
+	char *tmp;
 
 	/* Find parameter */
 	for (i = 0; i < num_params; i++) {
@@ -114,6 +115,16 @@ static int parse_one(char *param,
 		}
 	}
 
+	/*
+	 * Ignore ddebug module params and module.ddebug boot params:
+	 * Documentation/dynamic-debug-howto.txt
+	 */
+	tmp = strstr(param, ".ddebug");
+	if (parameq(param, "ddebug") || (tmp && strlen(tmp) == 7)) {
+		DEBUGP("Ignoring ddebug parameter %s\n", param);
+		return 0;
+	}
+
 	if (handle_unknown) {
 		DEBUGP("Unknown argument: calling %p\n", handle_unknown);
 		return handle_unknown(param, val);
@@ -125,7 +136,7 @@ static int parse_one(char *param,
 
 /* You can use " around spaces, but can't escape ". */
 /* Hyphens and underscores equivalent in parameter names. */
-static char *next_arg(char *args, char **param, char **val)
+char *next_arg(char *args, char **param, char **val)
 {
 	unsigned int i, equals = 0;
 	int in_quote = 0, quoted = 0;
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 516ad4e..bb66251 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -13,6 +13,7 @@
 #define pr_fmt(fmt) KBUILD_MODNAME ":%s: " fmt, __func__
 
 #include <linux/kernel.h>
+#include <linux/init.h>
 #include <linux/module.h>
 #include <linux/moduleparam.h>
 #include <linux/kallsyms.h>
@@ -34,6 +35,8 @@
 #include <linux/device.h>
 #include <linux/netdevice.h>
 
+#include <asm/setup.h>
+
 extern struct _ddebug __start___verbose[];
 extern struct _ddebug __stop___verbose[];
 
@@ -446,7 +449,7 @@ static int ddebug_parse_flags(const char *str, unsigned int *flagsp,
 	return 0;
 }
 
-static int ddebug_exec_query(char *query_string)
+int ddebug_exec_query(char *query_string)
 {
 	unsigned int flags = 0, mask = 0;
 	struct ddebug_query query;
@@ -454,6 +457,9 @@ static int ddebug_exec_query(char *query_string)
 	int nwords, nfound;
 	char *words[MAXWORDS];
 
+	if (verbose)
+		printk(KERN_INFO "%s: got query: %s\n", __func__, query_string);
+
 	nwords = ddebug_tokenize(query_string, words, MAXWORDS);
 	if (nwords <= 0)
 		return -EINVAL;
@@ -618,7 +624,7 @@ 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;
 	}
@@ -872,6 +878,76 @@ int ddebug_add_module(struct _ddebug *tab, unsigned int n,
 }
 EXPORT_SYMBOL_GPL(ddebug_add_module);
 
+/* We search for *ddebug* module params */
+void ddebug_module_parse_args(const char *name, char* args,
+			      struct kernel_param *params, unsigned num)
+{
+	char ddebug[DDEBUG_STRING_SIZE], *param, *val, *args_it, *arg_dup_ptr;
+	int i;
+
+	/*
+	 * We must not modify the passed args string and need to store the
+	 * kstrdup pointer to be able to free memory later, TBD: find a way
+	 * to do this nicer
+	 */
+	arg_dup_ptr = args_it = kstrdup(args, GFP_KERNEL);
+
+	if (verbose)
+		printk(KERN_INFO "%s: Parsing ARGS: -%s- of %s\n",
+		       __func__, args_it, name);
+
+	for (i = 0; i < num; i++) {
+		if (!strcmp("ddebug", params[i].name))
+			pr_warning("Module %s uses reserved keyword "
+				   "*ddebug* as parameter\n", name);
+	}
+
+	/* Chew leading spaces */
+	args_it = skip_spaces(args_it);
+
+	while (*args_it) {
+		args_it = next_arg(args_it, &param, &val);
+		if (verbose)
+			printk(KERN_INFO "%s: Param: %s, val: %s\n",
+			       __func__, param, val);
+		if (!strcmp(param, "ddebug")) {
+			pr_info("Enabling debugging for module %s\n", name);
+			snprintf(ddebug, DDEBUG_STRING_SIZE, "module %s +p",
+				 name);
+			ddebug_exec_query(ddebug);
+		}
+	}
+	kfree(arg_dup_ptr);
+	if (verbose)
+		printk(KERN_INFO "%s: Finished %s parsing\n",  __func__, name);
+}
+/* We search for module.ddebug kernel boot params */
+static void ddebug_boot_parse_args(void)
+{
+	char tmp_cmd_arr[COMMAND_LINE_SIZE], *tmp_cmd_ptr, *param, *val, *tmp;
+	char module[MODULE_NAME_LEN], ddebug[DDEBUG_STRING_SIZE];
+
+	/* next_arg touches the passed buffer and chops each argument */
+	strlcpy(tmp_cmd_arr, saved_command_line, COMMAND_LINE_SIZE);
+	/* Chew leading spaces */
+	tmp_cmd_ptr = skip_spaces(tmp_cmd_arr);
+
+	while (*tmp_cmd_ptr) {
+		tmp_cmd_ptr = next_arg(tmp_cmd_ptr, &param, &val);
+		if (verbose)
+			printk(KERN_INFO "%s: Param: %s, val: %s\n",
+			       __func__, param, val);
+		tmp = strstr(param, ".ddebug");
+		if (tmp && strlen(tmp) == 7) {
+			strlcpy(module, param, tmp - param + 1);
+			pr_info("Enabling debugging for module %s\n", module);
+			snprintf(ddebug, DDEBUG_STRING_SIZE, "module %s +p",
+				 module);
+			ddebug_exec_query(ddebug);
+		}
+	}
+}
+
 static void ddebug_table_free(struct ddebug_table *dt)
 {
 	list_del_init(&dt->link);
@@ -979,6 +1055,8 @@ static int __init dynamic_debug_init(void)
 		ret = 0;
 	}
 
+	ddebug_boot_parse_args();
+
 out_free:
 	if (ret)
 		ddebug_remove_all_tables();
-- 
1.7.7.3

[PATCH 19/25] pnp: if CONFIG_DYNAMIC_DEBUG, use pnp.ddebug instead of pnp.debug

[jim.cromie]

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

resubmit of https://lkml.org/lkml/2010/9/15/398

This allows usage of generic pnp.ddebug debug parameter instead of
pnp.debug PNP specific parameter.

I wonder whether CONFIG_PNP_DEBUG_MESSAGES can vanish totally with
this or at some time. Only advantage having it is: If you are
restricted and your kernel must not exceed X bytes, you cannot compile
in PNP debug messages only, but you have to compile in all debug
messages.

Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
CC: Thomas Renninger <trenn@suse.de>
---
 drivers/pnp/base.h |    8 ++++++--
 drivers/pnp/core.c |   13 +++++++++++++
 2 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/drivers/pnp/base.h b/drivers/pnp/base.h
index fa4e0a5..28e98aa 100644
--- a/drivers/pnp/base.h
+++ b/drivers/pnp/base.h
@@ -173,12 +173,16 @@ struct pnp_resource *pnp_add_bus_resource(struct pnp_dev *dev,
 					  resource_size_t start,
 					  resource_size_t end);
 
-extern int pnp_debug;
-
+#if defined(CONFIG_DYNAMIC_DEBUG)
+#define pnp_dbg(dev, format, arg...)					\
+	({ dev_dbg(dev, format, ## arg); 0; })
+#else
 #if defined(CONFIG_PNP_DEBUG_MESSAGES)
+extern int pnp_debug;
 #define pnp_dbg(dev, format, arg...)					\
 	({ if (pnp_debug) dev_printk(KERN_DEBUG, dev, format, ## arg); 0; })
 #else
 #define pnp_dbg(dev, format, arg...)					\
 	({ if (0) dev_printk(KERN_DEBUG, dev, format, ## arg); 0; })
 #endif
+#endif
diff --git a/drivers/pnp/core.c b/drivers/pnp/core.c
index cb6ce42..332010a 100644
--- a/drivers/pnp/core.c
+++ b/drivers/pnp/core.c
@@ -219,6 +219,19 @@ subsys_initcall(pnp_init);
 
 int pnp_debug;
 
+#if defined(CONFIG_DYNAMIC_DEBUG)
+static int __init pnp_debug_setup(char *__unused)
+{
+	printk(KERN_INFO "DYNAMIC_DEBUG enabled use pnp.ddebug instead of "
+		"pnp.debug boot param\n");
+	return 1;
+}
+__setup("pnp.debug", pnp_debug_setup);
+
+#else
+
 #if defined(CONFIG_PNP_DEBUG_MESSAGES)
 module_param_named(debug, pnp_debug, int, 0644);
 #endif
+
+#endif
-- 
1.7.7.3

[PATCH 20/25] dynamic_debug: rename ddebug param to dyndbg, plus minor tweaks

[jim.cromie]

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

user-visible:
- change param-name from "ddebug" to "dyndbg".  This is more clearly
  associated with dynamic-debug, and more distinct from other *debug*
  names.

minor internal tweaks:
- drop return 0 in the inline void ddebug_module_parse_args stub,
  in #ifndef CONFIG_DYNAMIC_DEBUG case, to silence warning.
- make ddebug_exec_query static again, its not used elsewhere.
- shrink stack in ddebug_boot_parse_args, move auto vars to __initdata.
- convert printk()s to pr_info()s.
- add "mod" prefix to ddebug_module_parse_args(name, args, ...) parameters.
- in ddebug_module_parse_args(), return early if no mod-params,
  to avoid non-pertinent pr_info()s and kstrdup.
- in ddebug_module_parse_args() and ddebug_boot_parse_args() arg scan
  loops, invert param-name == "ddebug" checks and continue.  This
  avoids pr_info()s for uninteresting args, and reduces indenting.
- use ddebug_exec_queries() instead of ddebug_exec_query().

Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
CC: Thomas Renninger <trenn@suse.de>
CC: Rusty Russell <rusty@rustcorp.com.au>
---
 Documentation/dynamic-debug-howto.txt |   57 +++++++++++++++++++--------
 include/linux/dynamic_debug.h         |    7 +---
 kernel/params.c                       |    8 ++--
 lib/dynamic_debug.c                   |   67 +++++++++++++++++----------------
 4 files changed, 79 insertions(+), 60 deletions(-)

diff --git a/Documentation/dynamic-debug-howto.txt b/Documentation/dynamic-debug-howto.txt
index 269065d..8363a56 100644
--- a/Documentation/dynamic-debug-howto.txt
+++ b/Documentation/dynamic-debug-howto.txt
@@ -241,28 +241,40 @@ this boot parameter for debugging purposes.
 Debug Messages at Module Initialization Time
 ============================================
 
-Enabling debug messages inside a module is only possible if the module itself
-is loaded already. If you unload a module, the dynamic debug flags associated
-to its debug messages are lost.
-Therefore, enabling debug messages that get processed at module initialization
-time through the <debugfs>/dynamic_debug/control interface is not possible.
-Instead, a "ddebug" module paramter can be passed:
+Enabling a module's debug messages via debug control file only works
+once the module is loaded; too late for callsites in init functions.
+And when module is unloaded, debug flag settings for the module are
+lost.  Instead, a "dyndbg" module parameter can be passed:
 
-	- via kernel boot parameter:
-	  module.ddebug
+	- via kernel boot parameter: (this form works on built-ins too)
+	  module.dyndbg=+mfp
+	  module.dyndbg	# defaults to +p
 
-	- as an ordinary module parameter via modprobe
-	  modprobe module ddebug
+	- as an ordinary module parameter via modprobe modprobe module
+	  $> modprobe module dyndbg +pmfl # defaults to +p
 
 	- or the parameter can be used permanently via modprobe.conf(.local)
-	  options module ddebug
+	  options module dyndbg=+pmflt
+	  options module dyndbg # defaults to +p
 
-The ddebug option is not implemented as an ordinary module parameter and thus
-will not show up in /sys/module/module_name/parameters/ddebug
-The settings can get reverted through the sysfs interface again when the
-module got loaded as soon as debug messages are not needed anymore:
-echo "module module_name -p" > <debugfs>/dynamic_debug/control
-as described in the "Command Language Reference" chapter above.
+The $modname.dyndbg="value" should exclude "module $modname", as the
+$modname is taken from the param-name, and only 1 spec of each type is
+allowed.
+
+The ddebug option is not implemented as an ordinary module parameter
+and thus will not show up in /sys/module/module_name/parameters/ddebug
+The settings can be reverted later via the sysfs interface if the
+debug messages are no longer needed:
+
+      echo "module module_name -p" > <debugfs>/dynamic_debug/control
+
+$module.dyndbg="..." on boot-line works on built-in modules as well as
+those loaded by modprobe (from either early or normal userspace), and
+somewhat overlaps debug_query functionality.
+
+Modprobed modules get ddebug flags given on boot-line after those
+given via modprobe (either explicitly, or from /etc/modprobe.d/*).
+This can surprise if boot-line arg subtracts flags.
 
 Examples
 ========
@@ -290,3 +302,14 @@ nullarbor:~ # echo -n 'func svc_process -p' >
 // enable messages for NFS calls READ, READLINK, READDIR and READDIR+.
 nullarbor:~ # echo -n 'format "nfsd: READ" +p' >
 				<debugfs>/dynamic_debug/control
+
+// boot-args example, with newlines and comments
+Kernel command line: ... 
+  ddebug_query="func i2c_del_adapter +p; func tboot_probe +p"
+  dynamic_debug.verbose=1 		// see whats going on
+  nouveau.dyndbg 			// implicit =+p
+  tsc_sync.dyndbg=+p 			// builtin on my kernel
+  i2c_core.dyndbg=+p			// loaded by udev
+  *.dyndbg="=_"				// wildcard applies to builtins
+  k10temp.dyndbg="+p # comment in query is stripped "
+  pnp.dyndbg="func pnpacpi_get_resources +p; func pnp_assign_mem +p" # multi
diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index d6f5b5c..29b57e5 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -47,7 +47,6 @@ extern int ddebug_remove_module(const char *mod_name);
 extern __printf(2, 3)
 int __dynamic_pr_debug(struct _ddebug *descriptor, const char *fmt, ...);
 
-extern int ddebug_exec_query(char *query_string);
 extern void ddebug_module_parse_args(const char *name, char* args,
 				struct kernel_param *params, unsigned num);
 
@@ -105,15 +104,11 @@ static inline int ddebug_remove_module(const char *mod)
 {
 	return 0;
 }
-static inline int ddebug_exec_query(char *query_string)
-{
-	return 0;
-}
+
 static inline void ddebug_module_parse_args(const char *name, char* args,
 					struct kernel_param *params,
 					unsigned num)
 {
-	return 0;
 }
 
 #define dynamic_pr_debug(fmt, ...)					\
diff --git a/kernel/params.c b/kernel/params.c
index 313ea84..010f167 100644
--- a/kernel/params.c
+++ b/kernel/params.c
@@ -116,12 +116,12 @@ static int parse_one(char *param,
 	}
 
 	/*
-	 * Ignore ddebug module params and module.ddebug boot params:
+	 * Ignore ddebug module params and module.dyndbg boot params:
 	 * Documentation/dynamic-debug-howto.txt
 	 */
-	tmp = strstr(param, ".ddebug");
-	if (parameq(param, "ddebug") || (tmp && strlen(tmp) == 7)) {
-		DEBUGP("Ignoring ddebug parameter %s\n", param);
+	tmp = strstr(param, ".dyndbg");
+	if (parameq(param, "dyndbg") || (tmp && strlen(tmp) == 7)) {
+		pr_debug("Ignoring ddebug parameter %s\n", param);
 		return 0;
 	}
 
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index bb66251..afc9bd3 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -449,7 +449,7 @@ static int ddebug_parse_flags(const char *str, unsigned int *flagsp,
 	return 0;
 }
 
-int ddebug_exec_query(char *query_string)
+static int ddebug_exec_query(char *query_string)
 {
 	unsigned int flags = 0, mask = 0;
 	struct ddebug_query query;
@@ -458,7 +458,7 @@ int ddebug_exec_query(char *query_string)
 	char *words[MAXWORDS];
 
 	if (verbose)
-		printk(KERN_INFO "%s: got query: %s\n", __func__, query_string);
+		pr_info("got query: %s\n", query_string);
 
 	nwords = ddebug_tokenize(query_string, words, MAXWORDS);
 	if (nwords <= 0)
@@ -879,53 +879,55 @@ int ddebug_add_module(struct _ddebug *tab, unsigned int n,
 EXPORT_SYMBOL_GPL(ddebug_add_module);
 
 /* We search for *ddebug* module params */
-void ddebug_module_parse_args(const char *name, char* args,
+void ddebug_module_parse_args(const char *modname, char* modargs,
 			      struct kernel_param *params, unsigned num)
 {
 	char ddebug[DDEBUG_STRING_SIZE], *param, *val, *args_it, *arg_dup_ptr;
 	int i;
 
+	if (!modargs || !*modargs)
+		return; /* no args, return now, avoid pr_infos */
 	/*
 	 * We must not modify the passed args string and need to store the
 	 * kstrdup pointer to be able to free memory later, TBD: find a way
 	 * to do this nicer
 	 */
-	arg_dup_ptr = args_it = kstrdup(args, GFP_KERNEL);
+	arg_dup_ptr = args_it = kstrdup(modargs, GFP_KERNEL);
+	args_it = skip_spaces(args_it);
 
 	if (verbose)
-		printk(KERN_INFO "%s: Parsing ARGS: -%s- of %s\n",
-		       __func__, args_it, name);
+		pr_info("%s ARGS: %s\n", modname, args_it);
 
+	/* warn if modules declare ddebug param - todo BUILD_BUG ?? */
 	for (i = 0; i < num; i++) {
 		if (!strcmp("ddebug", params[i].name))
 			pr_warning("Module %s uses reserved keyword "
-				   "*ddebug* as parameter\n", name);
+				   "*ddebug* as parameter\n", modname);
 	}
 
-	/* Chew leading spaces */
-	args_it = skip_spaces(args_it);
-
 	while (*args_it) {
 		args_it = next_arg(args_it, &param, &val);
+		if (strcmp(param, "dyndbg"))
+			continue;
+		pr_info("Enabling debugging for module %s\n", modname);
 		if (verbose)
-			printk(KERN_INFO "%s: Param: %s, val: %s\n",
-			       __func__, param, val);
-		if (!strcmp(param, "ddebug")) {
-			pr_info("Enabling debugging for module %s\n", name);
-			snprintf(ddebug, DDEBUG_STRING_SIZE, "module %s +p",
-				 name);
-			ddebug_exec_query(ddebug);
-		}
+			pr_info("Param: %s, val: %s\n", param, val);
+
+		snprintf(ddebug, DDEBUG_STRING_SIZE, "module %s +p", modname);
+		ddebug_exec_queries(ddebug);
 	}
 	kfree(arg_dup_ptr);
 	if (verbose)
-		printk(KERN_INFO "%s: Finished %s parsing\n",  __func__, name);
+		pr_info("Finished %s parsing\n", modname);
 }
-/* We search for module.ddebug kernel boot params */
-static void ddebug_boot_parse_args(void)
+
+char __initdata tmp_cmd_arr[COMMAND_LINE_SIZE];
+char __initdata module[MODULE_NAME_LEN], ddebug[DDEBUG_STRING_SIZE];
+
+/* We search for module.dyndbg kernel boot params */
+static __init void ddebug_boot_parse_args(void)
 {
-	char tmp_cmd_arr[COMMAND_LINE_SIZE], *tmp_cmd_ptr, *param, *val, *tmp;
-	char module[MODULE_NAME_LEN], ddebug[DDEBUG_STRING_SIZE];
+	char *tmp_cmd_ptr, *param, *val, *tmp;
 
 	/* next_arg touches the passed buffer and chops each argument */
 	strlcpy(tmp_cmd_arr, saved_command_line, COMMAND_LINE_SIZE);
@@ -934,17 +936,16 @@ static void ddebug_boot_parse_args(void)
 
 	while (*tmp_cmd_ptr) {
 		tmp_cmd_ptr = next_arg(tmp_cmd_ptr, &param, &val);
+		tmp = strstr(param, ".dyndbg");
+		if (!tmp || strlen(tmp) != 7)
+			continue;
+		strlcpy(module, param, tmp - param + 1);
+		pr_info("Enabling debugging for module %s\n", module);
 		if (verbose)
-			printk(KERN_INFO "%s: Param: %s, val: %s\n",
-			       __func__, param, val);
-		tmp = strstr(param, ".ddebug");
-		if (tmp && strlen(tmp) == 7) {
-			strlcpy(module, param, tmp - param + 1);
-			pr_info("Enabling debugging for module %s\n", module);
-			snprintf(ddebug, DDEBUG_STRING_SIZE, "module %s +p",
-				 module);
-			ddebug_exec_query(ddebug);
-		}
+			pr_info("Param: %s, val: %s\n", param, val);
+
+		snprintf(ddebug, DDEBUG_STRING_SIZE, "module %s +p", module);
+		ddebug_exec_queries(ddebug);
 	}
 }
 
-- 
1.7.7.3

[PATCH 21/25] dynamic_debug: handle $module.dyndbg="+mfp" boot-line args

[jim.cromie]

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

Handle explicit flag-arg given on boot-line, defaulting to "+p"
otherwize.  Note that the operator char is required in the flag-arg,
so these are legal, and the same (the latter being slightly odd):

  $module.dyndbg="=p"	# set p flag, clear others
  $module.dyndbg==p	# same

Note that modprobed modules will receive flag-args from the boot-line
*after* those given in /etc/modprobe.d/*.  This may be surprising;
boot-line args can alter previous flag settings, but is defensible, as
former are invocation specific, whereas latter are system-wide.
Options given on modprobe invocation are last, and correctly override
all previous flag settings.

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

diff --git a/Documentation/dynamic-debug-howto.txt b/Documentation/dynamic-debug-howto.txt
index 8363a56..56e496a 100644
--- a/Documentation/dynamic-debug-howto.txt
+++ b/Documentation/dynamic-debug-howto.txt
@@ -250,8 +250,9 @@ lost.  Instead, a "dyndbg" module parameter can be passed:
 	  module.dyndbg=+mfp
 	  module.dyndbg	# defaults to +p
 
-	- as an ordinary module parameter via modprobe modprobe module
-	  $> modprobe module dyndbg +pmfl # defaults to +p
+	- as an ordinary module parameter via modprobe
+	  modprobe module dyndbg=+pmfl
+	  modprobe module dyndbg # defaults to +p
 
 	- or the parameter can be used permanently via modprobe.conf(.local)
 	  options module dyndbg=+pmflt
@@ -261,8 +262,8 @@ The $modname.dyndbg="value" should exclude "module $modname", as the
 $modname is taken from the param-name, and only 1 spec of each type is
 allowed.
 
-The ddebug option is not implemented as an ordinary module parameter
-and thus will not show up in /sys/module/module_name/parameters/ddebug
+The dyndbg option is not implemented as an ordinary module parameter
+and thus will not show up in /sys/module/module_name/parameters/dyndbg
 The settings can be reverted later via the sysfs interface if the
 debug messages are no longer needed:
 
@@ -272,7 +273,7 @@ $module.dyndbg="..." on boot-line works on built-in modules as well as
 those loaded by modprobe (from either early or normal userspace), and
 somewhat overlaps debug_query functionality.
 
-Modprobed modules get ddebug flags given on boot-line after those
+Modprobed modules get dyndbg flags given on boot-line after those
 given via modprobe (either explicitly, or from /etc/modprobe.d/*).
 This can surprise if boot-line arg subtracts flags.
 
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index afc9bd3..9dcbb61 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -913,7 +913,8 @@ void ddebug_module_parse_args(const char *modname, char* modargs,
 		if (verbose)
 			pr_info("Param: %s, val: %s\n", param, val);
 
-		snprintf(ddebug, DDEBUG_STRING_SIZE, "module %s +p", modname);
+		snprintf(ddebug, DDEBUG_STRING_SIZE, "module %s %s",
+			modname, (val ? val : "+p"));
 		ddebug_exec_queries(ddebug);
 	}
 	kfree(arg_dup_ptr);
@@ -944,7 +945,8 @@ static __init void ddebug_boot_parse_args(void)
 		if (verbose)
 			pr_info("Param: %s, val: %s\n", param, val);
 
-		snprintf(ddebug, DDEBUG_STRING_SIZE, "module %s +p", module);
+		snprintf(ddebug, DDEBUG_STRING_SIZE, "module %s %s",
+			module, (val ? val : "+p"));
 		ddebug_exec_queries(ddebug);
 	}
 }
-- 
1.7.7.3

[PATCH 22/25] dynamic_debug: add modname arg to exec_query callchain

[jim.cromie]

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

Extract module name from param-name, and pass to ddebug_exec_query(),
ddebug_exec_queries() and ddebug_parse_query() as separate parameter.
This has 2 purposes:

1. support multiple queries:
   $modname.dyndbg="func foo +fp; func bar +fp"

In ddebug_parse_query(), the module name is added into the query
struct before each query-string is parsed, so "module $module" must be
omitted from the query-string (param-value), otherwize the
set-only-once test added previously will issue an error.

2. With module-name passed by ddebug_boot_parse_args() into
ddebug_exec_queries(), we no longer need to sprintf "module modname"
into the query-string, and can just drop the sprintf buffer.

Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
 lib/dynamic_debug.c |   29 ++++++++++++++---------------
 1 files changed, 14 insertions(+), 15 deletions(-)

diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 9dcbb61..fc3f68a 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -340,7 +340,7 @@ static int check_set(const char **dest, char *src, char *name)
  * Returns 0 on success, <0 on error.
  */
 static int ddebug_parse_query(char *words[], int nwords,
-			       struct ddebug_query *query)
+			struct ddebug_query *query, const char *modname)
 {
 	unsigned int i;
 	int rc;
@@ -350,6 +350,10 @@ static int ddebug_parse_query(char *words[], int nwords,
 		return -EINVAL;
 	memset(query, 0, sizeof(*query));
 
+	if (modname)
+		/* support $modname.dyndbg=<multiple queries> */
+		query->module = modname;
+
 	for (i = 0 ; i < nwords ; i += 2) {
 		if (!strcmp(words[i], "func"))
 			rc = check_set(&query->function, words[i+1], "func");
@@ -449,7 +453,7 @@ static int ddebug_parse_flags(const char *str, unsigned int *flagsp,
 	return 0;
 }
 
-static int ddebug_exec_query(char *query_string)
+static int ddebug_exec_query(char *query_string, const char *modname)
 {
 	unsigned int flags = 0, mask = 0;
 	struct ddebug_query query;
@@ -463,7 +467,7 @@ static int ddebug_exec_query(char *query_string)
 	nwords = ddebug_tokenize(query_string, words, MAXWORDS);
 	if (nwords <= 0)
 		return -EINVAL;
-	if (ddebug_parse_query(words, nwords-1, &query))
+	if (ddebug_parse_query(words, nwords-1, &query, modname))
 		return -EINVAL;
 	if (ddebug_parse_flags(words[nwords-1], &flags, &mask))
 		return -EINVAL;
@@ -479,7 +483,7 @@ static int ddebug_exec_query(char *query_string)
    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)
+static int ddebug_exec_queries(char *query, char *modname)
 {
 	char *split;
 	int i, errs = 0, exitcode = 0, rc, nfound = 0;
@@ -496,7 +500,7 @@ static int ddebug_exec_queries(char *query)
 		if (verbose)
 			pr_info("query %d: \"%s\"\n", i, query);
 
-		rc = ddebug_exec_query(query);
+		rc = ddebug_exec_query(query, modname);
 		if (rc < 0) {
 			errs++;
 			exitcode = rc;
@@ -662,7 +666,7 @@ 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_queries(tmpbuf);
+	ret = ddebug_exec_queries(tmpbuf, NULL);
 	kfree(tmpbuf);
 	if (ret < 0)
 		return ret;
@@ -882,7 +886,7 @@ EXPORT_SYMBOL_GPL(ddebug_add_module);
 void ddebug_module_parse_args(const char *modname, char* modargs,
 			      struct kernel_param *params, unsigned num)
 {
-	char ddebug[DDEBUG_STRING_SIZE], *param, *val, *args_it, *arg_dup_ptr;
+	char *param, *val, *args_it, *arg_dup_ptr;
 	int i;
 
 	if (!modargs || !*modargs)
@@ -894,7 +898,6 @@ void ddebug_module_parse_args(const char *modname, char* modargs,
 	 */
 	arg_dup_ptr = args_it = kstrdup(modargs, GFP_KERNEL);
 	args_it = skip_spaces(args_it);
-
 	if (verbose)
 		pr_info("%s ARGS: %s\n", modname, args_it);
 
@@ -913,9 +916,7 @@ void ddebug_module_parse_args(const char *modname, char* modargs,
 		if (verbose)
 			pr_info("Param: %s, val: %s\n", param, val);
 
-		snprintf(ddebug, DDEBUG_STRING_SIZE, "module %s %s",
-			modname, (val ? val : "+p"));
-		ddebug_exec_queries(ddebug);
+		ddebug_exec_queries((val ? val : "+p"), modname);
 	}
 	kfree(arg_dup_ptr);
 	if (verbose)
@@ -945,9 +946,7 @@ static __init void ddebug_boot_parse_args(void)
 		if (verbose)
 			pr_info("Param: %s, val: %s\n", param, val);
 
-		snprintf(ddebug, DDEBUG_STRING_SIZE, "module %s %s",
-			module, (val ? val : "+p"));
-		ddebug_exec_queries(ddebug);
+		ddebug_exec_queries((val ? val : "+p"), module);
 	}
 }
 
@@ -1047,7 +1046,7 @@ 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_queries(ddebug_setup_string);
+		ret = ddebug_exec_queries(ddebug_setup_string, NULL);
 		if (ret < 0)
 			pr_warn("Invalid ddebug boot param %s",
 				ddebug_setup_string);
-- 
1.7.7.3

[PATCH 23/25] dynamic_debug: allow wildcard modname in boot-line query

[jim.cromie]

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

Allow use of "*.dyndbg=+p" in boot-line, to turn on all debugging.
This works almost exactly like ddebug_query="+p"; it enables all
callsites for builtin modules, but does so after ddebug_query=... is
processed, and in the order of given mod.dyndbg args.

IOW, these are equivalent:

  ddebug_query="+p"		foo.dyndbg=+p
  ddebug_query="" *.dyndbg=+p	foo.dyndbg=+p

So this turns on module, function, line flags on all callsites,
which can then be enabled separately:

  *.dyndbg="+mfl"

Note that order matters if flags are set rather than added (= not +),
these are slightly different:

  " foo.dyndbg=p *.dyndbg=+mf "
  " *.dyndbg=+mf foo.dyndbg=p "

This obsoletes the remaining usage of ddebug_query; module specific
uses of ddebug_query were already covered by modname.dyndbg="...".  It
may however offend the tastes of some, or perhaps "*" should be
spelled as "dynamic_debug".  Removing ddebug_query probably also needs
a deprecation cycle, so I've deferred.

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

diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index fc3f68a..a077cd4 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -946,7 +946,8 @@ static __init void ddebug_boot_parse_args(void)
 		if (verbose)
 			pr_info("Param: %s, val: %s\n", param, val);
 
-		ddebug_exec_queries((val ? val : "+p"), module);
+		ddebug_exec_queries((val ? val : "+p"),
+				    !strcmp(module, "*") ? NULL : module);
 	}
 }
 
-- 
1.7.7.3

[PATCH 24/25] kernel/module: replace DEBUGP with pr_debug

[jim.cromie]

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

Use more flexible pr_debug

Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
 kernel/module.c |   44 +++++++++++++++++++-------------------------
 kernel/params.c |   14 ++++----------
 2 files changed, 23 insertions(+), 35 deletions(-)

diff --git a/kernel/module.c b/kernel/module.c
index 3806d7e..de3a056 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -62,12 +62,6 @@
 #define CREATE_TRACE_POINTS
 #include <trace/events/module.h>
 
-#if 0
-#define DEBUGP printk
-#else
-#define DEBUGP(fmt , a...)
-#endif
-
 #ifndef ARCH_SHF_SMALL
 #define ARCH_SHF_SMALL 0
 #endif
@@ -410,7 +404,7 @@ const struct kernel_symbol *find_symbol(const char *name,
 		return fsa.sym;
 	}
 
-	DEBUGP("Failed to find symbol %s\n", name);
+	pr_debug("Failed to find symbol %s\n", name);
 	return NULL;
 }
 EXPORT_SYMBOL_GPL(find_symbol);
@@ -600,11 +594,11 @@ static int already_uses(struct module *a, struct module *b)
 
 	list_for_each_entry(use, &b->source_list, source_list) {
 		if (use->source == a) {
-			DEBUGP("%s uses %s!\n", a->name, b->name);
+			pr_debug("%s uses %s!\n", a->name, b->name);
 			return 1;
 		}
 	}
-	DEBUGP("%s does not use %s!\n", a->name, b->name);
+	pr_debug("%s does not use %s!\n", a->name, b->name);
 	return 0;
 }
 
@@ -619,7 +613,7 @@ static int add_module_usage(struct module *a, struct module *b)
 {
 	struct module_use *use;
 
-	DEBUGP("Allocating new usage for %s.\n", a->name);
+	pr_debug("Allocating new usage for %s.\n", a->name);
 	use = kmalloc(sizeof(*use), GFP_ATOMIC);
 	if (!use) {
 		printk(KERN_WARNING "%s: out of memory loading\n", a->name);
@@ -663,7 +657,7 @@ static void module_unload_free(struct module *mod)
 	mutex_lock(&module_mutex);
 	list_for_each_entry_safe(use, tmp, &mod->target_list, target_list) {
 		struct module *i = use->target;
-		DEBUGP("%s unusing %s\n", mod->name, i->name);
+		pr_debug("%s unusing %s\n", mod->name, i->name);
 		module_put(i);
 		list_del(&use->source_list);
 		list_del(&use->target_list);
@@ -761,7 +755,7 @@ static void wait_for_zero_refcount(struct module *mod)
 	/* Since we might sleep for some time, release the mutex first */
 	mutex_unlock(&module_mutex);
 	for (;;) {
-		DEBUGP("Looking at refcount...\n");
+		pr_debug("Looking at refcount...\n");
 		set_current_state(TASK_UNINTERRUPTIBLE);
 		if (module_refcount(mod) == 0)
 			break;
@@ -804,7 +798,7 @@ SYSCALL_DEFINE2(delete_module, const char __user *, name_user,
 	if (mod->state != MODULE_STATE_LIVE) {
 		/* FIXME: if (force), slam module count and wake up
                    waiter --RR */
-		DEBUGP("%s already dying\n", mod->name);
+		pr_debug("%s already dying\n", mod->name);
 		ret = -EBUSY;
 		goto out;
 	}
@@ -1057,7 +1051,7 @@ static int check_version(Elf_Shdr *sechdrs,
 
 		if (versions[i].crc == maybe_relocated(*crc, crc_owner))
 			return 1;
-		DEBUGP("Found checksum %lX vs module %lX\n",
+		pr_debug("Found checksum %lX vs module %lX\n",
 		       maybe_relocated(*crc, crc_owner), versions[i].crc);
 		goto bad_version;
 	}
@@ -1834,7 +1828,7 @@ static int simplify_symbols(struct module *mod, const struct load_info *info)
 		case SHN_COMMON:
 			/* We compiled with -fno-common.  These are not
 			   supposed to happen.  */
-			DEBUGP("Common symbol: %s\n", name);
+			pr_debug("Common symbol: %s\n", name);
 			printk("%s: please compile with -fno-common\n",
 			       mod->name);
 			ret = -ENOEXEC;
@@ -1842,7 +1836,7 @@ static int simplify_symbols(struct module *mod, const struct load_info *info)
 
 		case SHN_ABS:
 			/* Don't need to do anything */
-			DEBUGP("Absolute symbol: 0x%08lx\n",
+			pr_debug("Absolute symbol: 0x%08lx\n",
 			       (long)sym[i].st_value);
 			break;
 
@@ -1966,7 +1960,7 @@ static void layout_sections(struct module *mod, struct load_info *info)
 	for (i = 0; i < info->hdr->e_shnum; i++)
 		info->sechdrs[i].sh_entsize = ~0UL;
 
-	DEBUGP("Core section allocation order:\n");
+	pr_debug("Core section allocation order:\n");
 	for (m = 0; m < ARRAY_SIZE(masks); ++m) {
 		for (i = 0; i < info->hdr->e_shnum; ++i) {
 			Elf_Shdr *s = &info->sechdrs[i];
@@ -1978,7 +1972,7 @@ static void layout_sections(struct module *mod, struct load_info *info)
 			    || strstarts(sname, ".init"))
 				continue;
 			s->sh_entsize = get_offset(mod, &mod->core_size, s, i);
-			DEBUGP("\t%s\n", sname);
+			pr_debug("\t%s\n", sname);
 		}
 		switch (m) {
 		case 0: /* executable */
@@ -1995,7 +1989,7 @@ static void layout_sections(struct module *mod, struct load_info *info)
 		}
 	}
 
-	DEBUGP("Init section allocation order:\n");
+	pr_debug("Init section allocation order:\n");
 	for (m = 0; m < ARRAY_SIZE(masks); ++m) {
 		for (i = 0; i < info->hdr->e_shnum; ++i) {
 			Elf_Shdr *s = &info->sechdrs[i];
@@ -2008,7 +2002,7 @@ static void layout_sections(struct module *mod, struct load_info *info)
 				continue;
 			s->sh_entsize = (get_offset(mod, &mod->init_size, s, i)
 					 | INIT_OFFSET_MASK);
-			DEBUGP("\t%s\n", sname);
+			pr_debug("\t%s\n", sname);
 		}
 		switch (m) {
 		case 0: /* executable */
@@ -2189,7 +2183,7 @@ static void layout_symtab(struct module *mod, struct load_info *info)
 	symsect->sh_flags |= SHF_ALLOC;
 	symsect->sh_entsize = get_offset(mod, &mod->init_size, symsect,
 					 info->index.sym) | INIT_OFFSET_MASK;
-	DEBUGP("\t%s\n", info->secstrings + symsect->sh_name);
+	pr_debug("\t%s\n", info->secstrings + symsect->sh_name);
 
 	src = (void *)info->hdr + symsect->sh_offset;
 	nsrc = symsect->sh_size / sizeof(*src);
@@ -2211,7 +2205,7 @@ static void layout_symtab(struct module *mod, struct load_info *info)
 	strsect->sh_flags |= SHF_ALLOC;
 	strsect->sh_entsize = get_offset(mod, &mod->init_size, strsect,
 					 info->index.str) | INIT_OFFSET_MASK;
-	DEBUGP("\t%s\n", info->secstrings + strsect->sh_name);
+	pr_debug("\t%s\n", info->secstrings + strsect->sh_name);
 
 	/* Append room for core symbols' strings at end of core part. */
 	info->stroffs = mod->core_size;
@@ -2621,7 +2615,7 @@ static int move_module(struct module *mod, struct load_info *info)
 	mod->module_init = ptr;
 
 	/* Transfer each section which specifies SHF_ALLOC */
-	DEBUGP("final section addresses:\n");
+	pr_debug("final section addresses:\n");
 	for (i = 0; i < info->hdr->e_shnum; i++) {
 		void *dest;
 		Elf_Shdr *shdr = &info->sechdrs[i];
@@ -2639,7 +2633,7 @@ static int move_module(struct module *mod, struct load_info *info)
 			memcpy(dest, (void *)shdr->sh_addr, shdr->sh_size);
 		/* Update sh_addr to point to copy in image. */
 		shdr->sh_addr = (unsigned long)dest;
-		DEBUGP("\t0x%p %s\n",
+		pr_debug("\t0x%p %s\n",
 		       (void *)shdr->sh_addr, info->secstrings + shdr->sh_name);
 	}
 
@@ -2811,7 +2805,7 @@ static struct module *load_module(void __user *umod,
 	struct module *mod;
 	long err;
 
-	DEBUGP("load_module: umod=%p, len=%lu, uargs=%p\n",
+	pr_debug("load_module: umod=%p, len=%lu, uargs=%p\n",
 	       umod, len, uargs);
 
 	/* Copy in the blobs from userspace, check they are vaguely sane. */
diff --git a/kernel/params.c b/kernel/params.c
index 010f167..b7f5ff6 100644
--- a/kernel/params.c
+++ b/kernel/params.c
@@ -25,12 +25,6 @@
 #include <linux/slab.h>
 #include <linux/ctype.h>
 
-#if 0
-#define DEBUGP printk
-#else
-#define DEBUGP(fmt, a...)
-#endif
-
 /* Protects all parameters, and incidentally kmalloced_param list. */
 static DEFINE_MUTEX(param_lock);
 
@@ -106,7 +100,7 @@ static int parse_one(char *param,
 			/* No one handled NULL, so do it here. */
 			if (!val && params[i].ops->set != param_set_bool)
 				return -EINVAL;
-			DEBUGP("They are equal!  Calling %p\n",
+			pr_debug("They are equal!  Calling %p\n",
 			       params[i].ops->set);
 			mutex_lock(&param_lock);
 			err = params[i].ops->set(val, &params[i]);
@@ -126,11 +120,11 @@ static int parse_one(char *param,
 	}
 
 	if (handle_unknown) {
-		DEBUGP("Unknown argument: calling %p\n", handle_unknown);
+		pr_debug("Unknown argument: calling %p\n", handle_unknown);
 		return handle_unknown(param, val);
 	}
 
-	DEBUGP("Unknown argument `%s'\n", param);
+	pr_debug("Unknown argument `%s'\n", param);
 	return -ENOENT;
 }
 
@@ -195,7 +189,7 @@ int parse_args(const char *name,
 {
 	char *param, *val;
 
-	DEBUGP("Parsing ARGS: %s\n", args);
+	pr_debug("Parsing ARGS: %s\n", args);
 
 	/* Chew leading spaces */
 	args = skip_spaces(args);
-- 
1.7.7.3

[PATCH 25/25] dynamic_debug: protect "dyndbg" fake module param name at compile-time

[jim.cromie]

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

Add BUILD_BUG_DECL(name, cond) to cause compile error if condition is
true.  Unlike other flavors of BUILD_BUG_*, this works at file scope;
it declares an unused 0-sized var: __BUILD_BUG_DECL_##name, whose
declaration is readily found if compile errors happen.

Use this macro in module_param_named() to insure that "dyndbg" is
reserved and not used by modules as a param name, and drop the
run-time check for same.

Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
 include/linux/kernel.h      |   10 ++++++++++
 include/linux/moduleparam.h |    1 +
 lib/dynamic_debug.c         |    8 --------
 3 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index e8b1597..62f210b 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -665,6 +665,7 @@ static inline void ftrace_dump(enum ftrace_dump_mode oops_dump_mode) { }
 #define BUILD_BUG_ON_ZERO(e) (0)
 #define BUILD_BUG_ON_NULL(e) ((void*)0)
 #define BUILD_BUG_ON(condition)
+#define BUILD_BUG_DECL(name, condition)
 #else /* __CHECKER__ */
 
 /* Force a compilation error if a constant expression is not a power of 2 */
@@ -703,6 +704,15 @@ extern int __build_bug_on_failed;
 		if (condition) __build_bug_on_failed = 1;	\
 	} while(0)
 #endif
+/* 
+ * BUILD_BUG_DECL is usable at file scope (by avoiding do {} loop).
+ * If condition is true, causes a compile error, otherwize it declares
+ * a 0-sized array (so it doesn't waste space)
+ */
+#define BUILD_BUG_DECL(name, condition)					\
+	static struct __BUILD_BUG_DECL_ ##name {			\
+		int __BUILD_BUG_DECL_ ##name[1 - 2*!!(condition)];	\
+  } __BUILD_BUG_DECL_ ##name[0] __attribute__((unused))
 #endif	/* __CHECKER__ */
 
 /* Trap pasters of __FUNCTION__ at compile-time */
diff --git a/include/linux/moduleparam.h b/include/linux/moduleparam.h
index e62e5ce..490e557 100644
--- a/include/linux/moduleparam.h
+++ b/include/linux/moduleparam.h
@@ -118,6 +118,7 @@ struct kparam_array
  * structure.  This allows exposure under a different name.
  */
 #define module_param_named(name, value, type, perm)			   \
+	BUILD_BUG_DECL(name, !__builtin_strcmp(#name, "dyndbg"));	   \
 	param_check_##type(name, &(value));				   \
 	module_param_cb(name, &param_ops_##type, &value, perm);		   \
 	__MODULE_PARM_TYPE(name, #type)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index a077cd4..9b07a3b 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -887,7 +887,6 @@ void ddebug_module_parse_args(const char *modname, char* modargs,
 			      struct kernel_param *params, unsigned num)
 {
 	char *param, *val, *args_it, *arg_dup_ptr;
-	int i;
 
 	if (!modargs || !*modargs)
 		return; /* no args, return now, avoid pr_infos */
@@ -901,13 +900,6 @@ void ddebug_module_parse_args(const char *modname, char* modargs,
 	if (verbose)
 		pr_info("%s ARGS: %s\n", modname, args_it);
 
-	/* warn if modules declare ddebug param - todo BUILD_BUG ?? */
-	for (i = 0; i < num; i++) {
-		if (!strcmp("ddebug", params[i].name))
-			pr_warning("Module %s uses reserved keyword "
-				   "*ddebug* as parameter\n", modname);
-	}
-
 	while (*args_it) {
 		args_it = next_arg(args_it, &param, &val);
 		if (strcmp(param, "dyndbg"))
-- 
1.7.7.3

Re: [PATCH 04/25] dynamic_debug: make dynamic-debug supersede DEBUG ccflag

[Jason Baron]

On Wed, Nov 30, 2011 at 12:56:33PM -0700, jim.cromie@gmail.com wrote:
> 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: Joe Perches <joe@perches.com>
> Signed-off-by: Jim Cromie <jim.cromie@gmail.com>

I think it's this patach that is causing a lot of "=p" when I boot up
using a 'make defconfig'. I'm a bit surprised at how many files have
'DEBUG' defined. Are we sure that we are setting the 'p' flag for the
same set of debug statements that would be set if CONFIG_DYNAMIC_DEBUG
were not set?

Also, it seems a bit strange to me, if I now enable all debug statements
and then clear everything (common pattern, I think), that I'm now
disabling stuff that was enabled by default.

Thanks,

-Jason 


> ---
>  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 cbeb586..f8b71a3c 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -2643,14 +2643,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
> 

Re: [PATCH 04/25] dynamic_debug: make dynamic-debug supersede DEBUG ccflag

[Joe Perches]

On Wed, 2011-11-30 at 17:03 -0500, Jason Baron wrote:
> I think it's this patach that is causing a lot of "=p" when I boot up
> using a 'make defconfig'.

=p?

>  I'm a bit surprised at how many files have
> 'DEBUG' defined.

Also look for DDEBUG in Makefiles.

> Are we sure that we are setting the 'p' flag for the
> same set of debug statements that would be set if CONFIG_DYNAMIC_DEBUG
> were not set?

?

> Also, it seems a bit strange to me, if I now enable all debug statements
> and then clear everything (common pattern, I think), that I'm now
> disabling stuff that was enabled by default.

I don't think that's a common pattern and I
also don't think that's bad.

Re: [PATCH 04/25] dynamic_debug: make dynamic-debug supersede DEBUG ccflag

[Jim Cromie]

On Wed, Nov 30, 2011 at 3:16 PM, Joe Perches <joe@perches.com> wrote:
> On Wed, 2011-11-30 at 17:03 -0500, Jason Baron wrote:
>> I think it's this patach that is causing a lot of "=p" when I boot up
>> using a 'make defconfig'.
>
> =p?
>

Thats referring to the new enabled indicator in control,
its more greppable than plain old 'p', and has better memonics
(p-flag is set)

root@voyage:~# grep =p /dbg/dynamic_debug/control
fs/sysfs/mount.c:66 [mount]sysfs_fill_super =p "%s: could not get root
dentry!\012"
fs/sysfs/mount.c:59 [mount]sysfs_fill_super =p "sysfs: could not get
root inode\012"
drivers/rtc/class.c:102 [rtc_core]rtc_resume =p "%s:  time travel!\012"
drivers/rtc/class.c:94 [rtc_core]rtc_resume =p "%s:  bogus resume time\012"

>>  I'm a bit surprised at how many files have
>> 'DEBUG' defined.
>

thats at least partly due to defconfig
I turn on a few things via bootline and /etc/modprobe.d/dyndbg.conf

handy for seeing boot-time dyndbg processing,
you;ll want to turn it off before catting control..
 dynamic_debug.verbose=1

root@voyage:~# more /etc/modprobe.d/dyndbg.conf
options scx200_acb dyndbg=+mfp
options pc87360 dyndbg=+p

root@voyage:~# grep =p /dbg/dynamic_debug/control  |wc
     42     302    3950


dynamic_debug:ddebug_change: changed drivers/hwmon/pc87360.c:1506
[pc87360]pc87360_update_device =p
dynamic_debug:ddebug_exec_query: applied: func="" file=""
module="pc87360" format="" lineno=0-0
dynamic_debug:ddebug_exec_queries: processed 1 queries, with 20 matches, 0 errs
dynamic_debug:ddebug_module_parse_args: Finished pc87360 parsing

root@voyage:~# grep =p /dbg/dynamic_debug/control  |grep pc87 |wc
     20     137    1867

> Also look for DDEBUG in Makefiles.
>
>> Are we sure that we are setting the 'p' flag for the
>> same set of debug statements that would be set if CONFIG_DYNAMIC_DEBUG
>> were not set?
>
> ?

FWIW, I have a few that Im not setting explicitly,
which are probably set by *_DEBUG, as Joe suggested

root@voyage:~# zgrep -i rtc /proc/config.gz | grep DEBUG
CONFIG_RTC_DEBUG=y

root@voyage:~# grep =p /dbg/dynamic_debug/control  |grep rtc |wc
     12      84     989


>
>> Also, it seems a bit strange to me, if I now enable all debug statements
>> and then clear everything (common pattern, I think), that I'm now
>> disabling stuff that was enabled by default.
>
> I don't think that's a common pattern and I
> also don't think that's bad.
>

moreover, we'd need another bit (at least) to distinguish default
flags from current flags.

the query-cmd language lets you constrain, but without any,
all sites *should* be affected

root@voyage:~# echo +p > /dbg/dynamic_debug/control
dynamic_debug:ddebug_exec_queries: processed 1 queries, with 613 matches, 0 errs
uart_wait_until_sent(0), jiffies=703813, expire=703855...
root@voyage:~# grep =p /dbg/dynamic_debug/control |wc
uart_wait_until_sent(0), jiffies=724988, expire=725030...
    613    4683   56064
uart_wait_until_sent(0), jiffies=725263, expire=725305...
root@voyage:~# echo -p > /dbg/dynamic_debug/control
uart_wait_until_sent(0), jiffies=742213, expire=742255...
dynamic_debug:ddebug_exec_queries: processed 1 queries, with 613 matches, 0 errs
root@voyage:~# grep =p /dbg/dynamic_debug/control |wc
      0       0       0

Re: [PATCH 18/25] dynamic_debug: Introduce global fake module param module.ddebug

[Rusty Russell]

On Wed, 30 Nov 2011 12:56:47 -0700, jim.cromie@gmail.com wrote:
> From: Jim Cromie <jim.cromie@gmail.com>
> 
> Rework Thomas Renninger's $module.ddebug boot-time debugging feature,
> from https://lkml.org/lkml/2010/9/15/397
> 
> Dynamic Debug allows enabling of pr_debug and dev_dbg messages at
> runtime.  This is controlled via $DBGFS/dynamic_debug/control.  One
> major drawback is that the whole initialization of a module cannot be
> tracked, because ddebug is only aware of debug strings of loaded
> modules.  But this is the most interesting part...
> 
> This patch introduces a fake module parameter module.ddebug (not shown
> in /sys/module/*/parameters, thus it does not use any resources).  If
> a module passes ddebug as a module parameter (e.g. via module.ddebug
> kernel boot param or via "modprobe module ddebug"), all debug strings
> of this module get activated by issuing "module module_name +p"
> internally (not via sysfs) when the module gets loaded.
> A later patch extends this with an arg: module.ddebug="+mfp"

I think you're missing a neat trick here: we should be able to handle
ddebug in the handle_unknown parameter to parse_args().

Or am I missing something?

Thanks,
Rusty.

Re: [PATCH 18/25] dynamic_debug: Introduce global fake module param module.ddebug

[Jim Cromie]

>
> I think you're missing a neat trick here: we should be able to handle
> ddebug in the handle_unknown parameter to parse_args().
>
> Or am I missing something?
>

Jason suggested that too, offlist.
I pretty much agreed, but wanted to get the 1st 17 sent.

What gave me pause was this:
		pr_debug("Unknown argument: calling %p\n", handle_unknown);

it made me think about a common handler to deal with it 1st,
before the /* Find parameter */ loop, so that modules couldnt hijack
the dyndbg param, defeating the common handler.
OTOH, the BUILD_BUG_DECL prevents that hijack.

Maybe the best answer is silence the print when param is properly handled.

	if (handle_unknown) {
                int rc = handle_unknown(param, val);
		if (rc)
                      pr_debug("Unknown argument: calling %p\n",
handle_unknown);
		return rc;
       }

I'll do this shortly, see how it works out..
thanks

> Thanks,
> Rusty.

Re: [PATCH 19/25] pnp: if CONFIG_DYNAMIC_DEBUG, use pnp.ddebug instead of pnp.debug

[Thomas Renninger]

On Wednesday 30 November 2011 20:56:48 jim.cromie@gmail.com wrote:
> From: Jim Cromie <jim.cromie@gmail.com>
> 
> resubmit of https://lkml.org/lkml/2010/9/15/398
> 
> This allows usage of generic pnp.ddebug debug parameter instead of
> pnp.debug PNP specific parameter.
It depends on what you compile in which pnp debug parameter one has to use
and both are doing more or less the same?

We could add two pnp parameters in !defined(CONFIG_DYNAMIC_DEBUG) case:
  - deprecate pnp.debug by a message:
    "pnp.debug deprecated, use pnp.ddebug" instead
  - pnp.ddebug doing what pnp.debug is doing currently
The only misleading would be that pnp.ddebug has nothing to do with
dynamic debug if not compiled in, but user would have one parameter
to rely on.

In Documentation/kernel-parameters:
        pnp.debug       [PNP]
                        Enable PNP debug messages.  This depends on the
                        CONFIG_PNP_DEBUG_MESSAGES option.

Would be wrong in defined(CONFIG_DYNAMIC_DEBUG) case with your patch,
but would always work with:
        pnp.ddebug
with my above suggestions.

It's not a big deal and not a perfect solution, just looks a bit confusing
to have 2 different parameters for the same thing.

Bjorn should have a look and acknowledge or sign the pnp parts off.

    Thomas

> I wonder whether CONFIG_PNP_DEBUG_MESSAGES can vanish totally with
> this or at some time. Only advantage having it is: If you are
> restricted and your kernel must not exceed X bytes, you cannot compile
> in PNP debug messages only, but you have to compile in all debug
> messages.
> 
> Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
> CC: Thomas Renninger <trenn@suse.de>
> ---
>  drivers/pnp/base.h |    8 ++++++--
>  drivers/pnp/core.c |   13 +++++++++++++
>  2 files changed, 19 insertions(+), 2 deletions(-)
> 

...

> +#if defined(CONFIG_DYNAMIC_DEBUG)
> +static int __init pnp_debug_setup(char *__unused)
> +{
> +	printk(KERN_INFO "DYNAMIC_DEBUG enabled use pnp.ddebug instead of "
> +		"pnp.debug boot param\n");
> +	return 1;
> +}
> +__setup("pnp.debug", pnp_debug_setup);

> +
> +#else
> +
>  #if defined(CONFIG_PNP_DEBUG_MESSAGES)
>  module_param_named(debug, pnp_debug, int, 0644);
>  #endif
> +
> +#endif
> -- 
> 1.7.7.3
> 
> 

Re: [patch 00/25] dynamic-debug during module initialization

[Jason Baron]

On Wed, Nov 30, 2011 at 12:56:29PM -0700, jim.cromie@gmail.com wrote:
> This patchset adds
> - dynamic-debug during module initialization
> - multiple queries
> 
> Unlike previous versions, this drops pending-query approach in
> favor of "fake module parameter" approach proposed by Thomas Renninger.
>     https://lkml.org/lkml/2010/9/15/397
> 
> Its based upon v3.2-rc3, cuz it includes a few adjustments to
> dynamic_debug.h which are not in driver-core-next atm.
> 
> 
> 1	bug-fix for kernel/module.c under DEBUGP
> 2	whitespace cleanup
> 3-12	dynamic-debug cleanups, should be relatively uncontroversial
> 13-17	multiple queries in ddebug_query="..."
> 
> 18-25	fake module parameter
> 20	maybe fold into 18 (kept separate since 18 is Thomas's work)
> 23	*.dyndbg=...
> 25	BUILD_BUG_DECL (likely discussion point ;-)
> 

I like 25, it truly makes 'dyndbg' a reserved module parameter, which is the
intention.

Re: [PATCH 18/25] dynamic_debug: Introduce global fake module param module.ddebug

[Rusty Russell]

On Thu, 1 Dec 2011 01:15:22 -0700, Jim Cromie <jim.cromie@gmail.com> wrote:
> >
> > I think you're missing a neat trick here: we should be able to handle
> > ddebug in the handle_unknown parameter to parse_args().
> >
> > Or am I missing something?
> >
> 
> Jason suggested that too, offlist.
> I pretty much agreed, but wanted to get the 1st 17 sent.
> 
> What gave me pause was this:
> 		pr_debug("Unknown argument: calling %p\n", handle_unknown);

In my code, it's DEBUG(), which is only for people debugging parameter
parsing.  Ignore it, it's normal.  You can delete it if it really
bothers you.

Thanks,
Rusty.

Re: [patch 00/25] dynamic-debug during module initialization

[Jim Cromie]

On Thu, Dec 1, 2011 at 2:20 PM, Jason Baron <jbaron@redhat.com> wrote:
> On Wed, Nov 30, 2011 at 12:56:29PM -0700, jim.cromie@gmail.com wrote:
>> This patchset adds
>> - dynamic-debug during module initialization
>> - multiple queries
>>
>> Unlike previous versions, this drops pending-query approach in
>> favor of "fake module parameter" approach proposed by Thomas Renninger.
>>     https://lkml.org/lkml/2010/9/15/397
>>
>> Its based upon v3.2-rc3, cuz it includes a few adjustments to
>> dynamic_debug.h which are not in driver-core-next atm.
>>
>>
>> 1     bug-fix for kernel/module.c under DEBUGP
>> 2     whitespace cleanup
>> 3-12  dynamic-debug cleanups, should be relatively uncontroversial
>> 13-17 multiple queries in ddebug_query="..."
>>
>> 18-25 fake module parameter
>> 20    maybe fold into 18 (kept separate since 18 is Thomas's work)
>> 23    *.dyndbg=...
>> 25    BUILD_BUG_DECL (likely discussion point ;-)
>>
>
> I like 25, it truly makes 'dyndbg' a reserved module parameter, which is the
> intention.
>

heres a failing usage:

...
#include <linux/device.h>
#include <linux/netdevice.h>

BUILD_BUG_DECL(foo,(1==1));

heres what it looks like expanded:

static struct __BUILD_BUG_DECL_foo { int __BUILD_BUG_DECL_foo[1 -
2*!!(1==1)]; } __BUILD_BUG_DECL_foo[0] __attribute__((unused));

and heres the error:

  CALL    /home/jimc/projects/lx/linux-2.6/scripts/checksyscalls.sh
  CC      lib/dynamic_debug.o
/home/jimc/projects/lx/linux-2.6/lib/dynamic_debug.c:37:42: error:
size of array ‘__BUILD_BUG_DECL_foo’ is negative
make[3]: *** [lib/dynamic_debug.o] Error 1


I havent seen any unused-var warnings, but maybe Ive not stimulated it enough.

Re: [PATCH 19/25] pnp: if CONFIG_DYNAMIC_DEBUG, use pnp.ddebug instead of pnp.debug

[Jim Cromie]

2011/12/1 Thomas Renninger <trenn@suse.de>:
> On Wednesday 30 November 2011 20:56:48 jim.cromie@gmail.com wrote:
>> From: Jim Cromie <jim.cromie@gmail.com>
>>
>> resubmit of https://lkml.org/lkml/2010/9/15/398
>>
>> This allows usage of generic pnp.ddebug debug parameter instead of
>> pnp.debug PNP specific parameter.

> It depends on what you compile in which pnp debug parameter one has to use
> and both are doing more or less the same?
>
> We could add two pnp parameters in !defined(CONFIG_DYNAMIC_DEBUG) case:
>  - deprecate pnp.debug by a message:
>    "pnp.debug deprecated, use pnp.ddebug" instead

Just to be clear, this patch (yours) does this deprecation.

>  - pnp.ddebug doing what pnp.debug is doing currently

FWIW, the patch after this changes the name .ddebug to .dyndbg.

Why is this better than just fixing kernel-parameters to
advise using dyndbg directly, and skipping the indirection ?

With the newer unknown-parameter approach that Jason, Rusty recommended
(now done), it is possible for a module to implement its own .dyndbg option
handler (using __setup only, not with nicer module_param_named() macro,
at least with patch 25 included), but that doesnt seem wise:

modname.dyndbg is a fake option,
it doesnt show up in /sys/module/pnp/parameters/debug.
Adding pnp.dyndbg using __setup would add the sys file, giving an
entirely different interface than the one implemented in
/dbg/dynamic_debug/control.
Explaining this special case sounds difficult to do clearly, a sign of trouble.



> The only misleading would be that pnp.ddebug has nothing to do with
> dynamic debug if not compiled in, but user would have one parameter
> to rely on.
>
> In Documentation/kernel-parameters:
>        pnp.debug       [PNP]
>                        Enable PNP debug messages.  This depends on the
>                        CONFIG_PNP_DEBUG_MESSAGES option.
>
> Would be wrong in defined(CONFIG_DYNAMIC_DEBUG) case with your patch,
> but would always work with:
>        pnp.ddebug
> with my above suggestions.

how about a something like this ?


	pnp.debug=1	[PNP]
			Enable PNP debug messages (depends on the
			CONFIG_PNP_DEBUG_MESSAGES option and
                       !CONFIG_DYNAMIC_DEBUG).   If
CONFIG_DYNAMIC_DEBUG use pnp.dyndbg instead.  ...

This approach doesnt add any new failures;
if ! CONFIG_DYNAMIC_DEBUG,
     pnp.dyndbg will fail/warn just like any.dyndbg would
else
     pnp.debug will warn, yndbg will fail/warn just like any.dyndbg

>
> It's not a big deal and not a perfect solution, just looks a bit confusing
> to have 2 different parameters for the same thing.

I think this is covered adequately by a doc update, and less confusing than
different behavior/usage of 1 parameter under 2 different configs.

>
> Bjorn should have a look and acknowledge or sign the pnp parts off.
>
>    Thomas
>

thanks
Jim

Re: [PATCH 19/25] pnp: if CONFIG_DYNAMIC_DEBUG, use pnp.ddebug instead of pnp.debug

[Thomas Renninger]

On Monday, December 05, 2011 06:42:42 AM Jim Cromie wrote:
> 2011/12/1 Thomas Renninger <trenn@suse.de>:
> > On Wednesday 30 November 2011 20:56:48 jim.cromie@gmail.com wrote:
> >> From: Jim Cromie <jim.cromie@gmail.com>
> >>
> >> resubmit of https://lkml.org/lkml/2010/9/15/398
> >>
> >> This allows usage of generic pnp.ddebug debug parameter instead of
> >> pnp.debug PNP specific parameter.
> 
> > It depends on what you compile in which pnp debug parameter one has to use
> > and both are doing more or less the same?
> >
> > We could add two pnp parameters in !defined(CONFIG_DYNAMIC_DEBUG) case:
> >  - deprecate pnp.debug by a message:
> >    "pnp.debug deprecated, use pnp.ddebug" instead
> 
> Just to be clear, this patch (yours) does this deprecation.
> 
> >  - pnp.ddebug doing what pnp.debug is doing currently
> 
> FWIW, the patch after this changes the name .ddebug to .dyndbg.
> 
> Why is this better than just fixing kernel-parameters to
> advise using dyndbg directly, and skipping the indirection ?
With this patch you'd have pnp.debug and pnp.dyndbg essentially doing
the same (from what I can see),
but you'd either have to use the one or the other, depending
on what is compiled in.

It's not a big deal, but imo it would be nice to have one pnp debug
option which would always work.

This could look like this (not even compile tested):

#if !defined(CONFIG_DYNAMIC_DEBUG)
static int __init pnp_debug_setup(char *__unused)
{
     pnp_debug = 1;
}
__setup("pnp.dyndbg", pnp_debug_setup);

static int __init pnp_old_debug_setup(char *__unused)
{
     printk(KERN_INFO "pnp.debug is deprecated, use pnp.dyndbg instead\n");
}
__setup("pnp.debug", pnp_old_debug_setup);
#endif


As said, not a big deal. Maybe nicer, not sure.

    Thomas

Re: [PATCH 19/25] pnp: if CONFIG_DYNAMIC_DEBUG, use pnp.ddebug instead of pnp.debug

[Jim Cromie]

On Mon, Dec 5, 2011 at 7:47 AM, Thomas Renninger <trenn@suse.de> wrote:
> On Monday, December 05, 2011 06:42:42 AM Jim Cromie wrote:
>> 2011/12/1 Thomas Renninger <trenn@suse.de>:
>> > On Wednesday 30 November 2011 20:56:48 jim.cromie@gmail.com wrote:
>> >> From: Jim Cromie <jim.cromie@gmail.com>
>> >>
>> >> resubmit of https://lkml.org/lkml/2010/9/15/398
>> >>
>> >> This allows usage of generic pnp.ddebug debug parameter instead of
>> >> pnp.debug PNP specific parameter.
>>
>> > It depends on what you compile in which pnp debug parameter one has to use
>> > and both are doing more or less the same?
>> >
>> > We could add two pnp parameters in !defined(CONFIG_DYNAMIC_DEBUG) case:
>> >  - deprecate pnp.debug by a message:
>> >    "pnp.debug deprecated, use pnp.ddebug" instead
>>
>> Just to be clear, this patch (yours) does this deprecation.
>>
>> >  - pnp.ddebug doing what pnp.debug is doing currently
>>
>> FWIW, the patch after this changes the name .ddebug to .dyndbg.
>>
>> Why is this better than just fixing kernel-parameters to
>> advise using dyndbg directly, and skipping the indirection ?
> With this patch you'd have pnp.debug and pnp.dyndbg essentially doing
> the same (from what I can see),
> but you'd either have to use the one or the other, depending
> on what is compiled in.
>
> It's not a big deal, but imo it would be nice to have one pnp debug
> option which would always work.
>
> This could look like this (not even compile tested):
>
> #if !defined(CONFIG_DYNAMIC_DEBUG)
> static int __init pnp_debug_setup(char *__unused)
> {
>     pnp_debug = 1;
> }
> __setup("pnp.dyndbg", pnp_debug_setup);
>
> static int __init pnp_old_debug_setup(char *__unused)
> {
>     printk(KERN_INFO "pnp.debug is deprecated, use pnp.dyndbg instead\n");
> }
> __setup("pnp.debug", pnp_old_debug_setup);
> #endif
>
>
> As said, not a big deal. Maybe nicer, not sure.

I agree with this size assessment :-)

However, you trimmed my argument against your approach;

$modname.dyndbg is generally fake, __setup("pnp.dyndbg"...) is not.
This makes a completely different control interface for 2 kinds of builds.

echo $setting > /sys/modules/pnp/parameters/dyndbg
vs
echo "module pnp +/-p" > /dbg/dynamic/control

its certainly possible to translate $setting into
ddebug_exec_query( (setting ? "+p" : "-p"), "pnp");
but this still leaves the issue of the different interface,
hidden below the skin-deep appearance of a single parameter.
It also doesnt address what this means:
echo "-pmf" > /sys/modules/pnp/parameters/dyndbg
I think its a rabbit-hole of special cases, which we shouldnt go down.

Summarizing, I think the choices are:

1- 2 different module-options for yes/no CONFIG_DYNAMIC_DEBUG builds
2- 1 module-option with 2 different interfaces
3- deeper but still imperfect translations of 1 to other.

Ive coded 1, and will submit it shortly,
but am willing to reconsider, or to just follow marching orders.

>
>    Thomas

thanks
Jim

Re: [PATCH 19/25] pnp: if CONFIG_DYNAMIC_DEBUG, use pnp.ddebug instead of pnp.debug

[Thomas Renninger]

On Monday 05 December 2011 20:15:48 Jim Cromie wrote:
> On Mon, Dec 5, 2011 at 7:47 AM, Thomas Renninger <trenn@suse.de> wrote:
> > On Monday, December 05, 2011 06:42:42 AM Jim Cromie wrote:
> >> 2011/12/1 Thomas Renninger <trenn@suse.de>:
...
> >
> > As said, not a big deal. Maybe nicer, not sure.
> 
> I agree with this size assessment :-)
Please ignore my nit-picking.

This patch, making pnp dynamic debug aware, is a nice feature.

  Thomas

Re: [PATCH 18/25] dynamic_debug: Introduce global fake module param module.ddebug

[Rusty Russell]

On Tue, 6 Mar 2012 09:47:54 -0500, Jason Baron <jbaron@redhat.com> wrote:
> On Tue, Mar 06, 2012 at 10:02:56AM +0100, Thomas Renninger wrote:
> > On Monday, March 05, 2012 06:22:06 PM Jim Cromie wrote:
> > > 2012/3/5 Thomas Renninger <trenn@suse.de>:
> > ...
> > > I have reworked 18-25 on top of linux-next, and have been
> > > casually boot testing it on my i586 based soekris, also for
> > > several weeks.
> > > 
> > > I have some misgivings about trivial things I added to params.c
> > > (primarily pr-debugs in init-level), and about a few other details,
> > > but I guess I should just write up an explanation and send it out.
> > Seeing the stuff in linux-next soon would be great.

Agreed.  OK, here's a partial review.

Patch 1: init: add a pr_info, pr_debug to initcall-levels
> add a pr_info to print current level of initcall,
> add a pr_debug and a counter to initcall_level() to indicate

Meh.  We have an initcall_debug flag.  Let's use it please.

Patch 2: params: add a pr-debug to parse_one's min/max level

No, this is a great deal of spew for little purpose.

Patch 3: params: parse_one's pr_debug("They are equal! ...") isnt informative

Actually, callback address is informative, since it's kernel devs who
are debugging this.  But as they can add it, we should just drop this
patch, or remove the line altogether.

[PATCH 04/15] params: add 3rd arg to option handler callback

This is fine, I'll take this as is.

[PATCH 05/15] dynamic_debug: make dynamic-debug work during module initialization:

> +static inline int ddebug_dyndbg_param_cb(char *param, char *val,
> +					const char *modname)
> +{
> +	if (strstr(param, "dyndbg"))
> +		pr_warn("dyndbg supported only in "
> +			"CONFIG_DYNAMIC_DEBUG builds\n");
> +	return 0; /* allow unknown options */
> +}

No, this breaks unknown module params!  Please split into two callbacks:

        check_for_dyndebug()

        check_for_module_dyndebug()

The latter may be in module.c and call a common helper, depends how neat it
is.

> +	char *param, *modname;
> +
> +	param = strstr(fqparam, "dyndbg");
> +	if (!param) {
> +		pr_debug("%s: skip %s\n", doing, fqparam);
> +		return 0; /* param is unknown, ignore it (for boot) */
> +	}
> +	if (param > fqparam) {
> +		/* fqparam has module prefix, split it in 2 */
> +		*(param-1) = '\0';
> +		modname = fqparam;
> +	}
> +	else
> +		modname = doing;
> +
> +	if (verbose)
> +		pr_info("module: %s %s=\"%s\"\n", modname, param, val);
> +
> +	ddebug_exec_queries(val ? val : "+p");
> +	return 0; /* query failure shouldnt stop module load */

Please don't hack-parse, this accepts all kinds of invalid crap.

Your wildcard patch is even worse.  A sane option is:

        "dyndbg[=...]" on boot line turns dyndbg for everything.
        "<modname>.dyndbg[=...]" on boot line turns on dyndbg for
        that module.
        "dyndbg[=...]" on module line turns dyndbg for that module.

Something like:
        const char *modname = NULL;
        char *sep;
       
        sep = strchr(fqparam, '.');
        if (sep) {
                *sep = '\0';
                modname = fqparam;
                fqparam = sep + 1;
        }

        ...

[PATCH 07/15] dynamic_debug: add modname arg to exec_query callchain

This looks sane.

[PATCH 08/15] dynamic_debug: allow *.dyndbg=+p in boot args

No, just make it "dyndbg=+p", as above.

[PATCH 09/15] dynamic_debug: protect "dyndbg" fake module param name at compile-time

BUILD_BUG_DECL is redundant, you should use BUILD_BUG_ZERO.  But that
insists on a constant expression, so you'd need a new variant.  Probably
easiest to drop this one.  If someone calls their parameter dyndbg they
probably mean exactly what they say.

[PATCH 11/15] dyndbg: init at core level, not arch

Please say why.

[PATCH 14/15] dyndbg: replace if (verbose) pr_info with macro vpr_info

>  #define pr_fmt(fmt) KBUILD_MODNAME ":%s: " fmt, __func__
> +#define vpr_info(fmt, ...) if (verbose) { pr_info(fmt, ##__VA_ARGS__); }

This is a bear trap waiting to happen.  Please do { } while(0) wrap!

Thanks,
Rusty.
-- 
  How could I marry someone with more hair than me?  http://baldalex.org