Bar Yasser
I am Bar Yasser, I have a very important
Business matters i will like to
discuss with you.If this your Email is valid
Contact me through my personal
email:rawashdeh...@w.cn Thank you Mr.
Yasser Al
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majo...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Josh Boyer
Cc: linux-...@vger.kernel.org
Bcc:
Subject: stable patch queues and submission
Reply-To:
Hi All,
With kernel.org being down, the sta...@kernel.org list is no longer
working. This leaves me wondering if we should send patches directly to
the various stable tree maintainers with [stable] in the prefix and
linux-kernel CC'd, or if the stable queues are even being maintained
during the outage.
Does anyone have a preference or suggestion on how to continue stable
releases until the list comes back? (I'm assuming 'wait' isn't really a
best all around option.)
josh
Josh Boyer
> <paul.go...@windriver.com>
> Cc: linux-...@vger.kernel.org
> Bcc:
> Subject: stable patch queues and submission
> Reply-To:
>
> Hi All,
Yeah... I blame Friday and mutt for that. Sorry for the lack of
subject.
Greg KH
>
> With kernel.org being down, the sta...@kernel.org list is no longer
> working. This leaves me wondering if we should send patches directly to
> the various stable tree maintainers with [stable] in the prefix and
> linux-kernel CC'd, or if the stable queues are even being maintained
> during the outage.
No, please do not, unless you need something "special", and even then,
please cc: lkml and the developers so it's not lost.
> Does anyone have a preference or suggestion on how to continue stable
> releases until the list comes back? (I'm assuming 'wait' isn't really a
> best all around option.)
Wait and continue to add the marking to your patches, my scripts are
still running and pulling the patches out and they are going into the
stable queue just fine. The queue just isn't public at the moment due
to kernel.org being out.
greg k-h
Josh Boyer
> > Hi All,
> >
> > With kernel.org being down, the sta...@kernel.org list is no longer
> > working. This leaves me wondering if we should send patches directly to
> > the various stable tree maintainers with [stable] in the prefix and
> > linux-kernel CC'd, or if the stable queues are even being maintained
> > during the outage.
>
> No, please do not, unless you need something "special", and even then,
> please cc: lkml and the developers so it's not lost.
Would "special" include patches that are backported for longterm
releases?
> > Does anyone have a preference or suggestion on how to continue stable
> > releases until the list comes back? (I'm assuming 'wait' isn't really a
> > best all around option.)
>
> Wait and continue to add the marking to your patches, my scripts are
> still running and pulling the patches out and they are going into the
> stable queue just fine. The queue just isn't public at the moment due
> to kernel.org being out.
That's good to hear, and works for new patches with CC in the commit
log. I should have been more clear in that I was more curious about
patches that are needed for longterm kernels but the upstream commits
don't directly apply. They need to be backported and sent somewhere.
josh
Willy Tarreau
> >
> > With kernel.org being down, the sta...@kernel.org list is no longer
> > working. This leaves me wondering if we should send patches directly to
> > the various stable tree maintainers with [stable] in the prefix and
> > linux-kernel CC'd, or if the stable queues are even being maintained
> > during the outage.
>
> No, please do not, unless you need something "special", and even then,
> please cc: lkml and the developers so it's not lost.
>
> > Does anyone have a preference or suggestion on how to continue stable
> > releases until the list comes back? (I'm assuming 'wait' isn't really a
> > best all around option.)
>
> Wait and continue to add the marking to your patches, my scripts are
> still running and pulling the patches out and they are going into the
> stable queue just fine. The queue just isn't public at the moment due
> to kernel.org being out.
I was thinking about an alternative if that can help. If the mails are
sent to our addresses followed by "+stable", they're easy to filter out
to another mailbox (eg: "willy+...@1wt.eu").
But there are a number of other reviewers on stable@k.o who provide a
lot of value with their reviews, and not having them check the patches
could easily lead to some nasty patches leaking into stable.
Regards,
Willy
Greg KH
> On Fri, Sep 16, 2011 at 08:23:50PM +0200, Greg KH wrote:
> > > Hi All,
> > >
> > > With kernel.org being down, the sta...@kernel.org list is no longer
> > > working. This leaves me wondering if we should send patches directly to
> > > the various stable tree maintainers with [stable] in the prefix and
> > > linux-kernel CC'd, or if the stable queues are even being maintained
> > > during the outage.
> >
> > No, please do not, unless you need something "special", and even then,
> > please cc: lkml and the developers so it's not lost.
> >
> > > Does anyone have a preference or suggestion on how to continue stable
> > > releases until the list comes back? (I'm assuming 'wait' isn't really a
> > > best all around option.)
> >
> > Wait and continue to add the marking to your patches, my scripts are
> > still running and pulling the patches out and they are going into the
> > stable queue just fine. The queue just isn't public at the moment due
> > to kernel.org being out.
>
> I was thinking about an alternative if that can help. If the mails are
> sent to our addresses followed by "+stable", they're easy to filter out
> to another mailbox (eg: "willy+...@1wt.eu").
>
> But there are a number of other reviewers on stable@k.o who provide a
> lot of value with their reviews, and not having them check the patches
> could easily lead to some nasty patches leaking into stable.
I agree, and the review of the final patches during the -rc period will
still happen.
I just don't want to see people start to drop using the "Cc: stable@..."
as part of their patches going to Linus if they somehow think they
should do something else. This proceedure is still working just fine,
and when kernel.org comes back up, it will return to being an automated
process.
thanks,
greg k-h
Greg KH
> On Fri, Sep 16, 2011 at 08:23:50PM +0200, Greg KH wrote:
> > > Hi All,
> > >
> > > With kernel.org being down, the sta...@kernel.org list is no longer
> > > working. This leaves me wondering if we should send patches directly to
> > > the various stable tree maintainers with [stable] in the prefix and
> > > linux-kernel CC'd, or if the stable queues are even being maintained
> > > during the outage.
> >
> > No, please do not, unless you need something "special", and even then,
> > please cc: lkml and the developers so it's not lost.
>
> Would "special" include patches that are backported for longterm
> releases?
Yes.
greg k-h
BBC One Mail
System Administrator.
http://www.bbc.co.uk/lottery/
Mrs. Tessy Brown
Win $500,000 in Coca Cola Company @West Africa end of year
promo.To qualify, Email the correct answer to the question given below
to Mr. Frank Morris via(frankm...@ymail.com)
Question: Who won the 2010 FIFA World Cup in South Africa?
(A)England (B)Spain (C)Germany (D)Brazil.
jim.c...@gmail.com
this reworked* patchset enhances dynamic-debug with:
- multiple queries on bootline, via ddebug_query='"...", formerly just
1 query was accepted. This also allows cat foo-queries > $CONTROL
to add numerous rules together.
- tolerance to errors in ddebug_query. Formerly, a bad rule would
kill the facility, now it stays up, you can correct the rule without
rebooting.
- pending queries. bootline can enable pr_debugs in an uninstalled
module, by adding 'a' flag. When module is loaded later, pr_debug
callsites are enabled, before module-init is run, so pr_debug can be
used in module-init. Currently pending queries are readable in
$DBGMT/dynamic_debug/pending
- filter flags. flags before the operator [+-=] are used to narrow
selection of callsites. This augments module, filename, function
filters, allowing:
echo p+t > $CONTROL # add TID to ALL enabled sites
echo ml+p > $CONTROL # enable sites marked earlier.
- src-dir relative paths in $CONTROL. Formerly, filenames were
printed with full path, and new rules had to use full path if
basename wasnt enough. Now theyre printed with relative paths, and
relative paths are accepted when writing new rules.
Minor things
- added warn if >1 function, filename, module filter is used. also
fix a pr_err() conditional on verbose.
- '_' (empty) flag added. $CONTROL now says '=_' for disabled
callsites, so your grep command can be more selective than '='
- printing is enabled by p flag only, formerly any flag enabled
callsite. this fix is needed for filter-flags as described above.
- dynamic debug supercedes DEBUG - Formerly ddebug couldnt control
callsites when module had DEBUG 1, now it can. DEBUG 1 now enables
callsites by default, but you can turn them off.
- shrink _ddebug.lineno:24 to 18
lineno:24 allows 16G-lines files to be 'pr_debug'd, which is silly.
Largest in tree is 29k-lines, future additions that large are
*unlikely*. Even allowing for out-of-tree machine-generated code
(which shouldnt need ddebug, right? ;-), 256k-lines should be
enough.
- pr_fmt_dbg() - like pr_fmt(), but used in dynamic_pr_debug().
Allows independent control of the prefix-text added by pr_debug vs
pr_info and others. TBD - Joe Perches had issues with this, maybe
addressed here. Its also at end of set, so can be trivially
excluded from upstream.
- internal ddebug verbosity - this modparam enables several levels of
internal debugging. Previous patchsets used the ddebug facilities
within ddebug (eat your own dogfood) but that was deemed too
aggressive.
Future additions.
- user flags: If we can free up extra bits (32bit is currently tight),
adding user-flags (say: x,y,z) would let users mark groups of
callsites, then enable and disable them in a single operation:
echo module foo +x > $CONTROL
echo module bar +x > $CONTROL
...
echo x+p > $CONTROL
Breakdown:
1-15 prep, cleanups, minor things
16-23 major enhancements, doc
24-26 non-essentials, worth considering, discussing.
If you like, you can also get it from github,
git://github.com/jimc/linux-2.6.git dyndbg/on-driver-core
This is a clone of GregKH's driver-core tree, circa -rc3,
which includes 8/12 of Jason Barons dynamic-debug patches.
Ive added Jason's last 4/12, and my 26 patches.
thanks
Jim Cromie
jim.c...@gmail.com
Currently any enabled dynamic-debug flag on a pr_debug callsite will enable
printing, even if _DPRINTK_FLAGS_PRINT is off. Checking print flag directly
allows "-p" to disable callsites without fussing with other flags, so
the following disables everything:
echo -p > $DBGFS/dynamic_debug/control
Signed-off-by: Jim Cromie <jim.c...@gmail.com>
---
include/linux/dynamic_debug.h | 8 +++-----
lib/dynamic_debug.c | 4 ----
2 files changed, 3 insertions(+), 9 deletions(-)
diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index 8aec680..ed7df3d 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -28,7 +28,6 @@ struct _ddebug {
#define _DPRINTK_FLAGS_INCL_TID (1<<4)
#define _DPRINTK_FLAGS_DEFAULT 0
unsigned int flags:8;
- char enabled;
} __attribute__((aligned(8)));
@@ -63,13 +62,12 @@ extern int __dynamic_netdev_dbg(struct _ddebug *descriptor,
.format = (fmt), \
.lineno = __LINE__, \
.flags = _DPRINTK_FLAGS_DEFAULT, \
- .enabled = false, \
}
#define dynamic_pr_debug(fmt, ...) \
do { \
DECLARE_DYNAMIC_DEBUG_METADATA(descriptor, fmt); \
- if (unlikely(descriptor.enabled)) \
+ if (unlikely(descriptor.flags & _DPRINTK_FLAGS_PRINT)) \
__dynamic_pr_debug(&descriptor, pr_fmt(fmt), \
##__VA_ARGS__); \
} while (0)
@@ -77,7 +75,7 @@ do { \
#define dynamic_dev_dbg(dev, fmt, ...) \
do { \
DECLARE_DYNAMIC_DEBUG_METADATA(descriptor, fmt); \
- if (unlikely(descriptor.enabled)) \
+ if (unlikely(descriptor.flags & _DPRINTK_FLAGS_PRINT)) \
__dynamic_dev_dbg(&descriptor, dev, fmt, \
##__VA_ARGS__); \
} while (0)
@@ -85,7 +83,7 @@ do { \
#define dynamic_netdev_dbg(dev, fmt, ...) \
do { \
DECLARE_DYNAMIC_DEBUG_METADATA(descriptor, fmt); \
- if (unlikely(descriptor.enabled)) \
+ if (unlikely(descriptor.flags & _DPRINTK_FLAGS_PRINT)) \
__dynamic_netdev_dbg(&descriptor, dev, fmt, \
##__VA_ARGS__); \
} while (0)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 4138574..f2fb0c0 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -152,10 +152,6 @@ static void ddebug_change(const struct ddebug_query *query,
if (newflags == dp->flags)
continue;
dp->flags = newflags;
- if (newflags)
- dp->enabled = 1;
- else
- dp->enabled = 0;
if (verbose)
pr_info("changed %s:%d [%s]%s %s\n",
dp->filename, dp->lineno,
--
1.7.4.4
jim.c...@gmail.com
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
No MODULE_PARM_DESC added, as modinfo doesnt report it for built-ins.
Signed-off-by: Jim Cromie <jim.c...@gmail.com>
---
lib/dynamic_debug.c | 10 ++++++----
1 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 74b395b..b88f918 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -61,6 +61,8 @@ struct ddebug_iter {
static DEFINE_MUTEX(ddebug_lock);
static LIST_HEAD(ddebug_tables);
static int verbose = 0;
+#define VERBOSE_PROC 10 /* enable per-page msgs on control file reads */
+#define VERBOSE_PROC_SHOW 11 /* enable per-line msgs on control file reads */
module_param(verbose, int, 0644);
/* Return the last part of a pathname */
@@ -636,7 +638,7 @@ static void *ddebug_proc_start(struct seq_file *m, loff_t *pos)
struct _ddebug *dp;
int n = *pos;
- if (verbose)
+ if (verbose >= VERBOSE_PROC)
pr_info("called m=%p *pos=%lld\n", m, (unsigned long long)*pos);
mutex_lock(&ddebug_lock);
@@ -661,7 +663,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 >= VERBOSE_PROC_SHOW)
pr_info("called m=%p p=%p *pos=%lld\n",
m, p, (unsigned long long)*pos);
@@ -685,7 +687,7 @@ static int ddebug_proc_show(struct seq_file *m, void *p)
struct _ddebug *dp = p;
char flagsbuf[8];
- if (verbose)
+ if (verbose >= VERBOSE_PROC_SHOW)
pr_info("called m=%p p=%p\n", m, p);
if (p == SEQ_START_TOKEN) {
@@ -710,7 +712,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 >= VERBOSE_PROC)
pr_info("called m=%p p=%p\n", m, p);
mutex_unlock(&ddebug_lock);
}
--
1.7.4.4
jim.c...@gmail.com
Process multiple commands per line, separated by ';' or '\n'.
All commands are processed, independent of errors, allowing individual
commands to fail, for example when a module is not installed. Errors
are counted, and last error code is returned.
With this, extensive command sets can be given on the boot-line.
Splitting on '\n' allows "cat cmd-file > /dbg/dynamic_debug/control"
where cmd-file contains multiple queries, one per line. Empty commands
are skipped, allowing ";\n\n" to not count as errors.
Splitting on '\n' prevents its use in a format-spec, but thats not very
useful anyway, searching for meaningful and selective substrings is typical.
Also allow comment lines, starting with '#', with optional leading whitespace.
Trailing comments are allowed, if preceded by ';' which terminates
the command on the line. For example:
root@voyage:~# cat debugfs-file
# blank lines ok, comments too
module foo +p
# multiple cmds on line, with ; to separate them
module bar +p ; module buz +p ; # trailing comments need cmd terminator too
root@voyage:~# cat debugfs-file > /dbg/dynamic_debug/control
Signed-off-by: Jim Cromie <jim.c...@gmail.com>
---
lib/dynamic_debug.c | 35 +++++++++++++++++++++++++++++++++--
1 files changed, 33 insertions(+), 2 deletions(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index b88f918..4bf27f3 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -444,6 +444,35 @@ 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) {
+ split = strpbrk(query, ";\n");
+ if (split)
+ *split++ = '\0';
+
+ query = skip_spaces(query);
+ if (!*query || *query == '#')
+ continue;
+ if (verbose)
+ pr_info("query %d: \"%s\"\n", i, query);
+
+ rc = ddebug_exec_query(query);
+ if (rc) {
+ errs++;
+ exitcode = rc;
+ }
+ i++;
+ }
+ pr_info("processed %d queries, with %d errs\n", i, errs);
+
+ return exitcode;
+}
+
#define PREFIX_SIZE 64
#define LEFT(wrote) ((PREFIX_SIZE - wrote) > 0) ? (PREFIX_SIZE - wrote) : 0
@@ -578,7 +607,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;
@@ -883,7 +912,9 @@ 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);
+ pr_info("ddebug initializing with string \"%s\"",
+ 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.4
jim.c...@gmail.com
Current write buffer is 256 bytes, on stack. Allocate it off heap,
and enlarge it to 4096 bytes, big enough for ~100 queries (at 40 bytes
each), and error out if not. This should be enough for most uses, and
others can be split into 2 writes.
This makes it play nicely with:
$> cat debug-queries-file > /dbg/dynamic_debug/control
Signed-off-by: Jim Cromie <jim.c...@gmail.com>
---
lib/dynamic_debug.c | 16 ++++++++++++----
1 files changed, 12 insertions(+), 4 deletions(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 4bf27f3..2b3393f 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -590,24 +590,32 @@ __setup("ddebug_query=", ddebug_setup_query);
* File_ops->write method for <debugfs>/dynamic_debug/conrol. Gathers the
* command text from userspace, parses and executes it.
*/
+#define USER_BUF_PAGE 4095
static ssize_t ddebug_proc_write(struct file *file, const char __user *ubuf,
size_t len, loff_t *offp)
{
- char tmpbuf[256];
+ char *tmpbuf;
int ret;
if (len == 0)
return 0;
- /* we don't check *offp -- multiple writes() are allowed */
- if (len > sizeof(tmpbuf)-1)
+ if (len > USER_BUF_PAGE - 1) {
+ pr_warn("expected <%d bytes into control \n", USER_BUF_PAGE);
return -E2BIG;
- if (copy_from_user(tmpbuf, ubuf, len))
+ }
+ tmpbuf = kmalloc(len + 1, GFP_KERNEL);
+ if (!tmpbuf)
+ return -ENOMEM;
+ if (copy_from_user(tmpbuf, ubuf, len)) {
+ kfree(tmpbuf);
return -EFAULT;
+ }
tmpbuf[len] = '\0';
if (verbose)
pr_info("read %d bytes from userspace\n", (int)len);
ret = ddebug_exec_queries(tmpbuf);
+ kfree(tmpbuf);
if (ret)
return ret;
--
1.7.4.4
jim.c...@gmail.com
Save queries with 'a' flag to pending-queries after applying them.
When a module is loaded later, ddebug_add_module() calls
apply_pending_queries() to scan pending_queries list and call
ddebug_change to apply them.
With this change, the loaded module's pr_debug()s are enabled before
its module_init is invoked, allowing use of pr_debug()s during
initialization.
Behavior:
If pending query matches existing one, the existing one is updated.
Pending queries stay on list through rmmod, modprobe cycles.
If the updated query has 0 flags, it is removed.
If pending query has 0 flags, it is discarded, not added.
Patch adds:
'a' flag to dynamic_debug.h
struct pending_query: like ddebug_query, but with storage for match specs.
ddebug_save_pending():
checks new pending query against existing, to update or remove
discards new do-nothing pending queries.
copies ddebug_query, saving match specs from stack.
adds new pending query to end of pending list (fifo)
apply_pending_queries():
called from ddebug_add_module()
(re)applies queries on newly loaded modules.
queries_match() - helper for ddebug_save_pending
ddebug_remove_all_tables() - clear pending-queries here.
Signed-off-by: Jim Cromie <jim.c...@gmail.com>
---
include/linux/dynamic_debug.h | 1 +
lib/dynamic_debug.c | 154 +++++++++++++++++++++++++++++++++++++++--
2 files changed, 150 insertions(+), 5 deletions(-)
diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index 12ef233..2c9c476 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 */
#if defined DEBUG
#define _DPRINTK_FLAGS_DEFAULT _DPRINTK_FLAGS_PRINT
#else
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 4c8e178..a59d48c 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -53,6 +53,12 @@ struct ddebug_query {
unsigned int first_lineno, last_lineno;
};
+struct pending_query {
+ struct list_head link;
+ struct ddebug_query query;
+ unsigned int flags, mask;
+};
+
struct ddebug_iter {
struct ddebug_table *table;
unsigned int idx;
@@ -65,6 +71,9 @@ static int verbose = 0;
#define VERBOSE_PROC_SHOW 11 /* enable per-line msgs on control file reads */
module_param(verbose, int, 0644);
+/* legal but inapplicable queries, save and test against new modules */
+static LIST_HEAD(pending_queries);
+
/* Return the last part of a pathname */
static inline const char *basename(const char *path)
{
@@ -89,6 +98,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 */
@@ -125,6 +135,25 @@ do { \
q->first_lineno, q->last_lineno); \
} while (0)
+#define vpr_info_pq(pq, msg) \
+do { \
+ struct ddebug_query *q = &pq->query; \
+ if (verbose) \
+ /* trim last char off format print */ \
+ pr_info("%s: func=\"%s\" file=\"%s\" " \
+ "module=\"%s\" format=\"%.*s\" " \
+ "lineno=%u-%u " \
+ "flags=0x%x mask=0x%x", \
+ msg, \
+ q->function ? q->function : "", \
+ q->filename ? q->filename : "", \
+ q->module ? q->module : "", \
+ (int)(q->format ? strlen(q->format) - 1 : 0), \
+ q->format ? q->format : "", \
+ q->first_lineno, q->last_lineno, \
+ pq->flags, pq->mask); \
+} while (0)
+
static bool query_matches_callsite(struct _ddebug *dp,
const struct ddebug_query *query)
{
@@ -159,7 +188,7 @@ static bool query_matches_callsite(struct _ddebug *dp,
* the user which ddebug's were changed, or whether none
* were matched. Called with ddebug_lock held.
*/
-static void ddebug_change(const struct ddebug_query *query,
+static int ddebug_change(const struct ddebug_query *query,
unsigned int flags, unsigned int mask)
{
int i;
@@ -196,8 +225,7 @@ static void ddebug_change(const struct ddebug_query *query,
sizeof(flagbuf)));
}
}
- if (!nfound && verbose)
- pr_info("no matches for query\n");
+ return nfound;
}
/*
@@ -435,6 +463,95 @@ static int ddebug_parse_flags(const char *str, unsigned int *flagsp,
return 0;
}
+/* check if new query exactly matches existing one */
+static bool queries_match(struct ddebug_query *qnew, struct ddebug_query *qcur)
+{
+ if (!qnew->module != !qcur->module ||
+ !qnew->filename != !qcur->filename ||
+ !qnew->function != !qcur->function ||
+ !qnew->format != !qcur->format)
+ return false; /* a match-spec set/unset state differs */
+
+ if (qnew->last_lineno != qcur->last_lineno ||
+ qnew->first_lineno != qcur->first_lineno)
+ return false;
+
+ if ((qnew->module && strcmp(qnew->module, qcur->module)) ||
+ (qnew->filename && strcmp(qnew->filename, qcur->filename)) ||
+ (qnew->function && strcmp(qnew->function, qcur->function)) ||
+ (qnew->format && strcmp(qnew->format, qcur->format)))
+ return false;
+
+ return true;
+}
+
+static void pqfree(struct pending_query *pq)
+{
+ if (pq->query.module)
+ kfree(pq->query.module);
+ if (pq->query.function)
+ kfree(pq->query.function);
+ if (pq->query.filename)
+ kfree(pq->query.filename);
+ if (pq->query.format)
+ kfree(pq->query.format);
+ kfree(pq);
+}
+
+/* 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)
+{
+ 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;
+ vpr_info_pq(pq, "already pending, updated it");
+ return 0;
+ }
+ }
+ if (!flags) {
+ vpr_info_pq(pq, "pending query has 0 flags, discarding");
+ return 0;
+ }
+ vpr_info_dq(query, "add to pending");
+
+ 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)
+ if (!(pq->query.module = kstrdup(query->module, GFP_KERNEL)))
+ return -ENOMEM;
+ if (query->function)
+ if (!(pq->query.function = kstrdup(query->function, GFP_KERNEL)))
+ return -ENOMEM;
+ if (query->filename)
+ if (!(pq->query.filename = kstrdup(query->filename, GFP_KERNEL)))
+ return -ENOMEM;
+ if (query->format)
+ if (!(pq->query.format = kstrdup(query->format, GFP_KERNEL)))
+ return -ENOMEM;
+
+ pq->flags = flags;
+ pq->mask = mask;
+
+ list_add_tail(&pq->link, &pending_queries);
+
+ if (verbose)
+ pr_info("query saved as pending, in %ld bytes\n",
+ sizeof(struct pending_query));
+ return 0;
+}
+
static int ddebug_exec_query(char *query_string)
{
unsigned int flags = 0, mask = 0;
@@ -442,6 +559,7 @@ static int ddebug_exec_query(char *query_string)
#define MAXWORDS 9
int nwords;
char *words[MAXWORDS];
+ int nfound, rc = 0;
nwords = ddebug_tokenize(query_string, words, MAXWORDS);
if (nwords <= 0)
@@ -452,8 +570,13 @@ static int ddebug_exec_query(char *query_string)
return -EINVAL;
/* actually go and implement the change */
- ddebug_change(&query, flags, mask);
- return 0;
+ nfound = ddebug_change(&query, flags, mask);
+ vpr_info_dq((&query), (nfound) ? "applied" : "no-match");
+
+ if (flags & _DPRINTK_FLAGS_APPEND)
+ rc = ddebug_save_pending(&query, flags, mask);
+
+ return rc;
}
/* handle multiple queries, continue on error, return last error */
@@ -811,6 +934,19 @@ 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;
+
+ list_for_each_entry_safe(pq, pqnext, &pending_queries, link) {
+ nfound = ddebug_change(&pq->query, pq->flags, pq->mask);
+ vpr_info_pq(pq, (nfound) ?
+ "applied pending" : "no-match on pending");
+ }
+}
+
/*
* Allocate a new ddebug_table for the given module
* and add it to the global list.
@@ -835,6 +971,7 @@ int ddebug_add_module(struct _ddebug *tab, unsigned int n,
mutex_lock(&ddebug_lock);
list_add_tail(&dt->link, &ddebug_tables);
+ apply_pending_queries(dt);
mutex_unlock(&ddebug_lock);
if (verbose)
@@ -876,6 +1013,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,
@@ -883,6 +1022,11 @@ static void ddebug_remove_all_tables(void)
link);
ddebug_table_free(dt);
}
+ list_for_each_entry_safe(pq, pqnext, &pending_queries, link) {
+ vpr_info_pq(pq, "delete pending");
+ list_del_init(&pq->link);
+ pqfree(pq);
+ }
mutex_unlock(&ddebug_lock);
}
--
1.7.4.4
jim.c...@gmail.com
Change describe_flags() to emit '=[pmflta_]+' for current callsite
flags, or just '=_' when they're disabled. Having '=' in output
allows a more selective grep expression, in contrast '-' may appear
in filenames, line-ranges, and format-strings. '=' also has better
mnemonics, saying; "the current setting is equal to <flags>".
The default allows grep "=_" <dbgfs>/dynamic_debug/control
to see disabled callsites while avoiding the many occurrences of " = "
seen in format strings.
Enlarge flagsbufs to handle additional flag chars, "=a_"
Signed-off-by: Jim Cromie <jim.c...@gmail.com>
---
lib/dynamic_debug.c | 12 +++++++-----
1 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 0c9e53a..29dbf69 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -99,6 +99,7 @@ static struct { unsigned flag:8; char opt_char; } opt_array[] = {
{ _DPRINTK_FLAGS_INCL_LINENO, 'l' },
{ _DPRINTK_FLAGS_INCL_TID, 't' },
{ _DPRINTK_FLAGS_APPEND, 'a' },
+ { _DPRINTK_FLAGS_DEFAULT, '_' },
};
/* format a string into buf[] which describes the _ddebug's flags */
@@ -108,12 +109,13 @@ static char *ddebug_describe_flags(struct _ddebug *dp, char *buf,
char *p = buf;
int i;
- BUG_ON(maxlen < 4);
+ BUG_ON(maxlen < 10);
+ *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++ = '-';
+ if (*(p-1) == '=')
+ *p++ = '_';
*p = '\0';
return buf;
@@ -195,7 +197,7 @@ static int ddebug_change(const struct ddebug_query *query,
struct ddebug_table *dt;
unsigned int newflags;
unsigned int nfound = 0;
- char flagbuf[8];
+ char flagbuf[10];
/* search for matching ddebugs */
list_for_each_entry(dt, &ddebug_tables, link) {
@@ -866,7 +868,7 @@ static int ddebug_proc_show(struct seq_file *m, void *p)
{
struct ddebug_iter *iter = m->private;
struct _ddebug *dp = p;
- char flagsbuf[8];
+ char flagsbuf[10];
if (verbose >= VERBOSE_PROC_SHOW)
pr_info("called m=%p p=%p\n", m, p);
--
1.7.4.4
jim.c...@gmail.com
Change definition of flags spec to [pmflta_]*[-+=][pmflta_]*
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 also allows rules like "a-a" to select pending queries and
remove them from the list.
Also allow 0 flags byte, so that active rules can be completely
reset by writing "p=" or "p=_".
Patch factors flag-scanning into ddebug_parse_flag_settings(),
and uses it for both filter-flags and new flags, and adds pr_err()
where useful.
Signed-off-by: Jim Cromie <jim.c...@gmail.com>
---
lib/dynamic_debug.c | 85 +++++++++++++++++++++++++++++++--------------------
1 files changed, 52 insertions(+), 33 deletions(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 29dbf69..3c9244d 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -191,7 +191,8 @@ static bool 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;
@@ -213,6 +214,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;
@@ -406,29 +410,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--) {
@@ -437,13 +421,46 @@ 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
+ * [pmflta_]*[-+=][pmflta_]+. 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;
- if (verbose)
- pr_info("flags=0x%x\n", flags);
+ } 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;
+ }
/* calculate final *flagsp, *maskp according to mask and op */
switch (op) {
@@ -461,7 +478,9 @@ static int ddebug_parse_flags(const char *str, unsigned int *flagsp,
break;
}
if (verbose)
- pr_info("*flagsp=0x%x *maskp=0x%x\n", *flagsp, *maskp);
+ pr_info("filter=0x%x op='%c' flags=0x%x *flagsp=0x%x *maskp=0x%x\n",
+ filter, op, flags, *flagsp, *maskp);
+
return 0;
}
@@ -563,7 +582,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;
@@ -575,14 +594,14 @@ 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 */
- nfound = ddebug_change(&query, flags, mask);
+ nfound = ddebug_change(&query, flags, mask, filter);
vpr_info_dq((&query), (nfound) ? "applied" : "no-match");
- if (flags & _DPRINTK_FLAGS_APPEND)
+ if (flags & _DPRINTK_FLAGS_APPEND || filter & _DPRINTK_FLAGS_APPEND)
rc = ddebug_save_pending(&query, flags, mask);
return rc;
@@ -950,7 +969,7 @@ static void apply_pending_queries(struct ddebug_table *dt)
int nfound;
list_for_each_entry_safe(pq, pqnext, &pending_queries, link) {
- nfound = ddebug_change(&pq->query, pq->flags, pq->mask);
+ nfound = ddebug_change(&pq->query, pq->flags, pq->mask, 0);
vpr_info_pq(pq, (nfound) ?
"applied pending" : "no-match on pending");
}
--
1.7.4.4
jim.c...@gmail.com
Add pending file so user can see queries that are pending. Output
format imitates that used to submit commands/queries. This simplifies
cut-paste-edit to delete pending queries when they're no longer wanted.
Signed-off-by: Jim Cromie <jim.c...@gmail.com>
---
lib/dynamic_debug.c | 191 ++++++++++++++++++++++++++++++++++++++++++++++++++-
1 files changed, 188 insertions(+), 3 deletions(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index d7b71a6..80b5322 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -64,6 +64,10 @@ struct ddebug_iter {
unsigned int idx;
};
+struct pending_iter {
+ struct pending_query *elem;
+};
+
static DEFINE_MUTEX(ddebug_lock);
static LIST_HEAD(ddebug_tables);
static int verbose = 0;
@@ -842,7 +846,8 @@ static void *ddebug_proc_start(struct seq_file *m, loff_t *pos)
int n = *pos;
if (verbose >= VERBOSE_PROC)
- pr_info("called m=%p *pos=%lld\n", m, (unsigned long long)*pos);
+ pr_info("called m=%p *pos=%lld\n",
+ m, (unsigned long long)*pos);
mutex_lock(&ddebug_lock);
@@ -963,7 +968,180 @@ static const struct file_operations ddebug_proc_fops = {
.write = ddebug_proc_write
};
-/* apply matching queries in pending-queries list */
+/*
+ * Set the iterator to point to the first pending_query object
+ * and return a pointer to that first object. Returns
+ * NULL if there are no pending_querys at all.
+ */
+static struct pending_query *pending_iter_first(struct pending_iter *iter)
+{
+ if (list_empty(&pending_queries)) {
+ iter->elem = NULL;
+ return NULL;
+ }
+ iter->elem = list_entry(pending_queries.next,
+ struct pending_query, link);
+ return iter->elem;
+}
+
+/*
+ * Advance the iterator to point to the next pending_query
+ * object from the one the iterator currently points at,
+ * and returns a pointer to the new pending_query. Returns
+ * NULL if the iterator has seen all the pending_querys.
+ */
+static struct pending_query *pending_iter_next(struct pending_iter *iter)
+{
+ if (iter->elem == NULL)
+ return NULL;
+ if (list_is_last(&iter->elem->link, &pending_queries)) {
+ iter->elem = NULL;
+ return NULL;
+ }
+ iter->elem = list_entry(iter->elem->link.next,
+ struct pending_query, link);
+ return iter->elem;
+}
+
+/*
+ * Seq_ops start method. Called at the start of every read() call on
+ * pending file from userspace. Takes the ddebug_lock and seeks the
+ * seq_file's iterator to the given position.
+ */
+static void *pending_proc_start(struct seq_file *m, loff_t *pos)
+{
+ struct pending_iter *iter = m->private;
+ struct pending_query *pq;
+ int n = *pos;
+
+ if (verbose >= VERBOSE_PROC)
+ pr_info("called m=%p *pos=%lld\n",
+ m, (unsigned long long)*pos);
+
+ mutex_lock(&ddebug_lock);
+
+ if (!n)
+ return SEQ_START_TOKEN;
+ if (n < 0)
+ return NULL;
+ pq = pending_iter_first(iter);
+ while (pq != NULL && --n > 0)
+ pq = pending_iter_next(iter);
+ return pq;
+}
+
+/*
+ * Seq_ops next method. Called several times within a read()
+ * call from userspace, with pending_lock held. Walks to the
+ * next pending_query object with a special case for the header line.
+ */
+static void *pending_proc_next(struct seq_file *m, void *p, loff_t *pos)
+{
+ struct pending_iter *iter = m->private;
+ struct pending_query *pq;
+
+ if (verbose >= VERBOSE_PROC_SHOW)
+ pr_info("called m=%p p=%p *pos=%lld\n",
+ m, p, (unsigned long long)*pos);
+
+ if (p == SEQ_START_TOKEN)
+ pq = pending_iter_first(iter);
+ else
+ pq = pending_iter_next(iter);
+ ++*pos;
+ return pq;
+}
+
+/*
+ * Seq_ops show method. Called several times within a read()
+ * call from userspace, with pending_lock held. Formats the
+ * current pending_query as a single human-readable line, with a
+ * special case for the header line.
+ */
+static int pending_proc_show(struct seq_file *m, void *p)
+{
+ struct pending_iter *iter = m->private;
+ struct pending_query *pq = iter->elem;
+ struct ddebug_query *q = &pq->query;
+ char flagsbuf[10];
+
+ if (verbose >= VERBOSE_PROC_SHOW)
+ pr_info("called m=%p p=%p\n", m, p);
+
+ if (p == SEQ_START_TOKEN) {
+ seq_puts(m, "# func file module format line flags mask\n");
+ return 0;
+ }
+
+ seq_printf(m, "%s%s%s%s%s%s%s%s line %d-%d %s\n",
+ q->module ? "module " : "",
+ q->module ? q->module : "",
+ q->function ? " func " : "",
+ q->function ? q->function : "",
+ q->filename ? " file " : "",
+ q->filename ? q->filename : "",
+ q->format ? " format " : "",
+ q->format ? q->format : "",
+ q->first_lineno, q->last_lineno,
+ ddebug_describe_flags(pq->flags, flagsbuf, sizeof(flagsbuf)));
+
+ return 0;
+}
+
+/*
+ * Seq_ops stop method. Called at the end of each read()
+ * call from userspace. Drops pending_lock.
+ */
+static void pending_proc_stop(struct seq_file *m, void *p)
+{
+ if (verbose >= VERBOSE_PROC)
+ pr_info("called m=%p p=%p\n", m, p);
+ mutex_unlock(&ddebug_lock);
+}
+
+static const struct seq_operations pending_proc_seqops = {
+ .start = pending_proc_start,
+ .next = pending_proc_next,
+ .show = pending_proc_show,
+ .stop = pending_proc_stop
+};
+
+/*
+ * File_ops->open method for <debugfs>/dynamic_debug/control. Does the seq_file
+ * setup dance, and also creates an iterator to walk the pending_querys.
+ * Note that we create a seq_file always, even for O_WRONLY files
+ * where it's not needed, as doing so simplifies the ->release method.
+ */
+static int pending_proc_open(struct inode *inode, struct file *file)
+{
+ struct pending_iter *iter;
+ int err;
+
+ if (verbose)
+ pr_info("called\n");
+
+ iter = kzalloc(sizeof(*iter), GFP_KERNEL);
+ if (iter == NULL)
+ return -ENOMEM;
+
+ err = seq_open(file, &pending_proc_seqops);
+ if (err) {
+ kfree(iter);
+ return err;
+ }
+ ((struct seq_file *) file->private_data)->private = iter;
+ return 0;
+}
+
+static const struct file_operations pending_proc_fops = {
+ .owner = THIS_MODULE,
+ .open = pending_proc_open,
+ .read = seq_read,
+ .llseek = seq_lseek,
+ .release = seq_release_private,
+};
+
+/* 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;
@@ -1063,7 +1241,7 @@ static __initdata int ddebug_init_success;
static int __init dynamic_debug_init_debugfs(void)
{
- struct dentry *dir, *file;
+ struct dentry *dir, *file, *file2;
if (!ddebug_init_success)
return -ENODEV;
@@ -1077,6 +1255,13 @@ static int __init dynamic_debug_init_debugfs(void)
debugfs_remove(dir);
return -ENOMEM;
}
+ file2 = debugfs_create_file("pending", 0444, dir, NULL,
+ &pending_proc_fops);
+ if (!file2) {
+ debugfs_remove(file);
+ debugfs_remove(dir);
+ return -ENOMEM;
+ }
return 0;
}
--
1.7.4.4
jim.c...@gmail.com
if _ddebug table is empty (which shouldn't happen in a CONFIG_DYNAMIC_DEBUG
build), then warn (BUILD_BUG??) and return early. This skips empty
table scan and parsing of setup-string, including the pr_info call
noting the parse.
Signed-off-by: Jim Cromie <jim.c...@gmail.com>
---
lib/dynamic_debug.c | 32 +++++++++++++++++---------------
1 files changed, 17 insertions(+), 15 deletions(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 80b5322..1a8941e 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -1272,23 +1272,25 @@ static int __init dynamic_debug_init(void)
int ret = 0;
int n = 0;
- if (__start___verbose != __stop___verbose) {
- iter = __start___verbose;
- modname = iter->modname;
- iter_start = iter;
- for (; iter < __stop___verbose; iter++) {
- if (strcmp(modname, iter->modname)) {
- ret = ddebug_add_module(iter_start, n, modname);
- if (ret)
- goto out_free;
- n = 0;
- modname = iter->modname;
- iter_start = iter;
- }
- n++;
+ if (__start___verbose == __stop___verbose) {
+ pr_warn("_ddebug table is empty in a CONFIG_DYNAMIC_DEBUG build");
+ return 1;
+ }
+ iter = __start___verbose;
+ modname = iter->modname;
+ iter_start = iter;
+ for (; iter < __stop___verbose; iter++) {
+ if (strcmp(modname, iter->modname)) {
+ ret = ddebug_add_module(iter_start, n, modname);
+ if (ret)
+ goto out_free;
+ n = 0;
+ modname = iter->modname;
+ iter_start = iter;
}
- ret = ddebug_add_module(iter_start, n, modname);
+ n++;
}
+ ret = ddebug_add_module(iter_start, n, modname);
/* ddebug_query boot param got passed -> set it up */
if (ddebug_setup_string[0] != '\0') {
--
1.7.4.4
jim.c...@gmail.com
dynamic_pr_debug can add module, function, file, and line selectively,
so theres no need to also add them via pr_fmt. Moreover, using pr_fmt
to add KBUILD_MODNAME ": ", as is typical for pr_info etc, causes
pr_debug() to double-print those fields added by the flag-settings.
So define pr_fmt_dbg(fmt), and use it in dynamic_pr_debug(). This lets
users use pr_fmt() for pr_info and friends, without affecting pr_debug().
For non-dynamic-debug builds, pr_fmt_dbg() uses pr_fmt(), preserving
existing print info.
It also allows incrementally converting modules by dropping NAME
from pr_debug(NAME ": ...") and adding it once via pr_fmt_dbg().
Later, if/when module-name is added to dynamic-debug's default,
modules can drop their pr_fmt_dbg()s, and get it via header defns
for both non- and dynamic-debug builds.
Signed-off-by: Jim Cromie <jim.c...@gmail.com>
---
include/linux/dynamic_debug.h | 16 +++++++++++++---
1 files changed, 13 insertions(+), 3 deletions(-)
diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index 5622e04..722a4f5 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -69,11 +69,15 @@ extern int __dynamic_netdev_dbg(struct _ddebug *descriptor,
.flags = _DPRINTK_FLAGS_DEFAULT, \
}
+#ifndef pr_fmt_dbg
+#define pr_fmt_dbg(fmt) fmt
+#endif
+
#define dynamic_pr_debug(fmt, ...) \
do { \
DECLARE_DYNAMIC_DEBUG_METADATA(descriptor, fmt); \
if (unlikely(descriptor.flags & _DPRINTK_FLAGS_PRINT)) \
- __dynamic_pr_debug(&descriptor, pr_fmt(fmt), \
+ __dynamic_pr_debug(&descriptor, pr_fmt_dbg(fmt),\
##__VA_ARGS__); \
} while (0)
@@ -100,10 +104,16 @@ static inline int ddebug_remove_module(const char *mod)
return 0;
}
+#ifndef pr_fmt_dbg
+#define pr_fmt_dbg(fmt) pr_fmt(fmt)
+#endif
+
#define dynamic_pr_debug(fmt, ...) \
- do { if (0) printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__); } while (0)
+ do { if (0) printk(KERN_DEBUG pr_fmt_dbg(fmt), ##__VA_ARGS__); \
+ } while (0)
#define dynamic_dev_dbg(dev, fmt, ...) \
- do { if (0) dev_printk(KERN_DEBUG, dev, fmt, ##__VA_ARGS__); } while (0)
+ do { if (0) dev_printk(KERN_DEBUG, dev, fmt, ##__VA_ARGS__); \
+ } while (0)
#endif
#endif
--
1.7.4.4
jim.c...@gmail.com
Switch this driver over to using more flexible dynamic-debug facility.
With previous patch and CONFIG_I2C_DEBUG_BUS=y, we get pr_debug()s
enabled by default:
root@voyage:~# modprobe scx200_acb
root@voyage:~# grep scx200_acb /dbg/dynamic_debug/control
drivers/i2c/busses/scx200_acb.c:419 [scx200_acb]scx200_acb_probe =p "..."
drivers/i2c/busses/scx200_acb.c:408 [scx200_acb]scx200_acb_probe =p "..."
drivers/i2c/busses/scx200_acb.c:399 [scx200_acb]scx200_acb_probe =p "..."
drivers/i2c/busses/scx200_acb.c:584 [scx200_acb]scx200_acb_init =p "..."
The pr_fmt() adds the module name to pr_err() output, the pr_fmt_dbg()
does the same for pr_debug(). If and when modname is added to the
dynamic-debug default under DEBUG, we can drop the pr_fmt_dbg().
dynamic_debug:ddebug_add_module: 4 debug prints in module scx200_acb
scx200_acb: NatSemi SCx200 ACCESS.bus Driver
scx200_acb: ACBCTL2 readback failed
scx200_acb: probe failed
scx200_acb: ACBCTL2 readback failed
scx200_acb: probe failed
Signed-off-by: Jim Cromie <jim.c...@gmail.com>
---
drivers/i2c/busses/scx200_acb.c | 21 +++++++++++----------
1 files changed, 11 insertions(+), 10 deletions(-)
diff --git a/drivers/i2c/busses/scx200_acb.c b/drivers/i2c/busses/scx200_acb.c
index 986e5f6..30a2c6b 100644
--- a/drivers/i2c/busses/scx200_acb.c
+++ b/drivers/i2c/busses/scx200_acb.c
@@ -23,6 +23,9 @@
Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
*/
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+#define pr_fmt_dbg(fmt) pr_fmt(fmt)
+
#include <linux/module.h>
#include <linux/errno.h>
#include <linux/kernel.h>
@@ -37,8 +40,6 @@
#include <linux/scx200.h>
-#define NAME "scx200_acb"
-
MODULE_AUTHOR("Christer Weinigel <win...@nano-system.com>");
MODULE_DESCRIPTION("NatSemi SCx200 ACCESS.bus Driver");
MODULE_ALIAS("platform:cs5535-smb");
@@ -398,7 +399,7 @@ static __devinit int scx200_acb_probe(struct scx200_acb_iface *iface)
outb(0x70, ACBCTL2);
if (inb(ACBCTL2) != 0x70) {
- pr_debug(NAME ": ACBCTL2 readback failed\n");
+ pr_debug("ACBCTL2 readback failed\n");
return -ENXIO;
}
@@ -406,7 +407,7 @@ static __devinit int scx200_acb_probe(struct scx200_acb_iface *iface)
val = inb(ACBCTL1);
if (val) {
- pr_debug(NAME ": disabled, but ACBCTL1=0x%02x\n",
+ pr_debug("disabled, but ACBCTL1=0x%02x\n",
val);
return -ENXIO;
}
@@ -417,7 +418,7 @@ static __devinit int scx200_acb_probe(struct scx200_acb_iface *iface)
val = inb(ACBCTL1);
if ((val & ACBCTL1_NMINTE) != ACBCTL1_NMINTE) {
- pr_debug(NAME ": enabled, but NMINTE won't be set, "
+ pr_debug("enabled, but NMINTE won't be set, "
"ACBCTL1=0x%02x\n", val);
return -ENXIO;
}
@@ -433,7 +434,7 @@ static __devinit struct scx200_acb_iface *scx200_create_iface(const char *text,
iface = kzalloc(sizeof(*iface), GFP_KERNEL);
if (!iface) {
- printk(KERN_ERR NAME ": can't allocate memory\n");
+ pr_err("can't allocate memory\n");
return NULL;
}
@@ -459,14 +460,14 @@ static int __devinit scx200_acb_create(struct scx200_acb_iface *iface)
rc = scx200_acb_probe(iface);
if (rc) {
- printk(KERN_WARNING NAME ": probe failed\n");
+ pr_warn("probe failed\n");
return rc;
}
scx200_acb_reset(iface);
if (i2c_add_adapter(adapter) < 0) {
- printk(KERN_ERR NAME ": failed to register\n");
+ pr_err("failed to register\n");
return -ENODEV;
}
@@ -493,7 +494,7 @@ static struct scx200_acb_iface * __devinit scx200_create_dev(const char *text,
return NULL;
if (!request_region(base, 8, iface->adapter.name)) {
- printk(KERN_ERR NAME ": can't allocate io 0x%lx-0x%lx\n",
+ pr_err("can't allocate io 0x%lx-0x%lx\n",
base, base + 8 - 1);
goto errout_free;
}
@@ -583,7 +584,7 @@ static __init void scx200_scan_isa(void)
static int __init scx200_acb_init(void)
{
- pr_debug(NAME ": NatSemi SCx200 ACCESS.bus Driver\n");
+ pr_debug("NatSemi SCx200 ACCESS.bus Driver\n");
/* First scan for ISA-based devices */
scx200_scan_isa(); /* XXX: should we care about errors? */
--
1.7.4.4
jim.c...@gmail.com
Rework description of flags-spec into 3 parts; flags-filter, flags-change,
flags-values. Add section on pending-queries. Explain flags filtering
in detail. Reflow some paragraphs.
Add example of a command-file which shows commenting,
multiple queries per line, and describes constraints.
Signed-off-by: Jim Cromie <jim.c...@gmail.com>
---
Documentation/dynamic-debug-howto.txt | 336 ++++++++++++++++++++++-----------
1 files changed, 222 insertions(+), 114 deletions(-)
diff --git a/Documentation/dynamic-debug-howto.txt b/Documentation/dynamic-debug-howto.txt
index f959909..bf9037f 100644
--- a/Documentation/dynamic-debug-howto.txt
+++ b/Documentation/dynamic-debug-howto.txt
@@ -4,117 +4,134 @@ Introduction
This document describes how to use the dynamic debug (ddebug) feature.
-Dynamic debug is designed to allow you to dynamically enable/disable kernel
-code to obtain additional kernel information. Currently, if
-CONFIG_DYNAMIC_DEBUG is set, then all pr_debug()/dev_dbg() calls can be
-dynamically enabled per-callsite.
+Dynamic debug is designed to allow you to dynamically enable/disable
+kernel code to obtain additional kernel information. If kernel is
+built with CONFIG_DYNAMIC_DEBUG=y, then all pr_debug()/dev_dbg() calls
+can be dynamically enabled per-callsite.
Dynamic debug has even more useful features:
- * Simple query language allows turning on and off debugging statements by
- matching any combination of:
+ * Simple query language allows turning on and off debugging statements
+ by matching any combination of 0 or 1 of:
- source filename
- function name
- line number (including ranges of line numbers)
- module name
- format string
+ - current debugging flags
- * Provides a debugfs control file: <debugfs>/dynamic_debug/control which can be
- read to display the complete list of known debug statements, to help guide you
+ * Provides a debugfs control file: <debugfs>/dynamic_debug/control
+ which can be read to display the complete list of known debug
+ statements, to help guide you.
Controlling dynamic debug Behaviour
===================================
-The behaviour of pr_debug()/dev_dbg()s are controlled via writing to a
-control file in the 'debugfs' filesystem. Thus, you must first mount the debugfs
-filesystem, in order to make use of this feature. Subsequently, we refer to the
-control file as: <debugfs>/dynamic_debug/control. For example, if you want to
-enable printing from source file 'svcsock.c', line 1603 you simply do:
+The behaviour of pr_debug()/dev_dbg()/net_dbg()s are controlled via
+writing to a control file in the 'debugfs' filesystem. Thus, you must
+first mount the debugfs filesystem, in order to make use of this
+feature. Subsequently, we refer to mount point as $DBGFS, and the
+control file as $CONTROL. So if you want to enable printing from
+source file 'svcsock.c', line 1603 you simply do:
-nullarbor:~ # echo 'file svcsock.c line 1603 +p' >
- <debugfs>/dynamic_debug/control
+nullarbor:~ # echo file svcsock.c line 1603 +p > $CONTROL
If you make a mistake with the syntax, the write will fail thus:
-nullarbor:~ # echo 'file svcsock.c wtf 1 +p' >
- <debugfs>/dynamic_debug/control
+nullarbor:~ # 'echo file svcsock.c wtf 1 +p' > $CONTROL
-bash: echo: write error: Invalid argument
Viewing Dynamic Debug Behaviour
-===========================
+===============================
-You can view the currently configured behaviour of all the debug statements
-via:
+You can view the currently configured behaviour by catting or grepping
+the $CONTROL file:
-nullarbor:~ # cat <debugfs>/dynamic_debug/control
+root@voyage:~# grep freezer $CONTROL
+kernel/freezer.c:128 [freezer]cancel_freezing =_ " clean up: %s\012"
+kernel/freezer.c:60 [freezer]refrigerator =_ "%s left refrigerator\012"
+kernel/freezer.c:41 [freezer]refrigerator =_ "%s entered refrigerator\012"
+
+The third column shows the current flag settings for each debug
+statement callsite (menmonic: what they are equal to, see below for
+definitions of the flags). The default value, with nothing enabled,
+is "=_". So you can view all the debug statement callsites with any
+non-default flags:
+
+nullarbor:~ # grep -v "=_" $CONTROL
# filename:lineno [module]function flags format
-/usr/src/packages/BUILD/sgi-enhancednfs-1.4/default/net/sunrpc/svc_rdma.c:323 [svcxprt_rdma]svc_rdma_cleanup - "SVCRDMA Module Removed, deregister RPC RDMA transport\012"
-/usr/src/packages/BUILD/sgi-enhancednfs-1.4/default/net/sunrpc/svc_rdma.c:341 [svcxprt_rdma]svc_rdma_init - "\011max_inline : %d\012"
-/usr/src/packages/BUILD/sgi-enhancednfs-1.4/default/net/sunrpc/svc_rdma.c:340 [svcxprt_rdma]svc_rdma_init - "\011sq_depth : %d\012"
-/usr/src/packages/BUILD/sgi-enhancednfs-1.4/default/net/sunrpc/svc_rdma.c:338 [svcxprt_rdma]svc_rdma_init - "\011max_requests : %d\012"
-...
+/usr/src/packages/BUILD/sgi-enhancednfs-1.4/default/net/sunrpc/svcsock.c:1603 [sunrpc]svc_send =p "svc_process: st_sendto returned %d\012"
+
+
+Command Language Reference
+==========================
+At the lexical level, a command comprises a sequence of words separated
+by whitespace characters.
-You can also apply standard Unix text manipulation filters to this
-data, e.g.
+nullarbor:~# echo file svcsock.c line 1603 +p > $CONTROL
-nullarbor:~ # grep -i rdma <debugfs>/dynamic_debug/control | wc -l
-62
+nullarbor:~# echo 'file svcsock.c line 1603 +p' > $CONTROL
-nullarbor:~ # grep -i tcp <debugfs>/dynamic_debug/control | wc -l
-42
+nullarbor:~# echo -n ' file svcsock.c line 1603 +p ' > $CONTROL
-Note in particular that the third column shows the enabled behaviour
-flags for each debug statement callsite (see below for definitions of the
-flags). The default value, no extra behaviour enabled, is "-". So
-you can view all the debug statement callsites with any non-default flags:
+nullarbor:~# echo -n 'file svcsock.c line 1603 +p' > $CONTROL
-nullarbor:~ # awk '$3 != "-"' <debugfs>/dynamic_debug/control
-# filename:lineno [module]function flags format
-/usr/src/packages/BUILD/sgi-enhancednfs-1.4/default/net/sunrpc/svcsock.c:1603 [sunrpc]svc_send p "svc_process: st_sendto returned %d\012"
+Multiple query/commands
+=======================
+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 ';' or '\n'
-Command Language Reference
-==========================
+voyage:~# echo "func cancel_freezing +p ; func refrigerator +p; " > $CONTROL
+voyage:~# printf "func cancel_freezing +p \n func refrigerator +p; " > $CONTROL
-At the lexical level, a command comprises a sequence of words separated
-by whitespace characters. Note that newlines are treated as word
-separators and do *not* end a command or allow multiple commands to
-be done together. So these are all equivalent:
+voyage:~# cat dyndbg-cmdfile
-nullarbor:~ # echo -c 'file svcsock.c line 1603 +p' >
- <debugfs>/dynamic_debug/control
-nullarbor:~ # echo -c ' file svcsock.c line 1603 +p ' >
- <debugfs>/dynamic_debug/control
-nullarbor:~ # echo -c 'file svcsock.c\nline 1603 +p' >
- <debugfs>/dynamic_debug/control
-nullarbor:~ # echo -n 'file svcsock.c line 1603 +p' >
- <debugfs>/dynamic_debug/control
+ # comments and blank lines ok, but cannot contain semicolon
+ # multiple cmds per line, 1st must be terminated by semicolon
+ func foo p=_ ; func buzz p=_
+ func bar p=_ ; func bum p=_
+ func yak p=_ ; # trailing comment, requires semicolon to terminate cmd
-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:
+ # heres the oddity: semicolon terminates comment, so this works
+ func foo +p ; # comment ; func bar +p
+ # allowing this keeps parsing simple
-nullarbor:~ # echo 'file svcsock.c line 1603 +p' > /proc/dprintk ;\
-> echo 'file svcsock.c line 1563 +p' > /proc/dprintk
+voyage:~# cat dyndbg-cmdfile > $CONTROL
+dynamic_debug:ddebug_exec_queries: processed 7 queries, with 0 errs
+
+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' > $CONTROL; \
+> echo 'file svcsock.c line 1563 +p' > $CONTROL
or even like:
nullarbor:~ # (
> echo 'file svcsock.c line 1603 +p' ;\
> echo 'file svcsock.c line 1563 +p' ;\
-> ) > /proc/dprintk
+> ) > $CONTROL
-At the syntactical level, a command comprises a sequence of match
+Query/command syntax
+====================
+
+At the syntactic level, a command comprises a sequence of match
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()
-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.
+The match-specs are used to choose a subset of the known callsites to
+which to apply the flags-spec. Think of them as a query 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 matches all callsites.
A match specification comprises a keyword, which controls the attribute
of the callsite to be compared, and a value to compare against. Possible
@@ -125,12 +142,13 @@ match-spec ::= 'func' string |
'module' string |
'format' string |
'line' line-range
+// Note: no wildcards, regexs are accepted
line-range ::= lineno |
'-'lineno |
lineno'-' |
lineno'-'lineno
-// Note: line-range cannot contain space, e.g.
+// Note: line-range cannot contain a space, e.g.
// "1-30" is valid range but "1 - 30" is not.
lineno ::= unsigned-int
@@ -144,11 +162,12 @@ func
func svc_tcp_accept
file
- The given string is compared against either the full
- pathname or the basename of the source file of each
- callsite. Examples:
+ The given string is compared against either the full pathname,
+ relative path from kernel source dir, or the basename of the
+ source file of each callsite. Examples:
file svcsock.c
+ file kernel/freezer.c
file /usr/src/packages/BUILD/sgi-enhancednfs-1.4/default/net/sunrpc/svcsock.c
module
@@ -157,7 +176,7 @@ module
seen in "lsmod", i.e. without the directory or the .ko
suffix and with '-' changed to '_'. Examples:
- module sunrpc
+ module ide_pci_generic
module nfsd
format
@@ -190,79 +209,168 @@ line
line -1605 // the 1605 lines from line 1 to line 1605
line 1600- // all lines from line 1600 to the end of the file
-The flags specification comprises a change operation followed
-by one or more flag characters. The change operation is one
-of the characters:
-
--
- remove the given flags
-
-+
- add the given flags
+flags specification
+===================
-=
- set the flags to the given flags
+The flags specification matches the regexp: ^[flmpta_]*[-+=][flmpta_]*$
+and has 3 parts:
-The flags are:
+flags filter (optional):
+ The filter precedes the operation [-+=], and constrains matching
+ to thoses callsite with given flags set. This allows altering
+ flags on callsites with filtered flag values
-f
- Include the function name in the printed message
-l
- Include line number in the printed message
-m
- Include module name in the printed message
-p
- Causes a printk() message to be emitted to dmesg
-t
- Include thread ID in messages not generated from interrupt context
+ pm+t # add t to enabled sites which have m
+ p+t # add t to all enabled sites, both m and !m
+ tmf-p # disable sites with 'tmf'
+ tmf+p # re-enable those disabled sites
-Note the regexp ^[-+=][flmpt]+$ matches a flags specification.
-Note also that there is no convenient syntax to remove all
-the flags at once, you need to use "-flmpt".
+flags change operation:
+ '-' remove the given flags
+ '+' add the given flags
+ '=' set the flags to the given flags
+The flags are:
+ 'p' Causes a printk() message to be emitted to dmesg
+ other flags can thus be left on, and used in filters.
+ 'm' Include module name in the printed message
+ 'f' Include the function name in the printed message
+ 'l' Include line number in the printed message
+ 't' Include thread ID in messages not generated from interrupt context
+ 'a' Also add query to pending queries, for later (re)application
+ '_' default/null flag.
+ Primarily for display, so grep "=_" can avoid " = " in format strings.
+ Also usable (but not required) to clear all flags.
+
+Pending queries
+===============
+
+Queries submitted with 'a' are applied to current callsites as above,
+but are also added to a pending list. When a module is loaded later,
+pending queries are applied to the module in the order given.
+
+This is done before the module_init() routine is run, so pr_debug()s
+can be active during initialization. To better support module
+debugging, pending queries remain on the list through modprobe-rmmod
+cycles.
+
+You can review currently pending queries by catting or grepping
+$DEBUFS/dynamic_debug/pending. To simplify removal via
+cut-paste-edit, its format is similar to the query syntax:
+
+ root@voyage:~# cat /dbg/dynamic_debug/pending
+ # func file module format line flags mask
+ ...
+ func a299 line 0-0 =pa
+ func a300 line 0-0 =pa
+
+To remove a pending query, resubmit it with 'a' in filter-spec
+and zeroed flag-specs:
+
+ # a simple helper shell-function, to shorten examples
+ DBGFS=${DBGFS:=/dbg}
+ CONTROL=$DBGFS/dynamic_debug/control
+ function dbg_query() {
+ echo "$*" > $CONTROL
+ # see relevant settings, effects of the query/command
+ grep $2 $DBGFS/dynamic_debug/*
+ }
+
+ voyage:~# dbg_query module foo +apt # original pending query
+ voyage:~# dbg_query module foo a= # zero 'a' flag to remove query
+ voyage:~# dbg_query module foo a-a # zero 'a', remove query
+ voyage:~# dbg_query module foo a-ap # zero 'a', also disable callsites
+
+Deleting or altering a pending query requires an exact match on most
+of the match-spec; the same string specs must be given, but 'line 0-0'
+matches with '' and vice-versa. The filter-spec is ignored for
+pending-list changes.
+
+ voyage:~# dbg_query module foo +apt # add PQ-1 before each of following..
+
+ # will not remove PQ-1 (mismatch on lines)
+ voyage:~# dbg_query module foo line 100-200 ap=
+
+ # removes PQ-1 (exact match with PQ-1), disables active callsites (if any)
+ voyage:~# dbg_query module foo line 0-0 ap=
+
+ # delete PQ-1, leave callsites active (none of 'ptmfl' subtracted)
+ voyage:~# dbg_query module foo a-a
+
+ # delete PQ-1, disable callsites (all flags zeroed)
+ voyage:~# dbg_query module foo a=_
+
+ # delete PQ-1, leave callsites active (no 'm' on active callsites)
+ voyage:~# dbg_query module foo am=_
+
+ # del PQ-1, and disable callsites (-p), leaving 't'
+ voyage:~# dbg_query module foo a-ap
+
+ # del PQ-1, disable and clear callsites (-pt removes flags set by PQ-1)
+ voyage:~# dbg_query module foo a-apt
+
+ # del PQ-1, disable callsites with 'pt', leave 't'
+ voyage:~# dbg_query module foo apt-ap
+
+ # reactivate foo's callsites marked with 't' (see "flags filter" above)
+ voyage:~# dbg_query module foo t+p
+
+ # add 'm' to foo's callsites with 't', add new pending query with 'pm'
+ voyage:~# dbg_query module foo at+pm
+
+Altering a pending query ('a' in filter) will also alter the callsites
+that it applies to (subject to filter match), but changing the active
+callsites (using a query without the 'a') does not change the pending
+query that applied it.
Debug messages during boot process
==================================
-To be able to activate debug messages during the boot process,
-even before userspace and debugfs exists, use the boot parameter:
-ddebug_query="QUERY"
+To be able to activate debug messages during the boot process, even
+before userspace and debugfs exists, use the boot parameter:
+ddebug_query="QUERY".
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
+characters. It may contain multiple commands separated by ';'
+('\n' seems to be eaten by my bootloader).
+
+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.
-
Examples
========
// enable the message at line 1603 of file svcsock.c
-nullarbor:~ # echo -n 'file svcsock.c line 1603 +p' >
- <debugfs>/dynamic_debug/control
+nullarbor:~ # echo -n 'file svcsock.c line 1603 +p' > $CONTROL
// enable all the messages in file svcsock.c
-nullarbor:~ # echo -n 'file svcsock.c +p' >
- <debugfs>/dynamic_debug/control
+nullarbor:~ # echo -n 'file svcsock.c +p' > $CONTROL
// enable all the messages in the NFS server module
-nullarbor:~ # echo -n 'module nfsd +p' >
- <debugfs>/dynamic_debug/control
+nullarbor:~ # echo -n 'module nfsd +p' > $CONTROL
// enable all 12 messages in the function svc_process()
-nullarbor:~ # echo -n 'func svc_process +p' >
- <debugfs>/dynamic_debug/control
+nullarbor:~ # echo -n 'func svc_process +p' > $CONTROL
// disable all 12 messages in the function svc_process()
-nullarbor:~ # echo -n 'func svc_process -p' >
- <debugfs>/dynamic_debug/control
+nullarbor:~ # echo -n 'func svc_process -p' > $CONTROL
// enable messages for NFS calls READ, READLINK, READDIR and READDIR+.
-nullarbor:~ # echo -n 'format "nfsd: READ" +p' >
- <debugfs>/dynamic_debug/control
+nullarbor:~ # echo -n 'format "nfsd: READ" +p' > $CONTROL
+
+// send query-command file to control
+root@voyage:~# cat dyndbg-cmdfile > $CONTROL
+dynamic_debug:ddebug_proc_open: called
+dynamic_debug:ddebug_proc_write: read 500 bytes from userspace
+dynamic_debug:ddebug_exec_queries: query 0: "func foo p=_ "
+dynamic_debug:ddebug_tokenize: split into words: "func" "foo" "p=_"
+dynamic_debug:ddebug_parse_query: parsed func="foo" file="" module="" format="" lineno=0-0
+...
--
1.7.4.4
jim.c...@gmail.com
lineno:24 allows files with 4 million lines, an insane file-size, even
for never-to-get-in-tree machine generated code. Reduce this to 18
bits, which still allows 256k lines. This is still insanely big, but
its not raving mad.
Signed-off-by: Jim Cromie <jim.c...@gmail.com>
---
include/linux/dynamic_debug.h | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index 2c9c476..5622e04 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -15,7 +15,7 @@ struct _ddebug {
const char *function;
const char *filename;
const char *format;
- unsigned int lineno:24;
+ unsigned int lineno:18;
/*
* The flags field controls the behaviour at the callsite.
* The bits here are changed dynamically when the user
--
1.7.4.4
jim.c...@gmail.com
replace strcpy with strlcpy, and add define for the size constant.
Signed-off-by: Jim Cromie <jim.c...@gmail.com>
---
lib/dynamic_debug.c | 8 +++++---
1 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index f2fb0c0..6372b18 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -516,14 +516,16 @@ EXPORT_SYMBOL(__dynamic_netdev_dbg);
#endif
-static __initdata char ddebug_setup_string[1024];
+#define BOOT_QUERY_SZ 1024
+static __initdata char ddebug_setup_string[BOOT_QUERY_SZ];
+
static __init int ddebug_setup_query(char *str)
{
- if (strlen(str) >= 1024) {
+ if (strlen(str) >= BOOT_QUERY_SZ) {
pr_warn("ddebug boot param string too large\n");
return 0;
}
- strcpy(ddebug_setup_string, str);
+ strlcpy(ddebug_setup_string, str, BOOT_QUERY_SZ);
return 1;
}
--
1.7.4.4
jim.c...@gmail.com
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.c...@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 6372b18..a52b78a 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -276,6 +276,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:
@@ -287,6 +295,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)
@@ -300,13 +311,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.4
jim.c...@gmail.com
trim_prefix(path) skips past the absolute source path root, and returns
the pointer to the relative path from there. Use it to shorten displayed
path in ddebug_change() and in ddebug_proc_show(). Function insures
common prefix, so out-of-tree module paths are preserved as absolute paths.
For example:
kernel/freezer.c:128 [freezer]cancel_freezing - " clean up: %s\012"
Use trim_prefix() in ddebug_change to allow use of a relative pathname
in a file match-spec, in addition to basename and full-path.
Signed-off-by: Jim Cromie <jim.c...@gmail.com>
---
lib/dynamic_debug.c | 22 +++++++++++++++++-----
1 files changed, 17 insertions(+), 5 deletions(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index e7090f2..2d26c10 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -69,6 +69,17 @@ static inline const char *basename(const char *path)
return tail ? tail+1 : path;
}
+/* Return the path relative to source root */
+static inline const char *trim_prefix(const char *path)
+{
+ int skip = strlen(__FILE__) - strlen("lib/dynamic_debug.c");
+
+ if (strncmp(path, __FILE__, skip))
+ skip = 0; /* prefix mismatch, don't skip */
+
+ return path + skip;
+}
+
static struct { unsigned flag:8; char opt_char; } opt_array[] = {
{ _DPRINTK_FLAGS_PRINT, 'p' },
{ _DPRINTK_FLAGS_INCL_MODNAME, 'm' },
@@ -125,7 +136,8 @@ static void ddebug_change(const struct ddebug_query *query,
/* match against the source filename */
if (query->filename != NULL &&
strcmp(query->filename, dp->filename) &&
- strcmp(query->filename, basename(dp->filename)))
+ strcmp(query->filename, basename(dp->filename)) &&
+ strcmp(query->filename, trim_prefix(dp->filename)))
continue;
/* match against the function */
@@ -154,7 +166,7 @@ static void ddebug_change(const struct ddebug_query *query,
dp->flags = newflags;
if (verbose)
pr_info("changed %s:%d [%s]%s %s\n",
- dp->filename, dp->lineno,
+ trim_prefix(dp->filename), dp->lineno,
dt->mod_name, dp->function,
ddebug_describe_flags(dp, flagbuf,
sizeof(flagbuf)));
@@ -682,9 +694,9 @@ static int ddebug_proc_show(struct seq_file *m, void *p)
}
seq_printf(m, "%s:%u [%s]%s %s \"",
- dp->filename, dp->lineno,
- iter->table->mod_name, dp->function,
- ddebug_describe_flags(dp, flagsbuf, sizeof(flagsbuf)));
+ trim_prefix(dp->filename), dp->lineno,
+ iter->table->mod_name, dp->function,
+ ddebug_describe_flags(dp, flagsbuf, sizeof(flagsbuf)));
seq_escape(m, dp->format, "\t\r\n\"");
seq_puts(m, "\"\n");
--
1.7.4.4
jim.c...@gmail.com
This makes no behavioral change, its just a code-move/refactor
to encapsulate matching behavior.
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.c...@gmail.com>
---
lib/dynamic_debug.c | 54 +++++++++++++++++++++++++++++---------------------
1 files changed, 31 insertions(+), 23 deletions(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 2f39b2f..329e18d 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -125,6 +125,36 @@ do { \
q->first_lineno, q->last_lineno); \
} while (0)
+static bool 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)) &&
+ strcmp(query->filename, trim_prefix(dp->filename)))
+ return false;
+
+ /* match against the function */
+ if (query->function != NULL &&
+ strcmp(query->function, dp->function))
+ return false;
+
+ /* match against the format */
+ if (query->format != NULL &&
+ strstr(dp->format, query->format) == NULL)
+ return false;
+
+ /* match against the line number range */
+ if (query->first_lineno && dp->lineno < query->first_lineno)
+ return false;
+
+ if (query->last_lineno && dp->lineno > query->last_lineno)
+ return false;
+
+ return true;
+}
+
/*
* Search the tables for _ddebug's which match the given
* `query' and apply the `flags' and `mask' to them. Tells
@@ -151,29 +181,7 @@ static void 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)) &&
- strcmp(query->filename, trim_prefix(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.4
jim.c...@gmail.com
Reduce != NULL relational tests to their 0/1 equivalents.
Done separately to preserve code-move character of previous patch.
No generated code difference.
Signed-off-by: Jim Cromie <jim.c...@gmail.com>
---
lib/dynamic_debug.c | 8 +++-----
1 files changed, 3 insertions(+), 5 deletions(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 329e18d..4c8e178 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -129,20 +129,18 @@ static bool 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)) &&
strcmp(query->filename, trim_prefix(dp->filename)))
return false;
/* match against the function */
- if (query->function != NULL &&
- strcmp(query->function, dp->function))
+ if (query->function && strcmp(query->function, dp->function))
return false;
/* match against the format */
- if (query->format != NULL &&
- strstr(dp->format, query->format) == NULL)
+ if (query->format && !strstr(dp->format, query->format))
return false;
/* match against the line number range */
--
1.7.4.4
jim.c...@gmail.com
When a pending-query is resubmitted with zeroed flags, remove it
from pending-queries list. The submission must have identical
match-specs, and like the original query, must have 'a' in the
filter-flags. If other filter-flags are given, they must match
the query to be removed, but filter can be underspecified; "p"
will match against "pt".
Signed-off-by: Jim Cromie <jim.c...@gmail.com>
---
lib/dynamic_debug.c | 9 ++++++++-
1 files changed, 8 insertions(+), 1 deletions(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index a59d48c..0c9e53a 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -508,7 +508,14 @@ 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 */
+ vpr_info_pq(pq, "delete pending");
+ list_del_init(&pq->link);
+ pqfree(pq);
+ return 0;
+ }
if (pq->flags != flags)
pq->flags = flags;
if (pq->mask != mask)
--
1.7.4.4
jim.c...@gmail.com
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.c...@gmail.com>
---
lib/dynamic_debug.c | 10 ++++------
1 files changed, 4 insertions(+), 6 deletions(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 0676de0..3f37b5e 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -922,12 +922,10 @@ static int __init dynamic_debug_init(void)
pr_info("ddebug initializing with string \"%s\"",
ddebug_setup_string);
ret = ddebug_exec_queries(ddebug_setup_string);
- if (ret)
- pr_warn("Invalid ddebug boot param %s",
- ddebug_setup_string);
- else
- pr_info("ddebug initialized with string %s",
- ddebug_setup_string);
+ if (ret) {
+ ret = 0; /* make this non-fatal */
+ pr_warn("invalid ddebug_query\n");
+ }
}
out_free:
--
1.7.4.4
jim.c...@gmail.com
Alter fn-sig to accept flags arg directly, instead of a ptr to
structure containing it. This lets us reuse it for pending-queries.
Signed-off-by: Jim Cromie <jim.c...@gmail.com>
---
lib/dynamic_debug.c | 9 +++++----
1 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 3c9244d..d7b71a6 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -103,7 +103,7 @@ static struct { unsigned flag:8; char opt_char; } opt_array[] = {
};
/* format a string into buf[] which describes the _ddebug's flags */
-static char *ddebug_describe_flags(struct _ddebug *dp, char *buf,
+static char *ddebug_describe_flags(unsigned int flags, char *buf,
size_t maxlen)
{
char *p = buf;
@@ -112,7 +112,7 @@ static char *ddebug_describe_flags(struct _ddebug *dp, char *buf,
BUG_ON(maxlen < 10);
*p++ = '=';
for (i = 0; i < ARRAY_SIZE(opt_array); ++i)
- if (dp->flags & opt_array[i].flag)
+ if (flags & opt_array[i].flag)
*p++ = opt_array[i].opt_char;
if (*(p-1) == '=')
*p++ = '_';
@@ -227,7 +227,8 @@ static int ddebug_change(const struct ddebug_query *query,
pr_info("changed %s:%d [%s]%s %s\n",
trim_prefix(dp->filename), dp->lineno,
dt->mod_name, dp->function,
- ddebug_describe_flags(dp, flagbuf,
+ ddebug_describe_flags(dp->flags,
+ flagbuf,
sizeof(flagbuf)));
}
}
@@ -901,7 +902,7 @@ static int ddebug_proc_show(struct seq_file *m, void *p)
seq_printf(m, "%s:%u [%s]%s %s \"",
trim_prefix(dp->filename), dp->lineno,
iter->table->mod_name, dp->function,
- ddebug_describe_flags(dp, flagsbuf, sizeof(flagsbuf)));
+ ddebug_describe_flags(dp->flags, flagsbuf, sizeof(flagsbuf)));
seq_escape(m, dp->format, "\t\r\n\"");
seq_puts(m, "\"\n");
--
1.7.4.4
--
jim.c...@gmail.com
Factor pr_info(query) out of ddebug_parse_query, into pr_info_dq(),
for reuse later. Also change the printed labels: file, func to agree
with the query-spec keywords accepted in the control file. Pass ""
when string is null, to avoid "(null)" output from sprintf. For
format print, use precision to skip last char, assuming its '\n', no
great harm if not, its a debug msg,
Signed-off-by: Jim Cromie <jim.c...@gmail.com>
---
lib/dynamic_debug.c | 27 ++++++++++++++++++---------
1 files changed, 18 insertions(+), 9 deletions(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 3f37b5e..2f39b2f 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -109,6 +109,22 @@ static char *ddebug_describe_flags(struct _ddebug *dp, char *buf,
return buf;
}
+#define vpr_info_dq(q, msg) \
+do { \
+ if (verbose) \
+ /* trim last char off format print */ \
+ pr_info("%s: func=\"%s\" file=\"%s\" " \
+ "module=\"%s\" format=\"%.*s\" " \
+ "lineno=%u-%u", \
+ msg, \
+ q->function ? q->function : "", \
+ q->filename ? q->filename : "", \
+ q->module ? q->module : "", \
+ (int)(q->format ? strlen(q->format) - 1 : 0), \
+ q->format ? q->format : "", \
+ q->first_lineno, q->last_lineno); \
+} while (0)
+
/*
* Search the tables for _ddebug's which match the given
* `query' and apply the `flags' and `mask' to them. Tells
@@ -116,7 +132,7 @@ static char *ddebug_describe_flags(struct _ddebug *dp, char *buf,
* were matched. Called with ddebug_lock held.
*/
static void ddebug_change(const struct ddebug_query *query,
- unsigned int flags, unsigned int mask)
+ unsigned int flags, unsigned int mask)
{
int i;
struct ddebug_table *dt;
@@ -350,14 +366,7 @@ static int ddebug_parse_query(char *words[], int nwords,
return -EINVAL;
}
}
-
- if (verbose)
- pr_info("q->function=\"%s\" q->filename=\"%s\" "
- "q->module=\"%s\" q->format=\"%s\" q->lineno=%u-%u\n",
- query->function, query->filename,
- query->module, query->format, query->first_lineno,
- query->last_lineno);
-
+ vpr_info_dq(query, "parsed");
return 0;
}
--
1.7.4.4
jim.c...@gmail.com
Hoist locking out of ddebug_change() into ddebug_exec_queries().
This reduces lock fiddling, but will increase hold-times slightly,
at least when many queries are submitted.
With verbose off, on a 266 MHz, takes, I get a reasonable:
#> time cat query-file-w-300-pending-queries > $CONTROL
real 0m0.508s
user 0m0.009s
sys 0m0.290s
Signed-off-by: Jim Cromie <jim.c...@gmail.com>
---
lib/dynamic_debug.c | 7 +++----
1 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 2b3393f..0676de0 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -113,7 +113,7 @@ static char *ddebug_describe_flags(struct _ddebug *dp, char *buf,
* 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 void ddebug_change(const struct ddebug_query *query,
unsigned int flags, unsigned int mask)
@@ -125,7 +125,6 @@ static void 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 */
@@ -175,8 +174,6 @@ static void ddebug_change(const struct ddebug_query *query,
sizeof(flagbuf)));
}
}
- mutex_unlock(&ddebug_lock);
-
if (!nfound && verbose)
pr_info("no matches for query\n");
}
@@ -450,6 +447,7 @@ static int ddebug_exec_queries(char *query)
char *split;
int i, errs = 0, exitcode = 0, rc;
+ mutex_lock(&ddebug_lock);
for (i = 0; query; query = split) {
split = strpbrk(query, ";\n");
if (split)
@@ -468,6 +466,7 @@ static int ddebug_exec_queries(char *query)
}
i++;
}
+ mutex_unlock(&ddebug_lock);
pr_info("processed %d queries, with %d errs\n", i, errs);
return exitcode;
--
1.7.4.4
jim.c...@gmail.com
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.c...@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 2d26c10..74b395b 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.4
jim.c...@gmail.com
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.c...@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 a52b78a..e7090f2 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -334,8 +334,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.4
jim.c...@gmail.com
if CONFIG_DYNAMIC_DEBUG is defined, honor it over DEBUG, so that
pr_debug()s are controllable, instead of always-on. When DEBUG is
also defined, change _DPRINTK_FLAGS_DEFAULT to enable printing by
default.
Also adding _DPRINTK_FLAGS_INCL_MODNAME would be nice, but there are
numerous cases of pr_debug(NAME ": ...), which would result in double
printing of module-name. So defer this until things settle.
CC: Joe Perches <j...@perches.com>
Signed-off-by: Jim Cromie <jim.c...@gmail.com>
---
include/linux/device.h | 8 ++++----
include/linux/dynamic_debug.h | 4 ++++
include/linux/netdevice.h | 8 ++++----
include/linux/printk.h | 8 ++++----
4 files changed, 16 insertions(+), 12 deletions(-)
diff --git a/include/linux/device.h b/include/linux/device.h
index 4639419..2e16503 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -857,14 +857,14 @@ static inline int _dev_info(const struct device *dev, const char *fmt, ...)
#define dev_info(dev, fmt, arg...) _dev_info(dev, fmt, ##arg)
-#if defined(DEBUG)
-#define dev_dbg(dev, format, arg...) \
- dev_printk(KERN_DEBUG, dev, format, ##arg)
-#elif defined(CONFIG_DYNAMIC_DEBUG)
+#if defined(CONFIG_DYNAMIC_DEBUG)
#define dev_dbg(dev, format, ...) \
do { \
dynamic_dev_dbg(dev, format, ##__VA_ARGS__); \
} while (0)
+#elif defined(DEBUG)
+#define dev_dbg(dev, format, arg...) \
+ dev_printk(KERN_DEBUG, dev, format, ##arg)
#else
#define dev_dbg(dev, format, arg...) \
({ \
diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index ed7df3d..12ef233 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -26,7 +26,11 @@ struct _ddebug {
#define _DPRINTK_FLAGS_INCL_FUNCNAME (1<<2)
#define _DPRINTK_FLAGS_INCL_LINENO (1<<3)
#define _DPRINTK_FLAGS_INCL_TID (1<<4)
+#if defined DEBUG
+#define _DPRINTK_FLAGS_DEFAULT _DPRINTK_FLAGS_PRINT
+#else
#define _DPRINTK_FLAGS_DEFAULT 0
+#endif
unsigned int flags:8;
} __attribute__((aligned(8)));
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 2797260..8c08fe0 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -2641,14 +2641,14 @@ extern int netdev_info(const struct net_device *dev, const char *format, ...)
#define MODULE_ALIAS_NETDEV(device) \
MODULE_ALIAS("netdev-" device)
-#if defined(DEBUG)
-#define netdev_dbg(__dev, format, args...) \
- netdev_printk(KERN_DEBUG, __dev, format, ##args)
-#elif defined(CONFIG_DYNAMIC_DEBUG)
+#if defined(CONFIG_DYNAMIC_DEBUG)
#define netdev_dbg(__dev, format, args...) \
do { \
dynamic_netdev_dbg(__dev, format, ##args); \
} while (0)
+#elif defined(DEBUG)
+#define netdev_dbg(__dev, format, args...) \
+ netdev_printk(KERN_DEBUG, __dev, format, ##args)
#else
#define netdev_dbg(__dev, format, args...) \
({ \
diff --git a/include/linux/printk.h b/include/linux/printk.h
index 0101d55..1224b1d 100644
--- a/include/linux/printk.h
+++ b/include/linux/printk.h
@@ -180,13 +180,13 @@ extern void dump_stack(void) __cold;
#endif
/* If you are writing a driver, please use dev_dbg instead */
-#if defined(DEBUG)
-#define pr_debug(fmt, ...) \
- printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
-#elif defined(CONFIG_DYNAMIC_DEBUG)
+#if defined(CONFIG_DYNAMIC_DEBUG)
/* dynamic_pr_debug() uses pr_fmt() internally so we don't need it here */
#define pr_debug(fmt, ...) \
dynamic_pr_debug(fmt, ##__VA_ARGS__)
+#elif defined(DEBUG)
+#define pr_debug(fmt, ...) \
+ printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
#else
#define pr_debug(fmt, ...) \
no_printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
Joe Perches
> From: Jim Cromie <jim.c...@gmail.com>
>
> Current write buffer is 256 bytes, on stack. Allocate it off heap,
> and enlarge it to 4096 bytes, big enough for ~100 queries (at 40 bytes
> each), and error out if not. This should be enough for most uses, and
> others can be split into 2 writes.
>
> This makes it play nicely with:
> $> cat debug-queries-file > /dbg/dynamic_debug/control
> diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
[]
> @@ -590,24 +590,32 @@ __setup("ddebug_query=", ddebug_setup_query);
> * File_ops->write method for <debugfs>/dynamic_debug/conrol. Gathers the
> * command text from userspace, parses and executes it.
> */
> +#define USER_BUF_PAGE 4095
Why not just read into a fifo and parse
it until empty?
> static ssize_t ddebug_proc_write(struct file *file, const char __user *ubuf,
> size_t len, loff_t *offp)
> {
> - char tmpbuf[256];
> + char *tmpbuf;
> int ret;
>
> if (len == 0)
> return 0;
> - /* we don't check *offp -- multiple writes() are allowed */
> - if (len > sizeof(tmpbuf)-1)
> + if (len > USER_BUF_PAGE - 1) {
> + pr_warn("expected <%d bytes into control \n", USER_BUF_PAGE);
> return -E2BIG;
> - if (copy_from_user(tmpbuf, ubuf, len))
> + }
kfifo_from_user?
Randy Dunlap
> From: Jim Cromie <jim.c...@gmail.com>
>
> Rework description of flags-spec into 3 parts; flags-filter, flags-change,
> flags-values. Add section on pending-queries. Explain flags filtering
> in detail. Reflow some paragraphs.
>
> Add example of a command-file which shows commenting,
> multiple queries per line, and describes constraints.
>
> Signed-off-by: Jim Cromie <jim.c...@gmail.com>
> ---
> Documentation/dynamic-debug-howto.txt | 336 ++++++++++++++++++++++-----------
> 1 files changed, 222 insertions(+), 114 deletions(-)
>
> diff --git a/Documentation/dynamic-debug-howto.txt b/Documentation/dynamic-debug-howto.txt
> index f959909..bf9037f 100644
> --- a/Documentation/dynamic-debug-howto.txt
> +++ b/Documentation/dynamic-debug-howto.txt
> @@ -4,117 +4,134 @@ Introduction
>
> This document describes how to use the dynamic debug (ddebug) feature.
>
> -Dynamic debug is designed to allow you to dynamically enable/disable kernel
> -code to obtain additional kernel information. Currently, if
> -CONFIG_DYNAMIC_DEBUG is set, then all pr_debug()/dev_dbg() calls can be
> -dynamically enabled per-callsite.
> +Dynamic debug is designed to allow you to dynamically enable/disable
> +kernel code to obtain additional kernel information. If kernel is
If {the | a} kernel is
The behaviour ... is controlled by
> +writing to a control file in the 'debugfs' filesystem. Thus, you must
> +first mount the debugfs filesystem, in order to make use of this
drop comma.
> +feature. Subsequently, we refer to mount point as $DBGFS, and the
to the mount point
also drop comma after $DBGFS.
> +control file as $CONTROL. So if you want to enable printing from
> +source file 'svcsock.c', line 1603 you simply do:
>
> -nullarbor:~ # echo 'file svcsock.c line 1603 +p' >
> - <debugfs>/dynamic_debug/control
> +nullarbor:~ # echo file svcsock.c line 1603 +p > $CONTROL
>
> If you make a mistake with the syntax, the write will fail thus:
>
> -nullarbor:~ # echo 'file svcsock.c wtf 1 +p' >
> - <debugfs>/dynamic_debug/control
> +nullarbor:~ # 'echo file svcsock.c wtf 1 +p' > $CONTROL
> -bash: echo: write error: Invalid argument
>
> Viewing Dynamic Debug Behaviour
> -===========================
> +===============================
>
> -You can view the currently configured behaviour of all the debug statements
> -via:
> +You can view the currently configured behaviour by catting or grepping
> +the $CONTROL file:
>
> -nullarbor:~ # cat <debugfs>/dynamic_debug/control
> +root@voyage:~# grep freezer $CONTROL
> +kernel/freezer.c:128 [freezer]cancel_freezing =_ " clean up: %s\012"
> +kernel/freezer.c:60 [freezer]refrigerator =_ "%s left refrigerator\012"
> +kernel/freezer.c:41 [freezer]refrigerator =_ "%s entered refrigerator\012"
> +
> +The third column shows the current flag settings for each debug
There are columns there??!!?? Hard to tell.
> +statement callsite (menmonic: what they are equal to, see below for
mnemonic:
s/bounded/limited/ ?
> +(or 1024 for boot-line parameter) you can send multiple commands,
Maybe you could rewrite this paragraph so that I can understand what
it means?
> +separated by ';' or '\n'
End sentence with period ('.').
>
> -Command Language Reference
> -==========================
> +voyage:~# echo "func cancel_freezing +p ; func refrigerator +p; " > $CONTROL
> +voyage:~# printf "func cancel_freezing +p \n func refrigerator +p; " > $CONTROL
>
> -At the lexical level, a command comprises a sequence of words separated
> -by whitespace characters. Note that newlines are treated as word
> -separators and do *not* end a command or allow multiple commands to
> -be done together. So these are all equivalent:
> +voyage:~# cat dyndbg-cmdfile
>
> -nullarbor:~ # echo -c 'file svcsock.c line 1603 +p' >
> - <debugfs>/dynamic_debug/control
> -nullarbor:~ # echo -c ' file svcsock.c line 1603 +p ' >
> - <debugfs>/dynamic_debug/control
> -nullarbor:~ # echo -c 'file svcsock.c\nline 1603 +p' >
> - <debugfs>/dynamic_debug/control
> -nullarbor:~ # echo -n 'file svcsock.c line 1603 +p' >
> - <debugfs>/dynamic_debug/control
> + # comments and blank lines ok, but cannot contain semicolon
> + # multiple cmds per line, 1st must be terminated by semicolon
Change "1st" to "all but last" ?
> + func foo p=_ ; func buzz p=_
> + func bar p=_ ; func bum p=_
> + func yak p=_ ; # trailing comment, requires semicolon to terminate cmd
>
> -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:
> + # heres the oddity: semicolon terminates comment, so this works
> + func foo +p ; # comment ; func bar +p
> + # allowing this keeps parsing simple
>
> -nullarbor:~ # echo 'file svcsock.c line 1603 +p' > /proc/dprintk ;\
> -> echo 'file svcsock.c line 1563 +p' > /proc/dprintk
> +voyage:~# cat dyndbg-cmdfile > $CONTROL
> +dynamic_debug:ddebug_exec_queries: processed 7 queries, with 0 errs
> +
> +Multiple commands are processed independently, this allows you to send
independently. This allows
those
> + flags on callsites with filtered flag values
>
> -f
> - Include the function name in the printed message
> -l
> - Include line number in the printed message
> -m
> - Include module name in the printed message
> -p
> - Causes a printk() message to be emitted to dmesg
> -t
> - Include thread ID in messages not generated from interrupt context
> + pm+t # add t to enabled sites which have m
> + p+t # add t to all enabled sites, both m and !m
> + tmf-p # disable sites with 'tmf'
> + tmf+p # re-enable those disabled sites
>
> -Note the regexp ^[-+=][flmpt]+$ matches a flags specification.
> -Note also that there is no convenient syntax to remove all
> -the flags at once, you need to use "-flmpt".
> +flags change operation:
> + '-' remove the given flags
> + '+' add the given flags
> + '=' set the flags to the given flags
>
> +The flags are:
> + 'p' Causes a printk() message to be emitted to dmesg
to the kernel log (or kernel log buffer);
[dmesg is a program that can print the kernel log.]
> + other flags can thus be left on, and used in filters.
> + 'm' Include module name in the printed message
> + 'f' Include the function name in the printed message
> + 'l' Include line number in the printed message
> + 't' Include thread ID in messages not generated from interrupt context
> + 'a' Also add query to pending queries, for later (re)application
> + '_' default/null flag.
> + Primarily for display, so grep "=_" can avoid " = " in format strings.
> + Also usable (but not required) to clear all flags.
> +
> +Pending queries
> +===============
> +
> +Queries submitted with 'a' are applied to current callsites as above,
> +but are also added to a pending list. When a module is loaded later,
> +pending queries are applied to the module in the order given.
> +
> +This is done before the module_init() routine is run, so pr_debug()s
> +can be active during initialization. To better support module
> +debugging, pending queries remain on the list through modprobe-rmmod
> +cycles.
> +
> +You can review currently pending queries by catting or grepping
> +$DEBUFS/dynamic_debug/pending. To simplify removal via
$DBGFS ?
> +cut-paste-edit, its format is similar to the query syntax:
> +
> + root@voyage:~# cat /dbg/dynamic_debug/pending
use $DBGFS in path name?
Sorry, I guess I'm dense today. What is this "PQ-1"?
thanks,
--
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***
Joe Perches
> dynamic_pr_debug can add module, function, file, and line selectively,
> diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
[]
> +#ifndef pr_fmt_dbg
> +#define pr_fmt_dbg(fmt) fmt
> +#endif
> +#ifndef pr_fmt_dbg
> +#define pr_fmt_dbg(fmt) pr_fmt(fmt)
> +#endif
This might better be placed in printk.h just
once.
I think pr_fmt_debug is better than pr_fmt_dbg
because the function/macro is named pr_debug.
Maybe add all the pr_<level> variants too because
some like to prefix __func__ to pr_err but not pr_info
etc.
#ifndef pr_fmt_emerg
#define pr_fmt_emerg(fmt) pr_fmt(fmt)
#endif
#ifndef pr_fmt_crit
#define pr_fmt_crit(fmt) pr_fmt(fmt)
#endif
#ifndef pr_fmt_alert
#define pr_fmt_alert(fmt) pr_fmt(fmt)
#endif
#ifndef pr_fmt_err
#define pr_fmt_err(fmt) pr_fmt(fmt)
#endif
#ifndef pr_fmt_notice
#define pr_fmt_notice(fmt) pr_fmt(fmt)
#endif
#ifndef pr_fmt_warn
#define pr_fmt_warn(fmt) pr_fmt(fmt)
#endif
#ifndef pr_fmt_info
#define pr_fmt_info(fmt) pr_fmt(fmt)
#endif
#ifndef pr_fmt_debug
#define pr_fmt_debug(fmt) pr_fmt(fmt)
#endif
and change the macros
#define pr_emerg(fmt, ...) \
printk(KERN_EMERG pr_fmt_emerg(fmt), ##__VA_ARGS__)
etc.
Bart Van Assche
Hi Joe,
Are you sure it makes sense to introduce all these new macros ?
Introducing pr_fmt_dbg() makes sense to me, but the usefulness of the
macros proposed above is not clear to me.
Bart.
WESTERN UNION OFFICE
Dear Beneficiary,
This is to re-notify you of the �300,000.00GBP that was deposited here in the western union office in
your name.You should contact Mr.James Parker to collect your money transfer control number (M.T.C.N).
Contact Person: Mr. Allen Parker Email: mr.jame...@hotmail.co.uk Tell; +44 703 180 9904
Joe Perches
On Fri, 2011-09-23 at 12:31 +0200, Bart Van Assche wrote:
> On Thu, Sep 22, 2011 at 10:57 PM, Joe Perches <j...@perches.com> wrote:
> > Maybe add all the pr_<level> variants too because
> > some like to prefix __func__ to pr_err but not pr_info
> > etc.
> Hi Joe,
Hi Bart.
> Are you sure it makes sense to introduce all these new macros ?
> Introducing pr_fmt_dbg() makes sense to me, but the usefulness of the
> macros proposed above is not clear to me.
Probably the most useful application of
pr_fmt_debug would be to prefix __func__ to
debug output but not to other pr_<level> uses.
dynamic_debug would not be able to detect that
__func__ duplication except by using vscnprintf
to a buffer and checking the initial output.
Perhaps a simpler way to avoid duplication of
KBUILD_MODNAME ":" in dynamic_debugging is to
scan the fmt string and move the fmt pointer
past any leading KBUILD_MODNAME ": " when
appropriate.
I think these pr_fmt_<level> macros have a
relatively low overall value but I prefer
comprehensive implementations rather than
partial ones.
There are a couple of modules where people
prefix __func__ to pr_err but not pr_info.
e.g.: drivers/net/ethernet/broadcom/cnic.c
Adding all the #define pr_fmt_<level>s could
simplify that.
cheers, Joe
Greg KH
> hi all,
>
> this reworked* patchset enhances dynamic-debug with:
I need acks from Jason before I can apply any of this...
jim.c...@gmail.com
thanks Randy,
This revision has tweaks per your proofreading.
jim.c...@gmail.com
Rework description of flags-spec into 3 parts; flags-filter, flags-change,
flags-values. Add section on pending-queries. Explain flags filtering
in detail. Reflow some paragraphs.
Add example of a command-file which shows commenting,
multiple queries per line, and describes constraints.
Signed-off-by: Jim Cromie <jim.c...@gmail.com>
---
Documentation/dynamic-debug-howto.txt | 334 ++++++++++++++++++++++-----------
1 files changed, 222 insertions(+), 112 deletions(-)
diff --git a/Documentation/dynamic-debug-howto.txt b/Documentation/dynamic-debug-howto.txt
index f959909..eaf2743 100644
--- a/Documentation/dynamic-debug-howto.txt
+++ b/Documentation/dynamic-debug-howto.txt
@@ -4,117 +4,134 @@ Introduction
This document describes how to use the dynamic debug (ddebug) feature.
-Dynamic debug is designed to allow you to dynamically enable/disable kernel
-code to obtain additional kernel information. Currently, if
-CONFIG_DYNAMIC_DEBUG is set, then all pr_debug()/dev_dbg() calls can be
-dynamically enabled per-callsite.
+Dynamic debug is designed to allow you to dynamically enable/disable
+kernel code to obtain additional kernel information. If the kernel is
+The behaviour of pr_debug()/dev_dbg()/net_dbg()s is controlled by
+writing to a control file in the 'debugfs' filesystem. Thus, you must
+first mount the debugfs filesystem in order to make use of this
+feature. Subsequently, we refer to mount point as $DBGFS, and the
+control file as $CONTROL. So if you want to enable printing from
+source file 'svcsock.c', line 1603 you simply do:
-nullarbor:~ # echo 'file svcsock.c line 1603 +p' >
- <debugfs>/dynamic_debug/control
+nullarbor:~ # echo file svcsock.c line 1603 +p > $CONTROL
-If you make a mistake with the syntax, the write will fail thus:
+If you make a mistake with the syntax, the write will fail:
-nullarbor:~ # echo 'file svcsock.c wtf 1 +p' >
- <debugfs>/dynamic_debug/control
+nullarbor:~ # 'echo file svcsock.c wtf 1 +p' > $CONTROL
-bash: echo: write error: Invalid argument
Viewing Dynamic Debug Behaviour
-===========================
+===============================
-You can view the currently configured behaviour of all the debug statements
-via:
+You can view the currently configured behaviour by catting or grepping
+the $CONTROL file:
-nullarbor:~ # cat <debugfs>/dynamic_debug/control
+root@voyage:~# grep freezer $CONTROL
+kernel/freezer.c:128 [freezer]cancel_freezing =_ " clean up: %s\012"
+kernel/freezer.c:60 [freezer]refrigerator =_ "%s left refrigerator\012"
+kernel/freezer.c:41 [freezer]refrigerator =_ "%s entered refrigerator\012"
+
+The third space delimited field shows the current flag settings for
+each debug statement callsite (mnemonic: what they are equal to, see
+below for definitions of the flags). The default value, with nothing
+enabled, is "=_". So you can view all the debug statement callsites
+with any non-default flags:
+
+nullarbor:~ # grep -v "=_" $CONTROL
# filename:lineno [module]function flags format
-/usr/src/packages/BUILD/sgi-enhancednfs-1.4/default/net/sunrpc/svc_rdma.c:323 [svcxprt_rdma]svc_rdma_cleanup - "SVCRDMA Module Removed, deregister RPC RDMA transport\012"
-/usr/src/packages/BUILD/sgi-enhancednfs-1.4/default/net/sunrpc/svc_rdma.c:341 [svcxprt_rdma]svc_rdma_init - "\011max_inline : %d\012"
-/usr/src/packages/BUILD/sgi-enhancednfs-1.4/default/net/sunrpc/svc_rdma.c:340 [svcxprt_rdma]svc_rdma_init - "\011sq_depth : %d\012"
-/usr/src/packages/BUILD/sgi-enhancednfs-1.4/default/net/sunrpc/svc_rdma.c:338 [svcxprt_rdma]svc_rdma_init - "\011max_requests : %d\012"
-...
+/usr/src/packages/BUILD/sgi-enhancednfs-1.4/default/net/sunrpc/svcsock.c:1603 [sunrpc]svc_send =p "svc_process: st_sendto returned %d\012"
-You can also apply standard Unix text manipulation filters to this
-data, e.g.
+Command Language Reference
+==========================
-nullarbor:~ # grep -i rdma <debugfs>/dynamic_debug/control | wc -l
-62
+At the lexical level, a command comprises a sequence of words separated
+by whitespace characters.
-nullarbor:~ # grep -i tcp <debugfs>/dynamic_debug/control | wc -l
-42
+nullarbor:~# echo file svcsock.c line 1603 +p > $CONTROL
-Note in particular that the third column shows the enabled behaviour
-flags for each debug statement callsite (see below for definitions of the
-flags). The default value, no extra behaviour enabled, is "-". So
-you can view all the debug statement callsites with any non-default flags:
+nullarbor:~# echo 'file svcsock.c line 1603 +p' > $CONTROL
-nullarbor:~ # awk '$3 != "-"' <debugfs>/dynamic_debug/control
-# filename:lineno [module]function flags format
-/usr/src/packages/BUILD/sgi-enhancednfs-1.4/default/net/sunrpc/svcsock.c:1603 [sunrpc]svc_send p "svc_process: st_sendto returned %d\012"
+nullarbor:~# echo -n ' file svcsock.c line 1603 +p ' > $CONTROL
+nullarbor:~# echo -n 'file svcsock.c line 1603 +p' > $CONTROL
-Command Language Reference
-==========================
+Multiple query/commands
+=======================
-At the lexical level, a command comprises a sequence of words separated
-by whitespace characters. Note that newlines are treated as word
-separators and do *not* end a command or allow multiple commands to
-be done together. So these are all equivalent:
+Multiple commands can be written together, separated by ';' or '\n',
+if they fit in a write() system call (or 1023 bytes for the boot-line
+parameter).
-nullarbor:~ # echo -c 'file svcsock.c line 1603 +p' >
- <debugfs>/dynamic_debug/control
-nullarbor:~ # echo -c ' file svcsock.c line 1603 +p ' >
- <debugfs>/dynamic_debug/control
-nullarbor:~ # echo -c 'file svcsock.c\nline 1603 +p' >
- <debugfs>/dynamic_debug/control
-nullarbor:~ # echo -n 'file svcsock.c line 1603 +p' >
- <debugfs>/dynamic_debug/control
+voyage:~# echo "func cancel_freezing +p ; func refrigerator +p; " > $CONTROL
+voyage:~# printf "func cancel_freezing +p \n func refrigerator +p; " > $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:
+voyage:~# cat dyndbg-cmdfile
-nullarbor:~ # echo 'file svcsock.c line 1603 +p' > /proc/dprintk ;\
-> echo 'file svcsock.c line 1563 +p' > /proc/dprintk
+ # comments and blank lines ok, but cannot contain semicolon
+ # multiple cmds per line ok, all but last must be terminated by semicolon
+ func foo p=_ ; func buzz p=_
+ func bar p=_ ; func bum p=_
+ func yak p=_ ; # trailing comment, requires semicolon to terminate cmd
+
+ # heres the oddity: semicolon terminates comment, so this adds 2 queries
+ func foo +p ; # comment ; func bar +p
+ # allowing this keeps parsing simple
+
+voyage:~# cat dyndbg-cmdfile > $CONTROL
+dynamic_debug:ddebug_exec_queries: processed 7 queries, with 0 errs
+
+Multiple commands are processed independently. This allows you to send
@@ -190,79 +209,170 @@ line
line -1605 // the 1605 lines from line 1 to line 1605
line 1600- // all lines from line 1600 to the end of the file
-The flags specification comprises a change operation followed
-by one or more flag characters. The change operation is one
-of the characters:
+flags specification
+===================
+
+The flags specification matches the regexp: ^[flmpta_]*[-+=][flmpta_]*$
+and has 3 parts:
--
- remove the given flags
+flags filter (optional):
+ The filter precedes the operation [-+=], and constrains matching
+ to thoses callsite with given flags set. This allows altering
+ flags on callsites with matching flag values
-+
- add the given flags
+ pm+t # add t to enabled sites which have m
+ p+t # add t to all enabled sites, both m and !m
+ tmf-p # disable sites with 'tmf'
+ tmf+p # re-enable those disabled sites
-=
- set the flags to the given flags
+flags change operation:
+ '-' remove the given flags
+ '+' add the given flags
+ '=' set the flags to the given flags
The flags are:
+ 'p' Causes a printk() message to be emitted to the kernel log.
+ other flags can thus be left on, and used in filters.
+ 'm' Include module name in the printed message
+ 'f' Include the function name in the printed message
+ 'l' Include line number in the printed message
+ 't' Include thread ID in messages not generated from interrupt context
+ 'a' Also add query to pending queries, for later (re)application
+ '_' default/null flag.
+ Primarily for display, so grep "=_" can avoid " = " in format strings.
+ Also usable (but not required) to clear all flags.
+
+Pending queries
+===============
+
+Queries submitted with 'a' are applied to current callsites as above,
+but are also added to a pending list. When a module is loaded later,
+pending queries are applied to the module in the order given.
+
+This is done before the module_init() routine is run, so pr_debug()s
+can be active during initialization. To better support module
+debugging, pending queries remain on the list through modprobe-rmmod
+cycles.
-f
- Include the function name in the printed message
-l
- Include line number in the printed message
-m
- Include module name in the printed message
-p
- Causes a printk() message to be emitted to dmesg
-t
- Include thread ID in messages not generated from interrupt context
+You can review currently pending queries by catting or grepping
+$DBGFS/dynamic_debug/pending. To simplify removal via cut-paste-edit,
+its format is similar to the query syntax:
-Note the regexp ^[-+=][flmpt]+$ matches a flags specification.
-Note also that there is no convenient syntax to remove all
-the flags at once, you need to use "-flmpt".
+ root@voyage:~# cat $DBGFS/dynamic_debug/pending
+ # func file module format line flags mask
+ ...
+ func a299 line 0-0 =pa
+ func a300 line 0-0 =pa
+To remove a pending query, resubmit it with 'a' in filter-spec
+and zeroed flag-specs:
+
+ # a simple helper shell-function, to shorten examples
+ DBGFS=${DBGFS:=/dbg}
+ CONTROL=$DBGFS/dynamic_debug/control
+ function dbg_query() {
+ echo "$*" > $CONTROL
+ # see relevant settings, effects of the query/command
+ grep $2 $DBGFS/dynamic_debug/*
+ }
+
+ voyage:~# dbg_query module foo +apt # original pending query
+ voyage:~# dbg_query module foo a= # zero 'a' flag to remove pending query
+ voyage:~# dbg_query module foo a-a # zero 'a', remove pending query
+ voyage:~# dbg_query module foo a-ap # zero 'a', also disable callsites
+
+Deleting or altering a pending query requires an exact match on most
+of the match-spec; the same string specs must be given, and line range
+must match also, except that 'line 0-0' matches with '' and vice-versa.
+
+ # add a pendinq query, called PQ-1 in comments below..
+ voyage:~# dbg_query module foo +apt
+
+ # starting with PQ-1, each following alters it ..
+
+ # will not remove PQ-1 (mismatch on lines)
+ voyage:~# dbg_query module foo line 100-200 ap=
+
+ # removes PQ-1 (exact match with PQ-1), disables active callsites (if any)
+ voyage:~# dbg_query module foo line 0-0 ap=
+
+ # delete PQ-1, leave callsites unchanged (none of 'ptmfl' subtracted)
+ voyage:~# dbg_query module foo a-a
+
+ # delete PQ-1, disable callsites (all flags zeroed)
+ voyage:~# dbg_query module foo a=_
+
+ # delete PQ-1, leave callsites active (no 'm' on active callsites)
+ voyage:~# dbg_query module foo am=_
+
+ # del PQ-1, and disable callsites (-p), leaving 't'
+ voyage:~# dbg_query module foo a-ap
+
+ # del PQ-1, disable and clear callsites (-pt removes flags set by PQ-1)
+ voyage:~# dbg_query module foo a-apt
+
+ # del PQ-1, disable callsites with 'pt', leave 't'
+ voyage:~# dbg_query module foo apt-ap
+
+ # reactivate foo's callsites marked with 't' (see "flags filter" above)
+ voyage:~# dbg_query module foo t+p
+
+ # add 'm' to foo's callsites with 't', add new pending query with 'pm'
+ voyage:~# dbg_query module foo at+pm
+
+Altering a pending query ('a' in filter) will also alter the callsites
+that it applies to (subject to matching), but changing the active
--
1.7.4.4
Jim Cromie
> On Wed, 2011-09-21 at 15:55 -0600, jim.c...@gmail.com wrote:
>> dynamic_pr_debug can add module, function, file, and line selectively,
> []
>> diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
> []
>> +#ifndef pr_fmt_dbg
>> +#define pr_fmt_dbg(fmt) fmt
>> +#endif
> []
>> +#ifndef pr_fmt_dbg
>> +#define pr_fmt_dbg(fmt) pr_fmt(fmt)
>> +#endif
>
> This might better be placed in printk.h just
> once.
>
yes. it seems to work smoothly.
Only minor annoyance is having to put local defns
above the #includes, but thats no different than previously.
> I think pr_fmt_debug is better than pr_fmt_dbg
> because the function/macro is named pr_debug.
Ok. Done.
> Maybe add all the pr_<level> variants too because
> some like to prefix __func__ to pr_err but not pr_info
> etc.
>
Done. Only wrinkle is wrt pr_warning() vs pr_warn()
I added to your suggestion:
+#define pr_fmt_warn pr_fmt_warning
the imperfection is that user has 4 combos of warn/warning
rather than 2 pairs, but the latter seems less defensible.
revised patch forthcoming shortly
seems I havent mastered reply-to with git send-email :-}
jim.c...@gmail.com
Heres those pr_fmt_*() defns, added to printk.
While not essential, they do have a certain symmetry.
The biggest issue (still small) is that pr_fmt_debug() no longer has
different defns for DYNAMIC_DEBUG=y/n builds, so either y-builds get
KBUILD_MODNAME or n-builds dont. Users can add an #if-else to get
both defns, which is less action-at-a-distance, so seemed better
overall. We can revisit this later.
[PATCH 25/26] dynamic_debug: add pr_fmt_debug() for dynamic_pr_debug
[PATCH 26/26] scx200_acb: use pr_(info|warn|err|debug) and
jim.c...@gmail.com
Switch this driver over to using more flexible dynamic-debug by
dropping NAME from pr_debug(NAME ": ...") calls, and adding it once
via pr_fmt(). With previous patch and CONFIG_I2C_DEBUG_BUS=y, we
get pr_debug()s enabled by default:
root@voyage:~# modprobe scx200_acb
root@voyage:~# grep scx200_acb /dbg/dynamic_debug/control
drivers/i2c/busses/scx200_acb.c:419 [scx200_acb]scx200_acb_probe =p "..."
drivers/i2c/busses/scx200_acb.c:408 [scx200_acb]scx200_acb_probe =p "..."
drivers/i2c/busses/scx200_acb.c:399 [scx200_acb]scx200_acb_probe =p "..."
drivers/i2c/busses/scx200_acb.c:584 [scx200_acb]scx200_acb_init =p "..."
The pr_fmt() adds the KBUILD_MODNAME to pr_err(), etc outputs, the
pr_fmt_debug() overrides that default, letting user do so selectively
via dynamic-debug. This override affects a non DYNAMIC_DEBUG build
too; we could fix this by making it conditional, but doing so in
printk.h may be better.
Signed-off-by: Jim Cromie <jim.c...@gmail.com>
---
drivers/i2c/busses/scx200_acb.c | 21 +++++++++++----------
1 files changed, 11 insertions(+), 10 deletions(-)
diff --git a/drivers/i2c/busses/scx200_acb.c b/drivers/i2c/busses/scx200_acb.c
index 986e5f6..34d89d6 100644
--- a/drivers/i2c/busses/scx200_acb.c
+++ b/drivers/i2c/busses/scx200_acb.c
@@ -23,6 +23,9 @@
Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
*/
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+#define pr_fmt_debug(fmt) fmt
+
#include <linux/module.h>
#include <linux/errno.h>
#include <linux/kernel.h>
@@ -37,8 +40,6 @@
#include <linux/scx200.h>
-#define NAME "scx200_acb"
-
MODULE_AUTHOR("Christer Weinigel <win...@nano-system.com>");
MODULE_DESCRIPTION("NatSemi SCx200 ACCESS.bus Driver");
MODULE_ALIAS("platform:cs5535-smb");
@@ -398,7 +399,7 @@ static __devinit int scx200_acb_probe(struct scx200_acb_iface *iface)
outb(0x70, ACBCTL2);
if (inb(ACBCTL2) != 0x70) {
- pr_debug(NAME ": ACBCTL2 readback failed\n");
+ pr_debug("ACBCTL2 readback failed\n");
return -ENXIO;
}
@@ -406,7 +407,7 @@ static __devinit int scx200_acb_probe(struct scx200_acb_iface *iface)
val = inb(ACBCTL1);
if (val) {
- pr_debug(NAME ": disabled, but ACBCTL1=0x%02x\n",
+ pr_debug("disabled, but ACBCTL1=0x%02x\n",
val);
return -ENXIO;
}
@@ -417,7 +418,7 @@ static __devinit int scx200_acb_probe(struct scx200_acb_iface *iface)
val = inb(ACBCTL1);
if ((val & ACBCTL1_NMINTE) != ACBCTL1_NMINTE) {
- pr_debug(NAME ": enabled, but NMINTE won't be set, "
+ pr_debug("enabled, but NMINTE won't be set, "
"ACBCTL1=0x%02x\n", val);
return -ENXIO;
}
@@ -433,7 +434,7 @@ static __devinit struct scx200_acb_iface *scx200_create_iface(const char *text,
iface = kzalloc(sizeof(*iface), GFP_KERNEL);
if (!iface) {
- printk(KERN_ERR NAME ": can't allocate memory\n");
+ pr_err("can't allocate memory\n");
return NULL;
}
@@ -459,14 +460,14 @@ static int __devinit scx200_acb_create(struct scx200_acb_iface *iface)
rc = scx200_acb_probe(iface);
if (rc) {
- printk(KERN_WARNING NAME ": probe failed\n");
+ pr_warn("probe failed\n");
return rc;
}
scx200_acb_reset(iface);
if (i2c_add_adapter(adapter) < 0) {
- printk(KERN_ERR NAME ": failed to register\n");
+ pr_err("failed to register\n");
return -ENODEV;
}
@@ -493,7 +494,7 @@ static struct scx200_acb_iface * __devinit scx200_create_dev(const char *text,
return NULL;
if (!request_region(base, 8, iface->adapter.name)) {
- printk(KERN_ERR NAME ": can't allocate io 0x%lx-0x%lx\n",
+ pr_err("can't allocate io 0x%lx-0x%lx\n",
base, base + 8 - 1);
goto errout_free;
}
@@ -583,7 +584,7 @@ static __init void scx200_scan_isa(void)
static int __init scx200_acb_init(void)
{
- pr_debug(NAME ": NatSemi SCx200 ACCESS.bus Driver\n");
+ pr_debug("NatSemi SCx200 ACCESS.bus Driver\n");
/* First scan for ISA-based devices */
scx200_scan_isa(); /* XXX: should we care about errors? */
--
1.7.4.4
jim.c...@gmail.com
dynamic_pr_debug can add module, function, file, and line selectively,
so theres no need to also add them via pr_fmt. Moreover, using pr_fmt
to add KBUILD_MODNAME ": ", as is typical for pr_info etc, causes
pr_debug() to double-print those fields added by the flag-settings.
So define pr_fmt_debug(fmt), and use it in dynamic_pr_debug(). This
lets users use pr_fmt() for pr_info and friends, without affecting
pr_debug(). Also add pr_fmt_*(fmt) defns for info etc, which default
to pr_fmt(). This lets user set a default for all pr_$level()s, and
then customize one of them.
Unlike the previous revision of this patch, pr_fmt_debug() is the same
for DEBUG and !DEBUG. User may want different formats, but doesnt get
them magically.
Signed-off-by: Jim Cromie <jim.c...@gmail.com>
---
include/linux/dynamic_debug.h | 8 ++++--
include/linux/printk.h | 51 ++++++++++++++++++++++++++++++++---------
2 files changed, 45 insertions(+), 14 deletions(-)
diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index 5622e04..4382b0a 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -73,7 +73,7 @@ extern int __dynamic_netdev_dbg(struct _ddebug *descriptor,
do { \
DECLARE_DYNAMIC_DEBUG_METADATA(descriptor, fmt); \
if (unlikely(descriptor.flags & _DPRINTK_FLAGS_PRINT)) \
- __dynamic_pr_debug(&descriptor, pr_fmt(fmt), \
+ __dynamic_pr_debug(&descriptor, pr_fmt_debug(fmt),\
##__VA_ARGS__); \
} while (0)
@@ -101,9 +101,11 @@ static inline int ddebug_remove_module(const char *mod)
}
#define dynamic_pr_debug(fmt, ...) \
- do { if (0) printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__); } while (0)
+ do { if (0) printk(KERN_DEBUG pr_fmt_debug(fmt), ##__VA_ARGS__);\
+ } while (0)
#define dynamic_dev_dbg(dev, fmt, ...) \
- do { if (0) dev_printk(KERN_DEBUG, dev, fmt, ##__VA_ARGS__); } while (0)
+ do { if (0) dev_printk(KERN_DEBUG, dev, fmt, ##__VA_ARGS__); \
+ } while (0)
#endif
#endif
diff --git a/include/linux/printk.h b/include/linux/printk.h
index 1224b1d..5ff654d 100644
--- a/include/linux/printk.h
+++ b/include/linux/printk.h
@@ -152,31 +152,60 @@ extern void dump_stack(void) __cold;
#define pr_fmt(fmt) fmt
#endif
+#ifndef pr_fmt_emerg
+#define pr_fmt_emerg(fmt) pr_fmt(fmt)
+#endif
+#ifndef pr_fmt_crit
+#define pr_fmt_crit(fmt) pr_fmt(fmt)
+#endif
+#ifndef pr_fmt_alert
+#define pr_fmt_alert(fmt) pr_fmt(fmt)
+#endif
+#ifndef pr_fmt_err
+#define pr_fmt_err(fmt) pr_fmt(fmt)
+#endif
+#ifndef pr_fmt_notice
+#define pr_fmt_notice(fmt) pr_fmt(fmt)
+#endif
+#ifndef pr_fmt_warning
+#define pr_fmt_warning(fmt) pr_fmt(fmt)
+#endif
+#define pr_fmt_warn pr_fmt_warning
+#ifndef pr_fmt_info
+#define pr_fmt_info(fmt) pr_fmt(fmt)
+#endif
+#ifndef pr_fmt_debug
+#define pr_fmt_debug(fmt) pr_fmt(fmt)
+#endif
+#ifndef pr_fmt_devel
+#define pr_fmt_devel(fmt) pr_fmt(fmt)
+#endif
+
#define pr_emerg(fmt, ...) \
- printk(KERN_EMERG pr_fmt(fmt), ##__VA_ARGS__)
+ printk(KERN_EMERG pr_fmt_emerg(fmt), ##__VA_ARGS__)
#define pr_alert(fmt, ...) \
- printk(KERN_ALERT pr_fmt(fmt), ##__VA_ARGS__)
+ printk(KERN_ALERT pr_fmt_alert(fmt), ##__VA_ARGS__)
#define pr_crit(fmt, ...) \
- printk(KERN_CRIT pr_fmt(fmt), ##__VA_ARGS__)
+ printk(KERN_CRIT pr_fmt_crit(fmt), ##__VA_ARGS__)
#define pr_err(fmt, ...) \
- printk(KERN_ERR pr_fmt(fmt), ##__VA_ARGS__)
+ printk(KERN_ERR pr_fmt_err(fmt), ##__VA_ARGS__)
#define pr_warning(fmt, ...) \
- printk(KERN_WARNING pr_fmt(fmt), ##__VA_ARGS__)
+ printk(KERN_WARNING pr_fmt_warn(fmt), ##__VA_ARGS__)
#define pr_warn pr_warning
#define pr_notice(fmt, ...) \
- printk(KERN_NOTICE pr_fmt(fmt), ##__VA_ARGS__)
+ printk(KERN_NOTICE pr_fmt_notice(fmt), ##__VA_ARGS__)
#define pr_info(fmt, ...) \
- printk(KERN_INFO pr_fmt(fmt), ##__VA_ARGS__)
+ printk(KERN_INFO pr_fmt_info(fmt), ##__VA_ARGS__)
#define pr_cont(fmt, ...) \
printk(KERN_CONT fmt, ##__VA_ARGS__)
/* pr_devel() should produce zero code unless DEBUG is defined */
#ifdef DEBUG
#define pr_devel(fmt, ...) \
- printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
+ printk(KERN_DEBUG pr_fmt_devel(fmt), ##__VA_ARGS__)
#else
#define pr_devel(fmt, ...) \
- no_printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
+ no_printk(KERN_DEBUG pr_fmt_devel(fmt), ##__VA_ARGS__)
#endif
/* If you are writing a driver, please use dev_dbg instead */
@@ -186,10 +215,10 @@ extern void dump_stack(void) __cold;
dynamic_pr_debug(fmt, ##__VA_ARGS__)
#elif defined(DEBUG)
#define pr_debug(fmt, ...) \
- printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
+ printk(KERN_DEBUG pr_fmt_debug(fmt), ##__VA_ARGS__)
#else
#define pr_debug(fmt, ...) \
- no_printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
+ no_printk(KERN_DEBUG pr_fmt_debug(fmt), ##__VA_ARGS__)
#endif
/*
--
1.7.4.4
Joe Perches
I'd just use pr_fmt_warn.
Instead of all this other extra stuff:
What did you think of avoiding all of this and
having __dynamic_pr_debug move the fmt pointer over
any initial KBUILD_MODULE ": "
int __dynamic_pr_debug(struct _ddebug *descriptor, const char *fmt, ...)
{
[...]
size_t modsize = strlen(descriptor->modname);
if (0 == strncmp(fmt, descriptor->modname, modsize) &&
0 == strncmp(fmt + modsize, ": ", 2))
fmt += modsize + 2;
vprintk(fmt, ...)
?
Jim Cromie
> On Tue, 2011-09-27 at 14:37 -0600, Jim Cromie wrote:
>>
>> Done. Only wrinkle is wrt pr_warning() vs pr_warn()
>> I added to your suggestion:
>> +#define pr_fmt_warn pr_fmt_warning
>>
>> the imperfection is that user has 4 combos of warn/warning
>> rather than 2 pairs, but the latter seems less defensible.
>
> I'd just use pr_fmt_warn.
>
> Instead of all this other extra stuff:
>
trouble is that pr_warn & pr_warning are synonyms,
and not having matching synonyms for pr_fmt_warn*
creates a few corner-case oddities that I think
somebody will trip over eventually.
Cleanest way is to drop the original synonym, and just use
warn (its shorter), but that creates some churn (havent grepped to see
how much).
I picked what looked like least effort & fewest corner-cases.
ICBW..
> What did you think of avoiding all of this and
> having __dynamic_pr_debug move the fmt pointer over
> any initial KBUILD_MODULE ": "
>
> int __dynamic_pr_debug(struct _ddebug *descriptor, const char *fmt, ...)
> {
> [...]
> � � � �size_t modsize = strlen(descriptor->modname);
> � � � �if (0 == strncmp(fmt, descriptor->modname, modsize) &&
> � � � � � �0 == strncmp(fmt + modsize, ": ", 2))
> � � � � � � � �fmt += modsize + 2;
>
> � � � �vprintk(fmt, ...)
>
> ?
I was getting to that... ;-)
Im not crazy about it. It feels like too much ..
Its a runtime workaround for what I think is a
problem in users' (or header's) #defines.
something like this would fix it at compile time,
similar to how I had it in dynamic-debug.h previously:
#if (CONFIG_DYNAMIC_DEBUG)
#define pr_fmt_debug(fmt) KBUILD_MODNAME ": " fmt
#endif
// defaulting to pr_fmt as given by user, or by printk.h
without dynamic-debug, pr_debug() would get same value
as as used by pr_info() etc.
With patchset as proposed, user needs to do the above
to get 'proper' output for both CONFIG_DYNAMIC_DEBUG=y/n.
It may be appropriate to do that in printk.h or dynamic_debug.h
TBDiscussed.
thanks
Jim
Joe Perches
> Cleanest way is to drop the original synonym, and just use
> warn (its shorter), but that creates some churn (havent grepped to see
> how much).
> I picked what looked like least effort & fewest corner-cases.
> ICBW..
#ifndef pr_fmt_warning
#define pr_fmt_warning pr_fmt_warn
#endif
> > What did you think of avoiding all of this and
> > having __dynamic_pr_debug move the fmt pointer over
> > any initial KBUILD_MODULE ": "
> >
> > int __dynamic_pr_debug(struct _ddebug *descriptor, const char *fmt, ...)
> > {
> > [...]
> > size_t modsize = strlen(descriptor->modname);
> > if (0 == strncmp(fmt, descriptor->modname, modsize) &&
> > 0 == strncmp(fmt + modsize, ": ", 2))
> > fmt += modsize + 2;
> > vprintk(fmt, ...)
> > ?
>
> I was getting to that... ;-)
> Im not crazy about it. It feels like too much ..
> Its a runtime workaround for what I think is a
> problem in users' (or header's) #defines.
I think exactly the opposite myself.
I think all of the '#define pr_fmt(fmt) KBUILD_MODNAME ": "'
are effectivly useless and I will eventually delete them.
The printk subsystem should look up the module name and
prefix them to the printk akin to how the dynamic_debug
system does. Except the module name should be a singleton.
Jim Cromie
> On Tue, 2011-09-27 at 20:54 -0600, Jim Cromie wrote:
>> Cleanest way is to drop the original synonym, and just use
>> warn (its shorter), but that creates some churn (havent grepped to see
>> how much).
>> I picked what looked like least effort & fewest corner-cases.
>> ICBW..
>
> #ifndef pr_fmt_warning
> #define pr_fmt_warning pr_fmt_warn
> #endif
>
you've made it conditional, where line 203 is not. why ?
201 #define pr_warning(fmt, ...) \
202 printk(KERN_WARNING pr_fmt_warn(fmt), ##__VA_ARGS__)
203 #define pr_warn pr_warning
204 #define pr_notice(fmt, ...) \
Also, though minor, 203 adds pr_warn as the shortcut,
youve suggested the opposite. I presume this is an oversight.
>> > What did you think of avoiding all of this and
>> > having __dynamic_pr_debug move the fmt pointer over
>> > any initial KBUILD_MODULE ": "
>> >
>> > int __dynamic_pr_debug(struct _ddebug *descriptor, const char *fmt, ...)
>> > {
>> > [...]
>> > � � � �size_t modsize = strlen(descriptor->modname);
>> > � � � �if (0 == strncmp(fmt, descriptor->modname, modsize) &&
>> > � � � � � �0 == strncmp(fmt + modsize, ": ", 2))
>> > � � � � � � � �fmt += modsize + 2;
>> > � � � �vprintk(fmt, ...)
>> > ?
>>
>> I was getting to that... ;-)
>> Im not crazy about it. �It feels like too much ..
>> Its a runtime workaround for what I think is a
>> problem in users' (or header's) #defines.
>
> I think exactly the opposite myself.
>
> I think all of the '#define pr_fmt(fmt) KBUILD_MODNAME ": "'
> are effectivly useless and I will eventually delete them.
>
> The printk subsystem should look up the module name and
> prefix them to the printk akin to how the dynamic_debug
> system does. �Except the module name should be a singleton.
hmm, maybe Ive missed something in your argument.
For non-dynamic-debug builds, it seems you still want
the MODNAME in the pr_(crit|warn|info|debug) output,
you just dont like the hundreds of defines throughout drivers/*
linux-2.6]$ grep -r pr_fmt drivers/ |wc
427 2556 32040
linux-2.6]$ grep -r pr_fmt drivers/ | grep KBUILD_MODNAME |wc
303 1827 22567
Your runtime check-and-skip-MODNAME code above
fixes dynamic-debug so that works around user defines
doing:
#define pr_fmt(fmt) KBUILD_MODNAME ": "
This would let user of a dynamic-debug enabled kernel
add MODNAME selectively, with +m >$CONTROL
But ISTM this does the same:
diff --git a/include/linux/printk.h b/include/linux/printk.h
index e21de7e..cf17986 100644
--- a/include/linux/printk.h
+++ b/include/linux/printk.h
@@ -148,8 +148,17 @@ static inline void setup_log_buf(int early)
extern void dump_stack(void) __cold;
+/* Set pr_fmt default to most common, useful case */
#ifndef pr_fmt
-#define pr_fmt(fmt) fmt
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+#endif
+/* except for dynamic-debug, where user can enable it, and we dont
+ want to print it 2x
+ */
+#if defined(CONFIG_DYNAMIC_DEBUG)
+#ifndef pr_fmt_debug
+#define pr_fmt_debug(fmt) fmt
+#endif
#endif
#ifndef pr_fmt_emerg
This sets default to work for the 303 common cases,
allowing the removal of their defines:
#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
It also allows existing non-standard uses to work as is:
drivers/sh/intc/dynamic.c:#define pr_fmt(fmt) "intc: " fmt
Your suggested pr_fmt_warning, etc allow severity specific
customizations to override, w/o changing all the others..
And in dynamic-debug kernels, the default pr_fmt_debug
is reset so MODNAME isnt printed, except by the user:
~]# echo +m > $CONTROL
IOW, it seems to take care of everything, w/o runtime workarounds.
With agreement on this, I can fold above patch into 25/26
(which Ive updated here to also fix pr_*_once, pr_*_ratelimited macros),
and we can move against those 303 irritants at our leisure (there is
no flag day)
thanks
Jim
Joe Perches
> On Tue, Sep 27, 2011 at 10:51 PM, Joe Perches <j...@perches.com> wrote:
> > On Tue, 2011-09-27 at 20:54 -0600, Jim Cromie wrote:
> >> Cleanest way is to drop the original synonym, and just use
> >> warn (its shorter), but that creates some churn (havent grepped to see
> >> how much).
> >> I picked what looked like least effort & fewest corner-cases.
> >> ICBW..
> >
> > #ifndef pr_fmt_warning
> > #define pr_fmt_warning pr_fmt_warn
> > #endif
> >
>
> you've made it conditional, where line 203 is not. why ?
> 201 #define pr_warning(fmt, ...) \
> 202 printk(KERN_WARNING pr_fmt_warn(fmt), ##__VA_ARGS__)
> 203 #define pr_warn pr_warning
> 204 #define pr_notice(fmt, ...) \
>
> Also, though minor, 203 adds pr_warn as the shortcut,
> youve suggested the opposite. I presume this is an oversight.
I think pr_warning should be deprecated.
I think not #defining pr_fmt_warning is just fine.
Yes. I've added most all of them.
It's because the current printk / pr_<level>
subsystem chose to not prefix by default.
Over the last couple of years, I've helped
slowly converted about half of the kernel to
pr_<level> and pr_fmt.
At some point next year or so, enough of the
kernel should be converted so that marking
the last few uses of pr_debug or other
pr_<level> without a preceding pr_fmt should
be be relatively easy to convert to a more
standard KBUILD_MODNAME or other user specified
prefix.
It takes awhile though.
> IOW, it seems to take care of everything, w/o runtime workarounds.
Nope. Try it with and without DEBUG.
Also, look at the pr_debug uses that include __func__.
These also duplicate output.
I'd prefer a solution that allows deduplication.
Having a dynamic_debug output to a fixed buffer
via vsnprintf, scanning that output for leading
KBUILD_MODNAME/__file__/__func__/__LINE__ prefixes
and then skipping them when enabled would seem
appropriate to me.
cheers, Joe
jim.c...@gmail.com
- drops scx200 - off topic
- adds pr_cont_once tweak - remove pr_fmt
as before, it has:
- Doc tweaks per Randy Dunlap
- pr_fmt_$severity, suggested by Joe Perches
- does not change pr_fmt() default, per Joe
Rebased onto git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/driver-core.git
+ Jason's 8-12/12 which havent made it into driver-core
Ive not resent them here.
[jimc@groucho linux-2.6]$ git diff --stat jb-on-driver-core
Documentation/dynamic-debug-howto.txt | 334 +++++++++++------
include/linux/device.h | 8 +-
include/linux/dynamic_debug.h | 23 +-
include/linux/netdevice.h | 8 +-
include/linux/printk.h | 95 +++--
lib/dynamic_debug.c | 685 +++++++++++++++++++++++++++------
6 files changed, 866 insertions(+), 287 deletions(-)
jim.c...@gmail.com
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.c...@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 6372b18..a52b78a 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -276,6 +276,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:
@@ -287,6 +295,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)
@@ -300,13 +311,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.4
jim.c...@gmail.com
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.c...@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 2d26c10..74b395b 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.4
--
jim.c...@gmail.com
Alter fn-sig to accept flags arg directly, instead of a ptr to
structure containing it. This lets us reuse it for pending-queries.
Signed-off-by: Jim Cromie <jim.c...@gmail.com>
---
lib/dynamic_debug.c | 9 +++++----
1 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 3c9244d..d7b71a6 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -103,7 +103,7 @@ static struct { unsigned flag:8; char opt_char; } opt_array[] = {
};
/* format a string into buf[] which describes the _ddebug's flags */
-static char *ddebug_describe_flags(struct _ddebug *dp, char *buf,
+static char *ddebug_describe_flags(unsigned int flags, char *buf,
size_t maxlen)
{
char *p = buf;
@@ -112,7 +112,7 @@ static char *ddebug_describe_flags(struct _ddebug *dp, char *buf,
BUG_ON(maxlen < 10);
*p++ = '=';
for (i = 0; i < ARRAY_SIZE(opt_array); ++i)
- if (dp->flags & opt_array[i].flag)
+ if (flags & opt_array[i].flag)
*p++ = opt_array[i].opt_char;
if (*(p-1) == '=')
*p++ = '_';
@@ -227,7 +227,8 @@ static int ddebug_change(const struct ddebug_query *query,
pr_info("changed %s:%d [%s]%s %s\n",
trim_prefix(dp->filename), dp->lineno,
dt->mod_name, dp->function,
- ddebug_describe_flags(dp, flagbuf,
+ ddebug_describe_flags(dp->flags,
+ flagbuf,
sizeof(flagbuf)));
}
}
@@ -901,7 +902,7 @@ static int ddebug_proc_show(struct seq_file *m, void *p)
seq_printf(m, "%s:%u [%s]%s %s \"",
trim_prefix(dp->filename), dp->lineno,
iter->table->mod_name, dp->function,
- ddebug_describe_flags(dp, flagsbuf, sizeof(flagsbuf)));
+ ddebug_describe_flags(dp->flags, flagsbuf, sizeof(flagsbuf)));
seq_escape(m, dp->format, "\t\r\n\"");
seq_puts(m, "\"\n");
--
1.7.4.4
jim.c...@gmail.com
if _ddebug table is empty (which shouldn't happen in a CONFIG_DYNAMIC_DEBUG
build), then warn (BUILD_BUG??) and return early. This skips empty
table scan and parsing of setup-string, including the pr_info call
noting the parse.
Signed-off-by: Jim Cromie <jim.c...@gmail.com>
---
lib/dynamic_debug.c | 32 +++++++++++++++++---------------
1 files changed, 17 insertions(+), 15 deletions(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 80b5322..1a8941e 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -1272,23 +1272,25 @@ static int __init dynamic_debug_init(void)
int ret = 0;
int n = 0;
- if (__start___verbose != __stop___verbose) {
- iter = __start___verbose;
- modname = iter->modname;
- iter_start = iter;
- for (; iter < __stop___verbose; iter++) {
- if (strcmp(modname, iter->modname)) {
- ret = ddebug_add_module(iter_start, n, modname);
- if (ret)
- goto out_free;
- n = 0;
- modname = iter->modname;
- iter_start = iter;
- }
- n++;
+ if (__start___verbose == __stop___verbose) {
+ pr_warn("_ddebug table is empty in a CONFIG_DYNAMIC_DEBUG build");
+ return 1;
+ }
+ iter = __start___verbose;
+ modname = iter->modname;
+ iter_start = iter;
+ for (; iter < __stop___verbose; iter++) {
+ if (strcmp(modname, iter->modname)) {
+ ret = ddebug_add_module(iter_start, n, modname);
+ if (ret)
+ goto out_free;
+ n = 0;
+ modname = iter->modname;
+ iter_start = iter;
}
- ret = ddebug_add_module(iter_start, n, modname);
+ n++;
}
+ ret = ddebug_add_module(iter_start, n, modname);
/* ddebug_query boot param got passed -> set it up */
if (ddebug_setup_string[0] != '\0') {
--
1.7.4.4
jim.c...@gmail.com
lineno:24 allows files with 4 million lines, an insane file-size, even
for never-to-get-in-tree machine generated code. Reduce this to 18
bits, which still allows 256k lines. This is still insanely big, but
its not raving mad.
Signed-off-by: Jim Cromie <jim.c...@gmail.com>
---
include/linux/dynamic_debug.h | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index 2c9c476..5622e04 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -15,7 +15,7 @@ struct _ddebug {
const char *function;
const char *filename;
const char *format;
- unsigned int lineno:24;
+ unsigned int lineno:18;
/*
* The flags field controls the behaviour at the callsite.
* The bits here are changed dynamically when the user
--
1.7.4.4
jim.c...@gmail.com
Hoist locking out of ddebug_change() into ddebug_exec_queries().
This reduces lock fiddling, but will increase hold-times slightly,
at least when many queries are submitted.
With verbose off, on a 266 MHz, takes, I get a reasonable:
#> time cat query-file-w-300-pending-queries > $CONTROL
real 0m0.508s
user 0m0.009s
sys 0m0.290s
Signed-off-by: Jim Cromie <jim.c...@gmail.com>
---
lib/dynamic_debug.c | 7 +++----
1 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 2b3393f..0676de0 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -113,7 +113,7 @@ static char *ddebug_describe_flags(struct _ddebug *dp, char *buf,
* 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 void ddebug_change(const struct ddebug_query *query,
unsigned int flags, unsigned int mask)
@@ -125,7 +125,6 @@ static void 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 */
@@ -175,8 +174,6 @@ static void ddebug_change(const struct ddebug_query *query,
sizeof(flagbuf)));
}
}
- mutex_unlock(&ddebug_lock);
-
if (!nfound && verbose)
pr_info("no matches for query\n");
}
@@ -450,6 +447,7 @@ static int ddebug_exec_queries(char *query)
char *split;
int i, errs = 0, exitcode = 0, rc;
+ mutex_lock(&ddebug_lock);
for (i = 0; query; query = split) {
split = strpbrk(query, ";\n");
if (split)
@@ -468,6 +466,7 @@ static int ddebug_exec_queries(char *query)
}
i++;
}
+ mutex_unlock(&ddebug_lock);
pr_info("processed %d queries, with %d errs\n", i, errs);
return exitcode;
--
1.7.4.4
jim.c...@gmail.com
Factor pr_info(query) out of ddebug_parse_query, into pr_info_dq(),
for reuse later. Also change the printed labels: file, func to agree
with the query-spec keywords accepted in the control file. Pass ""
when string is null, to avoid "(null)" output from sprintf. For
format print, use precision to skip last char, assuming its '\n', no
great harm if not, its a debug msg,
Signed-off-by: Jim Cromie <jim.c...@gmail.com>
---
lib/dynamic_debug.c | 27 ++++++++++++++++++---------
1 files changed, 18 insertions(+), 9 deletions(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 3f37b5e..2f39b2f 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -109,6 +109,22 @@ static char *ddebug_describe_flags(struct _ddebug *dp, char *buf,
return buf;
}
+#define vpr_info_dq(q, msg) \
+do { \
+ if (verbose) \
+ /* trim last char off format print */ \
+ pr_info("%s: func=\"%s\" file=\"%s\" " \
+ "module=\"%s\" format=\"%.*s\" " \
+ "lineno=%u-%u", \
+ msg, \
+ q->function ? q->function : "", \
+ q->filename ? q->filename : "", \
+ q->module ? q->module : "", \
+ (int)(q->format ? strlen(q->format) - 1 : 0), \
+ q->format ? q->format : "", \
+ q->first_lineno, q->last_lineno); \
+} while (0)
+
/*
* Search the tables for _ddebug's which match the given
* `query' and apply the `flags' and `mask' to them. Tells
@@ -116,7 +132,7 @@ static char *ddebug_describe_flags(struct _ddebug *dp, char *buf,
* were matched. Called with ddebug_lock held.
*/
static void ddebug_change(const struct ddebug_query *query,
- unsigned int flags, unsigned int mask)
+ unsigned int flags, unsigned int mask)
{
int i;
struct ddebug_table *dt;
@@ -350,14 +366,7 @@ static int ddebug_parse_query(char *words[], int nwords,
return -EINVAL;
}
}
-
- if (verbose)
- pr_info("q->function=\"%s\" q->filename=\"%s\" "
- "q->module=\"%s\" q->format=\"%s\" q->lineno=%u-%u\n",
- query->function, query->filename,
- query->module, query->format, query->first_lineno,
- query->last_lineno);
-
+ vpr_info_dq(query, "parsed");
return 0;
}
--
1.7.4.4
jim.c...@gmail.com
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.c...@gmail.com>
---
lib/dynamic_debug.c | 10 ++++------
1 files changed, 4 insertions(+), 6 deletions(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 0676de0..3f37b5e 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -922,12 +922,10 @@ static int __init dynamic_debug_init(void)
pr_info("ddebug initializing with string \"%s\"",
ddebug_setup_string);
ret = ddebug_exec_queries(ddebug_setup_string);
- if (ret)
- pr_warn("Invalid ddebug boot param %s",
- ddebug_setup_string);
- else
- pr_info("ddebug initialized with string %s",
- ddebug_setup_string);
+ if (ret) {
+ ret = 0; /* make this non-fatal */
+ pr_warn("invalid ddebug_query\n");
+ }
}
out_free:
--
1.7.4.4
jim.c...@gmail.com
When a pending-query is resubmitted with zeroed flags, remove it
from pending-queries list. The submission must have identical
match-specs, and like the original query, must have 'a' in the
filter-flags. If other filter-flags are given, they must match
the query to be removed, but filter can be underspecified; "p"
will match against "pt".
Signed-off-by: Jim Cromie <jim.c...@gmail.com>
---
lib/dynamic_debug.c | 9 ++++++++-
1 files changed, 8 insertions(+), 1 deletions(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index a59d48c..0c9e53a 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -508,7 +508,14 @@ 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 */
+ vpr_info_pq(pq, "delete pending");
+ list_del_init(&pq->link);
+ pqfree(pq);
+ return 0;
+ }
if (pq->flags != flags)
pq->flags = flags;
if (pq->mask != mask)
--
1.7.4.4
jim.c...@gmail.com
Change describe_flags() to emit '=[pmflta_]+' for current callsite
flags, or just '=_' when they're disabled. Having '=' in output
allows a more selective grep expression, in contrast '-' may appear
in filenames, line-ranges, and format-strings. '=' also has better
mnemonics, saying; "the current setting is equal to <flags>".
The default allows grep "=_" <dbgfs>/dynamic_debug/control
to see disabled callsites while avoiding the many occurrences of " = "
seen in format strings.
Enlarge flagsbufs to handle additional flag chars, "=a_"
Signed-off-by: Jim Cromie <jim.c...@gmail.com>
---
lib/dynamic_debug.c | 12 +++++++-----
1 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 0c9e53a..29dbf69 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -99,6 +99,7 @@ static struct { unsigned flag:8; char opt_char; } opt_array[] = {
{ _DPRINTK_FLAGS_INCL_LINENO, 'l' },
{ _DPRINTK_FLAGS_INCL_TID, 't' },
{ _DPRINTK_FLAGS_APPEND, 'a' },
+ { _DPRINTK_FLAGS_DEFAULT, '_' },
};
/* format a string into buf[] which describes the _ddebug's flags */
@@ -108,12 +109,13 @@ static char *ddebug_describe_flags(struct _ddebug *dp, char *buf,
char *p = buf;
int i;
- BUG_ON(maxlen < 4);
+ BUG_ON(maxlen < 10);
+ *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++ = '-';
+ if (*(p-1) == '=')
+ *p++ = '_';
*p = '\0';
return buf;
@@ -195,7 +197,7 @@ static int ddebug_change(const struct ddebug_query *query,
struct ddebug_table *dt;
unsigned int newflags;
unsigned int nfound = 0;
- char flagbuf[8];
+ char flagbuf[10];
/* search for matching ddebugs */
list_for_each_entry(dt, &ddebug_tables, link) {
@@ -866,7 +868,7 @@ static int ddebug_proc_show(struct seq_file *m, void *p)
{
struct ddebug_iter *iter = m->private;
struct _ddebug *dp = p;
- char flagsbuf[8];
+ char flagsbuf[10];
if (verbose >= VERBOSE_PROC_SHOW)
pr_info("called m=%p p=%p\n", m, p);
--
1.7.4.4
jim.c...@gmail.com
pr_cont() doesnt use pr_fmt(), since it continues printing a single
line, and the prefix has already been done. Fix pr_cont_once() to do
the same.
Signed-off-by: Jim Cromie <jim.c...@gmail.com>
---
include/linux/printk.h | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/include/linux/printk.h b/include/linux/printk.h
index e21de7e..4ba6cde 100644
--- a/include/linux/printk.h
+++ b/include/linux/printk.h
@@ -255,7 +255,7 @@ extern void dump_stack(void) __cold;
#define pr_info_once(fmt, ...) \
printk_once(KERN_INFO pr_fmt_info(fmt), ##__VA_ARGS__)
#define pr_cont_once(fmt, ...) \
- printk_once(KERN_CONT pr_fmt(fmt), ##__VA_ARGS__)
+ printk_once(KERN_CONT fmt, ##__VA_ARGS__)
/* If you are writing a driver, please use dev_dbg instead */
#if defined(DEBUG)
#define pr_debug_once(fmt, ...) \
--
1.7.4.4
jim.c...@gmail.com
Define separate pr_fmt_*()s for each severity, defaulting to pr_fmt(),
and use them in all pr_$severity(), pr_$severity_once(), and
pr_$severity_ratelimited() macros. This lets modules change the
printed prefix for the whole set and/or any subset.
Signed-off-by: Jim Cromie <jim.c...@gmail.com>
---
include/linux/dynamic_debug.h | 8 ++-
include/linux/printk.h | 87 +++++++++++++++++++++++++++--------------
2 files changed, 63 insertions(+), 32 deletions(-)
diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index 5622e04..4382b0a 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -73,7 +73,7 @@ extern int __dynamic_netdev_dbg(struct _ddebug *descriptor,
do { \
DECLARE_DYNAMIC_DEBUG_METADATA(descriptor, fmt); \
if (unlikely(descriptor.flags & _DPRINTK_FLAGS_PRINT)) \
- __dynamic_pr_debug(&descriptor, pr_fmt(fmt), \
+ __dynamic_pr_debug(&descriptor, pr_fmt_debug(fmt),\
##__VA_ARGS__); \
} while (0)
@@ -101,9 +101,11 @@ static inline int ddebug_remove_module(const char *mod)
}
#define dynamic_pr_debug(fmt, ...) \
- do { if (0) printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__); } while (0)
+ do { if (0) printk(KERN_DEBUG pr_fmt_debug(fmt), ##__VA_ARGS__);\
+ } while (0)
#define dynamic_dev_dbg(dev, fmt, ...) \
- do { if (0) dev_printk(KERN_DEBUG, dev, fmt, ##__VA_ARGS__); } while (0)
+ do { if (0) dev_printk(KERN_DEBUG, dev, fmt, ##__VA_ARGS__); \
+ } while (0)
#endif
#endif
diff --git a/include/linux/printk.h b/include/linux/printk.h
index 1224b1d..e21de7e 100644
--- a/include/linux/printk.h
+++ b/include/linux/printk.h
/* If you are writing a driver, please use dev_dbg instead */
@@ -186,10 +215,10 @@ extern void dump_stack(void) __cold;
dynamic_pr_debug(fmt, ##__VA_ARGS__)
#elif defined(DEBUG)
#define pr_debug(fmt, ...) \
- printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
+ printk(KERN_DEBUG pr_fmt_debug(fmt), ##__VA_ARGS__)
#else
#define pr_debug(fmt, ...) \
- no_printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
+ no_printk(KERN_DEBUG pr_fmt_debug(fmt), ##__VA_ARGS__)
#endif
/*
@@ -212,28 +241,28 @@ extern void dump_stack(void) __cold;
#endif
#define pr_emerg_once(fmt, ...) \
- printk_once(KERN_EMERG pr_fmt(fmt), ##__VA_ARGS__)
+ printk_once(KERN_EMERG pr_fmt_emerg(fmt), ##__VA_ARGS__)
#define pr_alert_once(fmt, ...) \
- printk_once(KERN_ALERT pr_fmt(fmt), ##__VA_ARGS__)
+ printk_once(KERN_ALERT pr_fmt_alert(fmt), ##__VA_ARGS__)
#define pr_crit_once(fmt, ...) \
- printk_once(KERN_CRIT pr_fmt(fmt), ##__VA_ARGS__)
+ printk_once(KERN_CRIT pr_fmt_crit(fmt), ##__VA_ARGS__)
#define pr_err_once(fmt, ...) \
- printk_once(KERN_ERR pr_fmt(fmt), ##__VA_ARGS__)
+ printk_once(KERN_ERR pr_fmt_err(fmt), ##__VA_ARGS__)
#define pr_warn_once(fmt, ...) \
- printk_once(KERN_WARNING pr_fmt(fmt), ##__VA_ARGS__)
+ printk_once(KERN_WARNING pr_fmt_warn(fmt), ##__VA_ARGS__)
#define pr_notice_once(fmt, ...) \
- printk_once(KERN_NOTICE pr_fmt(fmt), ##__VA_ARGS__)
+ printk_once(KERN_NOTICE pr_fmt_notice(fmt), ##__VA_ARGS__)
#define pr_info_once(fmt, ...) \
- printk_once(KERN_INFO pr_fmt(fmt), ##__VA_ARGS__)
+ printk_once(KERN_INFO pr_fmt_info(fmt), ##__VA_ARGS__)
#define pr_cont_once(fmt, ...) \
printk_once(KERN_CONT pr_fmt(fmt), ##__VA_ARGS__)
/* If you are writing a driver, please use dev_dbg instead */
#if defined(DEBUG)
#define pr_debug_once(fmt, ...) \
- printk_once(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
+ printk_once(KERN_DEBUG pr_fmt_debug(fmt), ##__VA_ARGS__)
#else
#define pr_debug_once(fmt, ...) \
- no_printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
+ no_printk(KERN_DEBUG pr_fmt_debug(fmt), ##__VA_ARGS__)
#endif
/*
@@ -256,27 +285,27 @@ extern void dump_stack(void) __cold;
#endif
#define pr_emerg_ratelimited(fmt, ...) \
- printk_ratelimited(KERN_EMERG pr_fmt(fmt), ##__VA_ARGS__)
+ printk_ratelimited(KERN_EMERG pr_fmt_emerg(fmt), ##__VA_ARGS__)
#define pr_alert_ratelimited(fmt, ...) \
- printk_ratelimited(KERN_ALERT pr_fmt(fmt), ##__VA_ARGS__)
+ printk_ratelimited(KERN_ALERT pr_fmt_alert(fmt), ##__VA_ARGS__)
#define pr_crit_ratelimited(fmt, ...) \
- printk_ratelimited(KERN_CRIT pr_fmt(fmt), ##__VA_ARGS__)
+ printk_ratelimited(KERN_CRIT pr_fmt_crit(fmt), ##__VA_ARGS__)
#define pr_err_ratelimited(fmt, ...) \
- printk_ratelimited(KERN_ERR pr_fmt(fmt), ##__VA_ARGS__)
+ printk_ratelimited(KERN_ERR pr_fmt_err(fmt), ##__VA_ARGS__)
#define pr_warn_ratelimited(fmt, ...) \
- printk_ratelimited(KERN_WARNING pr_fmt(fmt), ##__VA_ARGS__)
+ printk_ratelimited(KERN_WARNING pr_fmt_warn(fmt), ##__VA_ARGS__)
#define pr_notice_ratelimited(fmt, ...) \
- printk_ratelimited(KERN_NOTICE pr_fmt(fmt), ##__VA_ARGS__)
+ printk_ratelimited(KERN_NOTICE pr_fmt_notice(fmt), ##__VA_ARGS__)
#define pr_info_ratelimited(fmt, ...) \
- printk_ratelimited(KERN_INFO pr_fmt(fmt), ##__VA_ARGS__)
+ printk_ratelimited(KERN_INFO pr_fmt_info(fmt), ##__VA_ARGS__)
/* no pr_cont_ratelimited, don't do that... */
/* If you are writing a driver, please use dev_dbg instead */
#if defined(DEBUG)
#define pr_debug_ratelimited(fmt, ...) \
- printk_ratelimited(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
+ printk_ratelimited(KERN_DEBUG pr_fmt_debug(fmt), ##__VA_ARGS__)
#else
#define pr_debug_ratelimited(fmt, ...) \
- no_printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
+ no_printk(KERN_DEBUG pr_fmt_debug(fmt), ##__VA_ARGS__)
#endif
enum {
--
1.7.4.4
jim.c...@gmail.com
Save queries with 'a' flag to pending-queries after applying them.
When a module is loaded later, ddebug_add_module() calls
apply_pending_queries() to scan pending_queries list and call
ddebug_change to apply them.
With this change, the loaded module's pr_debug()s are enabled before
its module_init is invoked, allowing use of pr_debug()s during
initialization.
Behavior:
If pending query matches existing one, the existing one is updated.
Pending queries stay on list through rmmod, modprobe cycles.
If the updated query has 0 flags, it is removed.
If pending query has 0 flags, it is discarded, not added.
Patch adds:
'a' flag to dynamic_debug.h
struct pending_query: like ddebug_query, but with storage for match specs.
ddebug_save_pending():
checks new pending query against existing, to update or remove
discards new do-nothing pending queries.
copies ddebug_query, saving match specs from stack.
adds new pending query to end of pending list (fifo)
apply_pending_queries():
called from ddebug_add_module()
(re)applies queries on newly loaded modules.
queries_match() - helper for ddebug_save_pending
ddebug_remove_all_tables() - clear pending-queries here.
Signed-off-by: Jim Cromie <jim.c...@gmail.com>
---
include/linux/dynamic_debug.h | 1 +
lib/dynamic_debug.c | 154 +++++++++++++++++++++++++++++++++++++++--
2 files changed, 150 insertions(+), 5 deletions(-)
diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index 12ef233..2c9c476 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 */
#if defined DEBUG
#define _DPRINTK_FLAGS_DEFAULT _DPRINTK_FLAGS_PRINT
#else
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 4c8e178..a59d48c 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -53,6 +53,12 @@ struct ddebug_query {
unsigned int first_lineno, last_lineno;
};
+struct pending_query {
+ struct list_head link;
+ struct ddebug_query query;
+ unsigned int flags, mask;
+};
+
struct ddebug_iter {
struct ddebug_table *table;
unsigned int idx;
@@ -65,6 +71,9 @@ static int verbose = 0;
#define VERBOSE_PROC_SHOW 11 /* enable per-line msgs on control file reads */
module_param(verbose, int, 0644);
+/* legal but inapplicable queries, save and test against new modules */
+static LIST_HEAD(pending_queries);
+
/* Return the last part of a pathname */
static inline const char *basename(const char *path)
{
@@ -89,6 +98,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 */
@@ -125,6 +135,25 @@ do { \
q->first_lineno, q->last_lineno); \
} while (0)
+#define vpr_info_pq(pq, msg) \
+do { \
+ struct ddebug_query *q = &pq->query; \
+ if (verbose) \
+ /* trim last char off format print */ \
+ pr_info("%s: func=\"%s\" file=\"%s\" " \
+ "module=\"%s\" format=\"%.*s\" " \
+ "lineno=%u-%u " \
+ "flags=0x%x mask=0x%x", \
+ msg, \
+ q->function ? q->function : "", \
+ q->filename ? q->filename : "", \
+ q->module ? q->module : "", \
+ (int)(q->format ? strlen(q->format) - 1 : 0), \
+ q->format ? q->format : "", \
+ q->first_lineno, q->last_lineno, \
+ pq->flags, pq->mask); \
+} while (0)
+
static bool query_matches_callsite(struct _ddebug *dp,
const struct ddebug_query *query)
{
@@ -159,7 +188,7 @@ static bool query_matches_callsite(struct _ddebug *dp,
* the user which ddebug's were changed, or whether none
* were matched. Called with ddebug_lock held.
*/
-static void ddebug_change(const struct ddebug_query *query,
+static int ddebug_change(const struct ddebug_query *query,
unsigned int flags, unsigned int mask)
{
int i;
@@ -196,8 +225,7 @@ static void ddebug_change(const struct ddebug_query *query,
sizeof(flagbuf)));
}
}
- if (!nfound && verbose)
- pr_info("no matches for query\n");
+ return nfound;
}
/*
@@ -435,6 +463,95 @@ static int ddebug_parse_flags(const char *str, unsigned int *flagsp,
return 0;
}
+/* check if new query exactly matches existing one */
+static bool queries_match(struct ddebug_query *qnew, struct ddebug_query *qcur)
+{
+ if (!qnew->module != !qcur->module ||
+ !qnew->filename != !qcur->filename ||
+ !qnew->function != !qcur->function ||
+ !qnew->format != !qcur->format)
+ return false; /* a match-spec set/unset state differs */
+
+ if (qnew->last_lineno != qcur->last_lineno ||
+ qnew->first_lineno != qcur->first_lineno)
+ return false;
+
+ if ((qnew->module && strcmp(qnew->module, qcur->module)) ||
+ (qnew->filename && strcmp(qnew->filename, qcur->filename)) ||
+ (qnew->function && strcmp(qnew->function, qcur->function)) ||
+ (qnew->format && strcmp(qnew->format, qcur->format)))
+ return false;
+
+ return true;
+}
+
+static void pqfree(struct pending_query *pq)
+{
+ if (pq->query.module)
+ kfree(pq->query.module);
+ if (pq->query.function)
+ kfree(pq->query.function);
+ if (pq->query.filename)
+ kfree(pq->query.filename);
+ if (pq->query.format)
+ kfree(pq->query.format);
+ kfree(pq);
+}
+
+/* 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)
+{
+ 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;
+ vpr_info_pq(pq, "already pending, updated it");
+ return 0;
+ }
+ }
+ if (!flags) {
+ vpr_info_pq(pq, "pending query has 0 flags, discarding");
+ return 0;
+ }
+ vpr_info_dq(query, "add to pending");
+
+ 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)
+ if (!(pq->query.module = kstrdup(query->module, GFP_KERNEL)))
+ return -ENOMEM;
+ if (query->function)
+ if (!(pq->query.function = kstrdup(query->function, GFP_KERNEL)))
+ return -ENOMEM;
+ if (query->filename)
+ if (!(pq->query.filename = kstrdup(query->filename, GFP_KERNEL)))
+ return -ENOMEM;
+ if (query->format)
+ if (!(pq->query.format = kstrdup(query->format, GFP_KERNEL)))
+ return -ENOMEM;
+
+ pq->flags = flags;
+ pq->mask = mask;
+
+ list_add_tail(&pq->link, &pending_queries);
+
+ if (verbose)
+ pr_info("query saved as pending, in %ld bytes\n",
+ sizeof(struct pending_query));
+ return 0;
+}
+
static int ddebug_exec_query(char *query_string)
{
unsigned int flags = 0, mask = 0;
@@ -442,6 +559,7 @@ static int ddebug_exec_query(char *query_string)
#define MAXWORDS 9
int nwords;
char *words[MAXWORDS];
+ int nfound, rc = 0;
nwords = ddebug_tokenize(query_string, words, MAXWORDS);
if (nwords <= 0)
@@ -452,8 +570,13 @@ static int ddebug_exec_query(char *query_string)
return -EINVAL;
/* actually go and implement the change */
- ddebug_change(&query, flags, mask);
- return 0;
+ nfound = ddebug_change(&query, flags, mask);
+ vpr_info_dq((&query), (nfound) ? "applied" : "no-match");
+
+ if (flags & _DPRINTK_FLAGS_APPEND)
+ rc = ddebug_save_pending(&query, flags, mask);
+
+ return rc;
}
/* handle multiple queries, continue on error, return last error */
@@ -811,6 +934,19 @@ 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;
+
+ list_for_each_entry_safe(pq, pqnext, &pending_queries, link) {
+ nfound = ddebug_change(&pq->query, pq->flags, pq->mask);
+ vpr_info_pq(pq, (nfound) ?
+ "applied pending" : "no-match on pending");
+ }
+}
+
/*
* Allocate a new ddebug_table for the given module
* and add it to the global list.
@@ -835,6 +971,7 @@ int ddebug_add_module(struct _ddebug *tab, unsigned int n,
mutex_lock(&ddebug_lock);
list_add_tail(&dt->link, &ddebug_tables);
+ apply_pending_queries(dt);
mutex_unlock(&ddebug_lock);
if (verbose)
@@ -876,6 +1013,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,
@@ -883,6 +1022,11 @@ static void ddebug_remove_all_tables(void)
link);
ddebug_table_free(dt);
}
+ list_for_each_entry_safe(pq, pqnext, &pending_queries, link) {
+ vpr_info_pq(pq, "delete pending");
+ list_del_init(&pq->link);
+ pqfree(pq);
+ }
mutex_unlock(&ddebug_lock);
}
--
1.7.4.4
jim.c...@gmail.com
Rework description of flags-spec into 3 parts; flags-filter, flags-change,
flags-values. Add section on pending-queries. Explain flags filtering
in detail. Reflow some paragraphs.
Add example of a command-file which shows commenting,
multiple queries per line, and describes constraints.
Signed-off-by: Jim Cromie <jim.c...@gmail.com>
---
--
jim.c...@gmail.com
Add pending file so user can see queries that are pending. Output
format imitates that used to submit commands/queries. This simplifies
cut-paste-edit to delete pending queries when they're no longer wanted.
Signed-off-by: Jim Cromie <jim.c...@gmail.com>
---
lib/dynamic_debug.c | 191 ++++++++++++++++++++++++++++++++++++++++++++++++++-
1 files changed, 188 insertions(+), 3 deletions(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index d7b71a6..80b5322 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -64,6 +64,10 @@ struct ddebug_iter {
unsigned int idx;
};
+struct pending_iter {
+ struct pending_query *elem;
+};
+
static DEFINE_MUTEX(ddebug_lock);
static LIST_HEAD(ddebug_tables);
static int verbose = 0;
@@ -842,7 +846,8 @@ static void *ddebug_proc_start(struct seq_file *m, loff_t *pos)
int n = *pos;
if (verbose >= VERBOSE_PROC)
- pr_info("called m=%p *pos=%lld\n", m, (unsigned long long)*pos);
+ pr_info("called m=%p *pos=%lld\n",
+ m, (unsigned long long)*pos);
mutex_lock(&ddebug_lock);
@@ -963,7 +968,180 @@ static const struct file_operations ddebug_proc_fops = {
.write = ddebug_proc_write
};
-/* apply matching queries in pending-queries list */
+/*
+ * Set the iterator to point to the first pending_query object
+ * and return a pointer to that first object. Returns
+ * NULL if there are no pending_querys at all.
+ */
+static struct pending_query *pending_iter_first(struct pending_iter *iter)
+{
+ if (list_empty(&pending_queries)) {
+ iter->elem = NULL;
+ return NULL;
+ }
+ iter->elem = list_entry(pending_queries.next,
+ struct pending_query, link);
+ return iter->elem;
+}
+
+/*
+ * Advance the iterator to point to the next pending_query
+ * object from the one the iterator currently points at,
+ * and returns a pointer to the new pending_query. Returns
+ * NULL if the iterator has seen all the pending_querys.
+ */
+static struct pending_query *pending_iter_next(struct pending_iter *iter)
+{
+ if (iter->elem == NULL)
+ return NULL;
+ if (list_is_last(&iter->elem->link, &pending_queries)) {
+ iter->elem = NULL;
+ return NULL;
+ }
+ iter->elem = list_entry(iter->elem->link.next,
+ struct pending_query, link);
+ return iter->elem;
+}
+
+/*
+ * Seq_ops start method. Called at the start of every read() call on
+ * pending file from userspace. Takes the ddebug_lock and seeks the
+ * seq_file's iterator to the given position.
+ */
+static void *pending_proc_start(struct seq_file *m, loff_t *pos)
+{
+ struct pending_iter *iter = m->private;
+ struct pending_query *pq;
+ int n = *pos;
+
+ if (verbose >= VERBOSE_PROC)
+ pr_info("called m=%p *pos=%lld\n",
+ m, (unsigned long long)*pos);
+
+ mutex_lock(&ddebug_lock);
+
+ if (!n)
+ return SEQ_START_TOKEN;
+ if (n < 0)
+ return NULL;
+ pq = pending_iter_first(iter);
+ while (pq != NULL && --n > 0)
+ pq = pending_iter_next(iter);
+ return pq;
+}
+
+/*
+ * Seq_ops next method. Called several times within a read()
+ * call from userspace, with pending_lock held. Walks to the
+ * next pending_query object with a special case for the header line.
+ */
+static void *pending_proc_next(struct seq_file *m, void *p, loff_t *pos)
+{
+ struct pending_iter *iter = m->private;
+ struct pending_query *pq;
+
+ if (verbose >= VERBOSE_PROC_SHOW)
+ pr_info("called m=%p p=%p *pos=%lld\n",
+ m, p, (unsigned long long)*pos);
+
+ if (p == SEQ_START_TOKEN)
+ pq = pending_iter_first(iter);
+ else
+ pq = pending_iter_next(iter);
+ ++*pos;
+ return pq;
+}
+
+/*
+ * Seq_ops show method. Called several times within a read()
+ * call from userspace, with pending_lock held. Formats the
+ * current pending_query as a single human-readable line, with a
+ * special case for the header line.
+ */
+static int pending_proc_show(struct seq_file *m, void *p)
+{
+ struct pending_iter *iter = m->private;
+ struct pending_query *pq = iter->elem;
+ struct ddebug_query *q = &pq->query;
+ char flagsbuf[10];
+
+ if (verbose >= VERBOSE_PROC_SHOW)
+ pr_info("called m=%p p=%p\n", m, p);
+
+ if (p == SEQ_START_TOKEN) {
+ seq_puts(m, "# func file module format line flags mask\n");
+ return 0;
+ }
+
+ seq_printf(m, "%s%s%s%s%s%s%s%s line %d-%d %s\n",
+ q->module ? "module " : "",
+ q->module ? q->module : "",
+ q->function ? " func " : "",
+ q->function ? q->function : "",
+ q->filename ? " file " : "",
+ q->filename ? q->filename : "",
+ q->format ? " format " : "",
+ q->format ? q->format : "",
+ q->first_lineno, q->last_lineno,
+ ddebug_describe_flags(pq->flags, flagsbuf, sizeof(flagsbuf)));
+
+ return 0;
+}
+
+/*
+ * Seq_ops stop method. Called at the end of each read()
+ * call from userspace. Drops pending_lock.
+ */
+static void pending_proc_stop(struct seq_file *m, void *p)
+{
+ if (verbose >= VERBOSE_PROC)
+ pr_info("called m=%p p=%p\n", m, p);
+ mutex_unlock(&ddebug_lock);
+}
+
+static const struct seq_operations pending_proc_seqops = {
+ .start = pending_proc_start,
+ .next = pending_proc_next,
+ .show = pending_proc_show,
+ .stop = pending_proc_stop
+};
+
+/*
+ * File_ops->open method for <debugfs>/dynamic_debug/control. Does the seq_file
+ * setup dance, and also creates an iterator to walk the pending_querys.
+ * Note that we create a seq_file always, even for O_WRONLY files
+ * where it's not needed, as doing so simplifies the ->release method.
+ */
+static int pending_proc_open(struct inode *inode, struct file *file)
+{
+ struct pending_iter *iter;
+ int err;
+
+ if (verbose)
+ pr_info("called\n");
+
+ iter = kzalloc(sizeof(*iter), GFP_KERNEL);
+ if (iter == NULL)
+ return -ENOMEM;
+
+ err = seq_open(file, &pending_proc_seqops);
+ if (err) {
+ kfree(iter);
+ return err;
+ }
+ ((struct seq_file *) file->private_data)->private = iter;
+ return 0;
+}
+
+static const struct file_operations pending_proc_fops = {
+ .owner = THIS_MODULE,
+ .open = pending_proc_open,
+ .read = seq_read,
+ .llseek = seq_lseek,
+ .release = seq_release_private,
+};
+
+/* 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;
@@ -1063,7 +1241,7 @@ static __initdata int ddebug_init_success;
static int __init dynamic_debug_init_debugfs(void)
{
- struct dentry *dir, *file;
+ struct dentry *dir, *file, *file2;
if (!ddebug_init_success)
return -ENODEV;
@@ -1077,6 +1255,13 @@ static int __init dynamic_debug_init_debugfs(void)
debugfs_remove(dir);
return -ENOMEM;
}
+ file2 = debugfs_create_file("pending", 0444, dir, NULL,
+ &pending_proc_fops);
+ if (!file2) {
+ debugfs_remove(file);
+ debugfs_remove(dir);
+ return -ENOMEM;
+ }
return 0;
}
--
1.7.4.4
jim.c...@gmail.com
Change definition of flags spec to [pmflta_]*[-+=][pmflta_]*
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 also allows rules like "a-a" to select pending queries and
remove them from the list.
Also allow 0 flags byte, so that active rules can be completely
reset by writing "p=" or "p=_".
Patch factors flag-scanning into ddebug_parse_flag_settings(),
and uses it for both filter-flags and new flags, and adds pr_err()
where useful.
Signed-off-by: Jim Cromie <jim.c...@gmail.com>
---
lib/dynamic_debug.c | 85 +++++++++++++++++++++++++++++++--------------------
1 files changed, 52 insertions(+), 33 deletions(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 29dbf69..3c9244d 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -191,7 +191,8 @@ static bool 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;
@@ -213,6 +214,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;
@@ -406,29 +410,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--) {
@@ -437,13 +421,46 @@ 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
+ * [pmflta_]*[-+=][pmflta_]+. 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;
- if (verbose)
- pr_info("flags=0x%x\n", flags);
+ } 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;
+ }
/* calculate final *flagsp, *maskp according to mask and op */
switch (op) {
@@ -461,7 +478,9 @@ static int ddebug_parse_flags(const char *str, unsigned int *flagsp,
break;
}
if (verbose)
- pr_info("*flagsp=0x%x *maskp=0x%x\n", *flagsp, *maskp);
+ pr_info("filter=0x%x op='%c' flags=0x%x *flagsp=0x%x *maskp=0x%x\n",
+ filter, op, flags, *flagsp, *maskp);
+
return 0;
}
@@ -563,7 +582,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;
@@ -575,14 +594,14 @@ 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 */
- nfound = ddebug_change(&query, flags, mask);
+ nfound = ddebug_change(&query, flags, mask, filter);
vpr_info_dq((&query), (nfound) ? "applied" : "no-match");
- if (flags & _DPRINTK_FLAGS_APPEND)
+ if (flags & _DPRINTK_FLAGS_APPEND || filter & _DPRINTK_FLAGS_APPEND)
rc = ddebug_save_pending(&query, flags, mask);
return rc;
@@ -950,7 +969,7 @@ static void apply_pending_queries(struct ddebug_table *dt)
int nfound;
list_for_each_entry_safe(pq, pqnext, &pending_queries, link) {
- nfound = ddebug_change(&pq->query, pq->flags, pq->mask);
+ nfound = ddebug_change(&pq->query, pq->flags, pq->mask, 0);
vpr_info_pq(pq, (nfound) ?
"applied pending" : "no-match on pending");
}
--
1.7.4.4
jim.c...@gmail.com
This makes no behavioral change, its just a code-move/refactor
to encapsulate matching behavior.
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.c...@gmail.com>
---
lib/dynamic_debug.c | 54 +++++++++++++++++++++++++++++---------------------
1 files changed, 31 insertions(+), 23 deletions(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 2f39b2f..329e18d 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -125,6 +125,36 @@ do { \
q->first_lineno, q->last_lineno); \
} while (0)
+static bool 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)) &&
+ strcmp(query->filename, trim_prefix(dp->filename)))
+ return false;
+
+ /* match against the function */
+ if (query->function != NULL &&
+ strcmp(query->function, dp->function))
+ return false;
+
+ /* match against the format */
+ if (query->format != NULL &&
+ strstr(dp->format, query->format) == NULL)
+ return false;
+
+ /* match against the line number range */
+ if (query->first_lineno && dp->lineno < query->first_lineno)
+ return false;
+
+ if (query->last_lineno && dp->lineno > query->last_lineno)
+ return false;
+
+ return true;
+}
+
/*
* Search the tables for _ddebug's which match the given
* `query' and apply the `flags' and `mask' to them. Tells
@@ -151,29 +181,7 @@ static void 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)) &&
- strcmp(query->filename, trim_prefix(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.4
jim.c...@gmail.com
Reduce != NULL relational tests to their 0/1 equivalents.
Done separately to preserve code-move character of previous patch.
No generated code difference.
Signed-off-by: Jim Cromie <jim.c...@gmail.com>
---
lib/dynamic_debug.c | 8 +++-----
1 files changed, 3 insertions(+), 5 deletions(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 329e18d..4c8e178 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -129,20 +129,18 @@ static bool 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)) &&
strcmp(query->filename, trim_prefix(dp->filename)))
return false;
/* match against the function */
- if (query->function != NULL &&
- strcmp(query->function, dp->function))
+ if (query->function && strcmp(query->function, dp->function))
return false;
/* match against the format */
- if (query->format != NULL &&
- strstr(dp->format, query->format) == NULL)
+ if (query->format && !strstr(dp->format, query->format))
return false;
/* match against the line number range */
--
1.7.4.4
jim.c...@gmail.com
trim_prefix(path) skips past the absolute source path root, and returns
the pointer to the relative path from there. Use it to shorten displayed
path in ddebug_change() and in ddebug_proc_show(). Function insures
common prefix, so out-of-tree module paths are preserved as absolute paths.
For example:
kernel/freezer.c:128 [freezer]cancel_freezing - " clean up: %s\012"
Use trim_prefix() in ddebug_change to allow use of a relative pathname
in a file match-spec, in addition to basename and full-path.
Signed-off-by: Jim Cromie <jim.c...@gmail.com>
---
lib/dynamic_debug.c | 22 +++++++++++++++++-----
1 files changed, 17 insertions(+), 5 deletions(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index e7090f2..2d26c10 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -69,6 +69,17 @@ static inline const char *basename(const char *path)
return tail ? tail+1 : path;
}
+/* Return the path relative to source root */
+static inline const char *trim_prefix(const char *path)
+{
+ int skip = strlen(__FILE__) - strlen("lib/dynamic_debug.c");
+
+ if (strncmp(path, __FILE__, skip))
+ skip = 0; /* prefix mismatch, don't skip */
+
+ return path + skip;
+}
+
static struct { unsigned flag:8; char opt_char; } opt_array[] = {
{ _DPRINTK_FLAGS_PRINT, 'p' },
{ _DPRINTK_FLAGS_INCL_MODNAME, 'm' },
@@ -125,7 +136,8 @@ static void ddebug_change(const struct ddebug_query *query,
/* match against the source filename */
if (query->filename != NULL &&
strcmp(query->filename, dp->filename) &&
- strcmp(query->filename, basename(dp->filename)))
+ strcmp(query->filename, basename(dp->filename)) &&
+ strcmp(query->filename, trim_prefix(dp->filename)))
continue;
/* match against the function */
@@ -154,7 +166,7 @@ static void ddebug_change(const struct ddebug_query *query,
dp->flags = newflags;
if (verbose)
pr_info("changed %s:%d [%s]%s %s\n",
- dp->filename, dp->lineno,
+ trim_prefix(dp->filename), dp->lineno,
dt->mod_name, dp->function,
ddebug_describe_flags(dp, flagbuf,
sizeof(flagbuf)));
@@ -682,9 +694,9 @@ static int ddebug_proc_show(struct seq_file *m, void *p)
}
seq_printf(m, "%s:%u [%s]%s %s \"",
- dp->filename, dp->lineno,
- iter->table->mod_name, dp->function,
- ddebug_describe_flags(dp, flagsbuf, sizeof(flagsbuf)));
+ trim_prefix(dp->filename), dp->lineno,
+ iter->table->mod_name, dp->function,
+ ddebug_describe_flags(dp, flagsbuf, sizeof(flagsbuf)));
seq_escape(m, dp->format, "\t\r\n\"");
seq_puts(m, "\"\n");
--
1.7.4.4
--
jim.c...@gmail.com
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
No MODULE_PARM_DESC added, as modinfo doesnt report it for built-ins.
Signed-off-by: Jim Cromie <jim.c...@gmail.com>
---
lib/dynamic_debug.c | 10 ++++++----
1 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 74b395b..b88f918 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -61,6 +61,8 @@ struct ddebug_iter {
static DEFINE_MUTEX(ddebug_lock);
static LIST_HEAD(ddebug_tables);
static int verbose = 0;
+#define VERBOSE_PROC 10 /* enable per-page msgs on control file reads */
+#define VERBOSE_PROC_SHOW 11 /* enable per-line msgs on control file reads */
module_param(verbose, int, 0644);
/* Return the last part of a pathname */
@@ -636,7 +638,7 @@ static void *ddebug_proc_start(struct seq_file *m, loff_t *pos)
struct _ddebug *dp;
int n = *pos;
- if (verbose)
+ if (verbose >= VERBOSE_PROC)
pr_info("called m=%p *pos=%lld\n", m, (unsigned long long)*pos);
mutex_lock(&ddebug_lock);
@@ -661,7 +663,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 >= VERBOSE_PROC_SHOW)
pr_info("called m=%p p=%p *pos=%lld\n",
m, p, (unsigned long long)*pos);
@@ -685,7 +687,7 @@ static int ddebug_proc_show(struct seq_file *m, void *p)
struct _ddebug *dp = p;
char flagsbuf[8];
- if (verbose)
+ if (verbose >= VERBOSE_PROC_SHOW)
pr_info("called m=%p p=%p\n", m, p);
if (p == SEQ_START_TOKEN) {
@@ -710,7 +712,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 >= VERBOSE_PROC)
pr_info("called m=%p p=%p\n", m, p);
mutex_unlock(&ddebug_lock);
}
--
1.7.4.4
jim.c...@gmail.com
if CONFIG_DYNAMIC_DEBUG is defined, honor it over DEBUG, so that
pr_debug()s are controllable, instead of always-on. When DEBUG is
also defined, change _DPRINTK_FLAGS_DEFAULT to enable printing by
default.
Also adding _DPRINTK_FLAGS_INCL_MODNAME would be nice, but there are
numerous cases of pr_debug(NAME ": ...), which would result in double
printing of module-name. So defer this until things settle.
CC: Joe Perches <j...@perches.com>
Signed-off-by: Jim Cromie <jim.c...@gmail.com>
---
include/linux/device.h | 8 ++++----
include/linux/dynamic_debug.h | 4 ++++
include/linux/netdevice.h | 8 ++++----
include/linux/printk.h | 8 ++++----
4 files changed, 16 insertions(+), 12 deletions(-)
diff --git a/include/linux/device.h b/include/linux/device.h
index 4639419..2e16503 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -857,14 +857,14 @@ static inline int _dev_info(const struct device *dev, const char *fmt, ...)
#define dev_info(dev, fmt, arg...) _dev_info(dev, fmt, ##arg)
-#if defined(DEBUG)
-#define dev_dbg(dev, format, arg...) \
- dev_printk(KERN_DEBUG, dev, format, ##arg)
-#elif defined(CONFIG_DYNAMIC_DEBUG)
+#if defined(CONFIG_DYNAMIC_DEBUG)
#define dev_dbg(dev, format, ...) \
do { \
dynamic_dev_dbg(dev, format, ##__VA_ARGS__); \
} while (0)
+#elif defined(DEBUG)
+#define dev_dbg(dev, format, arg...) \
+ dev_printk(KERN_DEBUG, dev, format, ##arg)
#else
#define dev_dbg(dev, format, arg...) \
({ \
diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index ed7df3d..12ef233 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -26,7 +26,11 @@ struct _ddebug {
#define _DPRINTK_FLAGS_INCL_FUNCNAME (1<<2)
#define _DPRINTK_FLAGS_INCL_LINENO (1<<3)
#define _DPRINTK_FLAGS_INCL_TID (1<<4)
+#if defined DEBUG
+#define _DPRINTK_FLAGS_DEFAULT _DPRINTK_FLAGS_PRINT
+#else
#define _DPRINTK_FLAGS_DEFAULT 0
+#endif
unsigned int flags:8;
} __attribute__((aligned(8)));
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 2797260..8c08fe0 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -2641,14 +2641,14 @@ extern int netdev_info(const struct net_device *dev, const char *format, ...)
#define MODULE_ALIAS_NETDEV(device) \
MODULE_ALIAS("netdev-" device)
-#if defined(DEBUG)
-#define netdev_dbg(__dev, format, args...) \
- netdev_printk(KERN_DEBUG, __dev, format, ##args)
-#elif defined(CONFIG_DYNAMIC_DEBUG)
+#if defined(CONFIG_DYNAMIC_DEBUG)
#define netdev_dbg(__dev, format, args...) \
do { \
dynamic_netdev_dbg(__dev, format, ##args); \
} while (0)
+#elif defined(DEBUG)
+#define netdev_dbg(__dev, format, args...) \
+ netdev_printk(KERN_DEBUG, __dev, format, ##args)
#else
#define netdev_dbg(__dev, format, args...) \
({ \
diff --git a/include/linux/printk.h b/include/linux/printk.h
index 0101d55..1224b1d 100644
--- a/include/linux/printk.h
+++ b/include/linux/printk.h
@@ -180,13 +180,13 @@ extern void dump_stack(void) __cold;
#endif
/* If you are writing a driver, please use dev_dbg instead */
-#if defined(DEBUG)
-#define pr_debug(fmt, ...) \
- printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
-#elif defined(CONFIG_DYNAMIC_DEBUG)
+#if defined(CONFIG_DYNAMIC_DEBUG)
/* dynamic_pr_debug() uses pr_fmt() internally so we don't need it here */
#define pr_debug(fmt, ...) \
dynamic_pr_debug(fmt, ##__VA_ARGS__)
+#elif defined(DEBUG)
+#define pr_debug(fmt, ...) \
+ printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
#else
#define pr_debug(fmt, ...) \
no_printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
--
1.7.4.4
jim.c...@gmail.com
Process multiple commands per line, separated by ';' or '\n'.
All commands are processed, independent of errors, allowing individual
commands to fail, for example when a module is not installed. Errors
are counted, and last error code is returned.
With this, extensive command sets can be given on the boot-line.
Splitting on '\n' allows "cat cmd-file > /dbg/dynamic_debug/control"
where cmd-file contains multiple queries, one per line. Empty commands
are skipped, allowing ";\n\n" to not count as errors.
Splitting on '\n' prevents its use in a format-spec, but thats not very
useful anyway, searching for meaningful and selective substrings is typical.
Also allow comment lines, starting with '#', with optional leading whitespace.
Trailing comments are allowed, if preceded by ';' which terminates
the command on the line. For example:
root@voyage:~# cat debugfs-file
# blank lines ok, comments too
module foo +p
# multiple cmds on line, with ; to separate them
module bar +p ; module buz +p ; # trailing comments need cmd terminator too
root@voyage:~# cat debugfs-file > /dbg/dynamic_debug/control
Signed-off-by: Jim Cromie <jim.c...@gmail.com>
---
lib/dynamic_debug.c | 35 +++++++++++++++++++++++++++++++++--
1 files changed, 33 insertions(+), 2 deletions(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index b88f918..4bf27f3 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -444,6 +444,35 @@ 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) {
+ split = strpbrk(query, ";\n");
+ if (split)
+ *split++ = '\0';
+
+ query = skip_spaces(query);
+ if (!*query || *query == '#')
+ continue;
+ if (verbose)
+ pr_info("query %d: \"%s\"\n", i, query);
+
+ rc = ddebug_exec_query(query);
+ if (rc) {
+ errs++;
+ exitcode = rc;
+ }
+ i++;
+ }
+ pr_info("processed %d queries, with %d errs\n", i, errs);
+
+ return exitcode;
+}
+
#define PREFIX_SIZE 64
#define LEFT(wrote) ((PREFIX_SIZE - wrote) > 0) ? (PREFIX_SIZE - wrote) : 0
@@ -578,7 +607,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;
@@ -883,7 +912,9 @@ 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);
+ pr_info("ddebug initializing with string \"%s\"",
+ 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.4
jim.c...@gmail.com
Current write buffer is 256 bytes, on stack. Allocate it off heap,
and enlarge it to 4096 bytes, big enough for ~100 queries (at 40 bytes
each), and error out if not. This should be enough for most uses, and
others can be split into 2 writes.
This makes it play nicely with:
$> cat debug-queries-file > /dbg/dynamic_debug/control
Signed-off-by: Jim Cromie <jim.c...@gmail.com>
---
lib/dynamic_debug.c | 16 ++++++++++++----
1 files changed, 12 insertions(+), 4 deletions(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 4bf27f3..2b3393f 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -590,24 +590,32 @@ __setup("ddebug_query=", ddebug_setup_query);
* File_ops->write method for <debugfs>/dynamic_debug/conrol. Gathers the
* command text from userspace, parses and executes it.
*/
+#define USER_BUF_PAGE 4095
static ssize_t ddebug_proc_write(struct file *file, const char __user *ubuf,
size_t len, loff_t *offp)
{
- char tmpbuf[256];
+ char *tmpbuf;
int ret;
if (len == 0)
return 0;
- /* we don't check *offp -- multiple writes() are allowed */
- if (len > sizeof(tmpbuf)-1)
+ if (len > USER_BUF_PAGE - 1) {
+ pr_warn("expected <%d bytes into control \n", USER_BUF_PAGE);
return -E2BIG;
- if (copy_from_user(tmpbuf, ubuf, len))
+ }
+ tmpbuf = kmalloc(len + 1, GFP_KERNEL);
+ if (!tmpbuf)
+ return -ENOMEM;
+ if (copy_from_user(tmpbuf, ubuf, len)) {
+ kfree(tmpbuf);
return -EFAULT;
+ }
tmpbuf[len] = '\0';
if (verbose)
pr_info("read %d bytes from userspace\n", (int)len);
ret = ddebug_exec_queries(tmpbuf);
+ kfree(tmpbuf);
if (ret)
return ret;
--
1.7.4.4
jim.c...@gmail.com
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.c...@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 a52b78a..e7090f2 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -334,8 +334,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.4
jim.c...@gmail.com
replace strcpy with strlcpy, and add define for the size constant.
Signed-off-by: Jim Cromie <jim.c...@gmail.com>
---
lib/dynamic_debug.c | 8 +++++---
1 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index f2fb0c0..6372b18 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -516,14 +516,16 @@ EXPORT_SYMBOL(__dynamic_netdev_dbg);
#endif
-static __initdata char ddebug_setup_string[1024];
+#define BOOT_QUERY_SZ 1024
+static __initdata char ddebug_setup_string[BOOT_QUERY_SZ];
+
static __init int ddebug_setup_query(char *str)
{
- if (strlen(str) >= 1024) {
+ if (strlen(str) >= BOOT_QUERY_SZ) {
pr_warn("ddebug boot param string too large\n");
return 0;
}
- strcpy(ddebug_setup_string, str);
+ strlcpy(ddebug_setup_string, str, BOOT_QUERY_SZ);
return 1;
}
--
1.7.4.4
jim.c...@gmail.com
Currently any enabled dynamic-debug flag on a pr_debug callsite will enable
printing, even if _DPRINTK_FLAGS_PRINT is off. Checking print flag directly
allows "-p" to disable callsites without fussing with other flags, so
the following disables everything:
echo -p > $DBGFS/dynamic_debug/control
Signed-off-by: Jim Cromie <jim.c...@gmail.com>
---
include/linux/dynamic_debug.h | 8 +++-----
lib/dynamic_debug.c | 4 ----
2 files changed, 3 insertions(+), 9 deletions(-)
diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index 8aec680..ed7df3d 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -28,7 +28,6 @@ struct _ddebug {
#define _DPRINTK_FLAGS_INCL_TID (1<<4)
#define _DPRINTK_FLAGS_DEFAULT 0
unsigned int flags:8;
- char enabled;
} __attribute__((aligned(8)));
@@ -63,13 +62,12 @@ extern int __dynamic_netdev_dbg(struct _ddebug *descriptor,
.format = (fmt), \
.lineno = __LINE__, \
.flags = _DPRINTK_FLAGS_DEFAULT, \
- .enabled = false, \
}
#define dynamic_pr_debug(fmt, ...) \
do { \
DECLARE_DYNAMIC_DEBUG_METADATA(descriptor, fmt); \
- if (unlikely(descriptor.enabled)) \
+ if (unlikely(descriptor.flags & _DPRINTK_FLAGS_PRINT)) \
__dynamic_pr_debug(&descriptor, pr_fmt(fmt), \
##__VA_ARGS__); \
} while (0)
@@ -77,7 +75,7 @@ do { \
#define dynamic_dev_dbg(dev, fmt, ...) \
do { \
DECLARE_DYNAMIC_DEBUG_METADATA(descriptor, fmt); \
- if (unlikely(descriptor.enabled)) \
+ if (unlikely(descriptor.flags & _DPRINTK_FLAGS_PRINT)) \
__dynamic_dev_dbg(&descriptor, dev, fmt, \
##__VA_ARGS__); \
} while (0)
@@ -85,7 +83,7 @@ do { \
#define dynamic_netdev_dbg(dev, fmt, ...) \
do { \
DECLARE_DYNAMIC_DEBUG_METADATA(descriptor, fmt); \
- if (unlikely(descriptor.enabled)) \
+ if (unlikely(descriptor.flags & _DPRINTK_FLAGS_PRINT)) \
__dynamic_netdev_dbg(&descriptor, dev, fmt, \
##__VA_ARGS__); \
} while (0)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 4138574..f2fb0c0 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -152,10 +152,6 @@ static void ddebug_change(const struct ddebug_query *query,
if (newflags == dp->flags)
continue;
dp->flags = newflags;
- if (newflags)
- dp->enabled = 1;
- else
- dp->enabled = 0;
if (verbose)
pr_info("changed %s:%d [%s]%s %s\n",
dp->filename, dp->lineno,
Joe Perches
> diff --git a/include/linux/printk.h b/include/linux/printk.h
[]
> @@ -255,7 +255,7 @@ extern void dump_stack(void) __cold;
> #define pr_info_once(fmt, ...) \
> printk_once(KERN_INFO pr_fmt_info(fmt), ##__VA_ARGS__)
> #define pr_cont_once(fmt, ...) \
> - printk_once(KERN_CONT pr_fmt(fmt), ##__VA_ARGS__)
> + printk_once(KERN_CONT fmt, ##__VA_ARGS__)
As this is unused and defective, I think it'd be better
to remove it instead.
Joe Perches
Just a note on the changelog.
The original code already uses pr_err.
> diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
[]
> @@ -334,8 +334,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]);
Bart Van Assche
> I think all of the '#define pr_fmt(fmt) KBUILD_MODNAME ": "'
> are effectivly useless and I will eventually delete them.
>
> The printk subsystem should look up the module name and
> prefix them to the printk akin to how the dynamic_debug
> system does. �Except the module name should be a singleton.
Shouldn't pr_*() equivalents be introduced for sdev_printk(),
scmd_printk(), starget_printk() and shost_printk() before elimination
of pr_fmt() can start ?
Bart.
Joe Perches
> On Wed, Sep 28, 2011 at 6:51 AM, Joe Perches <j...@perches.com> wrote:
> > I think all of the '#define pr_fmt(fmt) KBUILD_MODNAME ": "'
> > are effectivly useless and I will eventually delete them.
> >
> > The printk subsystem should look up the module name and
> > prefix them to the printk akin to how the dynamic_debug
> > system does. Except the module name should be a singleton.
>
> Shouldn't pr_*() equivalents be introduced for sdev_printk(),
> scmd_printk(), starget_printk() and shost_printk() before elimination
> of pr_fmt() can start ?
I don't think that's necessary.
Is there a reason you might think there is?
I don't think there's anything wrong with
<subsystem>_printk uses. Those subsystems
don't have to use pr_fmt and can prefix
whatever is appropriate.
The scsi logging substem uses sdev_printk,
scmd_printk, starget_printk, and shost _printk.
All of these are mapped to dev_printk and so
don't use pr_fmt.
I did send a patch a few months ago
https://lkml.org/lkml/2011/7/9/14
to reduce text use by some of those.
Jonathan Corbet
context of the doc file changes.
> Dynamic debug has even more useful features:
>
> - * Simple query language allows turning on and off debugging statements by
> - matching any combination of:
> + * Simple query language allows turning on and off debugging statements
> + by matching any combination of 0 or 1 of:
>
> - source filename
> - function name
> - line number (including ranges of line numbers)
> - module name
> - format string
> + - current debugging flags
This is totally confusing; you've replaced all the things that can be
matched with a single-entry itemized list of "current debugging flags"?
> + * Provides a debugfs control file: <debugfs>/dynamic_debug/control
> + which can be read to display the complete list of known debug
> + statements, to help guide you.
>
> Controlling dynamic debug Behaviour
> ===================================
>
> -The behaviour of pr_debug()/dev_dbg()s are controlled via writing to a
> -control file in the 'debugfs' filesystem. Thus, you must first mount the debugfs
> -filesystem, in order to make use of this feature. Subsequently, we refer to the
> -control file as: <debugfs>/dynamic_debug/control. For example, if you want to
> -enable printing from source file 'svcsock.c', line 1603 you simply do:
> +The behaviour of pr_debug()/dev_dbg()/net_dbg()s are controlled via
> +writing to a control file in the 'debugfs' filesystem. Thus, you must
> +first mount the debugfs filesystem, in order to make use of this
> +feature. Subsequently, we refer to mount point as $DBGFS, and the
> +control file as $CONTROL. So if you want to enable printing from
Why the $CONTROL stuff? I can see parameterizing teh debugfs location,
even though that discussion seems to have settled down. But, if you miss
the line at the top of this section, you'll never know where $CONTROL is.
It doesn't change, why obfuscate it?
> +source file 'svcsock.c', line 1603 you simply do:
>
> -nullarbor:~ # echo 'file svcsock.c line 1603 +p' >
> - <debugfs>/dynamic_debug/control
> +nullarbor:~ # echo file svcsock.c line 1603 +p > $CONTROL
>
> If you make a mistake with the syntax, the write will fail thus:
>
> -nullarbor:~ # echo 'file svcsock.c wtf 1 +p' >
> - <debugfs>/dynamic_debug/control
> +nullarbor:~ # 'echo file svcsock.c wtf 1 +p' > $CONTROL
> -bash: echo: write error: Invalid argument
I don't think you tested that last one :) The bash error output will be
other than shown here.
> + # comments and blank lines ok, but cannot contain semicolon
> + # multiple cmds per line, 1st must be terminated by semicolon
> + func foo p=_ ; func buzz p=_
> + func bar p=_ ; func bum p=_
> + func yak p=_ ; # trailing comment, requires semicolon to terminate cmd
That seems like a slightly strange set of rules. By Least Surprise, I
would expect # to start a comment anywhere and to go the end of the line,
semicolons or not.
> +voyage:~# cat dyndbg-cmdfile > $CONTROL
> +dynamic_debug:ddebug_exec_queries: processed 7 queries, with 0 errs
Who produces the "processed 7 queries" message? It's not that "cat"
command...
> +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' > $CONTROL; \
> +> echo 'file svcsock.c line 1563 +p' > $CONTROL
Is there some reason for the backslash-escaped newline here?
> A match specification comprises a keyword, which controls the attribute
> of the callsite to be compared, and a value to compare against. Possible
> @@ -125,12 +142,13 @@ match-spec ::= 'func' string |
> 'module' string |
> 'format' string |
> 'line' line-range
> +// Note: no wildcards, regexs are accepted
Why the // notation? And what does that line mean? There's a full regular
expression interpreter in this code now? I note that there are no
regex-based examples in this file.
> +flags specification
> +===================
>
> -=
> - set the flags to the given flags
> +The flags specification matches the regexp: ^[flmpta_]*[-+=][flmpta_]*$
> +and has 3 parts:
>
> -The flags are:
> +flags filter (optional):
> + The filter precedes the operation [-+=], and constrains matching
> + to thoses callsite with given flags set. This allows altering
> + flags on callsites with filtered flag values
>
> -f
> - Include the function name in the printed message
> -l
> - Include line number in the printed message
> -m
> - Include module name in the printed message
> -p
> - Causes a printk() message to be emitted to dmesg
> -t
> - Include thread ID in messages not generated from interrupt context
> + pm+t # add t to enabled sites which have m
How about "add the thread ID to sites that already have module-name
printing enabled? Same with the rest.
> +Pending queries
> +===============
> +
> +Queries submitted with 'a' are applied to current callsites as above,
> +but are also added to a pending list. When a module is loaded later,
> +pending queries are applied to the module in the order given.
> +
> +This is done before the module_init() routine is run, so pr_debug()s
> +can be active during initialization. To better support module
> +debugging, pending queries remain on the list through modprobe-rmmod
> +cycles.
So this functionality seems to be the whole point of much of this patch
set. That's a lot of stuff to turn on printks during module_init(). I
can't help but wonder: wouldn't it be easier and better to just recognize
the ddebug_query= parameter at module load time? Then you could drop this
whole pending queries mechanism, the expanded syntax, new control file, and
so on...? What am I missing?
> +You can review currently pending queries by catting or grepping
> +$DEBUFS/dynamic_debug/pending. To simplify removal via
> +cut-paste-edit, its format is similar to the query syntax:
> +
> + root@voyage:~# cat /dbg/dynamic_debug/pending
> + # func file module format line flags mask
> + ...
> + func a299 line 0-0 =pa
> + func a300 line 0-0 =pa
I don't get the "line 0-0" stuff either. Why does it exist? Its meaning
is far from intuitive.
> +Deleting or altering a pending query requires an exact match on most
> +of the match-spec; the same string specs must be given, but 'line 0-0'
> +matches with '' and vice-versa. The filter-spec is ignored for
...and that doesn't really clarify things for me. Sorry for being so
dense.
> + # removes PQ-1 (exact match with PQ-1), disables active callsites (if any)
> + voyage:~# dbg_query module foo line 0-0 ap=
And again...how is "line 0-0" an "exact match"?
> +Altering a pending query ('a' in filter) will also alter the callsites
> +that it applies to (subject to filter match), but changing the active
> +callsites (using a query without the 'a') does not change the pending
> +query that applied it.
Why? If you really need to keep pending queries around, why wouldn't they
be either totally independent of active queries, or completely tied to
them? From this kind of inconsistency unpleasant surprises are born.
> // enable all 12 messages in the function svc_process()
> -nullarbor:~ # echo -n 'func svc_process +p' >
> - <debugfs>/dynamic_debug/control
> +nullarbor:~ # echo -n 'func svc_process +p' > $CONTROL
>
> // disable all 12 messages in the function svc_process()
> -nullarbor:~ # echo -n 'func svc_process -p' >
> - <debugfs>/dynamic_debug/control
> +nullarbor:~ # echo -n 'func svc_process -p' > $CONTROL
Unless you're going to take responsibility for updating the document
whenever somebody patches svc_process(), I'd take out the exact count of
the number of messages.
> +// send query-command file to control
> +root@voyage:~# cat dyndbg-cmdfile > $CONTROL
> +dynamic_debug:ddebug_proc_open: called
> +dynamic_debug:ddebug_proc_write: read 500 bytes from userspace
> +dynamic_debug:ddebug_exec_queries: query 0: "func foo p=_ "
> +dynamic_debug:ddebug_tokenize: split into words: "func" "foo" "p=_"
> +dynamic_debug:ddebug_parse_query: parsed func="foo" file="" module="" format="" lineno=0-0
This doesn't seem like a particularly useful example; one can probably
assume that readers know about cat and I/O redirection.
Thanks,
jon
Jason Baron
> > +
> > +Queries submitted with 'a' are applied to current callsites as above,
> > +but are also added to a pending list. When a module is loaded later,
> > +pending queries are applied to the module in the order given.
> > +
> > +This is done before the module_init() routine is run, so pr_debug()s
> > +can be active during initialization. To better support module
> > +debugging, pending queries remain on the list through modprobe-rmmod
> > +cycles.
>
> So this functionality seems to be the whole point of much of this patch
> set. That's a lot of stuff to turn on printks during module_init(). I
> can't help but wonder: wouldn't it be easier and better to just recognize
> the ddebug_query= parameter at module load time? Then you could drop this
> whole pending queries mechanism, the expanded syntax, new control file, and
> so on...? What am I missing?
>
Hi,
Thanks for providing feedback on these patches. There was indeed an
earlier approach to do this as a module parameter:
https://lkml.org/lkml/2010/9/15/397
This never quite got merged b/c I believe there were some build issues,
that never got completely resolved. I've added Thomas Renninger, the
author of that earlier appraoch to the 'cc list here.
One concern with the module parameter approach was that it reserves a
module parameter name for all modules, but perhaps if its
'ddebug_query=' that is ok. Also, I think the implmentation was
basically pass 'ddebug' as module param and it becomes:
'ddebug_query=module foo +p'. So perhaps, we could have it pass on a
full query string.
But yes, I agree the module param approach may be simpler...
Thanks,
-Jason
Greg KH
> This is essentially a rebase, resend, with trivial changes:
> - drops scx200 - off topic
> - adds pr_cont_once tweak - remove pr_fmt
> as before, it has:
> - Doc tweaks per Randy Dunlap
> - pr_fmt_$severity, suggested by Joe Perches
> - does not change pr_fmt() default, per Joe
>
> Rebased onto git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/driver-core.git
> + Jason's 8-12/12 which havent made it into driver-core
> Ive not resent them here.
As stated before, I'm not going to be able to accept these until I get
Jason's ack on them.
Jason?
Jim Cromie
thanks for the review, and sorry for the delayed response -
I had it in sitting in drafts, but got distracted by some build-box issues..
To summarize, I agree that your suggestion, and with Thomas Renninger's patches,
they are a simpler approach (much less code), which does substantially
everything
I was looking for with pending-list. Im in the process of reworking
the patchset on top of Thomas's patches.
The one issue with new approach is that it adds $module.ddebug parameters
that is not known to modinfo. Whether to expose it, and how, are open
questions,
which Im deferring. My hope is they can be added later.
Perhaps this is a good way to add $module.debug_level and
$module.debug_flags too
Below are point-by-point responses, which arent really relevant any more,
but which Im sending anyway - maybe theres something worth a followup.
Jim Cromie
On Mon, Oct 10, 2011 at 12:54 PM, Jonathan Corbet <cor...@lwn.net> wrote:
> Some of these comments are about the patch set as a whole, but done in the
> context of the doc file changes.
>
>> �Dynamic debug has even more useful features:
>>
>> - * Simple query language allows turning on and off debugging statements by
>> - � matching any combination of:
>> + * Simple query language allows turning on and off debugging statements
>> + � by matching any combination of 0 or 1 of:
>>
>> � � - source filename
>> � � - function name
>> � � - line number (including ranges of line numbers)
>> � � - module name
>> � � - format string
>> + � - current debugging flags
>
> This is totally confusing; you've replaced all the things that can be
> matched with a single-entry itemized list of "current debugging flags"?
I've not replaced the 5 lines, Ive added a single line.
the '-' here is a bullet-point, not a line removal.
Does that clarify ?
If not, I guess you dislike the ambiguity of "current debugging flags"
I think thats covered later..
>
>> + * Provides a debugfs control file: <debugfs>/dynamic_debug/control
>> + � which can be read to display the complete list of known debug
>> + � statements, to help guide you.
>>
>> �Controlling dynamic debug Behaviour
>> �===================================
>>
>> -The behaviour of pr_debug()/dev_dbg()s are controlled via writing to a
>> -control file in the 'debugfs' filesystem. Thus, you must first mount the debugfs
>> -filesystem, in order to make use of this feature. Subsequently, we refer to the
>> -control file as: <debugfs>/dynamic_debug/control. For example, if you want to
>> -enable printing from source file 'svcsock.c', line 1603 you simply do:
>> +The behaviour of pr_debug()/dev_dbg()/net_dbg()s are controlled via
>> +writing to a control file in the 'debugfs' filesystem. Thus, you must
>> +first mount the debugfs filesystem, in order to make use of this
>> +feature. �Subsequently, we refer to mount point as $DBGFS, and the
>> +control file as $CONTROL. �So if you want to enable printing from
>
> Why the $CONTROL stuff? �I can see parameterizing teh debugfs location,
> even though that discussion seems to have settled down. �But, if you miss
> the line at the top of this section, you'll never know where $CONTROL is.
> It doesn't change, why obfuscate it?
The reason was to shorten the ~20 commandline examples using it,
avoiding long lines and \ line continuations
>> +source file 'svcsock.c', line 1603 you simply do:
>>
>> -nullarbor:~ # echo 'file svcsock.c line 1603 +p' >
>> - � � � � � � � � � � � � � � <debugfs>/dynamic_debug/control
>> +nullarbor:~ # echo file svcsock.c line 1603 +p > $CONTROL
>>
>> �If you make a mistake with the syntax, the write will fail thus:
>>
>> -nullarbor:~ # echo 'file svcsock.c wtf 1 +p' >
>> - � � � � � � � � � � � � � � <debugfs>/dynamic_debug/control
>> +nullarbor:~ # 'echo file svcsock.c wtf 1 +p' > $CONTROL
>> �-bash: echo: write error: Invalid argument
>
> I don't think you tested that last one :) �The bash error output will be
> other than shown here.
thats the error I get, on 32 and 64bit x86.
what are you seeing ?
>> + �# comments and blank lines ok, but cannot contain semicolon
>> + �# multiple cmds per line, 1st must be terminated by semicolon
>> + �func foo p=_ � � � ; func buzz p=_
>> + �func bar p=_ � � � ; func bum �p=_
>> + �func yak p=_ � � � ; # trailing comment, requires semicolon to terminate cmd
>
> That seems like a slightly strange set of rules. �By Least Surprise, I
> would expect # to start a comment anywhere and to go the end of the line,
> semicolons or not.
yes, slightly strange.
I did it this way so that I could just loop over existing ddebug_exec_query
ie simpler code, smaller changeset.
>> +voyage:~# cat dyndbg-cmdfile > $CONTROL
>> +dynamic_debug:ddebug_exec_queries: processed 7 queries, with 0 errs
>
> Who produces the "processed 7 queries" message? �It's not that "cat"
> command...
>
thats the log-message, on console because I turned up /sys/proc/kernel/printk.
I thought it would be clear, esp with the ~# prompt and command preceding it.
If thats the sticking point, Im happy to remove it.
>> +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' > $CONTROL; \
>> +> echo 'file svcsock.c line 1563 +p' > $CONTROL
Mostly because it was in the file originally.
>
> Is there some reason for the backslash-escaped newline here?
>
>> �A match specification comprises a keyword, which controls the attribute
>> �of the callsite to be compared, and a value to compare against. �Possible
>> @@ -125,12 +142,13 @@ match-spec ::= 'func' string |
>> � � � � � � �'module' string |
>> � � � � � � �'format' string |
>> � � � � � � �'line' line-range
>> +// Note: no wildcards, regexs are accepted
>
> Why the // notation? �And what does that line mean? �There's a full regular
> expression interpreter in this code now? �I note that there are no
> regex-based examples in this file.
>
that comment syntax is the same as is used in the lineno explanation
and elsewhere.
As for deeper meaning, it just means literal string matches only.
Perhaps I should just say that.
>> +flags specification
>> +===================
>>
>> -=
>> - � �set the flags to the given flags
>> +The flags specification matches the regexp: ^[flmpta_]*[-+=][flmpta_]*$
>> +and has 3 parts:
>>
>> -The flags are:
>> +flags filter (optional):
>> + � �The filter precedes the operation [-+=], and constrains matching
>> + � �to thoses callsite with given flags set. �This allows altering
>> + � �flags on callsites with filtered flag values
ie matches against callsites' current debug-flags
>>
>> -f
>> - � �Include the function name in the printed message
>> -l
>> - � �Include line number in the printed message
>> -m
>> - � �Include module name in the printed message
>> -p
>> - � �Causes a printk() message to be emitted to dmesg
>> -t
>> - � �Include thread ID in messages not generated from interrupt context
>> + � � �pm+t � # add t to enabled sites which have m
>
> How about "add the thread ID to sites that already have module-name
> printing enabled? �Same with the rest.
OK. I can improve the wording, something like:
p - enable callsite, printk message to ..
f - add function name to output of enabled callsite
...
>> +Pending queries
>> +===============
>> +
>> +Queries submitted with 'a' are applied to current callsites as above,
>> +but are also added to a pending list. �When a module is loaded later,
>> +pending queries are applied to the module in the order given.
>> +
>> +This is done before the module_init() routine is run, so pr_debug()s
>> +can be active during initialization. �To better support module
>> +debugging, pending queries remain on the list through modprobe-rmmod
>> +cycles.
>
> So this functionality seems to be the whole point of much of this patch
> set. �That's a lot of stuff to turn on printks during module_init(). �I
> can't help but wonder: wouldn't it be easier and better to just recognize
> the ddebug_query= parameter at module load time? �Then you could drop this
> whole pending queries mechanism, the expanded syntax, new control file, and
> so on...? �What am I missing?
1 - filtering callsites based upon current-flags is useful,
either by itself, or with other selection constraints
echo l+p > $CONTROL # enable callsites with line-numbers selected
echo p+mf > $CONTROL
2 - with addition of user-flags, say x,y,z, user could define
arbitrary sets of callsites,
then modify and enable them in 1 operation
echo module foo +x > $CONTROL
echo module bar lineno 2-200 +x > $CONTROL
echo x+mfp > $CONTROL
3 - all modules get working pr_debug during module_init, without changes.
Perhaps no existing mods need this bad enough to have added it, but maybe
they just used printks during development, and removed them before submitting.
4 - the $module.ddebug=+p approach implements a hidden option to all modules,
which is unreported by modinfo $module, and can be used
On the whole, theres no significant advantage vs Thomas Renninger's
$module.ddebug approach. With his way, you'd add foo.ddebug into
/etc/modprobe.d/*
for each module you want to debug at initialization time.
Given that complexity is the argument against this patchset, its primarily these
that add the pending list; most of the others (modulo the # vs ;
issue) are useful
whether or not pending-query maked the cut.
0016-dynamic_debug-save-a-queries-to-pending-list-for-lat.patch:
lib/dynamic_debug.c | 154
+++++++++++++++++++++++++++++++++++++++--
0021-dynamic_debug-add-DBGFS-dynamic_debug-pending-file.patch:
lib/dynamic_debug.c | 191
++++++++++++++++++++++++++++++++++++++++++++++++++-
>
>> +You can review currently pending queries by catting or grepping
>> +$DEBUFS/dynamic_debug/pending. �To simplify removal via
>> +cut-paste-edit, its format is similar to the query syntax:
>> +
>> + �root@voyage:~# cat /dbg/dynamic_debug/pending
>> + �# func file module format line flags mask
>> + �...
>> + � func a299 line 0-0 =pa
>> + � func a300 line 0-0 =pa
>
> I don't get the "line 0-0" stuff either. �Why does it exist? �Its meaning
> is far from intuitive.
It gets printed cuz its part of the query/rule syntax,
and the intent was to make it simple to copy-paste to
add/delete/modify pending rules.
Its trivial to suppress when 0-0, and this would be consistent with
module, file, format etc,
which are not shown when not specified.
I dunno why this didnt outweigh the copy-paste "feature" when I wrote the code,
eliminating it, except when its something like 100-200,
would have simplified the explanation.
>
>> +Deleting or altering a pending query requires an exact match on most
>> +of the match-spec; the same string specs must be given, but 'line 0-0'
>> +matches with '' and vice-versa. �The filter-spec is ignored for
>
> ...and that doesn't really clarify things for me. �Sorry for being so
> dense.
yeah. Clarity and brevity are *hard*. Thats why I put in all the examples.
I guess they werent enough.
Let me hazard one more try.
lineno differs from other match-specs in that
echo lineno 0-0 +p > $CONTROL # is legal, but
echo func "" module "" +p > $CONTROL # are not legal
If module, file, func, format is to be unconstrained, just leave it out..
You can add "lineno 0-0" to the query, or leave it out, they are equivalent.
>
>> + �# removes PQ-1 (exact match with PQ-1), disables active callsites (if any)
>> + �voyage:~# dbg_query module foo line 0-0 ap=
>
> And again...how is "line 0-0" an "exact match"?
>
>> +Altering a pending query ('a' in filter) will also alter the callsites
>> +that it applies to (subject to filter match), but changing the active
>> +callsites (using a query without the 'a') does not change the pending
>> +query that applied it.
>
> Why? �If you really need to keep pending queries around, why wouldn't they
> be either totally independent of active queries, or completely tied to
> them? �From this kind of inconsistency unpleasant surprises are born.
>
what does totally independent mean in this context ?
if a pending rule is applicable (ie can effect changes upon the
module) when issued,
it seemed sensible to apply it - waiting til "later" raises many questions..
My 1st version did have them 'totally tied' insofar as any query was pending
if it did not apply (ie if module wasnt loaded).
Jason suggested 'a' flag, I agreed, because it made user intention
clear, and cuz it
eliminated questions about whether any non-applicable query should be pended.
Without explicit 'a' flag, this does NOT add query to pending-list
echo module no-such-module +p > $CONTROL
Note that non-matching query is not an error as of v3.0, nor with this patchset.
Its perhaps worth considering whether more distinct error-codes should
be returned.
>> �// enable all 12 messages in the function svc_process()
>> -nullarbor:~ # echo -n 'func svc_process +p' >
>> - � � � � � � � � � � � � � � <debugfs>/dynamic_debug/control
>> +nullarbor:~ # echo -n 'func svc_process +p' > $CONTROL
>>
>> �// disable all 12 messages in the function svc_process()
>> -nullarbor:~ # echo -n 'func svc_process -p' >
>> - � � � � � � � � � � � � � � <debugfs>/dynamic_debug/control
>> +nullarbor:~ # echo -n 'func svc_process -p' > $CONTROL
>
> Unless you're going to take responsibility for updating the document
> whenever somebody patches svc_process(), I'd take out the exact count of
> the number of messages.
fair enough - again, that is unchanged from the original.
>
>> +// send query-command file to control
>> +root@voyage:~# cat dyndbg-cmdfile > $CONTROL
>> +dynamic_debug:ddebug_proc_open: called
>> +dynamic_debug:ddebug_proc_write: read 500 bytes from userspace
>> +dynamic_debug:ddebug_exec_queries: query 0: "func foo p=_ � �"
>> +dynamic_debug:ddebug_tokenize: split into words: "func" "foo" "p=_"
>> +dynamic_debug:ddebug_parse_query: parsed func="foo" file="" module="" format="" lineno=0-0
>
> This doesn't seem like a particularly useful example; one can probably
> assume that readers know about cat and I/O redirection.
>
Ok.
I put it there to show representative debug output..
and for folks who skip to EXAMPLES
> Thanks,
>
> jon
>
thanks, even though it doesnt feel like the outcome Id hoped for.
Do/can my responses change the outcome, or is the
module.ddebug=<...> approach fundamentally better ?
I wish my search had found Thomas' patches :-/
Jason, can you look over these patches and suggest the subset
that you could sign-off on ? I;ll drop the troublesome ones and resubmit.
these should all be good:
0001-dynamic_debug-drop-enabled-field-from-struct-_ddebug.patch
0002-dynamic_debug-make-dynamic-debug-supersede-DEBUG-ccf.patch
0003-dynamic_debug-replace-strcpy-with-strlcpy-in-ddebug_.patch
0004-dynamic_debug-warn-when-1-of-each-type-of-match-spec.patch
0005-dynamic_debug-pr_err-call-should-not-depend-upon-ver.patch
0006-dynamic_debug-add-trim_prefix-to-provide-source-root.patch
maybe defer til pr_debug_level is done:
0007-dynamic_debug-change-verbosity-at-runtime.patch
0008-dynamic_debug-define-several-levels-of-verbosity.patch
sensible, not necessary
0011-dynamic_debug-hoist-locking-in-ddebug_change-to-call.patch
should be done:
0012-dynamic_debug-dont-kill-entire-facility-on-error-par.patch
ok, fluff
0013-dynamic_debug-factor-vpr_info_dq-out-of-ddebug_parse.patch
sensible, not needed,
0014-dynamic_debug-refactor-query_matches_callsite-out-of.patch
0015-dynamic_debug-drop-explicit-foo-NULL-checks.patch
if flags-filtering is ok, minor reworks needed (to drop 'a')
0018-dynamic_debug-describe_flags-with-pmflta_.patch
0019-dynamic_debug-add-flags-filtering-to-flags-spec.patch
0020-dynamic_debug-make-ddebug_describe_flags-more-generi.patch
sensible
0022-dynamic_debug-early-return-if-_ddebug-table-is-empty.patch
0024-dynamic_debug-reduce-lineno-field-to-a-saner-18-bits.patch
0025-dynamic_debug-add-pr_fmt_-for-each-severity.patch
0026-printk.h-drop-unused-pr_cont_once.patch
thanks
~jimc
Jason Baron
> Date: Tue, 18 Oct 2011 14:41:32 -0600
> From: Jim Cromie <jim.c...@gmail.com>
> To: Jonathan Corbet <cor...@lwn.net>
> Cc: jba...@redhat.com, j...@perches.com, bart.va...@gmail.com,
> gr...@kroah.com, linux-...@vger.kernel.org
> Subject: Re: [PATCH 23/26] dynamic_debug: document pending queries,
> flags-filter, multiple queries
agree, these all look good.
> maybe defer til pr_debug_level is done:
> 0007-dynamic_debug-change-verbosity-at-runtime.patch
> 0008-dynamic_debug-define-several-levels-of-verbosity.patch
>
yes, let's defer these and use them as a test case for pr_debug_level()
> sensible, not necessary
> 0011-dynamic_debug-hoist-locking-in-ddebug_change-to-call.patch
>
I would leave the locking unless there was a clear bug or performance
gain, which doesn't seem the case here.
> should be done:
> 0012-dynamic_debug-dont-kill-entire-facility-on-error-par.patch
>
ok, should probably add a 'goto out', if the last module fails to add, but
this pre-existed your patch
> ok, fluff
> 0013-dynamic_debug-factor-vpr_info_dq-out-of-ddebug_parse.patch
>
if not used more than once - yes, not needed
> sensible, not needed,
> 0014-dynamic_debug-refactor-query_matches_callsite-out-of.patch
> 0015-dynamic_debug-drop-explicit-foo-NULL-checks.patch
>
skip if function not used more than once
> if flags-filtering is ok, minor reworks needed (to drop 'a')
> 0018-dynamic_debug-describe_flags-with-pmflta_.patch
makes things more readable - keep
> 0019-dynamic_debug-add-flags-filtering-to-flags-spec.patch
19 - statement grouping should probably be done in userspace, or via
pr_debug_flag/level controls
> 0020-dynamic_debug-make-ddebug_describe_flags-more-generi.patch
>
20 - not needed, if we aren't implementing pending queries
> sensible
> 0022-dynamic_debug-early-return-if-_ddebug-table-is-empty.patch
makes sense
> 0024-dynamic_debug-reduce-lineno-field-to-a-saner-18-bits.patch
24 - ok, but still allocating int - doesn't change the struct size
> 0025-dynamic_debug-add-pr_fmt_-for-each-severity.patch
> 0026-printk.h-drop-unused-pr_cont_once.patch
>
25-26 I would defer to Joe and others, if they are ok with this.
Thanks,
-Jason
Jason Baron
> Date: Tue, 18 Oct 2011 11:52:18 -0700
> From: Greg KH <gr...@kroah.com>
> To: jim.c...@gmail.com, jba...@redhat.com
> Cc: j...@perches.com, bart.va...@gmail.com, linux-...@vger.kernel.org
> Subject: Re: [PATCH 00/26] dynamic_debug: add pending queries, etc (~resend)
> User-Agent: Mutt/1.5.21 (2010-09-15)
>
> On Fri, Oct 07, 2011 at 02:33:06PM -0600, jim.c...@gmail.com wrote:
> > This is essentially a rebase, resend, with trivial changes:
> > - drops scx200 - off topic
> > - adds pr_cont_once tweak - remove pr_fmt
> > as before, it has:
> > - Doc tweaks per Randy Dunlap
> > - pr_fmt_$severity, suggested by Joe Perches
> > - does not change pr_fmt() default, per Joe
> >
> > Rebased onto git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/driver-core.git
> > + Jason's 8-12/12 which havent made it into driver-core
> > Ive not resent them here.
>
> As stated before, I'm not going to be able to accept these until I get
> Jason's ack on them.
>
> Jason?
>
Hi Greg,
Yes, I just replied, in a separate thread on which patches in the series
I'd like to include, and which I'd like to drop for now. I expect we'll
need another re-post of the series once we've reached a consensus.
Thanks,
-Jason
Greg KH
Ok, I need a resend of what should be applied then, as I'm not going to
be able to dig through that list and properly match up what should, and
should not, be applied. That's for the maintainer of the subsystem to
do :)
thanks,
greg k-h