All of lore.kernel.org
 help / color / mirror / Atom feed
* [patch 00/25] dynamic-debug enhancements
@ 2011-11-30 20:27 Jim Cromie
  2011-12-06 18:59 ` Jim Cromie
  2011-12-12 23:12 ` [patch 00/23] dynamic-debug enhancements Jim Cromie
  0 siblings, 2 replies; 45+ messages in thread
From: Jim Cromie @ 2011-11-30 20:27 UTC (permalink / raw)
  To: LKML

this patchset adds
- dynamic-debug during module initialization
- multiple queries in ddebug_query, module.dyndbg

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

Its based upon 3.2-rc3, since that has several patches to
include/linux/dynamic_debug.h
that are not in driver-core-next, and theyd need to be handled eventually.

1 - trivial bug in kernel/module.c under DEBUGP
2 - whitespace
3-12 dyndbg cleanups
13-17 multiple queries
18-25 dynamic-debug during module initialization




[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(-)

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [patch 00/25] dynamic-debug enhancements
  2011-11-30 20:27 [patch 00/25] dynamic-debug enhancements Jim Cromie
@ 2011-12-06 18:59 ` Jim Cromie
  2011-12-06 19:11   ` [patch 00/24 ] dynamic debug enhancements: multi-queries during mod-init jim.cromie
  2011-12-12 23:12 ` [patch 00/23] dynamic-debug enhancements Jim Cromie
  1 sibling, 1 reply; 45+ messages in thread
From: Jim Cromie @ 2011-12-06 18:59 UTC (permalink / raw)
  To: LKML, Jason Baron; +Cc: Greg KH

On Wed, Nov 30, 2011 at 1:27 PM, Jim Cromie <jim.cromie@gmail.com> wrote:
> this patchset adds
> - dynamic-debug during module initialization
> - multiple queries in ddebug_query, module.dyndbg
>
> Unlike previous versions, it drops the pending-query approach in
> favor of  "fake module parameters"proposed by Thomas Renninger
>    https://lkml.org/lkml/2010/9/15/397
>
> Its based upon 3.2-rc3, since that has several patches to
> include/linux/dynamic_debug.h
> that are not in driver-core-next, and theyd need to be handled eventually.
>
> 1 - trivial bug in kernel/module.c under DEBUGP
> 2 - whitespace
> 3-12 dyndbg cleanups
> 13-17 multiple queries
> 18-25 dynamic-debug during module initialization
>

This revision follows Rusty's, Jason's advice, and drops previously added
ddebug_(boot|module)_parse_args(), instead using a new callback function,
ddebug_dyndbg_param_cb(), invoked by kernel/module.c: parse_one(),
to handle unknown args

Ive also consolidated Thomas's original patch with my adjustments,
mostly to reduce churn around the above, and the ddebug -> dyndbg
param-name change.

WRT pnp.debug, vs pnp.dyndbg parameters, Ive kept the 2 of them,
since latter is fake, former is real, and the usages are quite different.
It seems the least suboptimal of the choices, and easiest to explain.

Im resending entire set, although 1-~18 are identical.
One patch has trivial alteration fixing a const char * mismatch,
not sure which one - sorry about the bookkeeping fail.

You can also get this patchset here:
git://github.com/jimc/linux-2.6.git  dyndbg/modinit-1205


[dyndbg/modinit-x1 c44ca6c] dynamic_debug: remove unneeded includes
 1 files changed, 0 insertions(+), 10 deletions(-)
[jimc@groucho linux-2.6]$    git diff --stat v3.2-rc3..HEAD
 Documentation/dynamic-debug-howto.txt |  110 ++++++++++---
 Documentation/kernel-parameters.txt   |   23 ++-
 drivers/pnp/base.h                    |    8 +-
 drivers/pnp/core.c                    |   12 ++
 include/linux/device.h                |    8 +-
 include/linux/dynamic_debug.h         |   30 +++-
 include/linux/kernel.h                |   10 +
 include/linux/moduleparam.h           |    3 +-
 include/linux/netdevice.h             |    8 +-
 include/linux/printk.h                |    8 +-
 init/main.c                           |    7 +-
 kernel/module.c                       |   49 +++---
 kernel/params.c                       |   24 +--
 lib/Kconfig.debug                     |   15 +-
 lib/dynamic_debug.c                   |  306 ++++++++++++++++++++++----------
 15 files changed, 423 insertions(+), 198 deletions(-)


with a little luck, I'll get the in-reply-to correct on the
git-send-email to follow.

thanks
Jim Cromie

^ permalink raw reply	[flat|nested] 45+ messages in thread

* [patch 00/24 ] dynamic debug enhancements: multi-queries during mod-init
  2011-12-06 18:59 ` Jim Cromie
@ 2011-12-06 19:11   ` jim.cromie
  2011-12-06 19:11     ` [PATCH 01/25] kernel/module.c: fix compile err, warnings under ifdef DEBUGP jim.cromie
                       ` (24 more replies)
  0 siblings, 25 replies; 45+ messages in thread
From: jim.cromie @ 2011-12-06 19:11 UTC (permalink / raw)
  To: jbaron; +Cc: greg, joe, bart.vanassche, linux-kernel

0001-kernel-module.c-fix-compile-err-warnings-under-ifdef.patch
0002-dynamic_debug-fix-whitespace-complaints-from-scripts.patch
0003-dynamic_debug-drop-enabled-field-from-struct-_ddebug.patch
0004-dynamic_debug-make-dynamic-debug-supersede-DEBUG-ccf.patch
0005-dynamic_debug-change-verbosity-at-runtime.patch
0006-dynamic_debug-replace-strcpy-with-strlcpy-in-ddebug_.patch
0007-dynamic_debug-pr_err-call-should-not-depend-upon-ver.patch
0008-dynamic_debug-drop-explicit-NULL-checks.patch
0009-dynamic_debug-describe_flags-with-pmflt_.patch
0010-dynamic_debug-tighten-up-error-checking-on-debug-que.patch
0011-dynamic_debug-early-return-if-_ddebug-table-is-empty.patch
0012-dynamic_debug-reduce-lineno-field-to-a-saner-18-bits.patch
0013-dynamic_debug-chop-off-comments-in-ddebug_tokenize.patch
0014-dynamic_debug-enlarge-command-query-write-buffer.patch
0015-dynamic_debug-add-trim_prefix-to-provide-source-root.patch
0016-dynamic_debug-factor-vpr_info_dq-out-of-ddebug_parse.patch
0017-dynamic_debug-process-multiple-debug-queries-on-a-li.patch
0018-dynamic_debug-Introduce-global-fake-module-param-mod.patch
0019-pnp-if-CONFIG_DYNAMIC_DEBUG-use-pnp.dyndbg-instead-o.patch
0020-dynamic_debug-add-modname-arg-to-exec_query-callchai.patch
0021-kernel-module-replace-DEBUGP-with-pr_debug.patch
0022-dynamic_debug-protect-dyndbg-fake-module-param-name-.patch
0023-dynamic_debug-update-Documentation-kernel-parameters.patch
0024-dynamic_debug-remove-unneeded-includes.patch



^ permalink raw reply	[flat|nested] 45+ messages in thread

* [PATCH 01/25] kernel/module.c: fix compile err, warnings under ifdef DEBUGP
  2011-12-06 19:11   ` [patch 00/24 ] dynamic debug enhancements: multi-queries during mod-init jim.cromie
@ 2011-12-06 19:11     ` jim.cromie
  2011-12-06 19:11     ` [PATCH 02/25] dynamic_debug: fix whitespace complaints from scripts/cleanfile jim.cromie
                       ` (23 subsequent siblings)
  24 siblings, 0 replies; 45+ messages in thread
From: jim.cromie @ 2011-12-06 19:11 UTC (permalink / raw)
  To: jbaron; +Cc: greg, joe, bart.vanassche, linux-kernel, 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


^ permalink raw reply related	[flat|nested] 45+ messages in thread

* [PATCH 02/25] dynamic_debug: fix whitespace complaints from scripts/cleanfile
  2011-12-06 19:11   ` [patch 00/24 ] dynamic debug enhancements: multi-queries during mod-init jim.cromie
  2011-12-06 19:11     ` [PATCH 01/25] kernel/module.c: fix compile err, warnings under ifdef DEBUGP jim.cromie
@ 2011-12-06 19:11     ` jim.cromie
  2011-12-06 19:11     ` [PATCH 03/25] dynamic_debug: drop enabled field from struct _ddebug, use _DPRINTK_FLAGS_PRINT jim.cromie
                       ` (22 subsequent siblings)
  24 siblings, 0 replies; 45+ messages in thread
From: jim.cromie @ 2011-12-06 19:11 UTC (permalink / raw)
  To: jbaron; +Cc: greg, joe, bart.vanassche, linux-kernel, 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


^ permalink raw reply related	[flat|nested] 45+ messages in thread

* [PATCH 03/25] dynamic_debug: drop enabled field from struct _ddebug, use _DPRINTK_FLAGS_PRINT
  2011-12-06 19:11   ` [patch 00/24 ] dynamic debug enhancements: multi-queries during mod-init jim.cromie
  2011-12-06 19:11     ` [PATCH 01/25] kernel/module.c: fix compile err, warnings under ifdef DEBUGP jim.cromie
  2011-12-06 19:11     ` [PATCH 02/25] dynamic_debug: fix whitespace complaints from scripts/cleanfile jim.cromie
@ 2011-12-06 19:11     ` jim.cromie
  2011-12-06 19:11     ` [PATCH 04/25] dynamic_debug: make dynamic-debug supersede DEBUG ccflag jim.cromie
                       ` (21 subsequent siblings)
  24 siblings, 0 replies; 45+ messages in thread
From: jim.cromie @ 2011-12-06 19:11 UTC (permalink / raw)
  To: jbaron; +Cc: greg, joe, bart.vanassche, linux-kernel, 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


^ permalink raw reply related	[flat|nested] 45+ messages in thread

* [PATCH 04/25] dynamic_debug: make dynamic-debug supersede DEBUG ccflag
  2011-12-06 19:11   ` [patch 00/24 ] dynamic debug enhancements: multi-queries during mod-init jim.cromie
                       ` (2 preceding siblings ...)
  2011-12-06 19:11     ` [PATCH 03/25] dynamic_debug: drop enabled field from struct _ddebug, use _DPRINTK_FLAGS_PRINT jim.cromie
@ 2011-12-06 19:11     ` jim.cromie
  2011-12-06 19:11     ` [PATCH 05/25] dynamic_debug: change verbosity at runtime jim.cromie
                       ` (20 subsequent siblings)
  24 siblings, 0 replies; 45+ messages in thread
From: jim.cromie @ 2011-12-06 19:11 UTC (permalink / raw)
  To: jbaron; +Cc: greg, joe, bart.vanassche, linux-kernel, 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


^ permalink raw reply related	[flat|nested] 45+ messages in thread

* [PATCH 05/25] dynamic_debug: change verbosity at runtime
  2011-12-06 19:11   ` [patch 00/24 ] dynamic debug enhancements: multi-queries during mod-init jim.cromie
                       ` (3 preceding siblings ...)
  2011-12-06 19:11     ` [PATCH 04/25] dynamic_debug: make dynamic-debug supersede DEBUG ccflag jim.cromie
@ 2011-12-06 19:11     ` jim.cromie
  2011-12-06 19:11     ` [PATCH 06/25] dynamic_debug: replace strcpy with strlcpy, in ddebug_setup_query() jim.cromie
                       ` (19 subsequent siblings)
  24 siblings, 0 replies; 45+ messages in thread
From: jim.cromie @ 2011-12-06 19:11 UTC (permalink / raw)
  To: jbaron; +Cc: greg, joe, bart.vanassche, linux-kernel, 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


^ permalink raw reply related	[flat|nested] 45+ messages in thread

* [PATCH 06/25] dynamic_debug: replace strcpy with strlcpy, in ddebug_setup_query()
  2011-12-06 19:11   ` [patch 00/24 ] dynamic debug enhancements: multi-queries during mod-init jim.cromie
                       ` (4 preceding siblings ...)
  2011-12-06 19:11     ` [PATCH 05/25] dynamic_debug: change verbosity at runtime jim.cromie
@ 2011-12-06 19:11     ` jim.cromie
  2011-12-06 19:11     ` [PATCH 07/25] dynamic_debug: pr_err() call should not depend upon verbosity jim.cromie
                       ` (18 subsequent siblings)
  24 siblings, 0 replies; 45+ messages in thread
From: jim.cromie @ 2011-12-06 19:11 UTC (permalink / raw)
  To: jbaron; +Cc: greg, joe, bart.vanassche, linux-kernel, 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


^ permalink raw reply related	[flat|nested] 45+ messages in thread

* [PATCH 07/25] dynamic_debug: pr_err() call should not depend upon verbosity
  2011-12-06 19:11   ` [patch 00/24 ] dynamic debug enhancements: multi-queries during mod-init jim.cromie
                       ` (5 preceding siblings ...)
  2011-12-06 19:11     ` [PATCH 06/25] dynamic_debug: replace strcpy with strlcpy, in ddebug_setup_query() jim.cromie
@ 2011-12-06 19:11     ` jim.cromie
  2011-12-06 19:11     ` [PATCH 08/25] dynamic_debug: drop explicit !=NULL checks jim.cromie
                       ` (17 subsequent siblings)
  24 siblings, 0 replies; 45+ messages in thread
From: jim.cromie @ 2011-12-06 19:11 UTC (permalink / raw)
  To: jbaron; +Cc: greg, joe, bart.vanassche, linux-kernel, 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


^ permalink raw reply related	[flat|nested] 45+ messages in thread

* [PATCH 08/25] dynamic_debug: drop explicit !=NULL checks
  2011-12-06 19:11   ` [patch 00/24 ] dynamic debug enhancements: multi-queries during mod-init jim.cromie
                       ` (6 preceding siblings ...)
  2011-12-06 19:11     ` [PATCH 07/25] dynamic_debug: pr_err() call should not depend upon verbosity jim.cromie
@ 2011-12-06 19:11     ` jim.cromie
  2011-12-06 19:11     ` [PATCH 09/25] dynamic_debug: describe_flags with '=[pmflt_]*' jim.cromie
                       ` (16 subsequent siblings)
  24 siblings, 0 replies; 45+ messages in thread
From: jim.cromie @ 2011-12-06 19:11 UTC (permalink / raw)
  To: jbaron; +Cc: greg, joe, bart.vanassche, linux-kernel, 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


^ permalink raw reply related	[flat|nested] 45+ messages in thread

* [PATCH 09/25] dynamic_debug: describe_flags with '=[pmflt_]*'
  2011-12-06 19:11   ` [patch 00/24 ] dynamic debug enhancements: multi-queries during mod-init jim.cromie
                       ` (7 preceding siblings ...)
  2011-12-06 19:11     ` [PATCH 08/25] dynamic_debug: drop explicit !=NULL checks jim.cromie
@ 2011-12-06 19:11     ` jim.cromie
  2011-12-06 19:11     ` [PATCH 10/25] dynamic_debug: tighten up error checking on debug queries jim.cromie
                       ` (15 subsequent siblings)
  24 siblings, 0 replies; 45+ messages in thread
From: jim.cromie @ 2011-12-06 19:11 UTC (permalink / raw)
  To: jbaron; +Cc: greg, joe, bart.vanassche, linux-kernel, 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


^ permalink raw reply related	[flat|nested] 45+ messages in thread

* [PATCH 10/25] dynamic_debug: tighten up error checking on debug queries
  2011-12-06 19:11   ` [patch 00/24 ] dynamic debug enhancements: multi-queries during mod-init jim.cromie
                       ` (8 preceding siblings ...)
  2011-12-06 19:11     ` [PATCH 09/25] dynamic_debug: describe_flags with '=[pmflt_]*' jim.cromie
@ 2011-12-06 19:11     ` jim.cromie
  2011-12-06 19:11     ` [PATCH 11/25] dynamic_debug: early return if _ddebug table is empty jim.cromie
                       ` (14 subsequent siblings)
  24 siblings, 0 replies; 45+ messages in thread
From: jim.cromie @ 2011-12-06 19:11 UTC (permalink / raw)
  To: jbaron; +Cc: greg, joe, bart.vanassche, linux-kernel, 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


^ permalink raw reply related	[flat|nested] 45+ messages in thread

* [PATCH 11/25] dynamic_debug: early return if _ddebug table is empty
  2011-12-06 19:11   ` [patch 00/24 ] dynamic debug enhancements: multi-queries during mod-init jim.cromie
                       ` (9 preceding siblings ...)
  2011-12-06 19:11     ` [PATCH 10/25] dynamic_debug: tighten up error checking on debug queries jim.cromie
@ 2011-12-06 19:11     ` jim.cromie
  2011-12-06 19:11     ` [PATCH 12/25] dynamic_debug: reduce lineno field to a saner 18 bits jim.cromie
                       ` (13 subsequent siblings)
  24 siblings, 0 replies; 45+ messages in thread
From: jim.cromie @ 2011-12-06 19:11 UTC (permalink / raw)
  To: jbaron; +Cc: greg, joe, bart.vanassche, linux-kernel, 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


^ permalink raw reply related	[flat|nested] 45+ messages in thread

* [PATCH 12/25] dynamic_debug: reduce lineno field to a saner 18 bits
  2011-12-06 19:11   ` [patch 00/24 ] dynamic debug enhancements: multi-queries during mod-init jim.cromie
                       ` (10 preceding siblings ...)
  2011-12-06 19:11     ` [PATCH 11/25] dynamic_debug: early return if _ddebug table is empty jim.cromie
@ 2011-12-06 19:11     ` jim.cromie
  2011-12-06 19:11     ` [PATCH 13/25] dynamic_debug: chop off comments in ddebug_tokenize jim.cromie
                       ` (12 subsequent siblings)
  24 siblings, 0 replies; 45+ messages in thread
From: jim.cromie @ 2011-12-06 19:11 UTC (permalink / raw)
  To: jbaron; +Cc: greg, joe, bart.vanassche, linux-kernel, 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


^ permalink raw reply related	[flat|nested] 45+ messages in thread

* [PATCH 13/25] dynamic_debug: chop off comments in ddebug_tokenize
  2011-12-06 19:11   ` [patch 00/24 ] dynamic debug enhancements: multi-queries during mod-init jim.cromie
                       ` (11 preceding siblings ...)
  2011-12-06 19:11     ` [PATCH 12/25] dynamic_debug: reduce lineno field to a saner 18 bits jim.cromie
@ 2011-12-06 19:11     ` jim.cromie
  2011-12-06 19:11     ` [PATCH 14/25] dynamic_debug: enlarge command/query write buffer jim.cromie
                       ` (11 subsequent siblings)
  24 siblings, 0 replies; 45+ messages in thread
From: jim.cromie @ 2011-12-06 19:11 UTC (permalink / raw)
  To: jbaron; +Cc: greg, joe, bart.vanassche, linux-kernel, 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


^ permalink raw reply related	[flat|nested] 45+ messages in thread

* [PATCH 14/25] dynamic_debug: enlarge command/query write buffer
  2011-12-06 19:11   ` [patch 00/24 ] dynamic debug enhancements: multi-queries during mod-init jim.cromie
                       ` (12 preceding siblings ...)
  2011-12-06 19:11     ` [PATCH 13/25] dynamic_debug: chop off comments in ddebug_tokenize jim.cromie
@ 2011-12-06 19:11     ` jim.cromie
  2011-12-06 19:11     ` [PATCH 15/25] dynamic_debug: add trim_prefix() to provide source-root relative paths jim.cromie
                       ` (10 subsequent siblings)
  24 siblings, 0 replies; 45+ messages in thread
From: jim.cromie @ 2011-12-06 19:11 UTC (permalink / raw)
  To: jbaron; +Cc: greg, joe, bart.vanassche, linux-kernel, 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


^ permalink raw reply related	[flat|nested] 45+ messages in thread

* [PATCH 15/25] dynamic_debug: add trim_prefix() to provide source-root relative paths
  2011-12-06 19:11   ` [patch 00/24 ] dynamic debug enhancements: multi-queries during mod-init jim.cromie
                       ` (13 preceding siblings ...)
  2011-12-06 19:11     ` [PATCH 14/25] dynamic_debug: enlarge command/query write buffer jim.cromie
@ 2011-12-06 19:11     ` jim.cromie
  2011-12-06 19:11     ` [PATCH 16/25] dynamic_debug: factor vpr_info_dq out of ddebug_parse_query jim.cromie
                       ` (9 subsequent siblings)
  24 siblings, 0 replies; 45+ messages in thread
From: jim.cromie @ 2011-12-06 19:11 UTC (permalink / raw)
  To: jbaron; +Cc: greg, joe, bart.vanassche, linux-kernel, 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


^ permalink raw reply related	[flat|nested] 45+ messages in thread

* [PATCH 16/25] dynamic_debug: factor vpr_info_dq out of ddebug_parse_query
  2011-12-06 19:11   ` [patch 00/24 ] dynamic debug enhancements: multi-queries during mod-init jim.cromie
                       ` (14 preceding siblings ...)
  2011-12-06 19:11     ` [PATCH 15/25] dynamic_debug: add trim_prefix() to provide source-root relative paths jim.cromie
@ 2011-12-06 19:11     ` jim.cromie
  2011-12-06 19:11     ` [PATCH 17/25] dynamic_debug: process multiple debug-queries on a line jim.cromie
                       ` (8 subsequent siblings)
  24 siblings, 0 replies; 45+ messages in thread
From: jim.cromie @ 2011-12-06 19:11 UTC (permalink / raw)
  To: jbaron; +Cc: greg, joe, bart.vanassche, linux-kernel, 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


^ permalink raw reply related	[flat|nested] 45+ messages in thread

* [PATCH 17/25] dynamic_debug: process multiple debug-queries on a line
  2011-12-06 19:11   ` [patch 00/24 ] dynamic debug enhancements: multi-queries during mod-init jim.cromie
                       ` (15 preceding siblings ...)
  2011-12-06 19:11     ` [PATCH 16/25] dynamic_debug: factor vpr_info_dq out of ddebug_parse_query jim.cromie
@ 2011-12-06 19:11     ` jim.cromie
  2011-12-06 19:11     ` [PATCH 18/25] dynamic_debug: Introduce global fake module param $module.dyndbg jim.cromie
                       ` (7 subsequent siblings)
  24 siblings, 0 replies; 45+ messages in thread
From: jim.cromie @ 2011-12-06 19:11 UTC (permalink / raw)
  To: jbaron; +Cc: greg, joe, bart.vanassche, linux-kernel, 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


^ permalink raw reply related	[flat|nested] 45+ messages in thread

* [PATCH 18/25] dynamic_debug: Introduce global fake module param $module.dyndbg
  2011-12-06 19:11   ` [patch 00/24 ] dynamic debug enhancements: multi-queries during mod-init jim.cromie
                       ` (16 preceding siblings ...)
  2011-12-06 19:11     ` [PATCH 17/25] dynamic_debug: process multiple debug-queries on a line jim.cromie
@ 2011-12-06 19:11     ` jim.cromie
  2011-12-07  1:01       ` Rusty Russell
  2011-12-08 17:10       ` Jason Baron
  2011-12-06 19:11     ` [PATCH 19/25] pnp: if CONFIG_DYNAMIC_DEBUG, use pnp.dyndbg instead of pnp.debug jim.cromie
                       ` (6 subsequent siblings)
  24 siblings, 2 replies; 45+ messages in thread
From: jim.cromie @ 2011-12-06 19:11 UTC (permalink / raw)
  To: jbaron
  Cc: greg, joe, bart.vanassche, linux-kernel, Jim Cromie,
	Thomas Renninger, Rusty Russell

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

Extend dynamic-debug facility to work during module initialization:

  foo.dyndbg bar.dyndbg="$bar-query-string"

This patch introduces a 'fake' module parameter: $module.ddebug for
all modules.  It is not explicitly added to each module, but is
implemented as an unknown-parameter callback invoked by kernel/param's
parse_one(), and it applies each module's query-string as if it were
written to the $DBGFS/dynamic_debug/control file.

The module-name is added to the unknown-parameter callback signature
and to its caller: parse_one().  This lets the common callback know
what module its handling a common parameter for.

If no value is given (as in foo.dyndbg example above), "+p" is
assumed, which enables all pr_debug callsites in the module.

The parameter is not shown in /sys/module/*/parameters, thus it does
not use any resources.  Changes to the parameter are made via the
control file.

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 |   52 ++++++++++++++++++++++++++++++++-
 include/linux/dynamic_debug.h         |   11 +++++++
 include/linux/moduleparam.h           |    2 +-
 init/main.c                           |    7 ++--
 kernel/module.c                       |    3 +-
 kernel/params.c                       |   10 ++++--
 lib/dynamic_debug.c                   |   16 +++++++++-
 7 files changed, 89 insertions(+), 12 deletions(-)

diff --git a/Documentation/dynamic-debug-howto.txt b/Documentation/dynamic-debug-howto.txt
index 989a892..56e496a 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,45 @@ 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 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: (this form works on built-ins too)
+	  module.dyndbg=+mfp
+	  module.dyndbg	# 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
+	  options module dyndbg # defaults to +p
+
+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 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:
+
+      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 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.
+
 Examples
 ========
 
@@ -264,3 +303,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 7e3c53a..b77f43b 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -39,11 +39,16 @@ 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_dyndbg_param_cb(char *param, char *val, const char *modname);
+
 struct device;
 
 extern __printf(3, 4)
@@ -99,6 +104,12 @@ static inline int ddebug_remove_module(const char *mod)
 	return 0;
 }
 
+static inline int ddebug_dyndbg_param_cb(char *param, char *val, const char *modname)
+{
+	pr_warn("dyndbg supported only in CONFIG_DYNAMIC_DEBUG builds\n");
+	return 0; /* dont fail modprobe, warning is enough */
+}
+
 #define dynamic_pr_debug(fmt, ...)					\
 	do { if (0) printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__); } while (0)
 #define dynamic_dev_dbg(dev, fmt, ...)					\
