This patchset extends dynamic-debug facility to allow
use of pr_debug() within a loadable module's module_init()
function. Query/rules can be given on the boot-line,
and are saved to a pending list if they cannot be applied
immediately. Later, when the module is being loaded, the
pending list is scanned, and matching rules are applied.
Thus pr_debug() calls in the module's initialization function
are active when it is invoked.
Other features:
- dynamic_debug.verbose=1 at boot-time or while running
- ddebug_query can specify multiple queries, separated by ';'
- warn on multiple uses of a single match-spec in single query
- tolerate bad queries in ddebug_query w/o taking down facility
Possible additions (not done now)
- <dbgfs>/dynamic_debug/pending
to show pending query/rules
- scan existing pending list before adding new entry
currently same rule can be appended multiple times
- persistent queries
when module is unloaded, applied rules are deleted
[PATCH 01/11] dynamic_debug: allow changing of dynamic_debug verbosity any time
[PATCH 02/11] dynamic_debug: trim source-path prefix from dynamic_debug/control
[PATCH 03/11] dynamic_debug: process multiple commands on a line
[PATCH 04/11] dynamic_debug: warn when >1 of each type of match-spec is given
[PATCH 05/11] dynamic_debug: use pr_info instead of printk(KERN_INFO ..
[PATCH 06/11] dynamic_debug: KERN_ERR should not depend upon verbosity
[PATCH 07/11] dynamic_debug: dont kill entire facility on error parsing ddebug_query
[PATCH 08/11] dynamic_debug: return int from ddebug_change
[PATCH 09/11] dynamic_debug: add_to_pending() saves non-matching queries for later.
[PATCH 10/11] dynamic_debug: call apply_pending_queries from ddebug_add_module
[PATCH 11/11] dynamic_debug: document use of pendinq queries at boot-time
allow changing dynamic_debug verbosity
at boot-time, with: dynamic_debug.verbose=1
or at runtime with:
root@voyage:~# echo 1 > /sys/module/dynamic_debug/parameters/verbose
Signed-off-by: Jim Cromie <[email protected]>
---
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 75ca78f..a3b08d5 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -58,6 +58,7 @@ struct ddebug_iter {
static DEFINE_MUTEX(ddebug_lock);
static LIST_HEAD(ddebug_tables);
static int verbose = 0;
+module_param(verbose, int, 0744);
/* Return the last part of a pathname */
static inline const char *basename(const char *path)
--
1.7.4.1
Skip past unchanging absolute source-path prefix to print a path
thats relative to kernel source's root-dir. This makes the file
easier to read without a wide screen. For example:
kernel/freezer.c:128 [freezer]cancel_freezing - " clean up: %s\012"
Signed-off-by: Jim Cromie <[email protected]>
---
lib/dynamic_debug.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index a3b08d5..eb08a2f 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -601,6 +601,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];
+ const int offset = strlen(__FILE__) - strlen("lib/dynamic_debug.c");
if (verbose)
printk(KERN_INFO "%s: called m=%p p=%p\n",
@@ -613,7 +614,7 @@ static int ddebug_proc_show(struct seq_file *m, void *p)
}
seq_printf(m, "%s:%u [%s]%s %s \"",
- dp->filename, dp->lineno,
+ dp->filename + offset, 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.4.1
Process multiple commands per line, separated by ';'. All commands are
processed, independent of errors, allowing individual commands to fail,
for example when a module is not installed. Last error code is returned.
With this, extensive command sets can be given on the boot-line.
Signed-off-by: Jim Cromie <[email protected]>
---
Documentation/dynamic-debug-howto.txt | 14 ++++++++++-
lib/dynamic_debug.c | 39 +++++++++++++++++++++++++++++++-
2 files changed, 49 insertions(+), 4 deletions(-)
diff --git a/Documentation/dynamic-debug-howto.txt b/Documentation/dynamic-debug-howto.txt
index f959909..d0faf98 100644
--- a/Documentation/dynamic-debug-howto.txt
+++ b/Documentation/dynamic-debug-howto.txt
@@ -92,8 +92,18 @@ 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:
+Commands are bounded by a write() system call. Subject to this limit
+(or 1024 for boot-line parameter) you can send multiple commands,
+separated by ';'
+
+foo:~ # echo "module nsc_gpio +p ; module pc8736x_gpio +p ; " \
+ "module scx200_gpio +p " > /dbg/dynamic_debug/control
+
+Multiple commands are processed independently, this allows you to send
+commands which may fail, for example if a module is not present. The
+last failing command returns its error.
+
+Or you can do an "echo" for each, like:
nullarbor:~ # echo 'file svcsock.c line 1603 +p' > /proc/dprintk ;\
> echo 'file svcsock.c line 1563 +p' > /proc/dprintk
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index eb08a2f..0e567ad 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -428,6 +428,41 @@ static int ddebug_exec_query(char *query_string)
return 0;
}
+/* handle multiple queries, continue on error, return last error */
+static int ddebug_exec_queries(char *query)
+{
+ char *split = query;
+ int i, errs = 0, exitcode = 0, rc;
+
+ if (verbose)
+ /* clean up for logging */
+ for (; (split = strpbrk(split, "\t\n")); split++)
+ *split = ' ';
+
+ for (i = 0; query; query = split, i++) {
+
+ split = strchr(query, ';');
+ if (split)
+ *split++ = '\0';
+
+ if (verbose)
+ printk(KERN_INFO "%s: query %d: \"%s\"",
+ __func__, i, query);
+
+ rc = ddebug_exec_query(query);
+ if (rc) {
+ errs++;
+ exitcode = rc;
+ }
+ }
+ if (verbose)
+ printk(KERN_INFO
+ "%s: processed %d queries, with %d errs",
+ __func__, i, errs);
+
+ return exitcode;
+}
+
int __dynamic_pr_debug(struct _ddebug *descriptor, const char *fmt, ...)
{
va_list args;
@@ -492,7 +527,7 @@ static ssize_t ddebug_proc_write(struct file *file, const char __user *ubuf,
printk(KERN_INFO "%s: read %d bytes from userspace\n",
__func__, (int)len);
- ret = ddebug_exec_query(tmpbuf);
+ ret = ddebug_exec_queries(tmpbuf);
if (ret)
return ret;
@@ -804,7 +839,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_query(ddebug_setup_string);
+ ret = ddebug_exec_queries(ddebug_setup_string);
if (ret)
pr_warning("Invalid ddebug boot param %s",
ddebug_setup_string);
--
1.7.4.1
Issue warnings when any match-spec is given multiple times in a rule.
Code honoredi 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.
Signed-off-by: Jim Cromie <[email protected]>
---
lib/dynamic_debug.c | 19 +++++++++++++++----
1 files changed, 15 insertions(+), 4 deletions(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 0e567ad..2505232 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -284,6 +284,14 @@ static char *unescape(char *str)
return str;
}
+static inline void check_set(const char **dest, char *src, char *name)
+{
+ if (*dest)
+ pr_warning("match-spec:%s val:%s overridden by %s",
+ name, *dest, src);
+ *dest = src;
+}
+
/*
* Parse words[] as a ddebug query specification, which is a series
* of (keyword, value) pairs chosen from these possibilities:
@@ -295,6 +303,9 @@ 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 pair of each type is expected, subsequent ones elicit a
+ * warning, and override the setting.
*/
static int ddebug_parse_query(char *words[], int nwords,
struct ddebug_query *query)
@@ -308,13 +319,13 @@ 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];
+ check_set(&query->function, words[i+1], "func");
else if (!strcmp(words[i], "file"))
- query->filename = words[i+1];
+ check_set(&query->filename, words[i+1], "file");
else if (!strcmp(words[i], "module"))
- query->module = words[i+1];
+ check_set(&query->module, words[i+1], "module");
else if (!strcmp(words[i], "format"))
- query->format = unescape(words[i+1]);
+ check_set(&query->format, words[i+1], "format");
else if (!strcmp(words[i], "line")) {
char *first = words[i+1];
char *last = strchr(first, '-');
--
1.7.4.1
pr_info was already in use here, update most others.
Use pr_fmt to uniformly add __func__ to all calls.
A few printks were left alone, where line was left un-terminated,
and subsequent printks appended to it. These need separate attention.
Signed-off-by: Jim Cromie <[email protected]>
---
lib/dynamic_debug.c | 55 +++++++++++++++++++++++---------------------------
1 files changed, 25 insertions(+), 30 deletions(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 2505232..17bace8 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -31,6 +31,9 @@
#include <linux/hardirq.h>
#include <linux/sched.h>
+#undef pr_fmt
+#define pr_fmt(fmt) "%s: " fmt, __func__
+
extern struct _ddebug __start___verbose[];
extern struct _ddebug __stop___verbose[];
@@ -160,8 +163,7 @@ static void ddebug_change(const struct ddebug_query *query,
else
dp->enabled = 0;
if (verbose)
- printk(KERN_INFO
- "ddebug: changed %s:%d [%s]%s %s\n",
+ pr_info("changed %s:%d [%s]%s %s\n",
dp->filename, dp->lineno,
dt->mod_name, dp->function,
ddebug_describe_flags(dp, flagbuf,
@@ -171,7 +173,7 @@ static void ddebug_change(const struct ddebug_query *query,
mutex_unlock(&ddebug_lock);
if (!nfound && verbose)
- printk(KERN_INFO "ddebug: no matches for query\n");
+ pr_info("no matches for query\n");
}
/*
@@ -349,9 +351,9 @@ static int ddebug_parse_query(char *words[], int nwords,
}
if (verbose)
- printk(KERN_INFO "%s: q->function=\"%s\" q->filename=\"%s\" "
+ pr_info("q->function=\"%s\" q->filename=\"%s\" "
"q->module=\"%s\" q->format=\"%s\" q->lineno=%u-%u\n",
- __func__, query->function, query->filename,
+ query->function, query->filename,
query->module, query->format, query->first_lineno,
query->last_lineno);
@@ -380,7 +382,7 @@ static int ddebug_parse_flags(const char *str, unsigned int *flagsp,
return -EINVAL;
}
if (verbose)
- printk(KERN_INFO "%s: op='%c'\n", __func__, op);
+ pr_info("op='%c'\n", op);
for ( ; *str ; ++str) {
for (i = ARRAY_SIZE(opt_array) - 1; i >= 0; i--) {
@@ -395,7 +397,7 @@ static int ddebug_parse_flags(const char *str, unsigned int *flagsp,
if (flags == 0)
return -EINVAL;
if (verbose)
- printk(KERN_INFO "%s: flags=0x%x\n", __func__, flags);
+ pr_info("flags=0x%x\n", flags);
/* calculate final *flagsp, *maskp according to mask and op */
switch (op) {
@@ -413,8 +415,8 @@ static int ddebug_parse_flags(const char *str, unsigned int *flagsp,
break;
}
if (verbose)
- printk(KERN_INFO "%s: *flagsp=0x%x *maskp=0x%x\n",
- __func__, *flagsp, *maskp);
+ pr_info("*flagsp=0x%x *maskp=0x%x\n", *flagsp, *maskp);
+
return 0;
}
@@ -457,8 +459,7 @@ static int ddebug_exec_queries(char *query)
*split++ = '\0';
if (verbose)
- printk(KERN_INFO "%s: query %d: \"%s\"",
- __func__, i, query);
+ pr_info("query %d: \"%s\"\n", i, query);
rc = ddebug_exec_query(query);
if (rc) {
@@ -467,9 +468,7 @@ static int ddebug_exec_queries(char *query)
}
}
if (verbose)
- printk(KERN_INFO
- "%s: processed %d queries, with %d errs",
- __func__, i, errs);
+ pr_info("processed %d queries, with %d errs", i, errs);
return exitcode;
}
@@ -507,7 +506,7 @@ static __initdata char ddebug_setup_string[1024];
static __init int ddebug_setup_query(char *str)
{
if (strlen(str) >= 1024) {
- pr_warning("ddebug boot param string too large\n");
+ pr_warning("boot param string too large\n");
return 0;
}
strcpy(ddebug_setup_string, str);
@@ -535,8 +534,7 @@ static ssize_t ddebug_proc_write(struct file *file, const char __user *ubuf,
return -EFAULT;
tmpbuf[len] = '\0';
if (verbose)
- printk(KERN_INFO "%s: read %d bytes from userspace\n",
- __func__, (int)len);
+ pr_info("read %d bytes from userspace\n", (int)len);
ret = ddebug_exec_queries(tmpbuf);
if (ret)
@@ -599,8 +597,8 @@ static void *ddebug_proc_start(struct seq_file *m, loff_t *pos)
int n = *pos;
if (verbose)
- printk(KERN_INFO "%s: called m=%p *pos=%lld\n",
- __func__, m, (unsigned long long)*pos);
+ pr_info("called m=%p *pos=%lld\n",
+ m, (unsigned long long)*pos);
mutex_lock(&ddebug_lock);
@@ -625,8 +623,8 @@ static void *ddebug_proc_next(struct seq_file *m, void *p, loff_t *pos)
struct _ddebug *dp;
if (verbose)
- printk(KERN_INFO "%s: called m=%p p=%p *pos=%lld\n",
- __func__, m, p, (unsigned long long)*pos);
+ pr_info("called m=%p p=%p *pos=%lld\n",
+ m, p, (unsigned long long)*pos);
if (p == SEQ_START_TOKEN)
dp = ddebug_iter_first(iter);
@@ -650,8 +648,7 @@ static int ddebug_proc_show(struct seq_file *m, void *p)
const int offset = strlen(__FILE__) - strlen("lib/dynamic_debug.c");
if (verbose)
- printk(KERN_INFO "%s: called m=%p p=%p\n",
- __func__, m, p);
+ pr_info("called m=%p p=%p\n", m, p);
if (p == SEQ_START_TOKEN) {
seq_puts(m,
@@ -676,8 +673,8 @@ static int ddebug_proc_show(struct seq_file *m, void *p)
static void ddebug_proc_stop(struct seq_file *m, void *p)
{
if (verbose)
- printk(KERN_INFO "%s: called m=%p p=%p\n",
- __func__, m, p);
+ pr_info("called m=%p p=%p\n", m, p);
+
mutex_unlock(&ddebug_lock);
}
@@ -700,7 +697,7 @@ static int ddebug_proc_open(struct inode *inode, struct file *file)
int err;
if (verbose)
- printk(KERN_INFO "%s: called\n", __func__);
+ pr_info("called\n");
iter = kzalloc(sizeof(*iter), GFP_KERNEL);
if (iter == NULL)
@@ -752,8 +749,7 @@ int ddebug_add_module(struct _ddebug *tab, unsigned int n,
mutex_unlock(&ddebug_lock);
if (verbose)
- printk(KERN_INFO "%u debug prints in module %s\n",
- n, dt->mod_name);
+ pr_info("%u debug prints in module %s\n", n, dt->mod_name);
return 0;
}
EXPORT_SYMBOL_GPL(ddebug_add_module);
@@ -775,8 +771,7 @@ int ddebug_remove_module(const char *mod_name)
int ret = -ENOENT;
if (verbose)
- printk(KERN_INFO "%s: removing module \"%s\"\n",
- __func__, mod_name);
+ pr_info("removing module \"%s\"\n", mod_name);
mutex_lock(&ddebug_lock);
list_for_each_entry_safe(dt, nextdt, &ddebug_tables, link) {
--
1.7.4.1
issue keyword/parsing errors even w/o verbose set;
uncover otherwize mysterious non-functionality.
Also convert to pr_err(), which supplies __func__ via pr_fmt.
Signed-off-by: Jim Cromie <[email protected]>
---
lib/dynamic_debug.c | 4 +---
1 files changed, 1 insertions(+), 3 deletions(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 17bace8..7cf5fa7 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -343,9 +343,7 @@ static int ddebug_parse_query(char *words[], int nwords,
query->last_lineno = query->first_lineno;
}
} else {
- if (verbose)
- printk(KERN_ERR "%s: unknown keyword \"%s\"\n",
- __func__, words[i]);
+ pr_err("unknown keyword \"%s\"\n", words[i]);
return -EINVAL;
}
}
--
1.7.4.1
Currently, a parsing error on ddebug_query results in effective loss
of the facility; all tables are removed, and the init-call returns error.
This is way too severe, especially when an innocent quoting error can
be the cause:
Kernel command line: root=LABEL=ROOT_FS ... ddebug_query='file char_dev.c +p'
yields:
ddebug_exec_queries: query 0: "'file"
query before parse <'file>
ddebug_exec_queries: processed 1 queries, with 1 errs
Invalid ddebug boot param 'file
Fix this by zeroing return-code for parse error before returning.
Signed-off-by: Jim Cromie <[email protected]>
---
lib/dynamic_debug.c | 5 +++--
1 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 7cf5fa7..97f6a9b 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -844,10 +844,11 @@ 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);
- if (ret)
+ if (ret) {
+ ret = 0; /* dont kill entire facility */
pr_warning("Invalid ddebug boot param %s",
ddebug_setup_string);
- else
+ } else
pr_info("ddebug initialized with string %s",
ddebug_setup_string);
}
--
1.7.4.1
Alter ddebug_change to return number of matches found for query/rule.
This lets caller know whether rule applied, and potentially what
to do next.
Signed-off-by: Jim Cromie <[email protected]>
---
lib/dynamic_debug.c | 11 ++++++-----
1 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 97f6a9b..9b8b1e8 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -102,8 +102,8 @@ static char *ddebug_describe_flags(struct _ddebug *dp, char *buf,
* the user which ddebug's were changed, or whether none
* were matched.
*/
-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;
@@ -172,8 +172,7 @@ static void ddebug_change(const struct ddebug_query *query,
}
mutex_unlock(&ddebug_lock);
- if (!nfound && verbose)
- pr_info("no matches for query\n");
+ return nfound;
}
/*
@@ -425,6 +424,7 @@ static int ddebug_exec_query(char *query_string)
#define MAXWORDS 9
int nwords;
char *words[MAXWORDS];
+ int nfound;
nwords = ddebug_tokenize(query_string, words, MAXWORDS);
if (nwords <= 0)
@@ -435,7 +435,8 @@ static int ddebug_exec_query(char *query_string)
return -EINVAL;
/* actually go and implement the change */
- ddebug_change(&query, flags, mask);
+ nfound = ddebug_change(&query, flags, mask);
+
return 0;
}
--
1.7.4.1
When a query/rule doesnt match, call add_to_pending(new function)
to copy the query off the stack, into a (new) struct pending_query,
and add to pending_queries (new) list.
Patch adds: /sys/module/dynamic_debug/parameters/
verbose - rw, added previously, here for completeness
pending_ct - ro, shows current count of pending queries
pending_max - rw, set max pending, further pending queries are rejected
With this and previous patches, queries added on the boot-line which
do not match (because module is not built-in, and thus not present yet)
are added to pending_queries.
IE, with these boot-line additions:
dynamic_debug.verbose=1 ddebug_query="module scx200_hrt +p"
Kernel will log the following:
ddebug_exec_queries: query 0: "module scx200_hrt +p"
ddebug_tokenize: split into words: "module" "scx200_hrt" "+p"
ddebug_parse_query: parsed q->function="(null)" q->filename="(null)" q->module="scx200_hrt" q->format="(null)" q->lineno=0-0
ddebug_parse_flags: op='+'
ddebug_parse_flags: flags=0x1
ddebug_parse_flags: *flagsp=0x1 *maskp=0xffffffff
ddebug_exec_query: nfound 0 on q->function="(null)" q->filename="(null)" q->module="scx200_hrt" q->format="(null)" q->lineno=0-0
ddebug_add_to_pending: add to pending: q->function="(null)" q->filename="(null)" q->module="scx200_hrt" q->format="(null)" q->lineno=0-0
ddebug_add_to_pending: ddebug: query saved as pending 1
Signed-off-by: Jim Cromie <[email protected]>
---
lib/dynamic_debug.c | 78 +++++++++++++++++++++++++++++++++++++++++++++++----
1 files changed, 72 insertions(+), 6 deletions(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 9b8b1e8..37d0275 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -53,6 +53,16 @@ struct ddebug_query {
unsigned int first_lineno, last_lineno;
};
+struct pending_query {
+ struct list_head link;
+ struct ddebug_query query;
+ char filename[100];
+ char module[30];
+ char function[50];
+ char format[200];
+ unsigned int flags, mask;
+};
+
struct ddebug_iter {
struct ddebug_table *table;
unsigned int idx;
@@ -61,7 +71,13 @@ struct ddebug_iter {
static DEFINE_MUTEX(ddebug_lock);
static LIST_HEAD(ddebug_tables);
static int verbose = 0;
-module_param(verbose, int, 0744);
+module_param(verbose, int, 0644);
+
+/* legal but inapplicable queries, save and test against new modules */
+static LIST_HEAD(pending_queries);
+static int pending_ct, pending_max = 100;
+module_param(pending_ct, int, 0444);
+module_param(pending_max, int, 0644);
/* Return the last part of a pathname */
static inline const char *basename(const char *path)
@@ -96,6 +112,56 @@ static char *ddebug_describe_flags(struct _ddebug *dp, char *buf,
return buf;
}
+static char prbuf_query[300];
+
+static char *show_ddebug_query(const struct ddebug_query *q)
+{
+ sprintf(prbuf_query,
+ "q->function=\"%s\" q->filename=\"%s\" "
+ "q->module=\"%s\" q->format=\"%s\" q->lineno=%u-%u",
+ q->function, q->filename, q->module, q->format,
+ q->first_lineno, q->last_lineno);
+
+ return prbuf_query;
+}
+
+/* copy query off stack, save flags & mask, and store in pending-list */
+static void add_to_pending(const struct ddebug_query *query,
+ unsigned int flags, unsigned int mask)
+{
+ struct pending_query *pq;
+
+ if (pending_ct >= pending_max) {
+ pr_warn("ddebug: no matches for query, pending is full\n");
+ return;
+ }
+ if (verbose)
+ pr_info("add to pending: %s\n", show_ddebug_query(query));
+
+ pending_ct++;
+ pq = kzalloc(sizeof(struct pending_query), GFP_KERNEL);
+
+ /* copy non-null match-specs into allocd mem, update pointers */
+ if (query->module)
+ pq->query.module = strcpy(pq->module, query->module);
+ if (query->function)
+ pq->query.function = strcpy(pq->function, query->function);
+ if (query->filename)
+ pq->query.filename = strcpy(pq->filename, query->filename);
+ if (query->format)
+ pq->query.format = strcpy(pq->format, query->format);
+
+ pq->flags = flags;
+ pq->mask = mask;
+
+ mutex_lock(&ddebug_lock);
+ list_add(&pq->link, &pending_queries);
+ mutex_unlock(&ddebug_lock);
+
+ if (verbose)
+ pr_info("ddebug: query saved as pending %d\n", pending_ct);
+}
+
/*
* Search the tables for _ddebug's which match the given
* `query' and apply the `flags' and `mask' to them. Tells
@@ -348,11 +414,7 @@ static int ddebug_parse_query(char *words[], int nwords,
}
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);
+ pr_info("parsed %s\n", show_ddebug_query(query));
return 0;
}
@@ -437,6 +499,10 @@ static int ddebug_exec_query(char *query_string)
/* actually go and implement the change */
nfound = ddebug_change(&query, flags, mask);
+ pr_info("nfound %d on %s\n", nfound, show_ddebug_query(&query));
+ if (!nfound)
+ ddebug_add_to_pending(&query, flags, mask);
+
return 0;
}
--
1.7.4.1
When a module is loaded, ddebug_add_module calls apply_pending_queries
to scan pending_queries list and call ddebug_change to apply them.
Matching rules are removed from pending_queries.
With this change, the loaded module's pr_debugs are enabled when
its module_init is invoked, which is not possible otherwize.
With verbose=1, kernel logs look like:
apply_pending_queries: check: pc8736x_gpio <-> q->function="(null)" q->filename="(null)" q->module="pc8736x_gpio" q->format="(null)" q->lineno=0-0 q->flags=0x1 q->mask=0xffffffff
ddebug_change: changed /home/jimc/projects/lx/linux-2.6/drivers/char/pc8736x_gpio.c:342 [pc8736x_gpio]pc8736x_gpio_cleanup p
ddebug_change: changed /home/jimc/projects/lx/linux-2.6/drivers/char/pc8736x_gpio.c:319 [pc8736x_gpio]pc8736x_gpio_init p
...
platform pc8736x_gpio.0: NatSemi pc8736x GPIO Driver Initializing
platform pc8736x_gpio.0: GPIO ioport 6600 reserved
Signed-off-by: Jim Cromie <[email protected]>
---
lib/dynamic_debug.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++--
1 files changed, 48 insertions(+), 2 deletions(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 37d0275..252888a 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -126,8 +126,8 @@ static char *show_ddebug_query(const struct ddebug_query *q)
}
/* copy query off stack, save flags & mask, and store in pending-list */
-static void add_to_pending(const struct ddebug_query *query,
- unsigned int flags, unsigned int mask)
+static void ddebug_add_to_pending(struct ddebug_query *query,
+ unsigned int flags, unsigned int mask)
{
struct pending_query *pq;
@@ -351,6 +351,20 @@ static char *unescape(char *str)
return str;
}
+static char *show_pending_query(struct pending_query *pq)
+{
+ struct ddebug_query *dq = &pq->query;
+ sprintf(prbuf_query,
+ "q->function=\"%s\" q->filename=\"%s\" "
+ "q->module=\"%s\" q->format=\"%s\" q->lineno=%u-%u "
+ "q->flags=0x%x q->mask=0x%x\n",
+ dq->function, dq->filename, dq->module, dq->format,
+ dq->first_lineno, dq->last_lineno,
+ pq->flags, pq->mask);
+
+ return prbuf_query;
+}
+
static inline void check_set(const char **dest, char *src, char *name)
{
if (*dest)
@@ -786,6 +800,35 @@ static const struct file_operations ddebug_proc_fops = {
.write = ddebug_proc_write
};
+/* apply matching queries in pending-queries list */
+static void apply_pending_queries(struct ddebug_table *dt)
+{
+ struct pending_query *pq, *pqnext;
+ int nfound;
+
+ if (verbose)
+ pr_info("pending_ct: %d\n", pending_ct);
+
+ list_for_each_entry_safe(pq, pqnext, &pending_queries, link) {
+
+ if (verbose)
+ pr_info("check: %s <-> %s\n",
+ dt->mod_name, show_pending_query(pq));
+
+ nfound = ddebug_change(&pq->query, pq->flags, pq->mask);
+
+ if (nfound) {
+ mutex_lock(&ddebug_lock);
+ list_del(&pq->link);
+ mutex_unlock(&ddebug_lock);
+ kfree(pq);
+ pending_ct--;
+ }
+ else if (verbose)
+ pr_info("no-match: %s\n", show_pending_query(pq));
+ }
+
+}
/*
* Allocate a new ddebug_table for the given module
* and add it to the global list.
@@ -815,6 +858,9 @@ int ddebug_add_module(struct _ddebug *tab, unsigned int n,
if (verbose)
pr_info("%u debug prints in module %s\n", n, dt->mod_name);
+
+ apply_pending_queries(dt);
+
return 0;
}
EXPORT_SYMBOL_GPL(ddebug_add_module);
--
1.7.4.1
Add paragraph that pendinq-queries are checked when modules are loaded,
so debug statements can be used in module_init functions.
Signed-off-by: Jim Cromie <[email protected]>
---
Documentation/dynamic-debug-howto.txt | 20 ++++++++++++++------
1 files changed, 14 insertions(+), 6 deletions(-)
diff --git a/Documentation/dynamic-debug-howto.txt b/Documentation/dynamic-debug-howto.txt
index d0faf98..49b75b81 100644
--- a/Documentation/dynamic-debug-howto.txt
+++ b/Documentation/dynamic-debug-howto.txt
@@ -120,11 +120,12 @@ specifications, followed by a flags change specification.
command ::= match-spec* flags-spec
-The match-spec's are used to choose a subset of the known dprintk()
+The match-specs are used to choose a subset of the known dprintk()
callsites to which to apply the flags-spec. Think of them as a query
-with implicit ANDs between each pair. Note that an empty list of
-match-specs is possible, but is not very useful because it will not
-match any debug statement callsites.
+with implicit ANDs between each pair. This means that multiple specs
+of a given type are nonsense; a module cannot match A and B
+simultaneously. Note that an empty list of match-specs is legal but
+pointless, it will not match any debug statement callsites.
A match specification comprises a keyword, which controls the attribute
of the callsite to be compared, and a value to compare against. Possible
@@ -242,13 +243,20 @@ QUERY follows the syntax described above, but must not exceed 1023
characters. The enablement of debug messages is done as an arch_initcall.
Thus you can enable debug messages in all code processed after this
arch_initcall via this boot parameter.
-On an x86 system for example ACPI enablement is a subsys_initcall and
-ddebug_query="file ec.c +p"
+
+On an x86 system for example ACPI enablement is a subsys_initcall, so
+ ddebug_query="file ec.c +p"
will show early Embedded Controller transactions during ACPI setup if
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.
+You can also give queries for modules which are not yet loaded, but
+will be, via udev or /etc/modules. These queries are saved to a
+pending-queries list, and are applied when the module is loaded, and
+before the module's init function is invoked. Note that modules which
+have no debug messages do not trigger this, so queries for them will
+remain on the pending-list until reboot.
Examples
========
--
1.7.4.1
On Tue, Jun 28, 2011 at 9:09 AM, Jim Cromie <[email protected]> wrote:
>
> Skip past unchanging absolute source-path prefix to print a path
> thats relative to kernel source's root-dir. ?This makes the file
> easier to read without a wide screen. ?For example:
>
> kernel/freezer.c:128 [freezer]cancel_freezing - " ?clean up: %s\012"
>
> Signed-off-by: Jim Cromie <[email protected]>
> ---
> ?lib/dynamic_debug.c | ? ?3 ++-
> ?1 files changed, 2 insertions(+), 1 deletions(-)
>
> diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
> index a3b08d5..eb08a2f 100644
> --- a/lib/dynamic_debug.c
> +++ b/lib/dynamic_debug.c
> @@ -601,6 +601,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];
> + ? ? ? const int offset = strlen(__FILE__) - strlen("lib/dynamic_debug.c");
>
> ? ? ? ?if (verbose)
> ? ? ? ? ? ? ? ?printk(KERN_INFO "%s: called m=%p p=%p\n",
> @@ -613,7 +614,7 @@ static int ddebug_proc_show(struct seq_file *m, void *p)
> ? ? ? ?}
>
> ? ? ? ?seq_printf(m, "%s:%u [%s]%s %s \"",
> - ? ? ? ? ? ? ? ? ?dp->filename, dp->lineno,
> + ? ? ? ? ? ? ? ? ?dp->filename + offset, dp->lineno,
> ? ? ? ? ? ? ? ? ? iter->table->mod_name, dp->function,
> ? ? ? ? ? ? ? ? ? ddebug_describe_flags(dp, flagsbuf, sizeof(flagsbuf)));
> ? ? ? ?seq_escape(m, dp->format, "\t\r\n\"");
The above will break the output generated for out-of-tree kernel
modules that use the dynamic debug facility.
Bart.
On Tue, Jun 28, 2011 at 9:09 AM, Jim Cromie <[email protected]> wrote:
> - ? ? ? ? ? ? ? ? ? ? ? query->format = unescape(words[i+1]);
> + ? ? ? ? ? ? ? ? ? ? ? check_set(&query->format, words[i+1], "format");
Was it your intention to remove the call to "unescape" ?
Bart.
On Tue, Jun 28, 2011 at 9:09 AM, Jim Cromie <[email protected]> wrote:
> @@ -425,6 +424,7 @@ static int ddebug_exec_query(char *query_string)
> ?#define MAXWORDS 9
> ? ? ? ?int nwords;
> ? ? ? ?char *words[MAXWORDS];
> + ? ? ? int nfound;
>
> ? ? ? ?nwords = ddebug_tokenize(query_string, words, MAXWORDS);
> ? ? ? ?if (nwords <= 0)
> @@ -435,7 +435,8 @@ static int ddebug_exec_query(char *query_string)
> ? ? ? ? ? ? ? ?return -EINVAL;
>
> ? ? ? ?/* actually go and implement the change */
> - ? ? ? ddebug_change(&query, flags, mask);
> + ? ? ? nfound = ddebug_change(&query, flags, mask);
> +
> ? ? ? ?return 0;
Do these changes actually do anything, or did I miss something ?
Bart.
Hi Jim,
A number of ppl have expressed interest to me in this functionality...
I'm wondering if we should call out this usage explicitly with a new
flag like '+a'. Meaning 'add' as a pending query if it doesn't match
anything existing.
In this way, we can have error when things don't match in the normal case.
Otherwise, somebody might think they've enabled something when they've in fact
added a pending query.
Otherwise, it looks pretty good.
thanks,
-Jason
On Tue, Jun 28, 2011 at 01:09:50AM -0600, Jim Cromie wrote:
> When a query/rule doesnt match, call add_to_pending(new function)
> to copy the query off the stack, into a (new) struct pending_query,
> and add to pending_queries (new) list.
>
> Patch adds: /sys/module/dynamic_debug/parameters/
> verbose - rw, added previously, here for completeness
> pending_ct - ro, shows current count of pending queries
> pending_max - rw, set max pending, further pending queries are rejected
>
> With this and previous patches, queries added on the boot-line which
> do not match (because module is not built-in, and thus not present yet)
> are added to pending_queries.
>
> IE, with these boot-line additions:
> dynamic_debug.verbose=1 ddebug_query="module scx200_hrt +p"
>
> Kernel will log the following:
>
> ddebug_exec_queries: query 0: "module scx200_hrt +p"
> ddebug_tokenize: split into words: "module" "scx200_hrt" "+p"
> ddebug_parse_query: parsed q->function="(null)" q->filename="(null)" q->module="scx200_hrt" q->format="(null)" q->lineno=0-0
> ddebug_parse_flags: op='+'
> ddebug_parse_flags: flags=0x1
> ddebug_parse_flags: *flagsp=0x1 *maskp=0xffffffff
> ddebug_exec_query: nfound 0 on q->function="(null)" q->filename="(null)" q->module="scx200_hrt" q->format="(null)" q->lineno=0-0
> ddebug_add_to_pending: add to pending: q->function="(null)" q->filename="(null)" q->module="scx200_hrt" q->format="(null)" q->lineno=0-0
> ddebug_add_to_pending: ddebug: query saved as pending 1
>
> Signed-off-by: Jim Cromie <[email protected]>
> ---
> lib/dynamic_debug.c | 78 +++++++++++++++++++++++++++++++++++++++++++++++----
> 1 files changed, 72 insertions(+), 6 deletions(-)
>
> diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
> index 9b8b1e8..37d0275 100644
> --- a/lib/dynamic_debug.c
> +++ b/lib/dynamic_debug.c
> @@ -53,6 +53,16 @@ struct ddebug_query {
> unsigned int first_lineno, last_lineno;
> };
>
> +struct pending_query {
> + struct list_head link;
> + struct ddebug_query query;
> + char filename[100];
> + char module[30];
> + char function[50];
> + char format[200];
> + unsigned int flags, mask;
> +};
> +
> struct ddebug_iter {
> struct ddebug_table *table;
> unsigned int idx;
> @@ -61,7 +71,13 @@ struct ddebug_iter {
> static DEFINE_MUTEX(ddebug_lock);
> static LIST_HEAD(ddebug_tables);
> static int verbose = 0;
> -module_param(verbose, int, 0744);
> +module_param(verbose, int, 0644);
> +
> +/* legal but inapplicable queries, save and test against new modules */
> +static LIST_HEAD(pending_queries);
> +static int pending_ct, pending_max = 100;
> +module_param(pending_ct, int, 0444);
> +module_param(pending_max, int, 0644);
>
> /* Return the last part of a pathname */
> static inline const char *basename(const char *path)
> @@ -96,6 +112,56 @@ static char *ddebug_describe_flags(struct _ddebug *dp, char *buf,
> return buf;
> }
>
> +static char prbuf_query[300];
> +
> +static char *show_ddebug_query(const struct ddebug_query *q)
> +{
> + sprintf(prbuf_query,
> + "q->function=\"%s\" q->filename=\"%s\" "
> + "q->module=\"%s\" q->format=\"%s\" q->lineno=%u-%u",
> + q->function, q->filename, q->module, q->format,
> + q->first_lineno, q->last_lineno);
> +
> + return prbuf_query;
> +}
> +
> +/* copy query off stack, save flags & mask, and store in pending-list */
> +static void add_to_pending(const struct ddebug_query *query,
> + unsigned int flags, unsigned int mask)
> +{
> + struct pending_query *pq;
> +
> + if (pending_ct >= pending_max) {
> + pr_warn("ddebug: no matches for query, pending is full\n");
> + return;
> + }
> + if (verbose)
> + pr_info("add to pending: %s\n", show_ddebug_query(query));
> +
> + pending_ct++;
> + pq = kzalloc(sizeof(struct pending_query), GFP_KERNEL);
> +
> + /* copy non-null match-specs into allocd mem, update pointers */
> + if (query->module)
> + pq->query.module = strcpy(pq->module, query->module);
> + if (query->function)
> + pq->query.function = strcpy(pq->function, query->function);
> + if (query->filename)
> + pq->query.filename = strcpy(pq->filename, query->filename);
> + if (query->format)
> + pq->query.format = strcpy(pq->format, query->format);
> +
> + pq->flags = flags;
> + pq->mask = mask;
> +
> + mutex_lock(&ddebug_lock);
> + list_add(&pq->link, &pending_queries);
> + mutex_unlock(&ddebug_lock);
> +
> + if (verbose)
> + pr_info("ddebug: query saved as pending %d\n", pending_ct);
> +}
> +
> /*
> * Search the tables for _ddebug's which match the given
> * `query' and apply the `flags' and `mask' to them. Tells
> @@ -348,11 +414,7 @@ static int ddebug_parse_query(char *words[], int nwords,
> }
>
> 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);
> + pr_info("parsed %s\n", show_ddebug_query(query));
>
> return 0;
> }
> @@ -437,6 +499,10 @@ static int ddebug_exec_query(char *query_string)
> /* actually go and implement the change */
> nfound = ddebug_change(&query, flags, mask);
>
> + pr_info("nfound %d on %s\n", nfound, show_ddebug_query(&query));
> + if (!nfound)
> + ddebug_add_to_pending(&query, flags, mask);
> +
> return 0;
> }
>
> --
> 1.7.4.1
>
On Tue, Jun 28, 2011 at 12:24:56PM +0200, Bart Van Assche wrote:
> On Tue, Jun 28, 2011 at 9:09 AM, Jim Cromie <[email protected]> wrote:
> > @@ -425,6 +424,7 @@ static int ddebug_exec_query(char *query_string)
> > ?#define MAXWORDS 9
> > ? ? ? ?int nwords;
> > ? ? ? ?char *words[MAXWORDS];
> > + ? ? ? int nfound;
> >
> > ? ? ? ?nwords = ddebug_tokenize(query_string, words, MAXWORDS);
> > ? ? ? ?if (nwords <= 0)
> > @@ -435,7 +435,8 @@ static int ddebug_exec_query(char *query_string)
> > ? ? ? ? ? ? ? ?return -EINVAL;
> >
> > ? ? ? ?/* actually go and implement the change */
> > - ? ? ? ddebug_change(&query, flags, mask);
> > + ? ? ? nfound = ddebug_change(&query, flags, mask);
> > +
> > ? ? ? ?return 0;
>
> Do these changes actually do anything, or did I miss something ?
>
> Bart.
its used in a subsequent patch to decide whether or not to call
add_to_pending.
thanks,
-Jason
On Tue, Jun 28, 2011 at 4:17 AM, Bart Van Assche <[email protected]> wrote:
> On Tue, Jun 28, 2011 at 9:09 AM, Jim Cromie <[email protected]> wrote:
>> - ? ? ? ? ? ? ? ? ? ? ? query->format = unescape(words[i+1]);
>> + ? ? ? ? ? ? ? ? ? ? ? check_set(&query->format, words[i+1], "format");
>
> Was it your intention to remove the call to "unescape" ?
>
> Bart.
>
No, that was accidental. thanks for pointing it out.
On Tue, Jun 28, 2011 at 4:54 PM, Jason Baron <[email protected]> wrote:
> On Tue, Jun 28, 2011 at 12:24:56PM +0200, Bart Van Assche wrote:
> > On Tue, Jun 28, 2011 at 9:09 AM, Jim Cromie <[email protected]> wrote:
> > > @@ -425,6 +424,7 @@ static int ddebug_exec_query(char *query_string)
> > > ?#define MAXWORDS 9
> > > ? ? ? ?int nwords;
> > > ? ? ? ?char *words[MAXWORDS];
> > > + ? ? ? int nfound;
> > >
> > > ? ? ? ?nwords = ddebug_tokenize(query_string, words, MAXWORDS);
> > > ? ? ? ?if (nwords <= 0)
> > > @@ -435,7 +435,8 @@ static int ddebug_exec_query(char *query_string)
> > > ? ? ? ? ? ? ? ?return -EINVAL;
> > >
> > > ? ? ? ?/* actually go and implement the change */
> > > - ? ? ? ddebug_change(&query, flags, mask);
> > > + ? ? ? nfound = ddebug_change(&query, flags, mask);
> > > +
> > > ? ? ? ?return 0;
> >
> > Do these changes actually do anything, or did I miss something ?
>
> its used in a subsequent patch to decide whether or not to call
> add_to_pending.
As far as I can see your comment applies to the function
ddebug_change() while my comment applies to the function
ddebug_exec_query(). If you have a close look at the above changes you
will see that these do nothing more than adding a dead assignment.
Bart.
On Tue, Jun 28, 2011 at 11:48 AM, Bart Van Assche <[email protected]> wrote:
> On Tue, Jun 28, 2011 at 4:54 PM, Jason Baron <[email protected]> wrote:
>> On Tue, Jun 28, 2011 at 12:24:56PM +0200, Bart Van Assche wrote:
>> > On Tue, Jun 28, 2011 at 9:09 AM, Jim Cromie <[email protected]> wrote:
>> > > @@ -425,6 +424,7 @@ static int ddebug_exec_query(char *query_string)
>> > > ?#define MAXWORDS 9
>> > > ? ? ? ?int nwords;
>> > > ? ? ? ?char *words[MAXWORDS];
>> > > + ? ? ? int nfound;
>> > >
>> > > ? ? ? ?nwords = ddebug_tokenize(query_string, words, MAXWORDS);
>> > > ? ? ? ?if (nwords <= 0)
>> > > @@ -435,7 +435,8 @@ static int ddebug_exec_query(char *query_string)
>> > > ? ? ? ? ? ? ? ?return -EINVAL;
>> > >
>> > > ? ? ? ?/* actually go and implement the change */
>> > > - ? ? ? ddebug_change(&query, flags, mask);
>> > > + ? ? ? nfound = ddebug_change(&query, flags, mask);
>> > > +
>> > > ? ? ? ?return 0;
>> >
>> > Do these changes actually do anything, or did I miss something ?
>>
>> its used in a subsequent patch to decide whether or not to call
>> add_to_pending.
>
> As far as I can see your comment applies to the function
> ddebug_change() while my comment applies to the function
> ddebug_exec_query(). If you have a close look at the above changes you
> will see that these do nothing more than adding a dead assignment.
>
> Bart.
>
its dead in 8/11, but used in 9/11
nfound = ddebug_change(&query, flags, mask);
+ pr_info("nfound %d on %s\n", nfound, show_ddebug_query(&query));
+ if (!nfound)
+ ddebug_add_to_pending(&query, flags, mask);
+
return 0;
I can merge 8 & 9 if it matters,
but ISTM that the important part is that the patchset is bisectable.
- Things compile at each patch (with a couple warnings,
including unescape, which I fat-fingered and will fix)
$ for i in `seq 1 10`; do (cd - && git checkout HEAD~1 && git status);
make; done
- and functionality doesnt regress
AFAIK - I may have botched something while rebasing bits and pieces.
On Tue, Jun 28, 2011 at 4:08 AM, Bart Van Assche <[email protected]> wrote:
> On Tue, Jun 28, 2011 at 9:09 AM, Jim Cromie <[email protected]> wrote:
>>
>> Skip past unchanging absolute source-path prefix to print a path
>> thats relative to kernel source's root-dir. ?This makes the file
>> easier to read without a wide screen. ?For example:
>>
>> kernel/freezer.c:128 [freezer]cancel_freezing - " ?clean up: %s\012"
>
> The above will break the output generated for out-of-tree kernel
> modules that use the dynamic debug facility.
>
> Bart.
>
ack, yes.
I'll add a check to see prefix matches before skipping it.
this will reduce efficiency slightly more, but its not a hot path.
thanks.
On Tue, Jun 28, 2011 at 1:09 AM, Jim Cromie <[email protected]> wrote:
> When a query/rule doesnt match, call add_to_pending(new function)
> to copy the query off the stack, into a (new) struct pending_query,
> and add to pending_queries (new) list.
>
> Patch adds: /sys/module/dynamic_debug/parameters/
> ?verbose - rw, added previously, here for completeness
> ?pending_ct - ro, shows current count of pending queries
> ?pending_max - rw, set max pending, further pending queries are rejected
>
> With this and previous patches, queries added on the boot-line which
> do not match (because module is not built-in, and thus not present yet)
> are added to pending_queries.
>
> IE, with these boot-line additions:
> ?dynamic_debug.verbose=1 ddebug_query="module scx200_hrt +p"
>
> Kernel will log the following:
>
> ddebug_exec_queries: query 0: "module scx200_hrt +p"
> ddebug_tokenize: split into words: "module" "scx200_hrt" "+p"
> ddebug_parse_query: parsed q->function="(null)" q->filename="(null)" q->module="scx200_hrt" q->format="(null)" q->lineno=0-0
> ddebug_parse_flags: op='+'
> ddebug_parse_flags: flags=0x1
> ddebug_parse_flags: *flagsp=0x1 *maskp=0xffffffff
> ddebug_exec_query: nfound 0 on q->function="(null)" q->filename="(null)" q->module="scx200_hrt" q->format="(null)" q->lineno=0-0
> ddebug_add_to_pending: add to pending: q->function="(null)" q->filename="(null)" q->module="scx200_hrt" q->format="(null)" q->lineno=0-0
> ddebug_add_to_pending: ddebug: query saved as pending 1
>
> Signed-off-by: Jim Cromie <[email protected]>
> ---
> ?lib/dynamic_debug.c | ? 78 +++++++++++++++++++++++++++++++++++++++++++++++----
> ?1 files changed, 72 insertions(+), 6 deletions(-)
>
> diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
> index 9b8b1e8..37d0275 100644
> --- a/lib/dynamic_debug.c
> +++ b/lib/dynamic_debug.c
> @@ -53,6 +53,16 @@ struct ddebug_query {
> ? ? ? ?unsigned int first_lineno, last_lineno;
> ?};
>
> +struct pending_query {
> + ? ? ? struct list_head link;
> + ? ? ? struct ddebug_query query;
> + ? ? ? char filename[100];
> + ? ? ? char module[30];
> + ? ? ? char function[50];
> + ? ? ? char format[200];
> + ? ? ? unsigned int flags, mask;
> +};
> +
> ?struct ddebug_iter {
> ? ? ? ?struct ddebug_table *table;
> ? ? ? ?unsigned int idx;
> @@ -61,7 +71,13 @@ struct ddebug_iter {
> ?static DEFINE_MUTEX(ddebug_lock);
> ?static LIST_HEAD(ddebug_tables);
> ?static int verbose = 0;
> -module_param(verbose, int, 0744);
> +module_param(verbose, int, 0644);
> +
> +/* legal but inapplicable queries, save and test against new modules */
> +static LIST_HEAD(pending_queries);
> +static int pending_ct, pending_max = 100;
> +module_param(pending_ct, int, 0444);
> +module_param(pending_max, int, 0644);
>
> ?/* Return the last part of a pathname */
> ?static inline const char *basename(const char *path)
> @@ -96,6 +112,56 @@ static char *ddebug_describe_flags(struct _ddebug *dp, char *buf,
> ? ? ? ?return buf;
> ?}
>
> +static char prbuf_query[300];
> +
> +static char *show_ddebug_query(const struct ddebug_query *q)
> +{
this part (the static fixed-length var)
gives me some heartburn.
I almost went with macros using pr_info to replace the
2 show_*_query fns, but a <debugfs/dynamic_debug/pending file
might want to use show_pending_query
advice ?
On Tue, Jun 28, 2011 at 9:09 AM, Jim Cromie <[email protected]> wrote:
> allow changing dynamic_debug verbosity
> at boot-time, with: dynamic_debug.verbose=1
> or at runtime with:
> root@voyage:~# echo 1 > /sys/module/dynamic_debug/parameters/verbose
>
> Signed-off-by: Jim Cromie <[email protected]>
> ---
> ?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 75ca78f..a3b08d5 100644
> --- a/lib/dynamic_debug.c
> +++ b/lib/dynamic_debug.c
> @@ -58,6 +58,7 @@ struct ddebug_iter {
> ?static DEFINE_MUTEX(ddebug_lock);
> ?static LIST_HEAD(ddebug_tables);
> ?static int verbose = 0;
> +module_param(verbose, int, 0744);
Why 0744 and not 0644 ? Why to set the 'executable' bit ?
Bart.
On Tue, Jun 28, 2011 at 9:09 AM, Jim Cromie <[email protected]> wrote:
> Process multiple commands per line, separated by ';'. ?All commands are
> processed, independent of errors, allowing individual commands to fail,
> for example when a module is not installed. ?Last error code is returned.
> With this, extensive command sets can be given on the boot-line.
>
> Signed-off-by: Jim Cromie <[email protected]>
> ---
> ?Documentation/dynamic-debug-howto.txt | ? 14 ++++++++++-
> ?lib/dynamic_debug.c ? ? ? ? ? ? ? ? ? | ? 39 +++++++++++++++++++++++++++++++-
> ?2 files changed, 49 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/dynamic-debug-howto.txt b/Documentation/dynamic-debug-howto.txt
> index f959909..d0faf98 100644
> --- a/Documentation/dynamic-debug-howto.txt
> +++ b/Documentation/dynamic-debug-howto.txt
> @@ -92,8 +92,18 @@ 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:
> +Commands are bounded by a write() system call. ?Subject to this limit
> +(or 1024 for boot-line parameter) you can send multiple commands,
> +separated by ';'
> +
> +foo:~ # echo "module nsc_gpio +p ; module pc8736x_gpio +p ; " \
> + "module scx200_gpio +p " > /dbg/dynamic_debug/control
> +
> +Multiple commands are processed independently, this allows you to send
> +commands which may fail, for example if a module is not present. ?The
> +last failing command returns its error.
> +
> +Or you can do an "echo" for each, like:
>
> ?nullarbor:~ # echo 'file svcsock.c line 1603 +p' > /proc/dprintk ;\
> ?> echo 'file svcsock.c line 1563 +p' > /proc/dprintk
> diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
> index eb08a2f..0e567ad 100644
> --- a/lib/dynamic_debug.c
> +++ b/lib/dynamic_debug.c
> @@ -428,6 +428,41 @@ static int ddebug_exec_query(char *query_string)
> ? ? ? ?return 0;
> ?}
>
> +/* handle multiple queries, continue on error, return last error */
> +static int ddebug_exec_queries(char *query)
> +{
> + ? ? ? char *split = query;
> + ? ? ? int i, errs = 0, exitcode = 0, rc;
> +
> + ? ? ? if (verbose)
> + ? ? ? ? ? ? ? /* clean up for logging */
> + ? ? ? ? ? ? ? for (; (split = strpbrk(split, "\t\n")); split++)
> + ? ? ? ? ? ? ? ? ? ? ? *split = ' ';
The above will join multiple lines into a single line and hence will
cause a shell statement like the one below to be interpret as a single
query:
printf "module nsc_gpio +p\n module pc8736x_gpio +p\n" > /proc/dprintk
Are you sure that's how things should work ?
> + ? ? ? for (i = 0; query; query = split, i++) {
> +
No blank line past "for" please.
> + ? ? ? ? ? ? ? split = strchr(query, ';');
> + ? ? ? ? ? ? ? if (split)
> + ? ? ? ? ? ? ? ? ? ? ? *split++ = '\0';
> +
> + ? ? ? ? ? ? ? if (verbose)
> + ? ? ? ? ? ? ? ? ? ? ? printk(KERN_INFO "%s: query %d: \"%s\"",
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? __func__, i, query);
> +
> + ? ? ? ? ? ? ? rc = ddebug_exec_query(query);
> + ? ? ? ? ? ? ? if (rc) {
> + ? ? ? ? ? ? ? ? ? ? ? errs++;
> + ? ? ? ? ? ? ? ? ? ? ? exitcode = rc;
> + ? ? ? ? ? ? ? }
> + ? ? ? }
> + ? ? ? if (verbose)
> + ? ? ? ? ? ? ? printk(KERN_INFO
> + ? ? ? ? ? ? ? ? ? ? ? "%s: processed %d queries, with %d errs",
> + ? ? ? ? ? ? ? ? ? ? ? __func__, i, errs);
> +
> + ? ? ? return exitcode;
> +}
> +
> ?int __dynamic_pr_debug(struct _ddebug *descriptor, const char *fmt, ...)
> ?{
> ? ? ? ?va_list args;
> @@ -492,7 +527,7 @@ static ssize_t ddebug_proc_write(struct file *file, const char __user *ubuf,
> ? ? ? ? ? ? ? ?printk(KERN_INFO "%s: read %d bytes from userspace\n",
> ? ? ? ? ? ? ? ? ? ? ? ?__func__, (int)len);
>
> - ? ? ? ret = ddebug_exec_query(tmpbuf);
> + ? ? ? ret = ddebug_exec_queries(tmpbuf);
> ? ? ? ?if (ret)
> ? ? ? ? ? ? ? ?return ret;
>
> @@ -804,7 +839,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_query(ddebug_setup_string);
> + ? ? ? ? ? ? ? ret = ddebug_exec_queries(ddebug_setup_string);
> ? ? ? ? ? ? ? ?if (ret)
> ? ? ? ? ? ? ? ? ? ? ? ?pr_warning("Invalid ddebug boot param %s",
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ddebug_setup_string);
> --
> 1.7.4.1
>
>