* [PATCH 01/21] dynamic_debug: add pending flag 'a' to make pending queries explicit
2011-07-11 7:46 ` Jim Cromie
@ 2011-07-11 7:46 ` Jim Cromie
2011-07-11 7:46 ` [PATCH 02/21] dynamic_debug: allow changing of dynamic_debug verbosity any time Jim Cromie
` (19 subsequent siblings)
20 siblings, 0 replies; 72+ messages in thread
From: Jim Cromie @ 2011-07-11 7:46 UTC (permalink / raw)
To: jbaron; +Cc: linux-kernel, bvanassche, joe, gregkh, gnb, Jim Cromie
this just adds flag to header, so it can be rebased to the front of
the patchset, so that rebuilds within the set are faster.
Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
include/linux/dynamic_debug.h | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index fdd3c0e..079a9e9 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -26,6 +26,7 @@ struct _ddebug {
#define _DPRINTK_FLAGS_INCL_FUNCNAME (1<<2)
#define _DPRINTK_FLAGS_INCL_LINENO (1<<3)
#define _DPRINTK_FLAGS_INCL_TID (1<<4)
+#define _DPRINTK_FLAGS_APPEND (1<<5) /* add query to pending list */
#define _DPRINTK_FLAGS_DEFAULT 0
unsigned int flags:8;
char enabled;
--
1.7.4.1
^ permalink raw reply related [flat|nested] 72+ messages in thread* [PATCH 02/21] dynamic_debug: allow changing of dynamic_debug verbosity any time
2011-07-11 7:46 ` Jim Cromie
2011-07-11 7:46 ` [PATCH 01/21] dynamic_debug: add pending flag 'a' to make pending queries explicit Jim Cromie
@ 2011-07-11 7:46 ` Jim Cromie
2011-07-11 7:46 ` [PATCH 03/21] dynamic_debug: process multiple commands on a line Jim Cromie
` (18 subsequent siblings)
20 siblings, 0 replies; 72+ messages in thread
From: Jim Cromie @ 2011-07-11 7:46 UTC (permalink / raw)
To: jbaron; +Cc: linux-kernel, bvanassche, joe, gregkh, gnb, Jim Cromie
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 <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 fbb4547..1597d5a 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -61,6 +61,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.4.1
^ permalink raw reply related [flat|nested] 72+ messages in thread* [PATCH 03/21] dynamic_debug: process multiple commands on a line
2011-07-11 7:46 ` Jim Cromie
2011-07-11 7:46 ` [PATCH 01/21] dynamic_debug: add pending flag 'a' to make pending queries explicit Jim Cromie
2011-07-11 7:46 ` [PATCH 02/21] dynamic_debug: allow changing of dynamic_debug verbosity any time Jim Cromie
@ 2011-07-11 7:46 ` Jim Cromie
2011-07-12 5:54 ` Bart Van Assche
2011-07-11 7:46 ` [PATCH 04/21] dynamic_debug: warn when >1 of each type of match-spec is given Jim Cromie
` (17 subsequent siblings)
20 siblings, 1 reply; 72+ messages in thread
From: Jim Cromie @ 2011-07-11 7:46 UTC (permalink / raw)
To: jbaron; +Cc: linux-kernel, bvanassche, joe, gregkh, gnb, Jim Cromie
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 <jim.cromie@gmail.com>
---
lib/dynamic_debug.c | 30 ++++++++++++++++++++++++++++--
1 files changed, 28 insertions(+), 2 deletions(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 1597d5a..ae72402 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -423,6 +423,32 @@ 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;
+ int i, errs = 0, exitcode = 0, rc;
+
+ for (i = 0; query; query = split, i++) {
+ split = strchr(query, ';');
+ if (split)
+ *split++ = '\0';
+
+ if (verbose)
+ pr_info("query %d: \"%s\"\n", i, query);
+
+ rc = ddebug_exec_query(query);
+ if (rc) {
+ errs++;
+ exitcode = rc;
+ }
+ }
+ if (verbose)
+ pr_info("processed %d queries, with %d errs\n", i, errs);
+
+ return exitcode;
+}
+
static int dynamic_emit_prefix(const struct _ddebug *descriptor)
{
char tid[sizeof(int) + sizeof(int)/2 + 4];
@@ -557,7 +583,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_query(tmpbuf);
+ ret = ddebug_exec_queries(tmpbuf);
if (ret)
return ret;
@@ -862,7 +888,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_warn("Invalid ddebug boot param %s",
ddebug_setup_string);
--
1.7.4.1
^ permalink raw reply related [flat|nested] 72+ messages in thread* Re: [PATCH 03/21] dynamic_debug: process multiple commands on a line
2011-07-11 7:46 ` [PATCH 03/21] dynamic_debug: process multiple commands on a line Jim Cromie
@ 2011-07-12 5:54 ` Bart Van Assche
2011-07-13 7:25 ` Jim Cromie
0 siblings, 1 reply; 72+ messages in thread
From: Bart Van Assche @ 2011-07-12 5:54 UTC (permalink / raw)
To: Jim Cromie; +Cc: jbaron, linux-kernel, joe, gregkh, gnb
On Mon, Jul 11, 2011 at 9:46 AM, Jim Cromie <jim.cromie@gmail.com> 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 <jim.cromie@gmail.com>
> ---
> lib/dynamic_debug.c | 30 ++++++++++++++++++++++++++++--
> 1 files changed, 28 insertions(+), 2 deletions(-)
>
> diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
> index 1597d5a..ae72402 100644
> --- a/lib/dynamic_debug.c
> +++ b/lib/dynamic_debug.c
> @@ -423,6 +423,32 @@ 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;
> + int i, errs = 0, exitcode = 0, rc;
> +
> + for (i = 0; query; query = split, i++) {
> + split = strchr(query, ';');
> + if (split)
> + *split++ = '\0';
> +
> + if (verbose)
> + pr_info("query %d: \"%s\"\n", i, query);
> +
> + rc = ddebug_exec_query(query);
> + if (rc) {
> + errs++;
> + exitcode = rc;
> + }
> + }
> + if (verbose)
> + pr_info("processed %d queries, with %d errs\n", i, errs);
> +
> + return exitcode;
> +}
> +
Does the above mean that semicolon is considered a valid separator but
newline not ?
Bart.
^ permalink raw reply [flat|nested] 72+ messages in thread* Re: [PATCH 03/21] dynamic_debug: process multiple commands on a line
2011-07-12 5:54 ` Bart Van Assche
@ 2011-07-13 7:25 ` Jim Cromie
2011-07-13 17:44 ` Bart Van Assche
0 siblings, 1 reply; 72+ messages in thread
From: Jim Cromie @ 2011-07-13 7:25 UTC (permalink / raw)
To: Bart Van Assche; +Cc: jbaron, linux-kernel, joe, gregkh, gnb
On Mon, Jul 11, 2011 at 11:54 PM, Bart Van Assche <bvanassche@acm.org> wrote:
> On Mon, Jul 11, 2011 at 9:46 AM, Jim Cromie <jim.cromie@gmail.com> 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.
> Does the above mean that semicolon is considered a valid separator but
> newline not ?
>
> Bart.
>
no, your printf example still works.
Note that it actually issues 2 writes to the control-file.
root@voyage:~# echo 1 > /sys/module/dynamic_debug/parameters/verbose
trol voyage:~# printf "module foo +ap\n module bar +ap" >
/dbg/dynamic_debug/cont
dynamic_debug:ddebug_proc_open: called
dynamic_debug:ddebug_proc_write: read 15 bytes from userspace
dynamic_debug:ddebug_exec_queries: query 0: "module foo +ap
"
dynamic_debug:ddebug_tokenize: split into words: "module" "foo" "+ap"
dynamic_debug:ddebug_parse_query: parsed func="" file="" module="foo"
format="" lineno=0-0
dynamic_debug:ddebug_parse_flags: flags_filter=0x0 op='+' flags=0x21
dynamic_debug:ddebug_parse_flags: *flagsp=0x21 *maskp=0xffffffff
dynamic_debug:ddebug_save_pending: add to pending: func="" file=""
module="foo" format="" lineno=0-0
dynamic_debug:ddebug_save_pending: query saved as pending 1
dynamic_debug:ddebug_exec_query: nfound 0 on func="" file=""
module="foo" format="" lineno=0-0
dynamic_debug:ddebug_exec_queries: processed 1 queries, with 0 errs
dynamic_debug:ddebug_proc_write: read 15 bytes from userspace
dynamic_debug:ddebug_exec_queries: query 0: " module bar +ap"
dynamic_debug:ddebug_tokenize: split into words: "module" "bar" "+ap"
dynamic_debug:ddebug_parse_query: parsed func="" file="" module="bar"
format="" lineno=0-0
dynamic_debug:ddebug_parse_flags: flags_filter=0x0 op='+' flags=0x21
dynamic_debug:ddebug_parse_flags: *flagsp=0x21 *maskp=0xffffffff
dynamic_debug:ddebug_save_pending: add to pending: func="" file=""
module="bar" format="" lineno=0-0
dynamic_debug:ddebug_save_pending: query saved as pending 2
dynamic_debug:ddebug_exec_query: nfound 0 on func="" file=""
module="bar" format="" lineno=0-0
root@voyage:~#
FWIW, the \n approach doesnt help on the boot-line;
on mainline, the following will break the facility -
/dbg/dynamic_debug will not get created.
Kernel command line: ... ddebug_query="module pc8736x_gpio +p \n
module nsc_gpio +p "
^ permalink raw reply [flat|nested] 72+ messages in thread
* Re: [PATCH 03/21] dynamic_debug: process multiple commands on a line
2011-07-13 7:25 ` Jim Cromie
@ 2011-07-13 17:44 ` Bart Van Assche
0 siblings, 0 replies; 72+ messages in thread
From: Bart Van Assche @ 2011-07-13 17:44 UTC (permalink / raw)
To: Jim Cromie; +Cc: jbaron, linux-kernel, joe, gregkh, gnb
On Wed, Jul 13, 2011 at 9:25 AM, Jim Cromie <jim.cromie@gmail.com> wrote:
> On Mon, Jul 11, 2011 at 11:54 PM, Bart Van Assche <bvanassche@acm.org> wrote:
> > On Mon, Jul 11, 2011 at 9:46 AM, Jim Cromie <jim.cromie@gmail.com> 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.
> >
> > Does the above mean that semicolon is considered a valid separator but
> > newline not ?
>
> no, your printf example still works.
> Note that it actually issues 2 writes to the control-file.
>
> root@voyage:~# echo 1 > /sys/module/dynamic_debug/parameters/verbose
> trol voyage:~# printf "module foo +ap\n module bar +ap" >
> /dbg/dynamic_debug/cont
>
> FWIW, the \n approach doesnt help on the boot-line;
> on mainline, the following will break the facility -
> /dbg/dynamic_debug will not get created.
>
> Kernel command line: ... ddebug_query="module pc8736x_gpio +p \n
> module nsc_gpio +p "
Just as a note, I do not expect that anyone will pass a string
containing an embedded newline via the kernel command line but I do
expect that people will use cat or printf to send multiple queries at
once to /sys/kernel/debug/dynamic_debug/control.
Bart.
^ permalink raw reply [flat|nested] 72+ messages in thread
* [PATCH 04/21] dynamic_debug: warn when >1 of each type of match-spec is given
2011-07-11 7:46 ` Jim Cromie
` (2 preceding siblings ...)
2011-07-11 7:46 ` [PATCH 03/21] dynamic_debug: process multiple commands on a line Jim Cromie
@ 2011-07-11 7:46 ` Jim Cromie
2011-07-11 7:46 ` [PATCH 05/21] dynamic_debug: pr_err() call should not depend upon verbosity Jim Cromie
` (16 subsequent siblings)
20 siblings, 0 replies; 72+ messages in thread
From: Jim Cromie @ 2011-07-11 7:46 UTC (permalink / raw)
To: jbaron; +Cc: linux-kernel, bvanassche, joe, gregkh, gnb, Jim Cromie
Issue warning 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.
Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
lib/dynamic_debug.c | 20 ++++++++++++++++----
1 files changed, 16 insertions(+), 4 deletions(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index ae72402..0575113 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -281,6 +281,14 @@ static char *unescape(char *str)
return str;
}
+static inline void check_set(const char **dest, char *src, char *name)
+{
+ if (*dest)
+ pr_warn("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:
@@ -292,6 +300,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)
@@ -305,13 +316,14 @@ 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, unescape(words[i+1]),
+ "format");
else if (!strcmp(words[i], "line")) {
char *first = words[i+1];
char *last = strchr(first, '-');
--
1.7.4.1
^ permalink raw reply related [flat|nested] 72+ messages in thread* [PATCH 05/21] dynamic_debug: pr_err() call should not depend upon verbosity
2011-07-11 7:46 ` Jim Cromie
` (3 preceding siblings ...)
2011-07-11 7:46 ` [PATCH 04/21] dynamic_debug: warn when >1 of each type of match-spec is given Jim Cromie
@ 2011-07-11 7:46 ` Jim Cromie
2011-07-11 7:46 ` [PATCH 06/21] dynamic_debug: dont kill entire facility on error parsing ddebug_query Jim Cromie
` (15 subsequent siblings)
20 siblings, 0 replies; 72+ messages in thread
From: Jim Cromie @ 2011-07-11 7:46 UTC (permalink / raw)
To: jbaron; +Cc: linux-kernel, bvanassche, joe, gregkh, gnb, Jim Cromie
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 <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 0575113..fc31707 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -339,8 +339,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.4.1
^ permalink raw reply related [flat|nested] 72+ messages in thread* [PATCH 06/21] dynamic_debug: dont kill entire facility on error parsing ddebug_query
2011-07-11 7:46 ` Jim Cromie
` (4 preceding siblings ...)
2011-07-11 7:46 ` [PATCH 05/21] dynamic_debug: pr_err() call should not depend upon verbosity Jim Cromie
@ 2011-07-11 7:46 ` Jim Cromie
2011-07-11 7:46 ` [PATCH 07/21] dynamic_debug: return int from ddebug_change Jim Cromie
` (14 subsequent siblings)
20 siblings, 0 replies; 72+ messages in thread
From: Jim Cromie @ 2011-07-11 7:46 UTC (permalink / raw)
To: jbaron; +Cc: linux-kernel, bvanassche, joe, gregkh, gnb, Jim Cromie
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: ... 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 query parse errors before
returning from dynamic_debug_init().
Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
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 fc31707..bbee0aa 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -900,10 +900,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; /* make this non-fatal */
pr_warn("Invalid ddebug boot param %s",
ddebug_setup_string);
- else
+ } else
pr_info("ddebug initialized with string %s",
ddebug_setup_string);
}
--
1.7.4.1
^ permalink raw reply related [flat|nested] 72+ messages in thread* [PATCH 07/21] dynamic_debug: return int from ddebug_change
2011-07-11 7:46 ` Jim Cromie
` (5 preceding siblings ...)
2011-07-11 7:46 ` [PATCH 06/21] dynamic_debug: dont kill entire facility on error parsing ddebug_query Jim Cromie
@ 2011-07-11 7:46 ` Jim Cromie
2011-07-11 18:39 ` Bart Van Assche
2011-07-11 7:46 ` [PATCH 08/21] dynamic_debug: factor show_ddebug_query out of ddebug_parse_query Jim Cromie
` (13 subsequent siblings)
20 siblings, 1 reply; 72+ messages in thread
From: Jim Cromie @ 2011-07-11 7:46 UTC (permalink / raw)
To: jbaron; +Cc: linux-kernel, bvanassche, joe, gregkh, gnb, Jim Cromie
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 <jim.cromie@gmail.com>
---
lib/dynamic_debug.c | 10 +++++-----
1 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index bbee0aa..de2a679 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;
@@ -167,8 +167,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;
}
/*
@@ -420,6 +419,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)
@@ -430,7 +430,7 @@ 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
^ permalink raw reply related [flat|nested] 72+ messages in thread* Re: [PATCH 07/21] dynamic_debug: return int from ddebug_change
2011-07-11 7:46 ` [PATCH 07/21] dynamic_debug: return int from ddebug_change Jim Cromie
@ 2011-07-11 18:39 ` Bart Van Assche
0 siblings, 0 replies; 72+ messages in thread
From: Bart Van Assche @ 2011-07-11 18:39 UTC (permalink / raw)
To: Jim Cromie; +Cc: jbaron, linux-kernel, joe, gregkh, gnb
On Mon, Jul 11, 2011 at 9:46 AM, Jim Cromie <jim.cromie@gmail.com> wrote:
> 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 <jim.cromie@gmail.com>
> ---
> lib/dynamic_debug.c | 10 +++++-----
> 1 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
> index bbee0aa..de2a679 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;
> @@ -167,8 +167,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;
> }
>
> /*
> @@ -420,6 +419,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)
> @@ -430,7 +430,7 @@ 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;
> }
I still see the introduction of a dead assignment in the second half
of the above patch. After the first series had been posted you had
proposed yourself to merge this patch with the subsequent patch.
Apparently that hasn't been done yet ?
Bart.
^ permalink raw reply [flat|nested] 72+ messages in thread
* [PATCH 08/21] dynamic_debug: factor show_ddebug_query out of ddebug_parse_query
2011-07-11 7:46 ` Jim Cromie
` (6 preceding siblings ...)
2011-07-11 7:46 ` [PATCH 07/21] dynamic_debug: return int from ddebug_change Jim Cromie
@ 2011-07-11 7:46 ` Jim Cromie
2011-07-11 18:36 ` Bart Van Assche
2011-07-11 7:46 ` [PATCH 09/21] dynamic_debug: save_pending() saves non-matching queries for later Jim Cromie
` (12 subsequent siblings)
20 siblings, 1 reply; 72+ messages in thread
From: Jim Cromie @ 2011-07-11 7:46 UTC (permalink / raw)
To: jbaron; +Cc: linux-kernel, bvanassche, joe, gregkh, gnb, Jim Cromie
Will reuse for show_pending_query too. Alloc and free
print buffer space inside ddebug_exec_queries, instead of
a permanent static allocation.
Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
lib/dynamic_debug.c | 24 +++++++++++++++++++-----
1 files changed, 19 insertions(+), 5 deletions(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index de2a679..81268e2 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -96,6 +96,19 @@ static char *ddebug_describe_flags(struct _ddebug *dp, char *buf,
return buf;
}
+static char *prbuf_query;
+
+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;
+}
+
/*
* Search the tables for _ddebug's which match the given
* `query' and apply the `flags' and `mask' to them. Tells
@@ -344,11 +357,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;
}
@@ -440,6 +449,10 @@ static int ddebug_exec_queries(char *query)
char *split;
int i, errs = 0, exitcode = 0, rc;
+ prbuf_query = kmalloc(1024, GFP_KERNEL);
+ if (prbuf_query == NULL)
+ return -ENOMEM;
+
for (i = 0; query; query = split, i++) {
split = strchr(query, ';');
if (split)
@@ -454,6 +467,7 @@ static int ddebug_exec_queries(char *query)
exitcode = rc;
}
}
+ kfree(prbuf_query);
if (verbose)
pr_info("processed %d queries, with %d errs\n", i, errs);
--
1.7.4.1
^ permalink raw reply related [flat|nested] 72+ messages in thread* Re: [PATCH 08/21] dynamic_debug: factor show_ddebug_query out of ddebug_parse_query
2011-07-11 7:46 ` [PATCH 08/21] dynamic_debug: factor show_ddebug_query out of ddebug_parse_query Jim Cromie
@ 2011-07-11 18:36 ` Bart Van Assche
2011-07-16 21:32 ` Jim Cromie
0 siblings, 1 reply; 72+ messages in thread
From: Bart Van Assche @ 2011-07-11 18:36 UTC (permalink / raw)
To: Jim Cromie; +Cc: jbaron, linux-kernel, joe, gregkh, gnb
On Mon, Jul 11, 2011 at 9:46 AM, Jim Cromie <jim.cromie@gmail.com> wrote:
> Will reuse for show_pending_query too. Alloc and free
> print buffer space inside ddebug_exec_queries, instead of
> a permanent static allocation.
>
> Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
> ---
> lib/dynamic_debug.c | 24 +++++++++++++++++++-----
> 1 files changed, 19 insertions(+), 5 deletions(-)
>
> diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
> index de2a679..81268e2 100644
> --- a/lib/dynamic_debug.c
> +++ b/lib/dynamic_debug.c
> @@ -96,6 +96,19 @@ static char *ddebug_describe_flags(struct _ddebug *dp, char *buf,
> return buf;
> }
>
> +static char *prbuf_query;
> +
> +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;
> +}
> +
> /*
> * Search the tables for _ddebug's which match the given
> * `query' and apply the `flags' and `mask' to them. Tells
> @@ -344,11 +357,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;
> }
> @@ -440,6 +449,10 @@ static int ddebug_exec_queries(char *query)
> char *split;
> int i, errs = 0, exitcode = 0, rc;
>
> + prbuf_query = kmalloc(1024, GFP_KERNEL);
> + if (prbuf_query == NULL)
> + return -ENOMEM;
> +
> for (i = 0; query; query = split, i++) {
> split = strchr(query, ';');
> if (split)
> @@ -454,6 +467,7 @@ static int ddebug_exec_queries(char *query)
> exitcode = rc;
> }
> }
> + kfree(prbuf_query);
> if (verbose)
> pr_info("processed %d queries, with %d errs\n", i, errs);
Why to invoke kmalloc() with a magic constant as the first argument
while the above code can be simplified by replacing the sprintf() call
(that is missing an output buffer size check) by a kasprintf() call ?
And why does the static variable 'prbuf_query' exist at all ?
Bart.
^ permalink raw reply [flat|nested] 72+ messages in thread* Re: [PATCH 08/21] dynamic_debug: factor show_ddebug_query out of ddebug_parse_query
2011-07-11 18:36 ` Bart Van Assche
@ 2011-07-16 21:32 ` Jim Cromie
2011-07-25 11:10 ` Bart Van Assche
0 siblings, 1 reply; 72+ messages in thread
From: Jim Cromie @ 2011-07-16 21:32 UTC (permalink / raw)
To: Bart Van Assche; +Cc: jbaron, linux-kernel, joe, gregkh, gnb
On Mon, Jul 11, 2011 at 12:36 PM, Bart Van Assche <bvanassche@acm.org> wrote:
> On Mon, Jul 11, 2011 at 9:46 AM, Jim Cromie <jim.cromie@gmail.com> wrote:
>> Will reuse for show_pending_query too. Alloc and free
>> print buffer space inside ddebug_exec_queries, instead of
>> a permanent static allocation.
>>
>> Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
>> ---
>> lib/dynamic_debug.c | 24 +++++++++++++++++++-----
>> 1 files changed, 19 insertions(+), 5 deletions(-)
>>
>> diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
>> index de2a679..81268e2 100644
>> --- a/lib/dynamic_debug.c
>> +++ b/lib/dynamic_debug.c
>> @@ -96,6 +96,19 @@ static char *ddebug_describe_flags(struct _ddebug *dp, char *buf,
>> return buf;
>> }
>>
>> +static char *prbuf_query;
>> +
>> +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;
>> +}
>> +
>> /*
>> * Search the tables for _ddebug's which match the given
>> * `query' and apply the `flags' and `mask' to them. Tells
>> @@ -344,11 +357,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;
>> }
>> @@ -440,6 +449,10 @@ static int ddebug_exec_queries(char *query)
>> char *split;
>> int i, errs = 0, exitcode = 0, rc;
>>
>> + prbuf_query = kmalloc(1024, GFP_KERNEL);
>> + if (prbuf_query == NULL)
>> + return -ENOMEM;
>> +
>> for (i = 0; query; query = split, i++) {
>> split = strchr(query, ';');
>> if (split)
>> @@ -454,6 +467,7 @@ static int ddebug_exec_queries(char *query)
>> exitcode = rc;
>> }
>> }
>> + kfree(prbuf_query);
>> if (verbose)
>> pr_info("processed %d queries, with %d errs\n", i, errs);
>
> Why to invoke kmalloc() with a magic constant as the first argument
> while the above code can be simplified by replacing the sprintf() call
> (that is missing an output buffer size check) by a kasprintf() call ?
> And why does the static variable 'prbuf_query' exist at all ?
>
> Bart.
>
using kasprintf would entail;
- a local char* var,
- a call to the string-producer (show_ddebug_query above)
- a kfree(var)
all in the caller context.
Using a preallocated string buffer avoids this,
and is slightly more efficient wrt kallocs.
^ permalink raw reply [flat|nested] 72+ messages in thread* Re: [PATCH 08/21] dynamic_debug: factor show_ddebug_query out of ddebug_parse_query
2011-07-16 21:32 ` Jim Cromie
@ 2011-07-25 11:10 ` Bart Van Assche
2011-07-25 13:56 ` Jim Cromie
0 siblings, 1 reply; 72+ messages in thread
From: Bart Van Assche @ 2011-07-25 11:10 UTC (permalink / raw)
To: Jim Cromie; +Cc: jbaron, linux-kernel, joe, gregkh, gnb
On Sat, Jul 16, 2011 at 11:32 PM, Jim Cromie <jim.cromie@gmail.com> wrote:
> using kasprintf would entail;
> - a local char* var,
> - a call to the string-producer (show_ddebug_query above)
> - a kfree(var)
> all in the caller context.
> Using a preallocated string buffer avoids this,
> and is slightly more efficient wrt kallocs.
I'm still missing an explanation why the static variable "prbuf_query"
is necessary. Wouldn't eliminating that static variable make the code
more elegant ?
Bart.
^ permalink raw reply [flat|nested] 72+ messages in thread
* Re: [PATCH 08/21] dynamic_debug: factor show_ddebug_query out of ddebug_parse_query
2011-07-25 11:10 ` Bart Van Assche
@ 2011-07-25 13:56 ` Jim Cromie
0 siblings, 0 replies; 72+ messages in thread
From: Jim Cromie @ 2011-07-25 13:56 UTC (permalink / raw)
To: Bart Van Assche; +Cc: jbaron, linux-kernel, joe, gregkh, gnb
On Mon, Jul 25, 2011 at 5:10 AM, Bart Van Assche <bvanassche@acm.org> wrote:
> On Sat, Jul 16, 2011 at 11:32 PM, Jim Cromie <jim.cromie@gmail.com> wrote:
>> using kasprintf would entail;
>> - a local char* var,
>> - a call to the string-producer (show_ddebug_query above)
>> - a kfree(var)
>> all in the caller context.
>> Using a preallocated string buffer avoids this,
>> and is slightly more efficient wrt kallocs.
>
> I'm still missing an explanation why the static variable "prbuf_query"
> is necessary. Wouldn't eliminating that static variable make the code
> more elegant ?
>
> Bart.
>
After further consideration, Ive done it as you suggest.
Its a few more lines of code, but less complex -
all kfrees follow use immediately, no buffer mgmt at a distance.
^ permalink raw reply [flat|nested] 72+ messages in thread
* [PATCH 09/21] dynamic_debug: save_pending() saves non-matching queries for later.
2011-07-11 7:46 ` Jim Cromie
` (7 preceding siblings ...)
2011-07-11 7:46 ` [PATCH 08/21] dynamic_debug: factor show_ddebug_query out of ddebug_parse_query Jim Cromie
@ 2011-07-11 7:46 ` Jim Cromie
2011-07-11 18:32 ` Bart Van Assche
2011-07-12 5:55 ` Bart Van Assche
2011-07-11 7:46 ` [PATCH 10/21] dynamic_debug: call apply_pending_queries from ddebug_add_module Jim Cromie
` (11 subsequent siblings)
20 siblings, 2 replies; 72+ messages in thread
From: Jim Cromie @ 2011-07-11 7:46 UTC (permalink / raw)
To: jbaron; +Cc: linux-kernel, bvanassche, joe, gregkh, gnb, Jim Cromie
When a query/rule doesnt match, call save_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
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_save_pending: add to pending: q->function="(null)" q->filename="(null)"\
q->module="scx200_hrt" q->format="(null)" q->lineno=0-0
ddebug_save_pending: ddebug: query saved as pending 1
Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
lib/dynamic_debug.c | 56 +++++++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 56 insertions(+), 0 deletions(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 81268e2..b049ef2 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;
@@ -63,6 +73,11 @@ static LIST_HEAD(ddebug_tables);
static int verbose = 0;
module_param(verbose, int, 0644);
+/* legal but inapplicable queries, save and test against new modules */
+static LIST_HEAD(pending_queries);
+static int pending_ct;
+module_param(pending_ct, int, 0444);
+
/* Return the last part of a pathname */
static inline const char *basename(const char *path)
{
@@ -421,6 +436,42 @@ static int ddebug_parse_flags(const char *str, unsigned int *flagsp,
return 0;
}
+/* copy query off stack, save flags & mask, and store in pending-list */
+static int ddebug_save_pending(struct ddebug_query *query,
+ unsigned int flags, unsigned int mask)
+{
+ struct pending_query *pq;
+
+ if (verbose)
+ pr_info("add to pending: %s\n", show_ddebug_query(query));
+
+ pending_ct++;
+ pq = kzalloc(sizeof(struct pending_query), GFP_KERNEL);
+ if (pq == NULL)
+ return -ENOMEM;
+
+ /* 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("query saved as pending %d\n", pending_ct);
+ return 0;
+}
+
static int ddebug_exec_query(char *query_string)
{
unsigned int flags = 0, mask = 0;
@@ -440,6 +491,11 @@ 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)
+ return ddebug_save_pending(&query, flags, mask);
+
return 0;
}
--
1.7.4.1
^ permalink raw reply related [flat|nested] 72+ messages in thread* Re: [PATCH 09/21] dynamic_debug: save_pending() saves non-matching queries for later.
2011-07-11 7:46 ` [PATCH 09/21] dynamic_debug: save_pending() saves non-matching queries for later Jim Cromie
@ 2011-07-11 18:32 ` Bart Van Assche
2011-07-12 5:55 ` Bart Van Assche
1 sibling, 0 replies; 72+ messages in thread
From: Bart Van Assche @ 2011-07-11 18:32 UTC (permalink / raw)
To: Jim Cromie; +Cc: jbaron, linux-kernel, joe, gregkh, gnb
On Mon, Jul 11, 2011 at 9:46 AM, Jim Cromie <jim.cromie@gmail.com> wrote:
>
> When a query/rule doesnt match, call save_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
>
> 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_save_pending: add to pending: q->function="(null)" q->filename="(null)"\
> q->module="scx200_hrt" q->format="(null)" q->lineno=0-0
> ddebug_save_pending: ddebug: query saved as pending 1
>
> Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
> ---
> lib/dynamic_debug.c | 56 +++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 files changed, 56 insertions(+), 0 deletions(-)
>
> diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
> index 81268e2..b049ef2 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;
> @@ -63,6 +73,11 @@ static LIST_HEAD(ddebug_tables);
> static int verbose = 0;
> module_param(verbose, int, 0644);
>
> +/* legal but inapplicable queries, save and test against new modules */
> +static LIST_HEAD(pending_queries);
> +static int pending_ct;
> +module_param(pending_ct, int, 0444);
> +
> /* Return the last part of a pathname */
> static inline const char *basename(const char *path)
> {
> @@ -421,6 +436,42 @@ static int ddebug_parse_flags(const char *str, unsigned int *flagsp,
> return 0;
> }
>
> +/* copy query off stack, save flags & mask, and store in pending-list */
> +static int ddebug_save_pending(struct ddebug_query *query,
> + unsigned int flags, unsigned int mask)
> +{
> + struct pending_query *pq;
> +
> + if (verbose)
> + pr_info("add to pending: %s\n", show_ddebug_query(query));
> +
> + pending_ct++;
> + pq = kzalloc(sizeof(struct pending_query), GFP_KERNEL);
> + if (pq == NULL)
> + return -ENOMEM;
> +
> + /* 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);
I see here several calls of strcpy() without prior size check.
Shouldn't these be changed into strlcpy() ?
Bart.
^ permalink raw reply [flat|nested] 72+ messages in thread* Re: [PATCH 09/21] dynamic_debug: save_pending() saves non-matching queries for later.
2011-07-11 7:46 ` [PATCH 09/21] dynamic_debug: save_pending() saves non-matching queries for later Jim Cromie
2011-07-11 18:32 ` Bart Van Assche
@ 2011-07-12 5:55 ` Bart Van Assche
1 sibling, 0 replies; 72+ messages in thread
From: Bart Van Assche @ 2011-07-12 5:55 UTC (permalink / raw)
To: Jim Cromie; +Cc: jbaron, linux-kernel, joe, gregkh, gnb
On Mon, Jul 11, 2011 at 9:46 AM, Jim Cromie <jim.cromie@gmail.com> wrote:
> +/* copy query off stack, save flags & mask, and store in pending-list */
> +static int ddebug_save_pending(struct ddebug_query *query,
> + unsigned int flags, unsigned int mask)
> +{
> + struct pending_query *pq;
> +
> + if (verbose)
> + pr_info("add to pending: %s\n", show_ddebug_query(query));
> +
> + pending_ct++;
> + pq = kzalloc(sizeof(struct pending_query), GFP_KERNEL);
> + if (pq == NULL)
> + return -ENOMEM;
> +
> + /* 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("query saved as pending %d\n", pending_ct);
> + return 0;
> +}
As I wrote before, manipulating pending_ct without protecting it by
any kind of locking is racy.
Bart.
^ permalink raw reply [flat|nested] 72+ messages in thread
* [PATCH 10/21] dynamic_debug: call apply_pending_queries from ddebug_add_module
2011-07-11 7:46 ` Jim Cromie
` (8 preceding siblings ...)
2011-07-11 7:46 ` [PATCH 09/21] dynamic_debug: save_pending() saves non-matching queries for later Jim Cromie
@ 2011-07-11 7:46 ` Jim Cromie
2011-07-11 7:46 ` [PATCH 11/21] dynamic_debug: refactor query_matches_callsite out of ddebug_change Jim Cromie
` (10 subsequent siblings)
20 siblings, 0 replies; 72+ messages in thread
From: Jim Cromie @ 2011-07-11 7:46 UTC (permalink / raw)
To: jbaron; +Cc: linux-kernel, bvanassche, joe, gregkh, gnb, Jim Cromie
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 .../drivers/char/pc8736x_gpio.c:342 [pc8736x_gpio]pc8736x_gpio_cleanup p
ddebug_change: changed .../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 <jim.cromie@gmail.com>
---
lib/dynamic_debug.c | 41 +++++++++++++++++++++++++++++++++++++++++
1 files changed, 41 insertions(+), 0 deletions(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index b049ef2..cc8e157 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -124,6 +124,16 @@ static char *show_ddebug_query(const struct ddebug_query *q)
return prbuf_query;
}
+static char *show_pending_query(struct pending_query *pq)
+{
+ struct ddebug_query *dq = &pq->query;
+ char *tmp = show_ddebug_query(dq);
+
+ sprintf(tmp + strlen(tmp), " q->flags=0x%x q->mask=0x%x",
+ pq->flags, pq->mask);
+ return prbuf_query;
+}
+
/*
* Search the tables for _ddebug's which match the given
* `query' and apply the `flags' and `mask' to them. Tells
@@ -846,6 +856,34 @@ 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.
@@ -874,6 +912,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
^ permalink raw reply related [flat|nested] 72+ messages in thread* [PATCH 11/21] dynamic_debug: refactor query_matches_callsite out of ddebug_change
2011-07-11 7:46 ` Jim Cromie
` (9 preceding siblings ...)
2011-07-11 7:46 ` [PATCH 10/21] dynamic_debug: call apply_pending_queries from ddebug_add_module Jim Cromie
@ 2011-07-11 7:46 ` Jim Cromie
2011-07-11 7:46 ` [PATCH 12/21] dynamic_debug: document use of pending queries at boot-time Jim Cromie
` (9 subsequent siblings)
20 siblings, 0 replies; 72+ messages in thread
From: Jim Cromie @ 2011-07-11 7:46 UTC (permalink / raw)
To: jbaron; +Cc: linux-kernel, bvanassche, joe, gregkh, gnb, Jim Cromie
this makes no behavioral change, its just a code-move.
Originally done cuz it looked momentarily to be related to
query-vs-query check needed soon.
If this function is reused, note that it excludes check of the module;
that is done once for the table, this code refactors the test inside
the loop over the module's ddebug callsites.
Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
lib/dynamic_debug.c | 52 +++++++++++++++++++++++++++++---------------------
1 files changed, 30 insertions(+), 22 deletions(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index cc8e157..bee1376 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -134,6 +134,35 @@ static char *show_pending_query(struct pending_query *pq)
return prbuf_query;
}
+static int query_matches_callsite(struct _ddebug *dp,
+ const struct ddebug_query *query)
+{
+ /* match against the source filename */
+ if (query->filename != NULL &&
+ strcmp(query->filename, dp->filename) &&
+ strcmp(query->filename, basename(dp->filename)))
+ return 0;
+
+ /* match against the function */
+ if (query->function != NULL &&
+ strcmp(query->function, dp->function))
+ return 0;
+
+ /* match against the format */
+ if (query->format != NULL &&
+ strstr(dp->format, query->format) == NULL)
+ return 0;
+
+ /* match against the line number range */
+ if (query->first_lineno && dp->lineno < query->first_lineno)
+ return 0;
+
+ if (query->last_lineno && dp->lineno > query->last_lineno)
+ return 0;
+
+ return 1;
+}
+
/*
* Search the tables for _ddebug's which match the given
* `query' and apply the `flags' and `mask' to them. Tells
@@ -161,28 +190,7 @@ static int ddebug_change(const struct ddebug_query *query,
for (i = 0 ; i < dt->num_ddebugs ; i++) {
struct _ddebug *dp = &dt->ddebugs[i];
- /* match against the source filename */
- if (query->filename != NULL &&
- strcmp(query->filename, dp->filename) &&
- strcmp(query->filename, basename(dp->filename)))
- continue;
-
- /* match against the function */
- if (query->function != NULL &&
- strcmp(query->function, dp->function))
- continue;
-
- /* match against the format */
- if (query->format != NULL &&
- strstr(dp->format, query->format) == NULL)
- continue;
-
- /* match against the line number range */
- if (query->first_lineno &&
- dp->lineno < query->first_lineno)
- continue;
- if (query->last_lineno &&
- dp->lineno > query->last_lineno)
+ if (!query_matches_callsite(dp, query))
continue;
nfound++;
--
1.7.4.1
^ permalink raw reply related [flat|nested] 72+ messages in thread* [PATCH 12/21] dynamic_debug: document use of pending queries at boot-time
2011-07-11 7:46 ` Jim Cromie
` (10 preceding siblings ...)
2011-07-11 7:46 ` [PATCH 11/21] dynamic_debug: refactor query_matches_callsite out of ddebug_change Jim Cromie
@ 2011-07-11 7:46 ` Jim Cromie
2011-07-11 7:46 ` [PATCH 13/21] dynamic_debug: require 'a' flag to explicitly mark pending queries Jim Cromie
` (8 subsequent siblings)
20 siblings, 0 replies; 72+ messages in thread
From: Jim Cromie @ 2011-07-11 7:46 UTC (permalink / raw)
To: jbaron; +Cc: linux-kernel, bvanassche, joe, gregkh, gnb, Jim Cromie
Add paragraph that pending-queries are checked when modules are loaded,
so debug statements can be used in module_init functions.
Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
Documentation/dynamic-debug-howto.txt | 34 +++++++++++++++++++++++++-------
1 files changed, 26 insertions(+), 8 deletions(-)
diff --git a/Documentation/dynamic-debug-howto.txt b/Documentation/dynamic-debug-howto.txt
index f959909..49b75b81 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
@@ -110,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
@@ -232,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
^ permalink raw reply related [flat|nested] 72+ messages in thread* [PATCH 13/21] dynamic_debug: require 'a' flag to explicitly mark pending queries
2011-07-11 7:46 ` Jim Cromie
` (11 preceding siblings ...)
2011-07-11 7:46 ` [PATCH 12/21] dynamic_debug: document use of pending queries at boot-time Jim Cromie
@ 2011-07-11 7:46 ` Jim Cromie
2011-07-12 9:20 ` Bart Van Assche
2011-07-11 7:46 ` [PATCH 14/21] dynamic_debug: hoist locking in ddebug_change to callers Jim Cromie
` (7 subsequent siblings)
20 siblings, 1 reply; 72+ messages in thread
From: Jim Cromie @ 2011-07-11 7:46 UTC (permalink / raw)
To: jbaron; +Cc: linux-kernel, bvanassche, joe, gregkh, gnb, Jim Cromie
Require that user mark pending queries explicitly, otherwise warn
that they do not apply. Note that pending rules are still applied
by default, and are only added to pending-list if they do not apply,
and bear the flag.
Also test that query hasnt already been added to pending-list,
if it has (according to queries_match), just update the found query.
Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
lib/dynamic_debug.c | 52 +++++++++++++++++++++++++++++++++++++++++++++-----
1 files changed, 46 insertions(+), 6 deletions(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index bee1376..0faac83 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -91,6 +91,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_APPEND, 'a' },
};
/* format a string into buf[] which describes the _ddebug's flags */
@@ -454,12 +455,47 @@ static int ddebug_parse_flags(const char *str, unsigned int *flagsp,
return 0;
}
-/* copy query off stack, save flags & mask, and store in pending-list */
+static int queries_match(struct ddebug_query *q1, struct ddebug_query *q2)
+{
+ if (!q1->module ^ !q2->module ||
+ !q1->filename ^ !q2->filename ||
+ !q1->function ^ !q2->function ||
+ !q1->format ^ !q2->format)
+ return 0; /* a match-spec set/unset state differs */
+
+ if (q1->last_lineno != q2->last_lineno ||
+ q1->first_lineno != q2->first_lineno)
+ return 0;
+
+ if ((q1->module && strcmp(q1->module, q2->module)) ||
+ (q1->filename && strcmp(q1->filename, q2->filename)) ||
+ (q1->function && strcmp(q1->function, q2->function)) ||
+ (q1->format && strcmp(q1->format, q2->format)))
+ return 0;
+ return 1;
+}
+
+/* copy query off stack, save flags & mask, and store in pending-list,
+ after checking that query isnt already there
+ */
static int ddebug_save_pending(struct ddebug_query *query,
unsigned int flags, unsigned int mask)
{
- struct pending_query *pq;
+ struct pending_query *pq, *pqnext;
+ list_for_each_entry_safe(pq, pqnext, &pending_queries, link) {
+ if (queries_match(query, &pq->query)) {
+ /* query already in list, update flags */
+ if (pq->flags != flags)
+ pq->flags = flags;
+ if (pq->mask != mask)
+ pq->mask = mask;
+ if (verbose)
+ pr_info("already pending, updated it: %s\n",
+ show_pending_query(pq));
+ return 0;
+ }
+ }
if (verbose)
pr_info("add to pending: %s\n", show_ddebug_query(query));
@@ -511,9 +547,13 @@ static int ddebug_exec_query(char *query_string)
nfound = ddebug_change(&query, flags, mask);
pr_info("nfound %d on %s\n", nfound, show_ddebug_query(&query));
- if (!nfound)
- return ddebug_save_pending(&query, flags, mask);
-
+ if (!nfound) {
+ if (flags & _DPRINTK_FLAGS_APPEND)
+ return ddebug_save_pending(&query, flags, mask);
+ else
+ pr_warn("no match on: %s\n",
+ show_ddebug_query(&query));
+ }
return 0;
}
--
1.7.4.1
^ permalink raw reply related [flat|nested] 72+ messages in thread* Re: [PATCH 13/21] dynamic_debug: require 'a' flag to explicitly mark pending queries
2011-07-11 7:46 ` [PATCH 13/21] dynamic_debug: require 'a' flag to explicitly mark pending queries Jim Cromie
@ 2011-07-12 9:20 ` Bart Van Assche
0 siblings, 0 replies; 72+ messages in thread
From: Bart Van Assche @ 2011-07-12 9:20 UTC (permalink / raw)
To: Jim Cromie; +Cc: jbaron, linux-kernel, joe, gregkh, gnb
On Mon, Jul 11, 2011 at 9:46 AM, Jim Cromie <jim.cromie@gmail.com> wrote:
> +static int queries_match(struct ddebug_query *q1, struct ddebug_query *q2)
> +{
> + if (!q1->module ^ !q2->module ||
> + !q1->filename ^ !q2->filename ||
> + !q1->function ^ !q2->function ||
> + !q1->format ^ !q2->format)
> + return 0; /* a match-spec set/unset state differs */
> +
> + if (q1->last_lineno != q2->last_lineno ||
> + q1->first_lineno != q2->first_lineno)
> + return 0;
> +
> + if ((q1->module && strcmp(q1->module, q2->module)) ||
> + (q1->filename && strcmp(q1->filename, q2->filename)) ||
> + (q1->function && strcmp(q1->function, q2->function)) ||
> + (q1->format && strcmp(q1->format, q2->format)))
> + return 0;
> + return 1;
> +}
Is there a reason why the return type of this function is "int" and
not "bool" ? Also, in the Linux kernel you can use the symbolic
constants "true" and "false" instead of "1" and "0".
Bart.
^ permalink raw reply [flat|nested] 72+ messages in thread
* [PATCH 14/21] dynamic_debug: hoist locking in ddebug_change to callers
2011-07-11 7:46 ` Jim Cromie
` (12 preceding siblings ...)
2011-07-11 7:46 ` [PATCH 13/21] dynamic_debug: require 'a' flag to explicitly mark pending queries Jim Cromie
@ 2011-07-11 7:46 ` Jim Cromie
2011-07-11 7:46 ` [PATCH 15/21] dynamic_debug: describe_flags with '=[ptmfl]*' Jim Cromie
` (6 subsequent siblings)
20 siblings, 0 replies; 72+ messages in thread
From: Jim Cromie @ 2011-07-11 7:46 UTC (permalink / raw)
To: jbaron; +Cc: linux-kernel, bvanassche, joe, gregkh, gnb, Jim Cromie
Hoist locking out of ddebug_change(), into ddebug_exec_query(),
and fix return from inside the critical section.
Remove locking in ddebug_save_pending(), since its also
protected by ddebug_exec_query().
ddebug_add_module() also calls ddebug_change() via apply_pending_queries(),
move that call inside the critical section protecting list_add_tail()
Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
lib/dynamic_debug.c | 37 ++++++++++++++-----------------------
1 files changed, 14 insertions(+), 23 deletions(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 0faac83..d115f52 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -168,7 +168,7 @@ static int query_matches_callsite(struct _ddebug *dp,
* 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.
+ * were matched. Called with ddebug_lock held.
*/
static int ddebug_change(const struct ddebug_query *query,
unsigned int flags, unsigned int mask)
@@ -180,7 +180,6 @@ static int ddebug_change(const struct ddebug_query *query,
char flagbuf[8];
/* search for matching ddebugs */
- mutex_lock(&ddebug_lock);
list_for_each_entry(dt, &ddebug_tables, link) {
/* match against the module name */
@@ -212,8 +211,6 @@ static int ddebug_change(const struct ddebug_query *query,
sizeof(flagbuf)));
}
}
- mutex_unlock(&ddebug_lock);
-
return nfound;
}
@@ -475,8 +472,8 @@ static int queries_match(struct ddebug_query *q1, struct ddebug_query *q2)
return 1;
}
-/* copy query off stack, save flags & mask, and store in pending-list,
- after checking that query isnt already there
+/* copy query off stack, save flags & mask, and store or update in
+ pending-list. Called with ddebug_lock held.
*/
static int ddebug_save_pending(struct ddebug_query *query,
unsigned int flags, unsigned int mask)
@@ -499,7 +496,6 @@ static int ddebug_save_pending(struct ddebug_query *query,
if (verbose)
pr_info("add to pending: %s\n", show_ddebug_query(query));
- pending_ct++;
pq = kzalloc(sizeof(struct pending_query), GFP_KERNEL);
if (pq == NULL)
return -ENOMEM;
@@ -517,9 +513,8 @@ static int ddebug_save_pending(struct ddebug_query *query,
pq->flags = flags;
pq->mask = mask;
- mutex_lock(&ddebug_lock);
list_add(&pq->link, &pending_queries);
- mutex_unlock(&ddebug_lock);
+ pending_ct++;
if (verbose)
pr_info("query saved as pending %d\n", pending_ct);
@@ -533,7 +528,7 @@ static int ddebug_exec_query(char *query_string)
#define MAXWORDS 9
int nwords;
char *words[MAXWORDS];
- int nfound;
+ int nfound, rc = 0;
nwords = ddebug_tokenize(query_string, words, MAXWORDS);
if (nwords <= 0)
@@ -544,17 +539,18 @@ static int ddebug_exec_query(char *query_string)
return -EINVAL;
/* actually go and implement the change */
+ mutex_lock(&ddebug_lock);
nfound = ddebug_change(&query, flags, mask);
-
- pr_info("nfound %d on %s\n", nfound, show_ddebug_query(&query));
if (!nfound) {
if (flags & _DPRINTK_FLAGS_APPEND)
- return ddebug_save_pending(&query, flags, mask);
+ rc = ddebug_save_pending(&query, flags, mask);
else
pr_warn("no match on: %s\n",
show_ddebug_query(&query));
}
- return 0;
+ mutex_unlock(&ddebug_lock);
+ pr_info("nfound %d on %s\n", nfound, show_ddebug_query(&query));
+ return rc;
}
/* handle multiple queries, continue on error, return last error */
@@ -904,7 +900,7 @@ static const struct file_operations ddebug_proc_fops = {
.write = ddebug_proc_write
};
-/* apply matching queries in pending-queries list */
+/* apply matching queries in pending-queries list. Called with lock held */
static void apply_pending_queries(struct ddebug_table *dt)
{
struct pending_query *pq, *pqnext;
@@ -914,7 +910,6 @@ static void apply_pending_queries(struct ddebug_table *dt)
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));
@@ -922,15 +917,12 @@ static void apply_pending_queries(struct ddebug_table *dt)
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
@@ -954,14 +946,13 @@ int ddebug_add_module(struct _ddebug *tab, unsigned int n,
dt->num_ddebugs = n;
dt->ddebugs = tab;
- mutex_lock(&ddebug_lock);
- list_add_tail(&dt->link, &ddebug_tables);
- mutex_unlock(&ddebug_lock);
-
if (verbose)
pr_info("%u debug prints in module %s\n", n, dt->mod_name);
+ mutex_lock(&ddebug_lock);
+ list_add_tail(&dt->link, &ddebug_tables);
apply_pending_queries(dt);
+ mutex_unlock(&ddebug_lock);
return 0;
}
--
1.7.4.1
^ permalink raw reply related [flat|nested] 72+ messages in thread* [PATCH 15/21] dynamic_debug: describe_flags with '=[ptmfl]*'
2011-07-11 7:46 ` Jim Cromie
` (13 preceding siblings ...)
2011-07-11 7:46 ` [PATCH 14/21] dynamic_debug: hoist locking in ddebug_change to callers Jim Cromie
@ 2011-07-11 7:46 ` Jim Cromie
2011-07-11 7:46 ` [PATCH 16/21] dynamic_debug: define several levels of verbosity Jim Cromie
` (5 subsequent siblings)
20 siblings, 0 replies; 72+ messages in thread
From: Jim Cromie @ 2011-07-11 7:46 UTC (permalink / raw)
To: jbaron; +Cc: linux-kernel, bvanassche, joe, gregkh, gnb, Jim Cromie
Describe flags in text using =<flag-chars>. This has better mnemonics
than the current dash-unless-flags, it says "Flags are = <flag-chars>",
and =<flags> is more selective in a grep; '=' will not show up in module
or filenames, nor in line-ranges.
This isnt yet optimal; "grep ' = ' control" reports on a couple dozen
format-strings, this could be avoided by altering describe_flags()
to emit '-' or '_' for empty flags, such that the grep for empty
flags would be '=_'. TBD.
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 d115f52..31ee0fd 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -102,11 +102,10 @@ static char *ddebug_describe_flags(struct _ddebug *dp, char *buf,
int i;
BUG_ON(maxlen < 4);
+ *p++ = '=';
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 = '\0';
return buf;
--
1.7.4.1
^ permalink raw reply related [flat|nested] 72+ messages in thread* [PATCH 16/21] dynamic_debug: define several levels of verbosity
2011-07-11 7:46 ` Jim Cromie
` (14 preceding siblings ...)
2011-07-11 7:46 ` [PATCH 15/21] dynamic_debug: describe_flags with '=[ptmfl]*' Jim Cromie
@ 2011-07-11 7:46 ` Jim Cromie
2011-07-11 7:46 ` [PATCH 17/21] dynamic_debug: non-optimization - remove != NULL Jim Cromie
` (4 subsequent siblings)
20 siblings, 0 replies; 72+ messages in thread
From: Jim Cromie @ 2011-07-11 7:46 UTC (permalink / raw)
To: jbaron; +Cc: linux-kernel, bvanassche, joe, gregkh, gnb, Jim Cromie
define verbose levels 10, 11 to turn on noisy pr_info()s selectively
10: ddebug_proc_(start|stop) - once each per page of output
11: ddebug_proc_(show|next) - once each for every call-site
1-9: are unspecified, may be used for other pr_info()s
Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
lib/dynamic_debug.c | 14 +++++++++-----
1 files changed, 9 insertions(+), 5 deletions(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 31ee0fd..8dde999 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -70,7 +70,11 @@ struct ddebug_iter {
static DEFINE_MUTEX(ddebug_lock);
static LIST_HEAD(ddebug_tables);
-static int verbose = 0;
+
+#define VERB_BASIC 1
+#define VERB_PROC 10
+#define VERB_PROC_SHOW 11
+static int verbose;
module_param(verbose, int, 0644);
/* legal but inapplicable queries, save and test against new modules */
@@ -777,7 +781,7 @@ static void *ddebug_proc_start(struct seq_file *m, loff_t *pos)
struct _ddebug *dp;
int n = *pos;
- if (verbose)
+ if (verbose >= VERB_PROC)
pr_info("called m=%p *pos=%lld\n", m, (unsigned long long)*pos);
mutex_lock(&ddebug_lock);
@@ -802,7 +806,7 @@ static void *ddebug_proc_next(struct seq_file *m, void *p, loff_t *pos)
struct ddebug_iter *iter = m->private;
struct _ddebug *dp;
- if (verbose)
+ if (verbose >= VERB_PROC_SHOW)
pr_info("called m=%p p=%p *pos=%lld\n",
m, p, (unsigned long long)*pos);
@@ -826,7 +830,7 @@ static int ddebug_proc_show(struct seq_file *m, void *p)
struct _ddebug *dp = p;
char flagsbuf[8];
- if (verbose)
+ if (verbose >= VERB_PROC_SHOW)
pr_info("called m=%p p=%p\n", m, p);
if (p == SEQ_START_TOKEN) {
@@ -851,7 +855,7 @@ static int ddebug_proc_show(struct seq_file *m, void *p)
*/
static void ddebug_proc_stop(struct seq_file *m, void *p)
{
- if (verbose)
+ if (verbose >= VERB_PROC)
pr_info("called m=%p p=%p\n", m, p);
mutex_unlock(&ddebug_lock);
}
--
1.7.4.1
^ permalink raw reply related [flat|nested] 72+ messages in thread* [PATCH 17/21] dynamic_debug: non-optimization - remove != NULL
2011-07-11 7:46 ` Jim Cromie
` (15 preceding siblings ...)
2011-07-11 7:46 ` [PATCH 16/21] dynamic_debug: define several levels of verbosity Jim Cromie
@ 2011-07-11 7:46 ` Jim Cromie
2011-07-11 7:46 ` [PATCH 18/21] dynamic_debug: trim source-path prefix from dynamic_debug/control Jim Cromie
` (3 subsequent siblings)
20 siblings, 0 replies; 72+ messages in thread
From: Jim Cromie @ 2011-07-11 7:46 UTC (permalink / raw)
To: jbaron; +Cc: linux-kernel, bvanassche, joe, gregkh, gnb, Jim Cromie
Reduce != NULL relational test to its 0/1 equivalent.
No code difference, may not be worth including.
Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
lib/dynamic_debug.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 8dde999..1763cc2 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -142,18 +142,18 @@ static int query_matches_callsite(struct _ddebug *dp,
const struct ddebug_query *query)
{
/* match against the source filename */
- if (query->filename != NULL &&
+ if (query->filename &&
strcmp(query->filename, dp->filename) &&
strcmp(query->filename, basename(dp->filename)))
return 0;
/* match against the function */
- if (query->function != NULL &&
+ if (query->function &&
strcmp(query->function, dp->function))
return 0;
/* match against the format */
- if (query->format != NULL &&
+ if (query->format &&
strstr(dp->format, query->format) == NULL)
return 0;
--
1.7.4.1
^ permalink raw reply related [flat|nested] 72+ messages in thread* [PATCH 18/21] dynamic_debug: trim source-path prefix from dynamic_debug/control
2011-07-11 7:46 ` Jim Cromie
` (16 preceding siblings ...)
2011-07-11 7:46 ` [PATCH 17/21] dynamic_debug: non-optimization - remove != NULL Jim Cromie
@ 2011-07-11 7:46 ` Jim Cromie
2011-07-11 7:46 ` [PATCH 19/21] dynamic_debug: add flags filtering to flags spec handling Jim Cromie
` (2 subsequent siblings)
20 siblings, 0 replies; 72+ messages in thread
From: Jim Cromie @ 2011-07-11 7:46 UTC (permalink / raw)
To: jbaron; +Cc: linux-kernel, bvanassche, joe, gregkh, gnb, Jim Cromie
Check for a common source-path prefix, and skip past it, to print a
path thats relative to kernel source's root-dir. For in-tree modules,
this makes the control 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 <jim.cromie@gmail.com>
---
lib/dynamic_debug.c | 6 +++++-
1 files changed, 5 insertions(+), 1 deletions(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 1763cc2..44ccc02 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -829,6 +829,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];
+ int skip = strlen(__FILE__) - strlen("lib/dynamic_debug.c");
if (verbose >= VERB_PROC_SHOW)
pr_info("called m=%p p=%p\n", m, p);
@@ -839,8 +840,11 @@ static int ddebug_proc_show(struct seq_file *m, void *p)
return 0;
}
+ if (strncmp(dp->filename, __FILE__, skip))
+ skip = 0; /* prefix mismatch, don't skip */
+
seq_printf(m, "%s:%u [%s]%s %s \"",
- dp->filename, dp->lineno,
+ dp->filename + skip, 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
^ permalink raw reply related [flat|nested] 72+ messages in thread* [PATCH 19/21] dynamic_debug: add flags filtering to flags spec handling
2011-07-11 7:46 ` Jim Cromie
` (17 preceding siblings ...)
2011-07-11 7:46 ` [PATCH 18/21] dynamic_debug: trim source-path prefix from dynamic_debug/control Jim Cromie
@ 2011-07-11 7:46 ` Jim Cromie
2011-07-11 7:46 ` [PATCH 20/21] dynamic_debug: clear pending_queries list in remove_all_tables Jim Cromie
2011-07-11 7:46 ` [PATCH 21/21] dynamic_debug: delete pending queries Jim Cromie
20 siblings, 0 replies; 72+ messages in thread
From: Jim Cromie @ 2011-07-11 7:46 UTC (permalink / raw)
To: jbaron; +Cc: linux-kernel, bvanassche, joe, gregkh, gnb, Jim Cromie
Change definition of flags spec to [ptlmfa]*[-+=][ptlmfa]+
The flags preceding the op are optional filter-flags, if provided,
they add an additional constraint to call-site matching done
by ddebug_change; call-sites which do not have all specified flags
are skipped (additional flags are allowed).
This allows query/rules like "p+t" to turn on TID logging for all
currently enabled call-sites, while leaving the others alone.
This will also allow filtering on pending rules (with 'a' flag),
which will support removal of pending rules.
Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
lib/dynamic_debug.c | 81 +++++++++++++++++++++++++++++++-------------------
1 files changed, 50 insertions(+), 31 deletions(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 44ccc02..9514071 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -174,7 +174,8 @@ static int query_matches_callsite(struct _ddebug *dp,
* were matched. Called with ddebug_lock held.
*/
static int ddebug_change(const struct ddebug_query *query,
- unsigned int flags, unsigned int mask)
+ unsigned int flags, unsigned int mask,
+ unsigned int filter)
{
int i;
struct ddebug_table *dt;
@@ -196,6 +197,9 @@ static int ddebug_change(const struct ddebug_query *query,
if (!query_matches_callsite(dp, query))
continue;
+ if (filter && (dp->flags & filter) != filter)
+ continue;
+
nfound++;
newflags = (dp->flags & mask) | flags;
@@ -396,29 +400,9 @@ static int ddebug_parse_query(char *words[], int nwords,
return 0;
}
-/*
- * Parse `str' as a flags specification, format [-+=][p]+.
- * Sets up *maskp and *flagsp to be used when changing the
- * flags fields of matched _ddebug's. Returns 0 on success
- * or <0 on error.
- */
-static int ddebug_parse_flags(const char *str, unsigned int *flagsp,
- unsigned int *maskp)
+static int ddebug_parse_flag_settings(const char *str)
{
- unsigned flags = 0;
- int op = '=', i;
-
- switch (*str) {
- case '+':
- case '-':
- case '=':
- op = *str++;
- break;
- default:
- return -EINVAL;
- }
- if (verbose)
- pr_info("op='%c'\n", op);
+ int i, flags = 0;
for ( ; *str ; ++str) {
for (i = ARRAY_SIZE(opt_array) - 1; i >= 0; i--) {
@@ -427,13 +411,49 @@ static int ddebug_parse_flags(const char *str, unsigned int *flagsp,
break;
}
}
- if (i < 0)
+ if (i < 0 && *str) {
+ pr_err("unknown flag: %c\n", *str);
return -EINVAL;
+ }
}
- if (flags == 0)
+ return flags;
+}
+
+/*
+ * Parse `str' as a flags filter and specification, with format
+ * [ptlmfa]*[-+=][ptlmfa]+. Sets up *maskp, *flagsp and *filterp to
+ * be used when changing the flags fields of matched _ddebug's.
+ * Returns 0 on success or <0 on error.
+ */
+static int ddebug_parse_flags(const char *str, unsigned int *flagsp,
+ unsigned int *maskp, unsigned int *filterp)
+{
+ int flags, filter;
+ int op = '=';
+ char *s = strpbrk(str, "+-=");
+
+ if (!s) {
+ pr_err("flags spec missing op, expecting [+-=]\n");
return -EINVAL;
+ } else {
+ op = *s;
+ *s++ = '\0';
+ }
+ filter = ddebug_parse_flag_settings(str);
+ if (filter < 0) {
+ pr_err("flags_filter=0x%x\n", filter);
+ return -EINVAL;
+ }
+ *filterp = filter;
+
+ flags = ddebug_parse_flag_settings(s);
+ if (flags <= 0) {
+ pr_err("flags=0x%x\n", flags);
+ return -EINVAL;
+ }
if (verbose)
- pr_info("flags=0x%x\n", flags);
+ pr_info(" flags_filter=0x%x\n op='%c'\n flags=0x%x\n",
+ filter, op, flags);
/* calculate final *flagsp, *maskp according to mask and op */
switch (op) {
@@ -526,7 +546,7 @@ static int ddebug_save_pending(struct ddebug_query *query,
static int ddebug_exec_query(char *query_string)
{
- unsigned int flags = 0, mask = 0;
+ unsigned int flags = 0, mask = 0, filter = 0;
struct ddebug_query query;
#define MAXWORDS 9
int nwords;
@@ -538,12 +558,12 @@ static int ddebug_exec_query(char *query_string)
return -EINVAL;
if (ddebug_parse_query(words, nwords-1, &query))
return -EINVAL;
- if (ddebug_parse_flags(words[nwords-1], &flags, &mask))
+ if (ddebug_parse_flags(words[nwords-1], &flags, &mask, &filter))
return -EINVAL;
/* actually go and implement the change */
mutex_lock(&ddebug_lock);
- nfound = ddebug_change(&query, flags, mask);
+ nfound = ddebug_change(&query, flags, mask, filter);
if (!nfound) {
if (flags & _DPRINTK_FLAGS_APPEND)
rc = ddebug_save_pending(&query, flags, mask);
@@ -921,8 +941,7 @@ static void apply_pending_queries(struct ddebug_table *dt)
pr_info("check: %s <-> %s\n",
dt->mod_name, show_pending_query(pq));
- nfound = ddebug_change(&pq->query, pq->flags, pq->mask);
-
+ nfound = ddebug_change(&pq->query, pq->flags, pq->mask, 0);
if (nfound) {
list_del(&pq->link);
kfree(pq);
--
1.7.4.1
^ permalink raw reply related [flat|nested] 72+ messages in thread* [PATCH 20/21] dynamic_debug: clear pending_queries list in remove_all_tables
2011-07-11 7:46 ` Jim Cromie
` (18 preceding siblings ...)
2011-07-11 7:46 ` [PATCH 19/21] dynamic_debug: add flags filtering to flags spec handling Jim Cromie
@ 2011-07-11 7:46 ` Jim Cromie
2011-07-11 7:46 ` [PATCH 21/21] dynamic_debug: delete pending queries Jim Cromie
20 siblings, 0 replies; 72+ messages in thread
From: Jim Cromie @ 2011-07-11 7:46 UTC (permalink / raw)
To: jbaron; +Cc: linux-kernel, bvanassche, joe, gregkh, gnb, Jim Cromie
Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
lib/dynamic_debug.c | 9 +++++++++
1 files changed, 9 insertions(+), 0 deletions(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 9514071..f0c2b39 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -1017,6 +1017,8 @@ EXPORT_SYMBOL_GPL(ddebug_remove_module);
static void ddebug_remove_all_tables(void)
{
+ struct pending_query *pq, *pqnext;
+
mutex_lock(&ddebug_lock);
while (!list_empty(&ddebug_tables)) {
struct ddebug_table *dt = list_entry(ddebug_tables.next,
@@ -1024,6 +1026,13 @@ static void ddebug_remove_all_tables(void)
link);
ddebug_table_free(dt);
}
+ list_for_each_entry_safe(pq, pqnext, &pending_queries, link) {
+ if (verbose > 1)
+ pr_info("delete pending: %s\n",
+ show_pending_query(pq));
+ list_del_init(&pq->link);
+ kfree(pq);
+ }
mutex_unlock(&ddebug_lock);
}
--
1.7.4.1
^ permalink raw reply related [flat|nested] 72+ messages in thread* [PATCH 21/21] dynamic_debug: delete pending queries
2011-07-11 7:46 ` Jim Cromie
` (19 preceding siblings ...)
2011-07-11 7:46 ` [PATCH 20/21] dynamic_debug: clear pending_queries list in remove_all_tables Jim Cromie
@ 2011-07-11 7:46 ` Jim Cromie
2011-07-11 10:49 ` Bart Van Assche
2011-07-12 0:25 ` Joe Perches
20 siblings, 2 replies; 72+ messages in thread
From: Jim Cromie @ 2011-07-11 7:46 UTC (permalink / raw)
To: jbaron; +Cc: linux-kernel, bvanassche, joe, gregkh, gnb, Jim Cromie
delete pending queries by re-issuing it with disabled flags
with:
$> echo "module foo +ap" > <debugfs>/dynamic_debug/control
$> echo "module foo +apt" > <debugfs>/dynamic_debug/control
$> echo "module foo ap=" > <debugfs>/dynamic_debug/control
1st command adds a pending query on foo
2nd command modifies it by adding a TID flag
3rd command deletes it by setting flags to 0
Note that the pending query matches 'ap' as required by the flags-filter,
but additionally has 't'. An exact match on the query-spec is required,
"module foo line 10-20" would not match, and wouldnt be removed.
Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
lib/dynamic_debug.c | 17 ++++++++++++++---
1 files changed, 14 insertions(+), 3 deletions(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index f0c2b39..2fd207d 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -447,7 +447,7 @@ static int ddebug_parse_flags(const char *str, unsigned int *flagsp,
*filterp = filter;
flags = ddebug_parse_flag_settings(s);
- if (flags <= 0) {
+ if (flags < 0) {
pr_err("flags=0x%x\n", flags);
return -EINVAL;
}
@@ -505,7 +505,17 @@ static int ddebug_save_pending(struct ddebug_query *query,
list_for_each_entry_safe(pq, pqnext, &pending_queries, link) {
if (queries_match(query, &pq->query)) {
- /* query already in list, update flags */
+ /* query already in list */
+ if (!flags) {
+ /* zeroed flags, remove query */
+ if (verbose)
+ pr_info("delete pending: %s\n",
+ show_pending_query(pq));
+ list_del_init(&pq->link);
+ kfree(pq);
+ pending_ct--;
+ return 0;
+ }
if (pq->flags != flags)
pq->flags = flags;
if (pq->mask != mask)
@@ -565,7 +575,8 @@ static int ddebug_exec_query(char *query_string)
mutex_lock(&ddebug_lock);
nfound = ddebug_change(&query, flags, mask, filter);
if (!nfound) {
- if (flags & _DPRINTK_FLAGS_APPEND)
+ if (flags & _DPRINTK_FLAGS_APPEND ||
+ filter & _DPRINTK_FLAGS_APPEND)
rc = ddebug_save_pending(&query, flags, mask);
else
pr_warn("no match on: %s\n",
--
1.7.4.1
^ permalink raw reply related [flat|nested] 72+ messages in thread* Re: [PATCH 21/21] dynamic_debug: delete pending queries
2011-07-11 7:46 ` [PATCH 21/21] dynamic_debug: delete pending queries Jim Cromie
@ 2011-07-11 10:49 ` Bart Van Assche
2011-07-11 13:43 ` Jim Cromie
2011-07-12 0:25 ` Joe Perches
1 sibling, 1 reply; 72+ messages in thread
From: Bart Van Assche @ 2011-07-11 10:49 UTC (permalink / raw)
To: Jim Cromie; +Cc: jbaron, linux-kernel, joe, gregkh, gnb
On Mon, Jul 11, 2011 at 9:46 AM, Jim Cromie <jim.cromie@gmail.com> wrote:
> delete pending queries by re-issuing it with disabled flags
>
> with:
> $> echo "module foo +ap" > <debugfs>/dynamic_debug/control
> $> echo "module foo +apt" > <debugfs>/dynamic_debug/control
> $> echo "module foo ap=" > <debugfs>/dynamic_debug/control
>
> 1st command adds a pending query on foo
> 2nd command modifies it by adding a TID flag
> 3rd command deletes it by setting flags to 0
>
> Note that the pending query matches 'ap' as required by the flags-filter,
> but additionally has 't'. An exact match on the query-spec is required,
> "module foo line 10-20" would not match, and wouldnt be removed.
Maybe I've missed something, but do we need both 'a' and 'p' ? Would
'a' alone be sufficient ?
Bart.
^ permalink raw reply [flat|nested] 72+ messages in thread
* Re: [PATCH 21/21] dynamic_debug: delete pending queries
2011-07-11 10:49 ` Bart Van Assche
@ 2011-07-11 13:43 ` Jim Cromie
0 siblings, 0 replies; 72+ messages in thread
From: Jim Cromie @ 2011-07-11 13:43 UTC (permalink / raw)
To: Bart Van Assche; +Cc: jbaron, linux-kernel, joe, gregkh, gnb
On Mon, Jul 11, 2011 at 4:49 AM, Bart Van Assche <bvanassche@acm.org> wrote:
> On Mon, Jul 11, 2011 at 9:46 AM, Jim Cromie <jim.cromie@gmail.com> wrote:
>> delete pending queries by re-issuing it with disabled flags
>>
>> with:
>> $> echo "module foo +ap" > <debugfs>/dynamic_debug/control
>> $> echo "module foo +apt" > <debugfs>/dynamic_debug/control
>> $> echo "module foo ap=" > <debugfs>/dynamic_debug/control
>>
>> 1st command adds a pending query on foo
>> 2nd command modifies it by adding a TID flag
>> 3rd command deletes it by setting flags to 0
>>
>> Note that the pending query matches 'ap' as required by the flags-filter,
>> but additionally has 't'. An exact match on the query-spec is required,
>> "module foo line 10-20" would not match, and wouldnt be removed.
>
> Maybe I've missed something, but do we need both 'a' and 'p' ? Would
> 'a' alone be sufficient ?
p is for printing, not appending.
unfortunate mnemonics.
maybe 'k' for keep, instead of a for pending is better.
>
> Bart.
>
^ permalink raw reply [flat|nested] 72+ messages in thread
* Re: [PATCH 21/21] dynamic_debug: delete pending queries
2011-07-11 7:46 ` [PATCH 21/21] dynamic_debug: delete pending queries Jim Cromie
2011-07-11 10:49 ` Bart Van Assche
@ 2011-07-12 0:25 ` Joe Perches
2011-07-12 20:50 ` Jason Baron
2011-07-20 17:43 ` Jim Cromie
1 sibling, 2 replies; 72+ messages in thread
From: Joe Perches @ 2011-07-12 0:25 UTC (permalink / raw)
To: Jim Cromie; +Cc: jbaron, linux-kernel, bvanassche, gregkh, gnb
On Mon, 2011-07-11 at 01:46 -0600, Jim Cromie wrote:
> delete pending queries by re-issuing it with disabled flags
[]
> diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
[]
> @@ -505,7 +505,17 @@ static int ddebug_save_pending(struct ddebug_query *query,
>
> list_for_each_entry_safe(pq, pqnext, &pending_queries, link) {
> if (queries_match(query, &pq->query)) {
> - /* query already in list, update flags */
> + /* query already in list */
> + if (!flags) {
> + /* zeroed flags, remove query */
> + if (verbose)
> + pr_info("delete pending: %s\n",
> + show_pending_query(pq));
I think these should be pr_debug.
I know you're only using the current style.
Jason, any reason these can not be converted?
^ permalink raw reply [flat|nested] 72+ messages in thread* Re: [PATCH 21/21] dynamic_debug: delete pending queries
2011-07-12 0:25 ` Joe Perches
@ 2011-07-12 20:50 ` Jason Baron
2011-07-20 5:39 ` Jim Cromie
2011-07-20 17:43 ` Jim Cromie
1 sibling, 1 reply; 72+ messages in thread
From: Jason Baron @ 2011-07-12 20:50 UTC (permalink / raw)
To: Joe Perches; +Cc: Jim Cromie, linux-kernel, bvanassche, gregkh, gnb
On Mon, Jul 11, 2011 at 05:25:35PM -0700, Joe Perches wrote:
> On Mon, 2011-07-11 at 01:46 -0600, Jim Cromie wrote:
> > delete pending queries by re-issuing it with disabled flags
> []
> > diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
> []
> > @@ -505,7 +505,17 @@ static int ddebug_save_pending(struct ddebug_query *query,
> >
> > list_for_each_entry_safe(pq, pqnext, &pending_queries, link) {
> > if (queries_match(query, &pq->query)) {
> > - /* query already in list, update flags */
> > + /* query already in list */
> > + if (!flags) {
> > + /* zeroed flags, remove query */
> > + if (verbose)
> > + pr_info("delete pending: %s\n",
> > + show_pending_query(pq));
>
> I think these should be pr_debug.
> I know you're only using the current style.
>
> Jason, any reason these can not be converted?
>
it should be ok, although we have to be careful not to use them in the
printing path, since that will cause a recursion.
Also, if there is an issue with the dynamic debug code, it makes it more
of a pain to debug :)
-Jason
^ permalink raw reply [flat|nested] 72+ messages in thread* Re: [PATCH 21/21] dynamic_debug: delete pending queries
2011-07-12 20:50 ` Jason Baron
@ 2011-07-20 5:39 ` Jim Cromie
0 siblings, 0 replies; 72+ messages in thread
From: Jim Cromie @ 2011-07-20 5:39 UTC (permalink / raw)
To: Jason Baron; +Cc: Joe Perches, linux-kernel, bvanassche, gregkh, gnb
On Tue, Jul 12, 2011 at 2:50 PM, Jason Baron <jbaron@redhat.com> wrote:
> On Mon, Jul 11, 2011 at 05:25:35PM -0700, Joe Perches wrote:
>> On Mon, 2011-07-11 at 01:46 -0600, Jim Cromie wrote:
>> > delete pending queries by re-issuing it with disabled flags
>> []
>> > diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
>> []
>> > @@ -505,7 +505,17 @@ static int ddebug_save_pending(struct ddebug_query *query,
>> >
>> > list_for_each_entry_safe(pq, pqnext, &pending_queries, link) {
>> > if (queries_match(query, &pq->query)) {
>> > - /* query already in list, update flags */
>> > + /* query already in list */
>> > + if (!flags) {
>> > + /* zeroed flags, remove query */
>> > + if (verbose)
>> > + pr_info("delete pending: %s\n",
>> > + show_pending_query(pq));
>>
>> I think these should be pr_debug.
>> I know you're only using the current style.
>>
>> Jason, any reason these can not be converted?
>>
>
> it should be ok, although we have to be careful not to use them in the
> printing path, since that will cause a recursion.
>
> Also, if there is an issue with the dynamic debug code, it makes it more
> of a pain to debug :)
>
> -Jason
>
I replaced almost all pr_info with pr_debug,
works nicely.
One pr_info remains, cuz code also uses pr_cont(),
which has no corresponding fn
if (verbose) {
int i;
pr_info("split into words: %d ", i);
for (i = 0 ; i < nwords ; i++)
pr_cont(" \"%s\"", words[i]);
pr_cont("\n");
}
Would you consider adding it ?
If so, I can drop verbose var completely.
^ permalink raw reply [flat|nested] 72+ messages in thread
* Re: [PATCH 21/21] dynamic_debug: delete pending queries
2011-07-12 0:25 ` Joe Perches
2011-07-12 20:50 ` Jason Baron
@ 2011-07-20 17:43 ` Jim Cromie
2011-07-20 22:08 ` Joe Perches
1 sibling, 1 reply; 72+ messages in thread
From: Jim Cromie @ 2011-07-20 17:43 UTC (permalink / raw)
To: Joe Perches; +Cc: jbaron, linux-kernel, bvanassche, gregkh, gnb
On Mon, Jul 11, 2011 at 6:25 PM, Joe Perches <joe@perches.com> wrote:
> On Mon, 2011-07-11 at 01:46 -0600, Jim Cromie wrote:
>> delete pending queries by re-issuing it with disabled flags
> []
>> diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
> []
>> @@ -505,7 +505,17 @@ static int ddebug_save_pending(struct ddebug_query *query,
>>
>> list_for_each_entry_safe(pq, pqnext, &pending_queries, link) {
>> if (queries_match(query, &pq->query)) {
>> - /* query already in list, update flags */
>> + /* query already in list */
>> + if (!flags) {
>> + /* zeroed flags, remove query */
>> + if (verbose)
>> + pr_info("delete pending: %s\n",
>> + show_pending_query(pq));
>
> I think these should be pr_debug.
> I know you're only using the current style.
>
> Jason, any reason these can not be converted?
>
>
hi Joe,
one other corner case (in addition to pr_cont, which I probly should
have addressed to you)
+#define pr_fmt(fmt) KBUILD_MODNAME ":%s: " fmt, __func__
makes pr_debug unconditionally print stuff that can be
doubled up if flags are also used.
Ive done s/pr_info/pr_debug/ locally, when combined with:
ddebug_query="module dynamic_debug +pfmlt "
lines are too long, and are redundant
This is pretty trivial, and can be avoided by
"dont add those flags then", but I could imagine it
being a minor annoyance wherever both pr_debug
and pr_info|warn|err|etc are used together.
Perhaps pr_debug and friends should ignore pr_fmt
under CONFIG_DYNAMIC_DEBUG ?
^ permalink raw reply [flat|nested] 72+ messages in thread* Re: [PATCH 21/21] dynamic_debug: delete pending queries
2011-07-20 17:43 ` Jim Cromie
@ 2011-07-20 22:08 ` Joe Perches
0 siblings, 0 replies; 72+ messages in thread
From: Joe Perches @ 2011-07-20 22:08 UTC (permalink / raw)
To: Jim Cromie; +Cc: jbaron, linux-kernel, bvanassche, gregkh, gnb
On Wed, 2011-07-20 at 11:43 -0600, Jim Cromie wrote:
> On Mon, Jul 11, 2011 at 6:25 PM, Joe Perches <joe@perches.com> wrote:
> > On Mon, 2011-07-11 at 01:46 -0600, Jim Cromie wrote:
> >> diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
> > []
> >> @@ -505,7 +505,17 @@ static int ddebug_save_pending(struct ddebug_query *query,
[]
> >> + if (verbose)
> >> + pr_info("delete pending: %s\n",
> >> + show_pending_query(pq));
> > I think these should be pr_debug.
> > I know you're only using the current style.
> > Jason, any reason these can not be converted?
> one other corner case (in addition to pr_cont, which I probly should
> have addressed to you)
> +#define pr_fmt(fmt) KBUILD_MODNAME ":%s: " fmt, __func__
> makes pr_debug unconditionally print stuff that can be
> doubled up if flags are also used.
> Ive done s/pr_info/pr_debug/ locally, when combined with:
> ddebug_query="module dynamic_debug +pfmlt "
> lines are too long, and are redundant
> This is pretty trivial, and can be avoided by
> "dont add those flags then", but I could imagine it
> being a minor annoyance wherever both pr_debug
> and pr_info|warn|err|etc are used together.
> Perhaps pr_debug and friends should ignore pr_fmt
> under CONFIG_DYNAMIC_DEBUG ?
Better I think would be to remove __func__ from those
dynamic debug pr_fmt defines.
Actually, I think it's better to never use __func__,
but that's an argument for a different day.
I think the only reason Jason did it was to always
have __func__ printed. If those become pr_debug
then anyone that really wants to can use +pf.
cheers, Joe
^ permalink raw reply [flat|nested] 72+ messages in thread