diff --git a/include/linux/moduleparam.h b/include/linux/moduleparam.h
index 7939f63..91bd56a 100644
--- a/include/linux/moduleparam.h
+++ b/include/linux/moduleparam.h
@@ -292,7 +292,7 @@ extern int parse_args(const char *name,
 		      char *args,
 		      const struct kernel_param *params,
 		      unsigned num,
-		      int (*unknown)(char *param, char *val));
+		      int (*unknown)(char *param, char *val, const char *modname));
 
 /* Called by module remove. */
 #ifdef CONFIG_SYSFS
diff --git a/init/main.c b/init/main.c
index 217ed23..4a591ec 100644
--- a/init/main.c
+++ b/init/main.c
@@ -230,7 +230,7 @@ early_param("loglevel", loglevel);
  * Unknown boot options get handed to init, unless they look like
  * unused parameters (modprobe will find them in /proc/cmdline).
  */
-static int __init unknown_bootoption(char *param, char *val)
+static int __init unknown_bootoption(char *param, char *val, const char *modname)
 {
 	/* Change NUL term back to "=", to make "param" the whole string. */
 	if (val) {
@@ -387,7 +387,7 @@ static noinline void __init_refok rest_init(void)
 }
 
 /* Check for early params. */
-static int __init do_early_param(char *param, char *val)
+static int __init do_early_param(char *param, char *val, const char *modname)
 {
 	const struct obs_kernel_param *p;
 
@@ -510,8 +510,7 @@ asmlinkage void __init start_kernel(void)
 	printk(KERN_NOTICE "Kernel command line: %s\n", boot_command_line);
 	parse_early_param();
 	parse_args("Booting kernel", static_command_line, __start___param,
-		   __stop___param - __start___param,
-		   &unknown_bootoption);
+		   __stop___param - __start___param, &unknown_bootoption);
 
 	jump_label_init();
 
diff --git a/kernel/module.c b/kernel/module.c
index 30ffed4..c7b495e 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -2893,7 +2893,8 @@ static struct module *load_module(void __user *umod,
 	mutex_unlock(&module_mutex);
 
 	/* Module is ready to execute: parsing args may do that. */
-	err = parse_args(mod->name, mod->args, mod->kp, mod->num_kp, NULL);
+	err = parse_args(mod->name, mod->args, mod->kp, mod->num_kp,
+			 ddebug_dyndbg_param_cb);
 	if (err < 0)
 		goto unlink;
 
diff --git a/kernel/params.c b/kernel/params.c
index 65aae11..70a9878 100644
--- a/kernel/params.c
+++ b/kernel/params.c
@@ -92,9 +92,11 @@ bool parameq(const char *a, const char *b)
 
 static int parse_one(char *param,
 		     char *val,
+		     const char *modname,
 		     const struct kernel_param *params,
 		     unsigned num_params,
-		     int (*handle_unknown)(char *param, char *val))
+		     int (*handle_unknown)(char *param, char *val,
+					   const char *modname))
 {
 	unsigned int i;
 	int err;
@@ -116,7 +118,7 @@ static int parse_one(char *param,
 
 	if (handle_unknown) {
 		DEBUGP("Unknown argument: calling %p\n", handle_unknown);
-		return handle_unknown(param, val);
+		return handle_unknown(param, val, modname);
 	}
 
 	DEBUGP("Unknown argument `%s'\n", param);
@@ -180,7 +182,7 @@ int parse_args(const char *name,
 	       char *args,
 	       const struct kernel_param *params,
 	       unsigned num,
-	       int (*unknown)(char *param, char *val))
+	       int (*unknown)(char *param, char *val, const char *modname))
 {
 	char *param, *val;
 
@@ -195,7 +197,7 @@ int parse_args(const char *name,
 
 		args = next_arg(args, &param, &val);
 		irq_was_disabled = irqs_disabled();
-		ret = parse_one(param, val, params, num, unknown);
+		ret = parse_one(param, val, name, params, num, unknown);
 		if (irq_was_disabled && !irqs_disabled()) {
 			printk(KERN_WARNING "parse_args(): option '%s' enabled "
 					"irq's!\n", param);
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 516ad4e..b774849 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -618,7 +618,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 +872,20 @@ int ddebug_add_module(struct _ddebug *tab, unsigned int n,
 }
 EXPORT_SYMBOL_GPL(ddebug_add_module);
 
+int ddebug_dyndbg_param_cb(char *param, char* val, const char* modname)
+{
+	if (strcmp(param, "dyndbg")) {
+		pr_warn("bogus param %s=%s received for %s\n",
+			param, val, modname);
+		return -EINVAL;
+	}
+	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 */
+}
+
 static void ddebug_table_free(struct ddebug_table *dt)
 {
 	list_del_init(&dt->link);
-- 
1.7.7.3


^ permalink raw reply related	[flat|nested] 45+ messages in thread

* [PATCH 19/25] pnp: if CONFIG_DYNAMIC_DEBUG, use pnp.dyndbg instead of pnp.debug
  2011-12-06 19:11   ` [patch 00/24 ] dynamic debug enhancements: multi-queries during mod-init jim.cromie
                       ` (17 preceding siblings ...)
  2011-12-06 19:11     ` [PATCH 18/25] dynamic_debug: Introduce global fake module param $module.dyndbg jim.cromie
@ 2011-12-06 19:11     ` jim.cromie
  2011-12-06 19:11     ` [PATCH 20/25] dynamic_debug: add modname arg to exec_query callchain jim.cromie
                       ` (5 subsequent siblings)
  24 siblings, 0 replies; 45+ messages in thread
From: jim.cromie @ 2011-12-06 19:11 UTC (permalink / raw)
  To: jbaron
  Cc: greg, joe, bart.vanassche, linux-kernel, Jim Cromie,
	Thomas Renninger, Bjorn Helgaas

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

based upon https://lkml.org/lkml/2010/9/15/398

This patch splits control of pnp debug messages for 2 configs:

CONFIG_DYNAMIC_DEBUG:
  use pnp.dyndbg, using pnp.debug will warn
!CONFIG_DYNAMIC_DEBUG:
  use pnp.debug, using pnp.dyndbg will warn

2 separate boot options is perhaps suboptimal, but dyndbg is a 'fake'
parameter, and is special enough that adapting one to another is both
more suboptimal and harder to explain succinctly.

Thomas' original comments, still pertinent:

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>
CC: Bjorn Helgaas <bhelgaas@google.com>
---
 Documentation/kernel-parameters.txt |   14 ++++++++------
 drivers/pnp/base.h                  |    8 ++++++--
 drivers/pnp/core.c                  |   12 ++++++++++++
 3 files changed, 26 insertions(+), 8 deletions(-)

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index a0c5c5f..81c27b1 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -2123,12 +2123,14 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
 			Override pmtimer IOPort with a hex value.
 			e.g. pmtmr=0x508
 
-	pnp.debug=1	[PNP]
-			Enable PNP debug messages (depends on the
-			CONFIG_PNP_DEBUG_MESSAGES option).  Change at run-time
-			via /sys/module/pnp/parameters/debug.  We always show
-			current resource usage; turning this on also shows
-			possible settings and some assignment information.
+	pnp.debug=1	[PNP] Enable PNP debug messages
+		        (depends on CONFIG_PNP_DEBUG_MESSAGES and
+			!CONFIG_DYNAMIC_DEBUG options.  If latter, use
+			pnp.dyndbg instead).  Change at run-time via
+			/sys/module/pnp/parameters/debug.  We always
+			show current resource usage; turning this on
+			also shows possible settings and some
+			assignment information.
 
 	pnpacpi=	[ACPI]
 			{ off }
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..838d82c 100644
--- a/drivers/pnp/core.c
+++ b/drivers/pnp/core.c
@@ -219,6 +219,18 @@ subsys_initcall(pnp_init);
 
 int pnp_debug;
 
+#if defined(CONFIG_DYNAMIC_DEBUG)
+static int __init pnp_debug_setup(char *__unused)
+{
+	pr_info("DYNAMIC_DEBUG enabled, use pnp.dyndbg instead\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


^ permalink raw reply related	[flat|nested] 45+ messages in thread

* [PATCH 20/25] dynamic_debug: add modname arg to exec_query callchain
  2011-12-06 19:11   ` [patch 00/24 ] dynamic debug enhancements: multi-queries during mod-init jim.cromie
                       ` (18 preceding siblings ...)
  2011-12-06 19:11     ` [PATCH 19/25] pnp: if CONFIG_DYNAMIC_DEBUG, use pnp.dyndbg instead of pnp.debug jim.cromie
@ 2011-12-06 19:11     ` jim.cromie
  2011-12-06 19:11     ` [PATCH 21/25] kernel/module: replace DEBUGP with pr_debug jim.cromie
                       ` (4 subsequent siblings)
  24 siblings, 0 replies; 45+ messages in thread
From: jim.cromie @ 2011-12-06 19:11 UTC (permalink / raw)
  To: jbaron; +Cc: greg, joe, bart.vanassche, linux-kernel, Jim Cromie

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

Pass module name into ddebug_exec_queries(), ddebug_exec_query(), and
ddebug_parse_query() as separate parameter.  In ddebug_parse_query(),
the module name is added into the query struct before the query-string
is parsed.  This allows the query-string to be shorter:

instead of:
   $modname.dyndbg="module $modname +fp"
do this:
   $modname.dyndbg="+fp"

Omitting "module $modname" from the query string is actually required
for $modname.dyndbg rules; the set-only-once check added in a previous
patch will throw an error if its added again.  ddebug_query="..." has
no $modname associated with it, so the query string may include it.

This also plays well with multiple queries per string:

   $modname.dyndbg="func foo +fp; func bar +fp"

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

diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index b774849..baed922 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -337,7 +337,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;
@@ -347,6 +347,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");
@@ -446,7 +450,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;
@@ -457,7 +461,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;
@@ -473,7 +477,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, const char *modname)
 {
 	char *split;
 	int i, errs = 0, exitcode = 0, rc, nfound = 0;
@@ -490,7 +494,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;
@@ -656,7 +660,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,9 @@ int ddebug_dyndbg_param_cb(char *param, char* val, const char* modname)
 	if (verbose)
 		pr_info("module: %s %s=\"%s\"\n", modname, param, val);
 
-	ddebug_exec_queries((val ? val : "+p"));
+	ddebug_exec_queries((val ? val : "+p"), 
+			    !strcmp(modname, "*") ? NULL : modname);
+
 	return 0; /* query failure shouldnt stop module load */
 }
 
@@ -982,7 +988,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


^ permalink raw reply related	[flat|nested] 45+ messages in thread

* [PATCH 21/25] kernel/module: replace DEBUGP with pr_debug
  2011-12-06 19:11   ` [patch 00/24 ] dynamic debug enhancements: multi-queries during mod-init jim.cromie
                       ` (19 preceding siblings ...)
  2011-12-06 19:11     ` [PATCH 20/25] dynamic_debug: add modname arg to exec_query callchain jim.cromie
@ 2011-12-06 19:11     ` jim.cromie
  2011-12-07  1:07       ` Rusty Russell
  2011-12-06 19:11     ` [PATCH 22/25] dynamic_debug: protect "dyndbg" fake module param name at compile-time jim.cromie
                       ` (3 subsequent siblings)
  24 siblings, 1 reply; 45+ messages in thread
From: jim.cromie @ 2011-12-06 19:11 UTC (permalink / raw)
  To: jbaron; +Cc: greg, joe, bart.vanassche, linux-kernel, Jim Cromie

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

Use more flexible pr_debug.  This allows:

  echo "module module +p" > /dbg/dynamic_debug/control

to turn on debug messages when needed.

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 c7b495e..78b229e 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 70a9878..c649593 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);
 
@@ -107,7 +101,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]);
@@ -117,11 +111,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, modname);
 	}
 
-	DEBUGP("Unknown argument `%s'\n", param);
+	pr_debug("Unknown argument `%s'\n", param);
 	return -ENOENT;
 }
 
@@ -186,7 +180,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


^ permalink raw reply related	[flat|nested] 45+ messages in thread

* [PATCH 22/25] dynamic_debug: protect "dyndbg" fake module param name at compile-time
  2011-12-06 19:11   ` [patch 00/24 ] dynamic debug enhancements: multi-queries during mod-init jim.cromie
                       ` (20 preceding siblings ...)
  2011-12-06 19:11     ` [PATCH 21/25] kernel/module: replace DEBUGP with pr_debug jim.cromie
@ 2011-12-06 19:11     ` jim.cromie
  2011-12-06 19:11     ` [PATCH 23/25] dynamic_debug: update Documentation/kernel-parameters.txt, Kconfig.debug jim.cromie
                       ` (2 subsequent siblings)
  24 siblings, 0 replies; 45+ messages in thread
From: jim.cromie @ 2011-12-06 19:11 UTC (permalink / raw)
  To: jbaron; +Cc: greg, joe, bart.vanassche, linux-kernel, 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 +
 2 files changed, 11 insertions(+), 0 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 91bd56a..da24b78 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)
-- 
1.7.7.3


^ permalink raw reply related	[flat|nested] 45+ messages in thread

* [PATCH 23/25] dynamic_debug: update Documentation/kernel-parameters.txt, Kconfig.debug
  2011-12-06 19:11   ` [patch 00/24 ] dynamic debug enhancements: multi-queries during mod-init jim.cromie
                       ` (21 preceding siblings ...)
  2011-12-06 19:11     ` [PATCH 22/25] dynamic_debug: protect "dyndbg" fake module param name at compile-time jim.cromie
@ 2011-12-06 19:11     ` jim.cromie
  2011-12-06 19:11     ` [PATCH 24/25] dynamic_debug: remove unneeded includes jim.cromie
  2011-12-12 22:05     ` [patch 00/24 ] dynamic debug enhancements: multi-queries during mod-init Greg KH
  24 siblings, 0 replies; 45+ messages in thread
From: jim.cromie @ 2011-12-06 19:11 UTC (permalink / raw)
  To: jbaron; +Cc: greg, joe, bart.vanassche, linux-kernel, Jim Cromie

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

In kernel-parameters.txt:

- update flags indicators in example output, from '-' to '=_'.
- add sentence on _ flag-char
- more info in Module Initialization section
  $module.dyndbg, *.dyndbg

In Kconfig.debug, note enabled-by-default effect of -DDEBUG
compilation.

Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
 Documentation/dynamic-debug-howto.txt |   52 ++++++++++++++++++++++-----------
 Documentation/kernel-parameters.txt   |    9 ++++++
 lib/Kconfig.debug                     |   15 ++++++----
 3 files changed, 53 insertions(+), 23 deletions(-)

diff --git a/Documentation/dynamic-debug-howto.txt b/Documentation/dynamic-debug-howto.txt
index 56e496a..2a9da50 100644
--- a/Documentation/dynamic-debug-howto.txt
+++ b/Documentation/dynamic-debug-howto.txt
@@ -49,10 +49,10 @@ via:
 
 nullarbor:~ # cat <debugfs>/dynamic_debug/control
 # filename:lineno [module]function flags format
-/usr/src/packages/BUILD/sgi-enhancednfs-1.4/default/net/sunrpc/svc_rdma.c:323 [svcxprt_rdma]svc_rdma_cleanup - "SVCRDMA Module Removed, deregister RPC RDMA transport\012"
-/usr/src/packages/BUILD/sgi-enhancednfs-1.4/default/net/sunrpc/svc_rdma.c:341 [svcxprt_rdma]svc_rdma_init - "\011max_inline       : %d\012"
-/usr/src/packages/BUILD/sgi-enhancednfs-1.4/default/net/sunrpc/svc_rdma.c:340 [svcxprt_rdma]svc_rdma_init - "\011sq_depth         : %d\012"
-/usr/src/packages/BUILD/sgi-enhancednfs-1.4/default/net/sunrpc/svc_rdma.c:338 [svcxprt_rdma]svc_rdma_init - "\011max_requests     : %d\012"
+/usr/src/packages/BUILD/sgi-enhancednfs-1.4/default/net/sunrpc/svc_rdma.c:323 [svcxprt_rdma]svc_rdma_cleanup =_ "SVCRDMA Module Removed, deregister RPC RDMA transport\012"
+/usr/src/packages/BUILD/sgi-enhancednfs-1.4/default/net/sunrpc/svc_rdma.c:341 [svcxprt_rdma]svc_rdma_init =_ "\011max_inline       : %d\012"
+/usr/src/packages/BUILD/sgi-enhancednfs-1.4/default/net/sunrpc/svc_rdma.c:340 [svcxprt_rdma]svc_rdma_init =_ "\011sq_depth         : %d\012"
+/usr/src/packages/BUILD/sgi-enhancednfs-1.4/default/net/sunrpc/svc_rdma.c:338 [svcxprt_rdma]svc_rdma_init =_ "\011max_requests     : %d\012"
 ...
 
 
@@ -70,7 +70,7 @@ flags for each debug statement callsite (see below for definitions of the
 flags).  The default value, no extra behaviour enabled, is "-".  So
 you can view all the debug statement callsites with any non-default flags:
 
-nullarbor:~ # awk '$3 != "-"' <debugfs>/dynamic_debug/control
+nullarbor:~ # awk '$3 != "=_"' <debugfs>/dynamic_debug/control
 # filename:lineno [module]function flags format
 /usr/src/packages/BUILD/sgi-enhancednfs-1.4/default/net/sunrpc/svcsock.c:1603 [sunrpc]svc_send p "svc_process: st_sendto returned %d\012"
 
@@ -213,10 +213,11 @@ p
     Causes a printk() message to be emitted to dmesg
 t
     Include thread ID in messages not generated from interrupt context
+_
+    No flags are set
 
-Note the regexp ^[-+=][flmpt]+$ matches a flags specification.
-Note also that there is no convenient syntax to remove all
-the flags at once, you need to use "-flmpt".
+Note the regexp ^[-+=][flmpt_]+$ matches a flags specification.
+To clear all flags at once, use "=_" or "-flmpt".
 
 
 Debug messages during Boot Process
@@ -237,6 +238,8 @@ your machine (typically a laptop) has an Embedded Controller.
 PCI (or other devices) initialization also is a hot candidate for using
 this boot parameter for debugging purposes.
 
+Note: ddebug_query= is supplanted by the feature in the next section,
+and will be deprecated later.
 
 Debug Messages at Module Initialization Time
 ============================================
@@ -258,10 +261,16 @@ lost.  Instead, a "dyndbg" module parameter can be passed:
 	  options module dyndbg=+pmflt
 	  options module dyndbg # defaults to +p
 
-The $modname.dyndbg="value" should exclude "module $modname", as the
+The $modname.dyndbg="value" must exclude "module $modname", as the
 $modname is taken from the param-name, and only 1 spec of each type is
 allowed.
 
+*.dyndbg="+p" can be used to enable all builtin (only) debug
+callsites, or to set print-format defaults (="+mf") for those enabled
+separately.  Loadable modules are not affected by wildcard rules, the
+module loader selects only boot options whose module name matches that
+being loaded.
+
 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
@@ -273,9 +282,12 @@ $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 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.
+When a module is loaded, dyndbg flags are applied in following order,
+with last having final say:
+
+  - as given in /etc/modprobe.d/*	system-wide settings
+  - as given on boot-line		kernel, boot-time specific
+  - as given in modprobe command	explicit at load
 
 Examples
 ========
@@ -304,13 +316,19 @@ nullarbor:~ # echo -n 'func svc_process -p' >
 nullarbor:~ # echo -n 'format "nfsd: READ" +p' >
 				<debugfs>/dynamic_debug/control
 
+// enable all messages
+nullarbor:~ # echo -n '+p' > <debugfs>/dynamic_debug/control
+
+// add module, function to all enabled messages
+nullarbor:~ # echo -n '+mf' > <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
+  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="+mf"		// print module, function for enabled sites
   k10temp.dyndbg="+p # comment in query is stripped "
   pnp.dyndbg="func pnpacpi_get_resources +p; func pnp_assign_mem +p" # multi
diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index 81c27b1..387443d 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -691,6 +691,15 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
 
 	dscc4.setup=	[NET]
 
+	$modulename.dyndbg[=string]
+			[KNL,DYNAMIC_DEBUG] Enable debug messages in
+		        $modulename for builtin or loadable modules.  Debug
+		        during module initialization is supported.
+		        These parameters are not exposed in
+		        /sys/module/${modnm}/parameters/dyndbg.
+			See Documentation/dynamic-debug-howto.txt for
+			details of allowed strings, etc.
+
 	earlycon=	[KNL] Output early console device and options.
 		uart[8250],io,<addr>[,options]
 		uart[8250],mmio,<addr>[,options]
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 82928f5..f6fbd92 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -1184,8 +1184,11 @@ config DYNAMIC_DEBUG
 	  otherwise be available at runtime. These messages can then be
 	  enabled/disabled based on various levels of scope - per source file,
 	  function, module, format string, and line number. This mechanism
-	  implicitly enables all pr_debug() and dev_dbg() calls. The impact of
-	  this compile option is a larger kernel text size of about 2%.
+	  implicitly compiles in all pr_debug() and dev_dbg() calls, which
+	  enlarges the kernel text size by about 2%.
+
+	  If a source file is compiled with DEBUG flag set, any pr_debug()
+	  calls in it are enabled by default, but can be disabled as below.
 
 	  Usage:
 
@@ -1202,16 +1205,16 @@ config DYNAMIC_DEBUG
 	  lineno : line number of the debug statement
 	  module : module that contains the debug statement
 	  function : function that contains the debug statement
-          flags : 'p' means the line is turned 'on' for printing
+          flags : '=p' means the line is turned 'on' for printing
           format : the format used for the debug statement
 
 	  From a live system:
 
 		nullarbor:~ # cat <debugfs>/dynamic_debug/control
 		# filename:lineno [module]function flags format
-		fs/aio.c:222 [aio]__put_ioctx - "__put_ioctx:\040freeing\040%p\012"
-		fs/aio.c:248 [aio]ioctx_alloc - "ENOMEM:\040nr_events\040too\040high\012"
-		fs/aio.c:1770 [aio]sys_io_cancel - "calling\040cancel\012"
+		fs/aio.c:222 [aio]__put_ioctx =_ "__put_ioctx:\040freeing\040%p\012"
+		fs/aio.c:248 [aio]ioctx_alloc =_ "ENOMEM:\040nr_events\040too\040high\012"
+		fs/aio.c:1770 [aio]sys_io_cancel =_ "calling\040cancel\012"
 
 	  Example usage:
 
-- 
1.7.7.3


^ permalink raw reply related	[flat|nested] 45+ messages in thread

* [PATCH 24/25] dynamic_debug: remove unneeded includes
  2011-12-06 19:11   ` [patch 00/24 ] dynamic debug enhancements: multi-queries during mod-init jim.cromie
                       ` (22 preceding siblings ...)
  2011-12-06 19:11     ` [PATCH 23/25] dynamic_debug: update Documentation/kernel-parameters.txt, Kconfig.debug jim.cromie
@ 2011-12-06 19:11     ` jim.cromie
  2011-12-12 22:05     ` [patch 00/24 ] dynamic debug enhancements: multi-queries during mod-init Greg KH
  24 siblings, 0 replies; 45+ messages in thread
From: jim.cromie @ 2011-12-06 19:11 UTC (permalink / raw)
  To: jbaron; +Cc: greg, joe, bart.vanassche, linux-kernel, Jim Cromie

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

These arent currently needed, so drop them.  Some will get re-added
when jump-label is re-introduced.

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

diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index baed922..25023f1 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -14,24 +14,14 @@
 
 #include <linux/kernel.h>
 #include <linux/module.h>
-#include <linux/moduleparam.h>
-#include <linux/kallsyms.h>
-#include <linux/types.h>
 #include <linux/mutex.h>
-#include <linux/proc_fs.h>
 #include <linux/seq_file.h>
-#include <linux/list.h>
-#include <linux/sysctl.h>
 #include <linux/ctype.h>
-#include <linux/string.h>
-#include <linux/uaccess.h>
 #include <linux/dynamic_debug.h>
 #include <linux/debugfs.h>
 #include <linux/slab.h>
-#include <linux/jump_label.h>
 #include <linux/hardirq.h>
 #include <linux/sched.h>
-#include <linux/device.h>
 #include <linux/netdevice.h>
 
 extern struct _ddebug __start___verbose[];
-- 
1.7.7.3


^ permalink raw reply related	[flat|nested] 45+ messages in thread

* Re: [PATCH 18/25] dynamic_debug: Introduce global fake module param $module.dyndbg
  2011-12-06 19:11     ` [PATCH 18/25] dynamic_debug: Introduce global fake module param $module.dyndbg jim.cromie
@ 2011-12-07  1:01       ` Rusty Russell
  2011-12-07  8:33         ` Jim Cromie
  2011-12-08 17:10       ` Jason Baron
  1 sibling, 1 reply; 45+ messages in thread
From: Rusty Russell @ 2011-12-07  1:01 UTC (permalink / raw)
  To: jim.cromie, jbaron
  Cc: greg, joe, bart.vanassche, linux-kernel, Jim Cromie, Thomas Renninger

On Tue,  6 Dec 2011 12:11:28 -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

This worked out pretty neatly.  Thanks!

A few questions from your patches:

> diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
> index 7e3c53a..b77f43b 100644
> --- a/include/linux/dynamic_debug.h
> +++ b/include/linux/dynamic_debug.h
> @@ -39,11 +39,16 @@ 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, ...);

struct kernel_param def belongs in future patch?  And whitespace change
is kind of annoying.

> diff --git a/include/linux/moduleparam.h b/include/linux/moduleparam.h
> index 7939f63..91bd56a 100644
> --- a/include/linux/moduleparam.h
> +++ b/include/linux/moduleparam.h
> @@ -292,7 +292,7 @@ extern int parse_args(const char *name,
>  		      char *args,
>  		      const struct kernel_param *params,
>  		      unsigned num,
> -		      int (*unknown)(char *param, char *val));
> +		      int (*unknown)(char *param, char *val, const char *modname));

Do you really want the modname here, or a struct module?  I wonder if we
should make this a standard opaque void *.  Of course, we'd have to
modify parse_args callers, but you're abusing "name" arg here a bit
anyway.

> @@ -2893,7 +2893,8 @@ static struct module *load_module(void __user *umod,
> 	  mutex_unlock(&module_mutex);
> 
> 	  /* Module is ready to execute: parsing args may do that. */
> -	err = parse_args(mod->name, mod->args, mod->kp, mod->num_kp, NULL);
> +	err = parse_args(mod->name, mod->args, mod->kp, mod->num_kp,
> +			 ddebug_dyndbg_param_cb);
> 	  if (err < 0)
> 		  goto unlink;

You have to 



> @@ -510,8 +510,7 @@ asmlinkage void __init start_kernel(void)
>  	printk(KERN_NOTICE "Kernel command line: %s\n", boot_command_line);
>  	parse_early_param();
>  	parse_args("Booting kernel", static_command_line, __start___param,
> -		   __stop___param - __start___param,
> -		   &unknown_bootoption);
> +		   __stop___param - __start___param, &unknown_bootoption);
>  
>  	jump_label_init();

Gratuitous whitespace change.

> +int ddebug_dyndbg_param_cb(char *param, char* val, const char* modname)
> +{
> +	if (strcmp(param, "dyndbg")) {
> +		pr_warn("bogus param %s=%s received for %s\n",
> +			param, val, modname);
> +		return -EINVAL;
> +	}

This is already done in parse_args(); don't add a pr_warn here.

Thanks,
Rusty.

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH 21/25] kernel/module: replace DEBUGP with pr_debug
  2011-12-06 19:11     ` [PATCH 21/25] kernel/module: replace DEBUGP with pr_debug jim.cromie
@ 2011-12-07  1:07       ` Rusty Russell
  2011-12-08 15:41         ` Jason Baron
  0 siblings, 1 reply; 45+ messages in thread
From: Rusty Russell @ 2011-12-07  1:07 UTC (permalink / raw)
  To: jim.cromie, jbaron; +Cc: greg, joe, bart.vanassche, linux-kernel, Jim Cromie

On Tue,  6 Dec 2011 12:11:31 -0700, jim.cromie@gmail.com wrote:
> From: Jim Cromie <jim.cromie@gmail.com>
> 
> Use more flexible pr_debug.  This allows:
> 
>   echo "module module +p" > /dbg/dynamic_debug/control
> 
> to turn on debug messages when needed.
> 
> Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
> ---
>  kernel/module.c |   44 +++++++++++++++++++-------------------------
>  kernel/params.c |   14 ++++----------
>  2 files changed, 23 insertions(+), 35 deletions(-)

Split into 2 and applied.  This is a win, independent of the rest.

I reused the same commit message for the second patch,
with minor mods:

From: Jim Cromie <jim.cromie@gmail.com>
Subject: kernel/params: replace DEBUGP with pr_debug
Date: Tue, 6 Dec 2011 12:11:31 -0700

Use more flexible pr_debug.  This allows:

  echo "module params +p" > /dbg/dynamic_debug/control

to turn on debug messages when needed.

Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
---
 kernel/params.c |   14 ++++----------
 1 file changed, 4 insertions(+), 10 deletions(-)

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH 18/25] dynamic_debug: Introduce global fake module param $module.dyndbg
  2011-12-07  1:01       ` Rusty Russell
@ 2011-12-07  8:33         ` Jim Cromie
  2011-12-07 10:59           ` Rusty Russell
  0 siblings, 1 reply; 45+ messages in thread
From: Jim Cromie @ 2011-12-07  8:33 UTC (permalink / raw)
  To: Rusty Russell
  Cc: jbaron, greg, joe, bart.vanassche, linux-kernel, Thomas Renninger

On Tue, Dec 6, 2011 at 6:01 PM, Rusty Russell <rusty@rustcorp.com.au> wrote:
> On Tue,  6 Dec 2011 12:11:28 -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
>
> This worked out pretty neatly.  Thanks!
>

It did.  Almost like you planned it.

> A few questions from your patches:
>
>> diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
>> index 7e3c53a..b77f43b 100644
>> --- a/include/linux/dynamic_debug.h
>> +++ b/include/linux/dynamic_debug.h
>> @@ -39,11 +39,16 @@ 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, ...);
>
> struct kernel_param def belongs in future patch?  And whitespace change
> is kind of annoying.

kernel-param not needed - was part of old way.
whitespace cleaned out too.


>
>> diff --git a/include/linux/moduleparam.h b/include/linux/moduleparam.h
>> index 7939f63..91bd56a 100644
>> --- a/include/linux/moduleparam.h
>> +++ b/include/linux/moduleparam.h
>> @@ -292,7 +292,7 @@ extern int parse_args(const char *name,
>>                     char *args,
>>                     const struct kernel_param *params,
>>                     unsigned num,
>> -                   int (*unknown)(char *param, char *val));
>> +                   int (*unknown)(char *param, char *val, const char *modname));
>
> Do you really want the modname here, or a struct module?  I wonder if we
> should make this a standard opaque void *.  Of course, we'd have to
> modify parse_args callers, but you're abusing "name" arg here a bit
> anyway.
>

I thought about the possible abuse, but then saw existing uses:

linux-2.6]$ grep parse_args init/main.c
	parse_args("early options", cmdline, NULL, 0, do_early_param);
	parse_args("Booting kernel", static_command_line, __start___param,

"Booting kernel" isnt quite a name either.

wrt using a struct module arg, Id prefer not.
what I need is the name, so that the ddebug-queries being passed
can leave out the module name and thus be shorter and less redundant

forex:
foo.dyndbg=" func bar +p ; func buz +p ; func bang +p "

would otherwize need "module foo" in each of the 4 rules, which would
be tedious.
Since its been given already by user, and already available in load_module()'s
existing call to parse_args(), it seems straightforward to alter the
cb sig, and pass it down.



>> @@ -2893,7 +2893,8 @@ static struct module *load_module(void __user *umod,
>>         mutex_unlock(&module_mutex);
>>
>>         /* Module is ready to execute: parsing args may do that. */
>> -     err = parse_args(mod->name, mod->args, mod->kp, mod->num_kp, NULL);
>> +     err = parse_args(mod->name, mod->args, mod->kp, mod->num_kp,
>> +                      ddebug_dyndbg_param_cb);
>>         if (err < 0)
>>                 goto unlink;
>
> You have to
>

?? You left this thought unfinished.


>
>
>> @@ -510,8 +510,7 @@ asmlinkage void __init start_kernel(void)
>>       printk(KERN_NOTICE "Kernel command line: %s\n", boot_command_line);
>>       parse_early_param();
>>       parse_args("Booting kernel", static_command_line, __start___param,
>> -                __stop___param - __start___param,
>> -                &unknown_bootoption);
>> +                __stop___param - __start___param, &unknown_bootoption);
>>
>>       jump_label_init();
>
> Gratuitous whitespace change.
>

undone.

>> +int ddebug_dyndbg_param_cb(char *param, char* val, const char* modname)
>> +{
>> +     if (strcmp(param, "dyndbg")) {
>> +             pr_warn("bogus param %s=%s received for %s\n",
>> +                     param, val, modname);
>> +             return -EINVAL;
>> +     }
>
> This is already done in parse_args(); don't add a pr_warn here.
>

OK yes.  I'll change rc to -ENOENT.


> Thanks,
> Rusty.


thanks.

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH 18/25] dynamic_debug: Introduce global fake module param $module.dyndbg
  2011-12-07  8:33         ` Jim Cromie
@ 2011-12-07 10:59           ` Rusty Russell
  0 siblings, 0 replies; 45+ messages in thread
From: Rusty Russell @ 2011-12-07 10:59 UTC (permalink / raw)
  To: Jim Cromie
  Cc: jbaron, greg, joe, bart.vanassche, linux-kernel, Thomas Renninger

On Wed, 7 Dec 2011 01:33:45 -0700, Jim Cromie <jim.cromie@gmail.com> wrote:
> On Tue, Dec 6, 2011 at 6:01 PM, Rusty Russell <rusty@rustcorp.com.au> wrote:
> > This worked out pretty neatly.  Thanks!
> 
> It did.  Almost like you planned it.

I do get lucky sometimes...

> >> -                   int (*unknown)(char *param, char *val));
> >> +                   int (*unknown)(char *param, char *val, const char *modname));
> >
> > Do you really want the modname here, or a struct module?  I wonder if we
> > should make this a standard opaque void *.  Of course, we'd have to
> > modify parse_args callers, but you're abusing "name" arg here a bit
> > anyway.
> >
> 
> I thought about the possible abuse, but then saw existing uses:
> 
> linux-2.6]$ grep parse_args init/main.c
> 	parse_args("early options", cmdline, NULL, 0, do_early_param);
> 	parse_args("Booting kernel", static_command_line, __start___param,
> 
> "Booting kernel" isnt quite a name either.
> 
> wrt using a struct module arg, Id prefer not.
> what I need is the name, so that the ddebug-queries being passed
> can leave out the module name and thus be shorter and less redundant

But from the module you can get the name.  It's just that having a
cb/void* pair is pretty standard practice.  Still, it's a line ball, and
I'm not unhappy enough to change it until we need it.

Thanks,
Rusty.


^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH 21/25] kernel/module: replace DEBUGP with pr_debug
  2011-12-07  1:07       ` Rusty Russell
@ 2011-12-08 15:41         ` Jason Baron
  2011-12-08 18:17           ` Jim Cromie
  0 siblings, 1 reply; 45+ messages in thread
From: Jason Baron @ 2011-12-08 15:41 UTC (permalink / raw)
  To: Rusty Russell; +Cc: jim.cromie, greg, joe, bart.vanassche, linux-kernel

On Wed, Dec 07, 2011 at 11:37:52AM +1030, Rusty Russell wrote:
> On Tue,  6 Dec 2011 12:11:31 -0700, jim.cromie@gmail.com wrote:
> > From: Jim Cromie <jim.cromie@gmail.com>
> > 
> > Use more flexible pr_debug.  This allows:
> > 
> >   echo "module module +p" > /dbg/dynamic_debug/control
> > 
> > to turn on debug messages when needed.
> > 
> > Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
> > ---
> >  kernel/module.c |   44 +++++++++++++++++++-------------------------
> >  kernel/params.c |   14 ++++----------
> >  2 files changed, 23 insertions(+), 35 deletions(-)
> 
> Split into 2 and applied.  This is a win, independent of the rest.
> 

Probably also want to pick up patch 1 in the series too, which actually allows
this one to build ;)

Thanks,

-Jason

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH 18/25] dynamic_debug: Introduce global fake module param $module.dyndbg
  2011-12-06 19:11     ` [PATCH 18/25] dynamic_debug: Introduce global fake module param $module.dyndbg jim.cromie
  2011-12-07  1:01       ` Rusty Russell
@ 2011-12-08 17:10       ` Jason Baron
  2011-12-08 18:12         ` Jim Cromie
  1 sibling, 1 reply; 45+ messages in thread
From: Jason Baron @ 2011-12-08 17:10 UTC (permalink / raw)
  To: jim.cromie
  Cc: greg, joe, bart.vanassche, linux-kernel, Thomas Renninger, Rusty Russell

On Tue, Dec 06, 2011 at 12:11:28PM -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
> 
> Extend dynamic-debug facility to work during module initialization:
> 
>   foo.dyndbg bar.dyndbg="$bar-query-string"
> 
> This patch introduces a 'fake' module parameter: $module.ddebug for
> all modules.  It is not explicitly added to each module, but is
> implemented as an unknown-parameter callback invoked by kernel/param's
> parse_one(), and it applies each module's query-string as if it were
> written to the $DBGFS/dynamic_debug/control file.
> 
> The module-name is added to the unknown-parameter callback signature
> and to its caller: parse_one().  This lets the common callback know
> what module its handling a common parameter for.
> 
> If no value is given (as in foo.dyndbg example above), "+p" is
> assumed, which enables all pr_debug callsites in the module.
> 
> The parameter is not shown in /sys/module/*/parameters, thus it does
> not use any resources.  Changes to the parameter are made via the
> control file.
> 
> 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 |   52 ++++++++++++++++++++++++++++++++-
>  include/linux/dynamic_debug.h         |   11 +++++++
>  include/linux/moduleparam.h           |    2 +-
>  init/main.c                           |    7 ++--
>  kernel/module.c                       |    3 +-
>  kernel/params.c                       |   10 ++++--
>  lib/dynamic_debug.c                   |   16 +++++++++-
>  7 files changed, 89 insertions(+), 12 deletions(-)
> 
> diff --git a/Documentation/dynamic-debug-howto.txt b/Documentation/dynamic-debug-howto.txt
> index 989a892..56e496a 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,45 @@ 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 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: (this form works on built-ins too)
> +	  module.dyndbg=+mfp
> +	  module.dyndbg	# 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
> +	  options module dyndbg # defaults to +p
> +
> +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 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:
> +
> +      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 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.
> +
>  Examples
>  ========
>  
> @@ -264,3 +303,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

I see you've added docs for the 'dyndbg' kernel command-line options,
but then I don't see any changes in the code to 'unkown_bootoption'.
What am I missing?

Thanks,

-Jason


^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH 18/25] dynamic_debug: Introduce global fake module param $module.dyndbg
  2011-12-08 17:10       ` Jason Baron
@ 2011-12-08 18:12         ` Jim Cromie
  0 siblings, 0 replies; 45+ messages in thread
From: Jim Cromie @ 2011-12-08 18:12 UTC (permalink / raw)
  To: Jason Baron
  Cc: greg, joe, bart.vanassche, linux-kernel, Thomas Renninger, Rusty Russell

On Thu, Dec 8, 2011 at 10:10 AM, Jason Baron <jbaron@redhat.com> wrote:
> On Tue, Dec 06, 2011 at 12:11:28PM -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
>>
>> Extend dynamic-debug facility to work during module initialization:
>>
>>   foo.dyndbg bar.dyndbg="$bar-query-string"
>>
>> This patch introduces a 'fake' module parameter: $module.ddebug for
>> all modules.  It is not explicitly added to each module, but is
>> implemented as an unknown-parameter callback invoked by kernel/param's
>> parse_one(), and it applies each module's query-string as if it were
>> written to the $DBGFS/dynamic_debug/control file.
>>
>> The module-name is added to the unknown-parameter callback signature
>> and to its caller: parse_one().  This lets the common callback know
>> what module its handling a common parameter for.
>>
>> If no value is given (as in foo.dyndbg example above), "+p" is
>> assumed, which enables all pr_debug callsites in the module.
>>
>> The parameter is not shown in /sys/module/*/parameters, thus it does
>> not use any resources.  Changes to the parameter are made via the
>> control file.
>>
>> 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 |   52 ++++++++++++++++++++++++++++++++-
>>  include/linux/dynamic_debug.h         |   11 +++++++
>>  include/linux/moduleparam.h           |    2 +-
>>  init/main.c                           |    7 ++--
>>  kernel/module.c                       |    3 +-
>>  kernel/params.c                       |   10 ++++--
>>  lib/dynamic_debug.c                   |   16 +++++++++-
>>  7 files changed, 89 insertions(+), 12 deletions(-)
>>
>> diff --git a/Documentation/dynamic-debug-howto.txt b/Documentation/dynamic-debug-howto.txt
>> index 989a892..56e496a 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,45 @@ 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 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: (this form works on built-ins too)
>> +       module.dyndbg=+mfp
>> +       module.dyndbg # 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
>> +       options module dyndbg # defaults to +p
>> +
>> +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 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:
>> +
>> +      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 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.
>> +
>>  Examples
>>  ========
>>
>> @@ -264,3 +303,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
>
> I see you've added docs for the 'dyndbg' kernel command-line options,
> but then I don't see any changes in the code to 'unkown_bootoption'.
> What am I missing?
>
> Thanks,
>
> -Jason
>

Hmm.

I didnt make changes there cuz I didnt see any test case that
tripped up there.  All the $mod.dyndbg=... boot-args are picked
up by the load-module path.  IE I didnt see any that were missed.

Ive tested both built-in and loadable modules,
but may have missed the $mod.dyndbg (with default +p)
in the most recent revisions.
I'll retest everything, and make sure Ive covered all the corners.


/*
 * Unknown boot options get handed to init, unless they look like
 * unused parameters (modprobe will find them in /proc/cmdline).
 */

...

	/* Unused module parameter. */
	if (strchr(param, '.') && (!val || strchr(param, '.') < val))
		return 0;



BTW, I fixed a few checkpatch probs that crept in during revisions.
The only complaints that now remain are regarding
absolute paths in the howto, which are justified.
I dont want to spam the list, so held them back
pending more semantic issues (like this)

I'll report back on any missed use-cases.
Of course, if you see something specific missing, please say so.

thanks
Jim

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH 21/25] kernel/module: replace DEBUGP with pr_debug
  2011-12-08 15:41         ` Jason Baron
@ 2011-12-08 18:17           ` Jim Cromie
  0 siblings, 0 replies; 45+ messages in thread
From: Jim Cromie @ 2011-12-08 18:17 UTC (permalink / raw)
  To: Jason Baron; +Cc: Rusty Russell, greg, joe, bart.vanassche, linux-kernel

On Thu, Dec 8, 2011 at 8:41 AM, Jason Baron <jbaron@redhat.com> wrote:
> On Wed, Dec 07, 2011 at 11:37:52AM +1030, Rusty Russell wrote:
>> On Tue,  6 Dec 2011 12:11:31 -0700, jim.cromie@gmail.com wrote:
>> > From: Jim Cromie <jim.cromie@gmail.com>
>> >
>> > Use more flexible pr_debug.  This allows:
>> >
>> >   echo "module module +p" > /dbg/dynamic_debug/control
>> >
>> > to turn on debug messages when needed.
>> >
>> > Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
>> > ---
>> >  kernel/module.c |   44 +++++++++++++++++++-------------------------
>> >  kernel/params.c |   14 ++++----------
>> >  2 files changed, 23 insertions(+), 35 deletions(-)
>>
>> Split into 2 and applied.  This is a win, independent of the rest.
>>
>
> Probably also want to pick up patch 1 in the series too, which actually allows
> this one to build ;)
>
> Thanks,
>
> -Jason

He did, in effect.

sorry for not ccing Rusty on 01/
I was thinking that it would go to trivial, and be done by then...

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [patch 00/24 ] dynamic debug enhancements: multi-queries during mod-init
  2011-12-06 19:11   ` [patch 00/24 ] dynamic debug enhancements: multi-queries during mod-init jim.cromie
                       ` (23 preceding siblings ...)
  2011-12-06 19:11     ` [PATCH 24/25] dynamic_debug: remove unneeded includes jim.cromie
@ 2011-12-12 22:05     ` Greg KH
       [not found]       ` <CAJfuBxzeKzmOCo00Ew8xAp9EmU0QZ3zjOkRcdEMzYrtx_b9+kA@mail.gmail.com>
  2011-12-13 14:48       ` Jason Baron
  24 siblings, 2 replies; 45+ messages in thread
From: Greg KH @ 2011-12-12 22:05 UTC (permalink / raw)
  To: jim.cromie; +Cc: jbaron, joe, bart.vanassche, linux-kernel

On Tue, Dec 06, 2011 at 12:11:10PM -0700, jim.cromie@gmail.com wrote:
> 0001-kernel-module.c-fix-compile-err-warnings-under-ifdef.patch
> 0002-dynamic_debug-fix-whitespace-complaints-from-scripts.patch
> 0003-dynamic_debug-drop-enabled-field-from-struct-_ddebug.patch
> 0004-dynamic_debug-make-dynamic-debug-supersede-DEBUG-ccf.patch
> 0005-dynamic_debug-change-verbosity-at-runtime.patch
> 0006-dynamic_debug-replace-strcpy-with-strlcpy-in-ddebug_.patch
> 0007-dynamic_debug-pr_err-call-should-not-depend-upon-ver.patch
> 0008-dynamic_debug-drop-explicit-NULL-checks.patch
> 0009-dynamic_debug-describe_flags-with-pmflt_.patch
> 0010-dynamic_debug-tighten-up-error-checking-on-debug-que.patch
> 0011-dynamic_debug-early-return-if-_ddebug-table-is-empty.patch
> 0012-dynamic_debug-reduce-lineno-field-to-a-saner-18-bits.patch
> 0013-dynamic_debug-chop-off-comments-in-ddebug_tokenize.patch
> 0014-dynamic_debug-enlarge-command-query-write-buffer.patch
> 0015-dynamic_debug-add-trim_prefix-to-provide-source-root.patch
> 0016-dynamic_debug-factor-vpr_info_dq-out-of-ddebug_parse.patch
> 0017-dynamic_debug-process-multiple-debug-queries-on-a-li.patch
> 0018-dynamic_debug-Introduce-global-fake-module-param-mod.patch
> 0019-pnp-if-CONFIG_DYNAMIC_DEBUG-use-pnp.dyndbg-instead-o.patch
> 0020-dynamic_debug-add-modname-arg-to-exec_query-callchai.patch
> 0021-kernel-module-replace-DEBUGP-with-pr_debug.patch
> 0022-dynamic_debug-protect-dyndbg-fake-module-param-name-.patch
> 0023-dynamic_debug-update-Documentation-kernel-parameters.patch
> 0024-dynamic_debug-remove-unneeded-includes.patch
> 

Now that is just about the most unhelpful 00/XX email I've seen in a
long time.

Jason, I'm going to wait for you to test these and forward them on to me
if you really want me to apply any of them.

greg k-h

^ permalink raw reply	[flat|nested] 45+ messages in thread

* [patch 00/23] dynamic-debug enhancements
  2011-11-30 20:27 [patch 00/25] dynamic-debug enhancements Jim Cromie
  2011-12-06 18:59 ` Jim Cromie
@ 2011-12-12 23:12 ` Jim Cromie
  1 sibling, 0 replies; 45+ messages in thread
From: Jim Cromie @ 2011-12-12 23:12 UTC (permalink / raw)
  To: LKML, Jason Baron; +Cc: Greg KH

On Wed, Nov 30, 2011 at 1:27 PM, Jim Cromie <jim.cromie@gmail.com> wrote:
> this patchset adds
> - dynamic-debug during module initialization
via $module.dyndbg
> - multiple queries in ddebug_query, module.dyndbg
>
> Unlike previous versions, it drops the pending-query approach in
> favor of  "fake module parameters"proposed by Thomas Renninger
>    https://lkml.org/lkml/2010/9/15/397
>

This revision fixes an overzealous rework in the previous;
the unknown-parameter callback from parse-args/parse-one
is not invoked in the boot-process, so boot-args were not picked up
for builtin modules, only for loadable modules.
So, this restores ddebug_boot_parse_args() to do that.

The char *modname added previously to the callback signature remains
(added in patch 18), this is needed by the callback (in patch 20), and is
unavailable in the param-name, which has already been stripped of its
module. prefix.
Im hoping this minor feature-before-use issue is forgettable; I could have
put patch 20 1st, but its useless unless modname is available,
which seemed to be a bigger violation.

I briefly looked at using a struct module* as pondered by Rusty,
and module:load_module() already uses it to fill args of params:parse_args(),
which would reduce sig to just (struct module*, callback*)
but other callers of parse_args, "booting kernel", "early options",
dont have a struct mod, and it seemed like massive overkill to dummy one up.
So I stuck with char *modname.



> Its based upon 3.2-rc3, since that has several patches to
> include/linux/dynamic_debug.h
> that are not in driver-core-next, and theyd need to be handled eventually.
>
> 1 - trivial bug in kernel/module.c under DEBUGP
> 2 - whitespace
> 3-12 dyndbg cleanups
> 13-17 multiple queries
> 18-23 dynamic-debug during module initialization
>
[jimc@groucho linux-2.6]$ git diff --stat v3.2-rc3..HEAD
 Documentation/dynamic-debug-howto.txt |  156 +++++++++++-----
 Documentation/kernel-parameters.txt   |   23 ++-
 drivers/pnp/base.h                    |    8 +-
 drivers/pnp/core.c                    |   12 ++
 include/linux/device.h                |    8 +-
 include/linux/dynamic_debug.h         |   30 +++-
 include/linux/kernel.h                |   10 +
 include/linux/moduleparam.h           |    7 +-
 include/linux/netdevice.h             |    8 +-
 include/linux/printk.h                |    8 +-
 init/main.c                           |    5 +-
 kernel/module.c                       |   49 +++---
 kernel/params.c                       |   36 ++--
 lib/Kconfig.debug                     |   17 +-
 lib/dynamic_debug.c                   |  335 +++++++++++++++++++++++----------
 15 files changed, 483 insertions(+), 229 deletions(-)

^ permalink raw reply	[flat|nested] 45+ messages in thread

* RE: [patch 00/24 ] dynamic debug enhancements: multi-queries during mod-init
       [not found]       ` <CAJfuBxzeKzmOCo00Ew8xAp9EmU0QZ3zjOkRcdEMzYrtx_b9+kA@mail.gmail.com>
@ 2011-12-13  4:17         ` Jim Cromie
  2011-12-13  4:25           ` Greg KH
  0 siblings, 1 reply; 45+ messages in thread
From: Jim Cromie @ 2011-12-13  4:17 UTC (permalink / raw)
  To: LKML, Greg KH; +Cc: Jason Baron

On Mon, Dec 12, 2011 at 3:05 PM, Greg KH <greg@kroah.com> wrote:
> On Tue, Dec 06, 2011 at 12:11:10PM -0700, jim.cromie@gmail.com wrote:
>> 0001-kernel-module.c-fix-compile-err-warnings-under-ifdef.patch
>> 0002-dynamic_debug-fix-whitespace-complaints-from-scripts.patch
>> 0024-dynamic_debug-remove-unneeded-includes.patch
>>
>
> Now that is just about the most unhelpful 00/XX email I've seen in a
> long time.
>

Sorry about that.
That was an attempt to thread onto a previous message.

In-Reply-To: <CAJfuBxyyriN6vYgwzEft9PKFjWsa2E4FZmbFMmtO6G_DP=nosg@mail.gmail.com>,
<CAJfuBxzM6rg21Rpq+ww+=2qK_am44V-Btvhp3Z3hbP2TKP8JTw@mail.gmail.com>
References: <CAJfuBxyyriN6vYgwzEft9PKFjWsa2E4FZmbFMmtO6G_DP=nosg@mail.gmail.com>,
<CAJfuBxzM6rg21Rpq+ww+=2qK_am44V-Btvhp3Z3hbP2TKP8JTw@mail.gmail.com>

IE this:

http://article.gmane.org/gmane.linux.kernel/1171277/match=dynamic_debug

I'll use URLs in the future.

> Jason, I'm going to wait for you to test these and forward them on to me
> if you really want me to apply any of them.
>
> greg k-h

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [patch 00/24 ] dynamic debug enhancements: multi-queries during mod-init
  2011-12-13  4:17         ` Jim Cromie
@ 2011-12-13  4:25           ` Greg KH
  2011-12-13  7:00             ` Jim Cromie
  0 siblings, 1 reply; 45+ messages in thread
From: Greg KH @ 2011-12-13  4:25 UTC (permalink / raw)
  To: Jim Cromie; +Cc: LKML, Jason Baron

On Mon, Dec 12, 2011 at 09:17:43PM -0700, Jim Cromie wrote:
> On Mon, Dec 12, 2011 at 3:05 PM, Greg KH <greg@kroah.com> wrote:
> > On Tue, Dec 06, 2011 at 12:11:10PM -0700, jim.cromie@gmail.com wrote:
> >> 0001-kernel-module.c-fix-compile-err-warnings-under-ifdef.patch
> >> 0002-dynamic_debug-fix-whitespace-complaints-from-scripts.patch
> >> 0024-dynamic_debug-remove-unneeded-includes.patch
> >>
> >
> > Now that is just about the most unhelpful 00/XX email I've seen in a
> > long time.
> >
> 
> Sorry about that.
> That was an attempt to thread onto a previous message.
> 
> In-Reply-To: <CAJfuBxyyriN6vYgwzEft9PKFjWsa2E4FZmbFMmtO6G_DP=nosg@mail.gmail.com>,
> <CAJfuBxzM6rg21Rpq+ww+=2qK_am44V-Btvhp3Z3hbP2TKP8JTw@mail.gmail.com>
> References: <CAJfuBxyyriN6vYgwzEft9PKFjWsa2E4FZmbFMmtO6G_DP=nosg@mail.gmail.com>,
> <CAJfuBxzM6rg21Rpq+ww+=2qK_am44V-Btvhp3Z3hbP2TKP8JTw@mail.gmail.com>
> 
> IE this:
> 
> http://article.gmane.org/gmane.linux.kernel/1171277/match=dynamic_debug
> 
> I'll use URLs in the future.

Use git-send-email and you will not have to do any of this by hand, it
handles it all for you automatically.

greg k-h

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [patch 00/24 ] dynamic debug enhancements: multi-queries during mod-init
  2011-12-13  4:25           ` Greg KH
@ 2011-12-13  7:00             ` Jim Cromie
  2011-12-13  7:44               ` Greg KH
  0 siblings, 1 reply; 45+ messages in thread
From: Jim Cromie @ 2011-12-13  7:00 UTC (permalink / raw)
  To: Greg KH; +Cc: LKML, Jason Baron

On Mon, Dec 12, 2011 at 9:25 PM, Greg KH <greg@kroah.com> wrote:
> On Mon, Dec 12, 2011 at 09:17:43PM -0700, Jim Cromie wrote:
>> On Mon, Dec 12, 2011 at 3:05 PM, Greg KH <greg@kroah.com> wrote:
>> > On Tue, Dec 06, 2011 at 12:11:10PM -0700, jim.cromie@gmail.com wrote:
>> >> 0001-kernel-module.c-fix-compile-err-warnings-under-ifdef.patch
>> >> 0002-dynamic_debug-fix-whitespace-complaints-from-scripts.patch
>> >> 0024-dynamic_debug-remove-unneeded-includes.patch
>> >>
>> >
>> > Now that is just about the most unhelpful 00/XX email I've seen in a
>> > long time.
>> >
>>
>> Sorry about that.
>> That was an attempt to thread onto a previous message.
>>
>> In-Reply-To: <CAJfuBxyyriN6vYgwzEft9PKFjWsa2E4FZmbFMmtO6G_DP=nosg@mail.gmail.com>,
>> <CAJfuBxzM6rg21Rpq+ww+=2qK_am44V-Btvhp3Z3hbP2TKP8JTw@mail.gmail.com>
>> References: <CAJfuBxyyriN6vYgwzEft9PKFjWsa2E4FZmbFMmtO6G_DP=nosg@mail.gmail.com>,
>> <CAJfuBxzM6rg21Rpq+ww+=2qK_am44V-Btvhp3Z3hbP2TKP8JTw@mail.gmail.com>
>>
>> IE this:
>>
>> http://article.gmane.org/gmane.linux.kernel/1171277/match=dynamic_debug
>>
>> I'll use URLs in the future.
>
> Use git-send-email and you will not have to do any of this by hand, it
> handles it all for you automatically.
>
> greg k-h


I did use git-send-email.
What I seem to miss on every send is which reference to use in the
reply-to part,
at least when trying to tie it to an email sent from the web-interface.
It doesnt help that gmail isnt displaying them in a thread for me
using the headers that git-send-email is adding (it threads by subject)

(for most recent batch..)
Message-Id: <1323731569-20644-22-git-send-email-jim.cromie@gmail.com>
X-Mailer: git-send-email 1.7.7.3
In-Reply-To: <1323731569-20644-1-git-send-email-jim.cromie@gmail.com>
References: <1323731569-20644-1-git-send-email-jim.cromie@gmail.com>

On previous batch (the one you commented on), IIRC, I deleted all
--compose content
after realizing the patch-count was wrong, thinking it would behave like
a rebase -i with an empty to-do file.  Or, it was some other operator error.

Looking now at the headers, I'll trust that --compose will work the same,
and use that only (should the latest set need another resend)

thanks
Jim

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [patch 00/24 ] dynamic debug enhancements: multi-queries during mod-init
  2011-12-13  7:00             ` Jim Cromie
@ 2011-12-13  7:44               ` Greg KH
  0 siblings, 0 replies; 45+ messages in thread
From: Greg KH @ 2011-12-13  7:44 UTC (permalink / raw)
  To: Jim Cromie; +Cc: LKML, Jason Baron

On Tue, Dec 13, 2011 at 12:00:19AM -0700, Jim Cromie wrote:
> I did use git-send-email.
> What I seem to miss on every send is which reference to use in the
> reply-to part,
> at least when trying to tie it to an email sent from the web-interface.

Don't use the web-interface, it only confuses this type of thing.

good luck,

greg k-h

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [patch 00/24 ] dynamic debug enhancements: multi-queries during mod-init
  2011-12-12 22:05     ` [patch 00/24 ] dynamic debug enhancements: multi-queries during mod-init Greg KH
       [not found]       ` <CAJfuBxzeKzmOCo00Ew8xAp9EmU0QZ3zjOkRcdEMzYrtx_b9+kA@mail.gmail.com>
@ 2011-12-13 14:48       ` Jason Baron
  1 sibling, 0 replies; 45+ messages in thread
From: Jason Baron @ 2011-12-13 14:48 UTC (permalink / raw)
  To: Greg KH; +Cc: jim.cromie, joe, bart.vanassche, linux-kernel

On Mon, Dec 12, 2011 at 02:05:37PM -0800, Greg KH wrote:
> Jason, I'm going to wait for you to test these and forward them on to me
> if you really want me to apply any of them.
> 
> greg k-h

Hi Greg,

Ok, I'm going to test the latest series today. The ability to pass a
module param to turn on dynamic debug is a feature I've been asked about
a lot - so I think its an important ehancement.

Thanks,

-Jason

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH 18/25] dynamic_debug: Introduce global fake module param $module.dyndbg
  2011-12-16  0:00   ` Rusty Russell
@ 2011-12-17  5:10     ` Jim Cromie
  0 siblings, 0 replies; 45+ messages in thread
From: Jim Cromie @ 2011-12-17  5:10 UTC (permalink / raw)
  To: Rusty Russell; +Cc: jbaron, greg, linux-kernel, Thomas Renninger

On Thu, Dec 15, 2011 at 5:00 PM, Rusty Russell <rusty@rustcorp.com.au> wrote:
> On Mon, 12 Dec 2011 16:12:44 -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
>
> OK, this looks cleaner!
>
> s/modname/doing/ here, since it's a generic callback.
>
> s/modname/doing/ (or /s/modname/unused/).
>
> Same as above.
>
> s/modname/doing/ here too, makes it clear.
>
> ... again....

All done, to be sent shortly.
Thanks


>
>> +/* find & process .dyndbg params (prefixed by modname) in cmdline */
>> +static __init void ddebug_boot_parse_args(void)
>> +{
>
> OK, I don't get why you are doing your own parsing for builtins?
>

After some poking around I didnt want to thrash too much,
and went for Thomas' original approach for builtins.


> Can't you call something from init/main.c:unknown_bootoption()?
>
> If that's too early, I have a patch we're working on which adds
> parameter parsing to each initlevel.
>

It looks too early (unless Ive misunderstood)
dynamic_debug is enabled via arch_initcall

    977 static int __init dynamic_debug_init(void)
    978 {
...
   1018         }
   1019         ddebug_boot_parse_args();
...
   1027 }

   1028 /* Allow early initialization for boot messages via boot param */
   1029 arch_initcall(dynamic_debug_init);
   1030 /* Debugfs setup must be done later */
   1031 module_init(dynamic_debug_init_debugfs);


Do you have a pointer to that patch ?
Is it targetted for v3.3 ?
If so, it sounds like I should aim to use it.

BTW, is this/your desc of arg-processing still accurate/current/complete ?
http://thread.gmane.org/gmane.linux.kernel/767172/focus=767361



> Thanks,
> Rusty.

thank you
Jim Cromie

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH 18/25] dynamic_debug: Introduce global fake module param $module.dyndbg
  2011-12-12 23:12 ` [PATCH 18/25] dynamic_debug: Introduce global fake module param $module.dyndbg jim.cromie
@ 2011-12-16  0:00   ` Rusty Russell
  2011-12-17  5:10     ` Jim Cromie
  0 siblings, 1 reply; 45+ messages in thread
From: Rusty Russell @ 2011-12-16  0:00 UTC (permalink / raw)
  To: jim.cromie, jbaron; +Cc: greg, linux-kernel, Jim Cromie, Thomas Renninger

On Mon, 12 Dec 2011 16:12:44 -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

OK, this looks cleaner!

> @@ -292,7 +292,8 @@ extern int parse_args(const char *name,
>  		      char *args,
>  		      const struct kernel_param *params,
>  		      unsigned num,
> -		      int (*unknown)(char *param, char *val));
> +		      int (*unknown)(char *param, char *val,
> +			      const char *modname));

s/modname/doing/ here, since it's a generic callback.

> -static int __init unknown_bootoption(char *param, char *val)
> +static int __init unknown_bootoption(char *param, char *val,
> +				const char *modname)
>  {

s/modname/doing/ (or /s/modname/unused/).

>  	/* Change NUL term back to "=", to make "param" the whole string. */
>  	if (val) {
> @@ -387,7 +388,7 @@ static noinline void __init_refok rest_init(void)
>  }
>  
>  /* Check for early params. */
> -static int __init do_early_param(char *param, char *val)
> +static int __init do_early_param(char *param, char *val, const char *modname)
>  {

Same as above.

> diff --git a/kernel/params.c b/kernel/params.c
> index 9240664..4c39c0e 100644
> --- a/kernel/params.c
> +++ b/kernel/params.c
> @@ -86,9 +86,11 @@ bool parameq(const char *a, const char *b)
>  
>  static int parse_one(char *param,
>  		     char *val,
> +		     const char *doing,
>  		     const struct kernel_param *params,
>  		     unsigned num_params,
> -		     int (*handle_unknown)(char *param, char *val))
> +		     int (*handle_unknown)(char *param, char *val,
> +					   const char *modname))
>  {

s/modname/doing/ here too, makes it clear.

>  /* Args looks like "foo=bar,bar2 baz=fuz wiz". */
> -int parse_args(const char *name,
> +int parse_args(const char *doing, /* modname, Booting kernel, early options */
>  	       char *args,
>  	       const struct kernel_param *params,
>  	       unsigned num,
> -	       int (*unknown)(char *param, char *val))
> +	       int (*unknown)(char *param, char *val, const char *modname))
>  {

... again....

> +/* find & process .dyndbg params (prefixed by modname) in cmdline */
> +static __init void ddebug_boot_parse_args(void)
> +{

OK, I don't get why you are doing your own parsing for builtins?

Can't you call something from init/main.c:unknown_bootoption()?

If that's too early, I have a patch we're working on which adds
parameter parsing to each initlevel.

Thanks,
Rusty.

^ permalink raw reply	[flat|nested] 45+ messages in thread

* [PATCH 18/25] dynamic_debug: Introduce global fake module param $module.dyndbg
  2011-12-12 23:12 [PATCH 01/25] kernel/module.c: fix compile err, warnings under ifdef DEBUGP, switch to pr_debug jim.cromie
@ 2011-12-12 23:12 ` jim.cromie
  2011-12-16  0:00   ` Rusty Russell
  0 siblings, 1 reply; 45+ messages in thread
From: jim.cromie @ 2011-12-12 23:12 UTC (permalink / raw)
  To: jbaron; +Cc: greg, linux-kernel, Jim Cromie, Thomas Renninger, Rusty Russell

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

Extend dynamic-debug facility to work during module initialization:

   foo.dyndbg bar.dyndbg="$bar_query_string"

This patch introduces a 'fake' module parameter: $module.dyndbg for
all modules, whether or not they need it.  It is not explicitly added
to each module, but is implemented in common code, in 2 ways:

For builtin modules, dynamic_debug.c:ddebug_boot_parse_args() copies
and parses the command-line for each $module.dyndbg, and calls
ddebug_exec_queries to apply its value as a query-string.

For loadable modules, kernel/param's parse_args(), via parse_one(),
invokes an unknown-parameter callback, ddebug_dyndbg_param_cb(),
for each of $module's options given in /etc/modprobe.d/*, for
$module.dyndbg given in boot command-line, and for options given
in the modprobe command, with each augmenting or overriding
previous flag settings.

In both cases, if no value is given (as in foo.dyndbg example above),
"+p" is assumed, which enables all pr_debug callsites in the module.

The unknown-parameter callback signature is altered, adding module-name.
This lets ddebug_dyndbg_param_cb() know what module its applying a
query-string for, and is needed because the module-name prefix is
stripped by parse_args()/parse_one().  The callback returns 0 if the
unknown param is "dyndbg", even if query-string is wrong; modprobe
shouldnt fail just because the dyndbg query is bad.

The parameter is not shown in /sys/module/*/parameters, thus it does
not use any resources.  Changes to the parameter are made via the
control file.

Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
CC: Thomas Renninger <trenn@suse.de>
CC: Rusty Russell <rusty@rustcorp.com.au>
---
 include/linux/dynamic_debug.h |   11 ++++++++++
 include/linux/moduleparam.h   |    6 ++++-
 init/main.c                   |    5 ++-
 kernel/module.c               |    3 +-
 kernel/params.c               |   26 ++++++++++++++----------
 lib/dynamic_debug.c           |   44 ++++++++++++++++++++++++++++++++++++++++-
 6 files changed, 79 insertions(+), 16 deletions(-)

diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index 7e3c53a..3027349 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -44,6 +44,8 @@ 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_dyndbg_param_cb(char *param, char *val, const char *modname);
+
 struct device;
 
 extern __printf(3, 4)
@@ -99,6 +101,15 @@ static inline int ddebug_remove_module(const char *mod)
 	return 0;
 }
 
+static inline int ddebug_dyndbg_param_cb(char *param, char *val,
+					const char *modname)
+{
+	if (strcmp(param, "dyndbg"))
+		return -ENOENT;
+	pr_warn("dyndbg supported only in CONFIG_DYNAMIC_DEBUG builds\n");
+	return 0; /* dont fail modprobe, warning is enough */
+}
+
 #define dynamic_pr_debug(fmt, ...)					\
 	do { if (0) printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__); } while (0)
 #define dynamic_dev_dbg(dev, fmt, ...)					\
diff --git a/include/linux/moduleparam.h b/include/linux/moduleparam.h
index 7939f63..3e272c7 100644
--- a/include/linux/moduleparam.h
+++ b/include/linux/moduleparam.h
@@ -292,7 +292,8 @@ extern int parse_args(const char *name,
 		      char *args,
 		      const struct kernel_param *params,
 		      unsigned num,
-		      int (*unknown)(char *param, char *val));
+		      int (*unknown)(char *param, char *val,
+			      const char *modname));
 
 /* Called by module remove. */
 #ifdef CONFIG_SYSFS
@@ -433,4 +434,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/init/main.c b/init/main.c
index 217ed23..928fab6 100644
--- a/init/main.c
+++ b/init/main.c
@@ -230,7 +230,8 @@ early_param("loglevel", loglevel);
  * Unknown boot options get handed to init, unless they look like
  * unused parameters (modprobe will find them in /proc/cmdline).
  */
-static int __init unknown_bootoption(char *param, char *val)
+static int __init unknown_bootoption(char *param, char *val,
+				const char *modname)
 {
 	/* Change NUL term back to "=", to make "param" the whole string. */
 	if (val) {
@@ -387,7 +388,7 @@ static noinline void __init_refok rest_init(void)
 }
 
 /* Check for early params. */
-static int __init do_early_param(char *param, char *val)
+static int __init do_early_param(char *param, char *val, const char *modname)
 {
 	const struct obs_kernel_param *p;
 
diff --git a/kernel/module.c b/kernel/module.c
index 39a7888..78b229e 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -2887,7 +2887,8 @@ static struct module *load_module(void __user *umod,
 	mutex_unlock(&module_mutex);
 
 	/* Module is ready to execute: parsing args may do that. */
-	err = parse_args(mod->name, mod->args, mod->kp, mod->num_kp, NULL);
+	err = parse_args(mod->name, mod->args, mod->kp, mod->num_kp,
+			 ddebug_dyndbg_param_cb);
 	if (err < 0)
 		goto unlink;
 
diff --git a/kernel/params.c b/kernel/params.c
index 9240664..4c39c0e 100644
--- a/kernel/params.c
+++ b/kernel/params.c
@@ -86,9 +86,11 @@ bool parameq(const char *a, const char *b)
 
 static int parse_one(char *param,
 		     char *val,
+		     const char *doing,
 		     const struct kernel_param *params,
 		     unsigned num_params,
-		     int (*handle_unknown)(char *param, char *val))
+		     int (*handle_unknown)(char *param, char *val,
+					   const char *modname))
 {
 	unsigned int i;
 	int err;
@@ -109,8 +111,10 @@ static int parse_one(char *param,
 	}
 
 	if (handle_unknown) {
-		pr_debug("Unknown argument: calling %p\n", handle_unknown);
-		return handle_unknown(param, val);
+		/* Booting kernel, early options are too early for this */
+		pr_debug("doing %s: %s = %s, callback %p\n",
+			doing, param, val, handle_unknown);
+		return handle_unknown(param, val, doing);
 	}
 
 	pr_debug("Unknown argument `%s'\n", param);
@@ -119,7 +123,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;
@@ -170,15 +174,15 @@ static char *next_arg(char *args, char **param, char **val)
 }
 
 /* Args looks like "foo=bar,bar2 baz=fuz wiz". */
-int parse_args(const char *name,
+int parse_args(const char *doing, /* modname, Booting kernel, early options */
 	       char *args,
 	       const struct kernel_param *params,
 	       unsigned num,
-	       int (*unknown)(char *param, char *val))
+	       int (*unknown)(char *param, char *val, const char *modname))
 {
 	char *param, *val;
 
-	pr_debug("Parsing ARGS: %s\n", args);
+	pr_debug("doing %s, parsing ARGS: %s\n", doing, args);
 
 	/* Chew leading spaces */
 	args = skip_spaces(args);
@@ -189,7 +193,7 @@ int parse_args(const char *name,
 
 		args = next_arg(args, &param, &val);
 		irq_was_disabled = irqs_disabled();
-		ret = parse_one(param, val, params, num, unknown);
+		ret = parse_one(param, val, doing, params, num, unknown);
 		if (irq_was_disabled && !irqs_disabled()) {
 			printk(KERN_WARNING "parse_args(): option '%s' enabled "
 					"irq's!\n", param);
@@ -197,19 +201,19 @@ int parse_args(const char *name,
 		switch (ret) {
 		case -ENOENT:
 			printk(KERN_ERR "%s: Unknown parameter `%s'\n",
-			       name, param);
+			       doing, param);
 			return ret;
 		case -ENOSPC:
 			printk(KERN_ERR
 			       "%s: `%s' too large for parameter `%s'\n",
-			       name, val ?: "", param);
+			       doing, val ?: "", param);
 			return ret;
 		case 0:
 			break;
 		default:
 			printk(KERN_ERR
 			       "%s: `%s' invalid for parameter `%s'\n",
-			       name, val ?: "", param);
+			       doing, val ?: "", param);
 			return ret;
 		}
 	}
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 516ad4e..4cdcc64 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -618,7 +618,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 +872,47 @@ int ddebug_add_module(struct _ddebug *tab, unsigned int n,
 }
 EXPORT_SYMBOL_GPL(ddebug_add_module);
 
+/* called from params.c:parse_one for unknown params, gets bare params */
+int ddebug_dyndbg_param_cb(char *param, char* val, const char* modname)
+{
+	if (strcmp(param, "dyndbg")) {
+		pr_info("skip modname: %s %s=\"%s\"\n", modname, param, val);
+		return -ENOENT;
+	}
+	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 */
+}
+
+#define COMMAND_LINE_SIZE 1024
+static __initdata char tmp_cmd_arr[COMMAND_LINE_SIZE];
+
+/* find & process .dyndbg params (prefixed by modname) in cmdline */
+static __init void ddebug_boot_parse_args(void)
+{
+	char *tmp_cmd_ptr, *param, *val, *modname;
+
+	/* copy cmdline before mangling it */
+	strlcpy(tmp_cmd_arr, saved_command_line, COMMAND_LINE_SIZE);
+	tmp_cmd_ptr = skip_spaces(tmp_cmd_arr);
+
+	while (*tmp_cmd_ptr) {
+		tmp_cmd_ptr = next_arg(tmp_cmd_ptr, &param, &val);
+		modname = param;
+		param = strstr(param, ".dyndbg");
+		if (!param)
+			continue;
+		*(param++) = '\0';
+		pr_info("Enabling debugging for module %s\n", modname);
+		if (verbose)
+			pr_info("Param: %s, val: %s\n", param, val);
+
+		ddebug_exec_queries(val ? val : "+p");
+	}
+}
+
 static void ddebug_table_free(struct ddebug_table *dt)
 {
 	list_del_init(&dt->link);
@@ -978,6 +1019,7 @@ static int __init dynamic_debug_init(void)
 		/* keep tables even on ddebug_query parse error */
 		ret = 0;
 	}
+	ddebug_boot_parse_args();
 
 out_free:
 	if (ret)
-- 
1.7.7.3


^ permalink raw reply related	[flat|nested] 45+ messages in thread

end of thread, other threads:[~2011-12-17  5:11 UTC | newest]

Thread overview: 45+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-11-30 20:27 [patch 00/25] dynamic-debug enhancements Jim Cromie
2011-12-06 18:59 ` Jim Cromie
2011-12-06 19:11   ` [patch 00/24 ] dynamic debug enhancements: multi-queries during mod-init jim.cromie
2011-12-06 19:11     ` [PATCH 01/25] kernel/module.c: fix compile err, warnings under ifdef DEBUGP jim.cromie
2011-12-06 19:11     ` [PATCH 02/25] dynamic_debug: fix whitespace complaints from scripts/cleanfile jim.cromie
2011-12-06 19:11     ` [PATCH 03/25] dynamic_debug: drop enabled field from struct _ddebug, use _DPRINTK_FLAGS_PRINT jim.cromie
2011-12-06 19:11     ` [PATCH 04/25] dynamic_debug: make dynamic-debug supersede DEBUG ccflag jim.cromie
2011-12-06 19:11     ` [PATCH 05/25] dynamic_debug: change verbosity at runtime jim.cromie
2011-12-06 19:11     ` [PATCH 06/25] dynamic_debug: replace strcpy with strlcpy, in ddebug_setup_query() jim.cromie
2011-12-06 19:11     ` [PATCH 07/25] dynamic_debug: pr_err() call should not depend upon verbosity jim.cromie
2011-12-06 19:11     ` [PATCH 08/25] dynamic_debug: drop explicit !=NULL checks jim.cromie
2011-12-06 19:11     ` [PATCH 09/25] dynamic_debug: describe_flags with '=[pmflt_]*' jim.cromie
2011-12-06 19:11     ` [PATCH 10/25] dynamic_debug: tighten up error checking on debug queries jim.cromie
2011-12-06 19:11     ` [PATCH 11/25] dynamic_debug: early return if _ddebug table is empty jim.cromie
2011-12-06 19:11     ` [PATCH 12/25] dynamic_debug: reduce lineno field to a saner 18 bits jim.cromie
2011-12-06 19:11     ` [PATCH 13/25] dynamic_debug: chop off comments in ddebug_tokenize jim.cromie
2011-12-06 19:11     ` [PATCH 14/25] dynamic_debug: enlarge command/query write buffer jim.cromie
2011-12-06 19:11     ` [PATCH 15/25] dynamic_debug: add trim_prefix() to provide source-root relative paths jim.cromie
2011-12-06 19:11     ` [PATCH 16/25] dynamic_debug: factor vpr_info_dq out of ddebug_parse_query jim.cromie
2011-12-06 19:11     ` [PATCH 17/25] dynamic_debug: process multiple debug-queries on a line jim.cromie
2011-12-06 19:11     ` [PATCH 18/25] dynamic_debug: Introduce global fake module param $module.dyndbg jim.cromie
2011-12-07  1:01       ` Rusty Russell
2011-12-07  8:33         ` Jim Cromie
2011-12-07 10:59           ` Rusty Russell
2011-12-08 17:10       ` Jason Baron
2011-12-08 18:12         ` Jim Cromie
2011-12-06 19:11     ` [PATCH 19/25] pnp: if CONFIG_DYNAMIC_DEBUG, use pnp.dyndbg instead of pnp.debug jim.cromie
2011-12-06 19:11     ` [PATCH 20/25] dynamic_debug: add modname arg to exec_query callchain jim.cromie
2011-12-06 19:11     ` [PATCH 21/25] kernel/module: replace DEBUGP with pr_debug jim.cromie
2011-12-07  1:07       ` Rusty Russell
2011-12-08 15:41         ` Jason Baron
2011-12-08 18:17           ` Jim Cromie
2011-12-06 19:11     ` [PATCH 22/25] dynamic_debug: protect "dyndbg" fake module param name at compile-time jim.cromie
2011-12-06 19:11     ` [PATCH 23/25] dynamic_debug: update Documentation/kernel-parameters.txt, Kconfig.debug jim.cromie
2011-12-06 19:11     ` [PATCH 24/25] dynamic_debug: remove unneeded includes jim.cromie
2011-12-12 22:05     ` [patch 00/24 ] dynamic debug enhancements: multi-queries during mod-init Greg KH
     [not found]       ` <CAJfuBxzeKzmOCo00Ew8xAp9EmU0QZ3zjOkRcdEMzYrtx_b9+kA@mail.gmail.com>
2011-12-13  4:17         ` Jim Cromie
2011-12-13  4:25           ` Greg KH
2011-12-13  7:00             ` Jim Cromie
2011-12-13  7:44               ` Greg KH
2011-12-13 14:48       ` Jason Baron
2011-12-12 23:12 ` [patch 00/23] dynamic-debug enhancements Jim Cromie
2011-12-12 23:12 [PATCH 01/25] kernel/module.c: fix compile err, warnings under ifdef DEBUGP, switch to pr_debug jim.cromie
2011-12-12 23:12 ` [PATCH 18/25] dynamic_debug: Introduce global fake module param $module.dyndbg jim.cromie
2011-12-16  0:00   ` Rusty Russell
2011-12-17  5:10     ` Jim Cromie

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.