changes from v2:
only claim one user flag, dont really need more
fix patchset grooming err Reported-by: kbuild test robot <[email protected]>
quoting tweaks in Docs (unvalidated as better, but likely)
rename to lib/dyndbg.c, update docs accordingly
changes from v1:
dont drop trim_prefix yet, its harmless, and better supports old compilers.
dont move externs to header, despite checkpatch
v1: https://lkml.org/lkml/2019/10/29/989
v2: https://lkml.org/lkml/2019/11/27/547
New Features (review):
accept new query input:
file inode.c:100-200 # & line-range
file inode.c:start_* # & function
add 'u' user flag, allowing user to compose an arbitrary set of
callsites by marking them with 'u', without altering current
print-modifying flags.
extend flags-spec to allow filter-flags, which select callsites for
modification based upon their current flags. This lets user activate
the set of callsites composed previously (u+p).
add negating flags to filter on absence of flags, or to negate a modification.
Jim Cromie (17):
dyndbg-docs: eschew file /full/path query in docs
dyndbg: drop obsolete comment on ddebug_proc_open
dyndbg: raise verbosity needed to enable ddebug_proc_* logging
dyndbg: rename __verbose section to __dyndbg
dyndbg: fix overcounting of ram used by dyndbg
dyndbg: fix a BUG_ON in ddebug_describe_flags
dyndbg: refactor parse_linerange out of ddebug_parse_query
dyndbg: accept 'file foo.c:func1' and 'file foo.c:10-100'
dyndbg: refactor ddebug_read_flags out of ddebug_parse_flags
dyndbg: combine flags & mask into a struct, use that
dyndbg: add filter parameter to ddebug_parse_flags
dyndbg: extend ddebug_parse_flags to accept optional filter-flags
dyndbg: prefer declarative init in caller, to memset in callee
dyndbg: add user-flag, negating-flags, and filtering on flags
dyndbg: allow negating flag-chars in modflags
dyndbg: make ddebug_tables list LIFO for add/remove_module
dyndbg: rename lib/dynamic_debug.c to lib/dyndbg.c
dyndbg-docs: normalize comments in examples
Documentation/admin-guide/dynamic-debug-howto.rst | 176 ++++++++++++++++-----------
include/asm-generic/vmlinux.lds.h | 6 +-
include/linux/dynamic_debug.h | 5 +-
kernel/module.c | 2 +-
lib/Makefile | 4 +-
lib/{dynamic_debug.c => dyndbg.c} | 285 ++++++++++++++++++++++++++------------------
6 files changed, 285 insertions(+), 193 deletions(-)
--
2.23.0
The verbose/debug logging done by `cat $DBGFS/dynamic_debug/control`
is voluminous, and clutters logging done during parsing of queries,
which is much more useful when manipulating/enabling callsites.
So increase the required verbosity to 8&9 for per-page and per-line
logging; ie ddebug_proc_(start|stop) and ddebug_proc_(show|next)
respectively. This leaves 2-7 for any further logging tweaks to the
query parsing process.
Signed-off-by: Jim Cromie <[email protected]>
---
lib/dynamic_debug.c | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 6cefceffadcb..c86c97154657 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -105,12 +105,16 @@ static char *ddebug_describe_flags(struct _ddebug *dp, char *buf,
return buf;
}
-#define vpr_info(fmt, ...) \
+#define vnpr_info(lvl, fmt, ...) \
do { \
- if (verbose) \
+ if (verbose >= lvl) \
pr_info(fmt, ##__VA_ARGS__); \
} while (0)
+#define vpr_info(fmt, ...) vnpr_info(1, fmt, ##__VA_ARGS__)
+#define v8pr_info(fmt, ...) vnpr_info(8, fmt, ##__VA_ARGS__)
+#define v9pr_info(fmt, ...) vnpr_info(9, fmt, ##__VA_ARGS__)
+
static void vpr_info_dq(const struct ddebug_query *query, const char *msg)
{
/* trim any trailing newlines */
@@ -771,7 +775,7 @@ static void *ddebug_proc_start(struct seq_file *m, loff_t *pos)
struct _ddebug *dp;
int n = *pos;
- vpr_info("called m=%p *pos=%lld\n", m, (unsigned long long)*pos);
+ v8pr_info("called m=%p *pos=%lld\n", m, (unsigned long long)*pos);
mutex_lock(&ddebug_lock);
@@ -795,7 +799,7 @@ static void *ddebug_proc_next(struct seq_file *m, void *p, loff_t *pos)
struct ddebug_iter *iter = m->private;
struct _ddebug *dp;
- vpr_info("called m=%p p=%p *pos=%lld\n",
+ v9pr_info("called m=%p p=%p *pos=%lld\n",
m, p, (unsigned long long)*pos);
if (p == SEQ_START_TOKEN)
@@ -818,7 +822,7 @@ static int ddebug_proc_show(struct seq_file *m, void *p)
struct _ddebug *dp = p;
char flagsbuf[10];
- vpr_info("called m=%p p=%p\n", m, p);
+ v9pr_info("called m=%p p=%p\n", m, p);
if (p == SEQ_START_TOKEN) {
seq_puts(m,
@@ -842,7 +846,7 @@ static int ddebug_proc_show(struct seq_file *m, void *p)
*/
static void ddebug_proc_stop(struct seq_file *m, void *p)
{
- vpr_info("called m=%p p=%p\n", m, p);
+ v8pr_info("called m=%p p=%p\n", m, p);
mutex_unlock(&ddebug_lock);
}
--
2.23.0
Regarding:
commit 2b6783191da7 ("dynamic_debug: add trim_prefix() to provide source-root relative paths")
commit a73619a845d5 ("kbuild: use -fmacro-prefix-map to make __FILE__ a relative path")
2nd commit broke dynamic-debug's "file $fullpath" query form, but
nobody noticed because 1st commit trimmed prefixes from control-file
output, so the click-copy-pasting of fullpaths into new queries had
ceased; that query form became unused.
Removing the function is cleanest, but it could be useful in
old-compiler corner cases, where __FILE__ still has /full/path,
and it safely does nothing otherwize.
So instead, quietly deprecate "file /full/path" query form, by
removing all /full/paths examples in the docs. I skipped adding a
back-compat note.
Signed-off-by: Jim Cromie <[email protected]>
---
.../admin-guide/dynamic-debug-howto.rst | 19 +++++++++----------
1 file changed, 9 insertions(+), 10 deletions(-)
diff --git a/Documentation/admin-guide/dynamic-debug-howto.rst b/Documentation/admin-guide/dynamic-debug-howto.rst
index 252e5ef324e5..e011f8907116 100644
--- a/Documentation/admin-guide/dynamic-debug-howto.rst
+++ b/Documentation/admin-guide/dynamic-debug-howto.rst
@@ -62,10 +62,10 @@ statements via::
nullarbor:~ # cat <debugfs>/dynamic_debug/control
# filename:lineno [module]function flags format
- /usr/src/packages/BUILD/sgi-enhancednfs-1.4/default/net/sunrpc/svc_rdma.c:323 [svcxprt_rdma]svc_rdma_cleanup =_ "SVCRDMA Module Removed, deregister RPC RDMA transport\012"
- /usr/src/packages/BUILD/sgi-enhancednfs-1.4/default/net/sunrpc/svc_rdma.c:341 [svcxprt_rdma]svc_rdma_init =_ "\011max_inline : %d\012"
- /usr/src/packages/BUILD/sgi-enhancednfs-1.4/default/net/sunrpc/svc_rdma.c:340 [svcxprt_rdma]svc_rdma_init =_ "\011sq_depth : %d\012"
- /usr/src/packages/BUILD/sgi-enhancednfs-1.4/default/net/sunrpc/svc_rdma.c:338 [svcxprt_rdma]svc_rdma_init =_ "\011max_requests : %d\012"
+ net/sunrpc/svc_rdma.c:323 [svcxprt_rdma]svc_rdma_cleanup =_ "SVCRDMA Module Removed, deregister RPC RDMA transport\012"
+ net/sunrpc/svc_rdma.c:341 [svcxprt_rdma]svc_rdma_init =_ "\011max_inline : %d\012"
+ net/sunrpc/svc_rdma.c:340 [svcxprt_rdma]svc_rdma_init =_ "\011sq_depth : %d\012"
+ net/sunrpc/svc_rdma.c:338 [svcxprt_rdma]svc_rdma_init =_ "\011max_requests : %d\012"
...
@@ -85,7 +85,7 @@ the debug statement callsites with any non-default flags::
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"
+ net/sunrpc/svcsock.c:1603 [sunrpc]svc_send p "svc_process: st_sendto returned %d\012"
Command Language Reference
==========================
@@ -158,13 +158,12 @@ func
func svc_tcp_accept
file
- The given string is compared against either the full pathname, the
- src-root relative pathname, or the basename of the source file of
- each callsite. Examples::
+ The given string is compared against either the src-root relative
+ pathname, or the basename of the source file of each callsite.
+ Examples::
file svcsock.c
- file kernel/freezer.c
- file /usr/src/packages/BUILD/sgi-enhancednfs-1.4/default/net/sunrpc/svcsock.c
+ file kernel/freezer.c # ie column 1 of control file
module
The given string is compared against the module name
--
2.23.0
dyndbg populates its callsite info into __verbose section, change that
to a more specific and descriptive name, __dyndbg.
Also, per checkpatch:
simplify __attribute(..) to __section(__dyndbg) declaration.
and 1 spelling fix
Signed-off-by: Jim Cromie <[email protected]>
---
include/asm-generic/vmlinux.lds.h | 6 +++---
include/linux/dynamic_debug.h | 4 ++--
kernel/module.c | 2 +-
lib/dynamic_debug.c | 12 ++++++------
4 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index dae64600ccbf..82694efe3a83 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -285,9 +285,9 @@
*(__tracepoints) \
/* implement dynamic printk debug */ \
. = ALIGN(8); \
- __start___verbose = .; \
- KEEP(*(__verbose)) \
- __stop___verbose = .; \
+ __start___dyndbg = .; \
+ KEEP(*(__dyndbg)) \
+ __stop___dyndbg = .; \
LIKELY_PROFILE() \
BRANCH_PROFILE() \
TRACE_PRINTKS() \
diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index 4cf02ecd67de..802480ea8708 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -80,7 +80,7 @@ void __dynamic_ibdev_dbg(struct _ddebug *descriptor,
#define DEFINE_DYNAMIC_DEBUG_METADATA(name, fmt) \
static struct _ddebug __aligned(8) \
- __attribute__((section("__verbose"))) name = { \
+ __section(__dyndbg) name = { \
.modname = KBUILD_MODNAME, \
.function = __func__, \
.filename = __FILE__, \
@@ -133,7 +133,7 @@ void __dynamic_ibdev_dbg(struct _ddebug *descriptor,
/*
* "Factory macro" for generating a call to func, guarded by a
- * DYNAMIC_DEBUG_BRANCH. The dynamic debug decriptor will be
+ * DYNAMIC_DEBUG_BRANCH. The dynamic debug descriptor will be
* initialized using the fmt argument. The function will be called with
* the address of the descriptor as first argument, followed by all
* the varargs. Note that fmt is repeated in invocations of this
diff --git a/kernel/module.c b/kernel/module.c
index ff2d7359a418..a9c052cc30c5 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -3237,7 +3237,7 @@ static int find_module_sections(struct module *mod, struct load_info *info)
if (section_addr(info, "__obsparm"))
pr_warn("%s: Ignoring obsolete parameters\n", mod->name);
- info->debug = section_objs(info, "__verbose",
+ info->debug = section_objs(info, "__dyndbg",
sizeof(*info->debug), &info->num_debug);
return 0;
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index c86c97154657..0a4588fe342e 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -39,8 +39,8 @@
#include <rdma/ib_verbs.h>
-extern struct _ddebug __start___verbose[];
-extern struct _ddebug __stop___verbose[];
+extern struct _ddebug __start___dyndbg[];
+extern struct _ddebug __stop___dyndbg[];
struct ddebug_table {
struct list_head link;
@@ -1010,14 +1010,14 @@ static int __init dynamic_debug_init(void)
int n = 0, entries = 0, modct = 0;
int verbose_bytes = 0;
- if (__start___verbose == __stop___verbose) {
+ if (__start___dyndbg == __stop___dyndbg) {
pr_warn("_ddebug table is empty in a CONFIG_DYNAMIC_DEBUG build\n");
return 1;
}
- iter = __start___verbose;
+ iter = __start___dyndbg;
modname = iter->modname;
iter_start = iter;
- for (; iter < __stop___verbose; iter++) {
+ for (; iter < __stop___dyndbg; iter++) {
entries++;
verbose_bytes += strlen(iter->modname) + strlen(iter->function)
+ strlen(iter->filename) + strlen(iter->format);
@@ -1040,7 +1040,7 @@ static int __init dynamic_debug_init(void)
ddebug_init_success = 1;
vpr_info("%d modules, %d entries and %d bytes in ddebug tables, %d bytes in (readonly) verbose section\n",
modct, entries, (int)(modct * sizeof(struct ddebug_table)),
- verbose_bytes + (int)(__stop___verbose - __start___verbose));
+ verbose_bytes + (int)(__stop___dyndbg - __start___dyndbg));
/* apply ddebug_query boot param, dont unload tables on err */
if (ddebug_setup_string[0] != '\0') {
--
2.23.0
ddebug_describe_flags currently fills a caller provided string buffer,
after testing its size (also passed) in a BUG_ON. Fix this with a
struct containing a known-big-enough string buffer, and passing it
instead.
Also simplify ddebug_describe_flags sig, and de-ref the struct in the
caller; this makes function reusable (soon) in contexts where flags
are already unpacked.
-v3 fix compile err introduced in patchset grooming.
Reported-by: kbuild test robot <[email protected]>
Signed-off-by: Jim Cromie <[email protected]>
---
lib/dynamic_debug.c | 31 +++++++++++++++----------------
1 file changed, 15 insertions(+), 16 deletions(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index b5fb0aa0fbc3..49cb24948e12 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -62,6 +62,8 @@ struct ddebug_iter {
unsigned int idx;
};
+struct flagsbuf { char buf[12]; }; /* big enough to hold all the flags */
+
static DEFINE_MUTEX(ddebug_lock);
static LIST_HEAD(ddebug_tables);
static int verbose;
@@ -88,21 +90,19 @@ 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,
- size_t maxlen)
+static char *ddebug_describe_flags(unsigned int flags, struct flagsbuf *fb)
{
- char *p = buf;
+ char *p = fb->buf;
int i;
- BUG_ON(maxlen < 6);
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 == buf)
+ if (p == fb->buf)
*p++ = '_';
*p = '\0';
- return buf;
+ return fb->buf;
}
#define vnpr_info(lvl, fmt, ...) \
@@ -142,13 +142,13 @@ static void vpr_info_dq(const struct ddebug_query *query, const char *msg)
* logs the changes. Takes ddebug_lock.
*/
static int ddebug_change(const struct ddebug_query *query,
- unsigned int flags, unsigned int mask)
+ unsigned int pflags, unsigned int mask)
{
int i;
struct ddebug_table *dt;
unsigned int newflags;
unsigned int nfound = 0;
- char flagbuf[10];
+ struct flagsbuf flags;
/* search for matching ddebugs */
mutex_lock(&ddebug_lock);
@@ -191,22 +191,21 @@ static int ddebug_change(const struct ddebug_query *query,
nfound++;
- newflags = (dp->flags & mask) | flags;
+ newflags = (dp->flags & mask) | pflags;
if (newflags == dp->flags)
continue;
#ifdef CONFIG_JUMP_LABEL
if (dp->flags & _DPRINTK_FLAGS_PRINT) {
- if (!(flags & _DPRINTK_FLAGS_PRINT))
+ if (!(pflags & _DPRINTK_FLAGS_PRINT))
static_branch_disable(&dp->key.dd_key_true);
- } else if (flags & _DPRINTK_FLAGS_PRINT)
+ } else if (pflags & _DPRINTK_FLAGS_PRINT)
static_branch_enable(&dp->key.dd_key_true);
#endif
dp->flags = newflags;
vpr_info("changed %s:%d [%s]%s =%s\n",
trim_prefix(dp->filename), dp->lineno,
dt->mod_name, dp->function,
- ddebug_describe_flags(dp, flagbuf,
- sizeof(flagbuf)));
+ ddebug_describe_flags(dp->flags, &flags));
}
}
mutex_unlock(&ddebug_lock);
@@ -820,7 +819,7 @@ static int ddebug_proc_show(struct seq_file *m, void *p)
{
struct ddebug_iter *iter = m->private;
struct _ddebug *dp = p;
- char flagsbuf[10];
+ struct flagsbuf flags;
v9pr_info("called m=%p p=%p\n", m, p);
@@ -833,7 +832,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, &flags));
seq_escape(m, dp->format, "\t\r\n\"");
seq_puts(m, "\"\n");
--
2.23.0
make the code-block reusable to later handle "file foo.c:101-200" etc.
This should be a 90%+ code-move, with minimal adaptations; reindent,
and maybe fixes for compile, behavior.
Signed-off-by: Jim Cromie <[email protected]>
---
lib/dynamic_debug.c | 61 +++++++++++++++++++++++++--------------------
1 file changed, 34 insertions(+), 27 deletions(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 49cb24948e12..f0cf90e672b8 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -292,6 +292,39 @@ static inline int parse_lineno(const char *str, unsigned int *val)
return 0;
}
+static int parse_linerange(struct ddebug_query *query, const char *first)
+{
+ char *last = strchr(first, '-');
+
+ if (query->first_lineno || query->last_lineno) {
+ pr_err("match-spec: line used 2x\n");
+ return -EINVAL;
+ }
+ if (last)
+ *last++ = '\0';
+ if (parse_lineno(first, &query->first_lineno) < 0)
+ return -EINVAL;
+ if (last) {
+ /* range <first>-<last> */
+ if (parse_lineno(last, &query->last_lineno) < 0)
+ return -EINVAL;
+
+ /* special case for last lineno not specified */
+ if (query->last_lineno == 0)
+ query->last_lineno = UINT_MAX;
+
+ if (query->last_lineno < query->first_lineno) {
+ pr_err("last-line:%d < 1st-line:%d\n",
+ query->last_lineno,
+ query->first_lineno);
+ return -EINVAL;
+ }
+ } else {
+ query->last_lineno = query->first_lineno;
+ }
+ return 0;
+}
+
static int check_set(const char **dest, char *src, char *name)
{
int rc = 0;
@@ -350,34 +383,8 @@ static int ddebug_parse_query(char *words[], int nwords,
UNESCAPE_SPECIAL);
rc = check_set(&query->format, words[i+1], "format");
} else if (!strcmp(words[i], "line")) {
- char *first = words[i+1];
- char *last = strchr(first, '-');
- if (query->first_lineno || query->last_lineno) {
- pr_err("match-spec: line used 2x\n");
- return -EINVAL;
- }
- if (last)
- *last++ = '\0';
- if (parse_lineno(first, &query->first_lineno) < 0)
+ if (parse_linerange(query, words[i+1]))
return -EINVAL;
- if (last) {
- /* range <first>-<last> */
- if (parse_lineno(last, &query->last_lineno) < 0)
- return -EINVAL;
-
- /* special case for last lineno not specified */
- if (query->last_lineno == 0)
- query->last_lineno = UINT_MAX;
-
- if (query->last_lineno < query->first_lineno) {
- pr_err("last-line:%d < 1st-line:%d\n",
- query->last_lineno,
- query->first_lineno);
- return -EINVAL;
- }
- } else {
- query->last_lineno = query->first_lineno;
- }
} else {
pr_err("unknown keyword \"%s\"\n", words[i]);
return -EINVAL;
--
2.23.0
Currently, ddebug_parse_flags accepts [+-=][pflmt_]+ as flag-spec
strings. If we allow [pflmt_]*[+-=][pflmt_]+ instead, the (new) 1st
flagset can be used as a filter to select callsites, before applying
changes in the 2nd flagset. 1st step is to split out the flags-reader
so we can use it again.
The point of this is to allow user to compose an arbitrary set of
changes, by marking callsites with [fmlt] flags, and then to
activate that composed set in a single query.
#> echo '=_' > control # clear all flags
#> echo 'module usb* +fmlt' > control # build the marked set, repeat
#> echo 'fmlt+p' > control # activate
Signed-off-by: Jim Cromie <[email protected]>
---
lib/dynamic_debug.c | 37 +++++++++++++++++++++++--------------
1 file changed, 23 insertions(+), 14 deletions(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 9fa6d4eeae5c..839f89b24474 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -414,6 +414,26 @@ static int ddebug_parse_query(char *words[], int nwords,
return 0;
}
+static int ddebug_read_flags(const char *str, unsigned int *flags)
+{
+ int i;
+
+ for (; *str ; ++str) {
+ for (i = ARRAY_SIZE(opt_array) - 1; i >= 0; i--) {
+ if (*str == opt_array[i].opt_char) {
+ *flags |= opt_array[i].flag;
+ break;
+ }
+ }
+ if (i < 0) {
+ pr_err("unknown flag '%c' in \"%s\"\n", *str, str);
+ return -EINVAL;
+ }
+ }
+ vpr_info("flags=0x%x\n", *flags);
+ return 0;
+}
+
/*
* Parse `str' as a flags specification, format [-+=][p]+.
* Sets up *maskp and *flagsp to be used when changing the
@@ -424,7 +444,7 @@ static int ddebug_parse_flags(const char *str, unsigned int *flagsp,
unsigned int *maskp)
{
unsigned flags = 0;
- int op = '=', i;
+ int op;
switch (*str) {
case '+':
@@ -438,19 +458,8 @@ static int ddebug_parse_flags(const char *str, unsigned int *flagsp,
}
vpr_info("op='%c'\n", op);
- for (; *str ; ++str) {
- for (i = ARRAY_SIZE(opt_array) - 1; i >= 0; i--) {
- if (*str == opt_array[i].opt_char) {
- flags |= opt_array[i].flag;
- break;
- }
- }
- if (i < 0) {
- pr_err("unknown flag '%c' in \"%s\"\n", *str, str);
- return -EINVAL;
- }
- }
- vpr_info("flags=0x%x\n", flags);
+ if (ddebug_read_flags(str, &flags))
+ return -EINVAL;
/* calculate final *flagsp, *maskp according to mask and op */
switch (op) {
--
2.23.0
combine flags & mask into a struct, and replace those 2 parameters in
3 functions: ddebug_change, ddebug_parse_flags, ddebug_read_flags,
altering the derefs in them accordingly.
This simplifies the 3 function sigs, preparing for more changes.
We dont yet need mask from ddebug_read_flags, but will soon.
Signed-off-by: Jim Cromie <[email protected]>
---
lib/dynamic_debug.c | 46 +++++++++++++++++++++++----------------------
1 file changed, 24 insertions(+), 22 deletions(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 839f89b24474..0d1b3dbdec1d 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -64,6 +64,11 @@ struct ddebug_iter {
struct flagsbuf { char buf[12]; }; /* big enough to hold all the flags */
+struct flagsettings {
+ unsigned int flags;
+ unsigned int mask;
+};
+
static DEFINE_MUTEX(ddebug_lock);
static LIST_HEAD(ddebug_tables);
static int verbose;
@@ -142,7 +147,7 @@ static void vpr_info_dq(const struct ddebug_query *query, const char *msg)
* logs the changes. Takes ddebug_lock.
*/
static int ddebug_change(const struct ddebug_query *query,
- unsigned int pflags, unsigned int mask)
+ struct flagsettings *mods)
{
int i;
struct ddebug_table *dt;
@@ -191,14 +196,14 @@ static int ddebug_change(const struct ddebug_query *query,
nfound++;
- newflags = (dp->flags & mask) | pflags;
+ newflags = (dp->flags & mods->mask) | mods->flags;
if (newflags == dp->flags)
continue;
#ifdef CONFIG_JUMP_LABEL
if (dp->flags & _DPRINTK_FLAGS_PRINT) {
- if (!(pflags & _DPRINTK_FLAGS_PRINT))
+ if (!(mods->flags & _DPRINTK_FLAGS_PRINT))
static_branch_disable(&dp->key.dd_key_true);
- } else if (pflags & _DPRINTK_FLAGS_PRINT)
+ } else if (mods->flags & _DPRINTK_FLAGS_PRINT)
static_branch_enable(&dp->key.dd_key_true);
#endif
dp->flags = newflags;
@@ -414,14 +419,14 @@ static int ddebug_parse_query(char *words[], int nwords,
return 0;
}
-static int ddebug_read_flags(const char *str, unsigned int *flags)
+static int ddebug_read_flags(const char *str, struct flagsettings *f)
{
int i;
for (; *str ; ++str) {
for (i = ARRAY_SIZE(opt_array) - 1; i >= 0; i--) {
if (*str == opt_array[i].opt_char) {
- *flags |= opt_array[i].flag;
+ f->flags |= opt_array[i].flag;
break;
}
}
@@ -430,7 +435,7 @@ static int ddebug_read_flags(const char *str, unsigned int *flags)
return -EINVAL;
}
}
- vpr_info("flags=0x%x\n", *flags);
+ vpr_info("flags=0x%x mask=0x%x\n", f->flags, f->mask);
return 0;
}
@@ -440,10 +445,8 @@ static int ddebug_read_flags(const char *str, unsigned int *flags)
* 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_flags(const char *str, struct flagsettings *mods)
{
- unsigned flags = 0;
int op;
switch (*str) {
@@ -458,31 +461,30 @@ static int ddebug_parse_flags(const char *str, unsigned int *flagsp,
}
vpr_info("op='%c'\n", op);
- if (ddebug_read_flags(str, &flags))
+ if (ddebug_read_flags(str, mods))
return -EINVAL;
- /* calculate final *flagsp, *maskp according to mask and op */
+ /* calculate final flags, mask based upon op */
switch (op) {
case '=':
- *maskp = 0;
- *flagsp = flags;
+ mods->mask = 0;
break;
case '+':
- *maskp = ~0U;
- *flagsp = flags;
+ mods->mask = ~0U;
break;
case '-':
- *maskp = ~flags;
- *flagsp = 0;
+ mods->mask = ~mods->flags;
+ mods->flags = 0;
break;
}
- vpr_info("*flagsp=0x%x *maskp=0x%x\n", *flagsp, *maskp);
+ vpr_info("*flagsp=0x%x *maskp=0x%x\n", mods->flags, mods->mask);
+
return 0;
}
static int ddebug_exec_query(char *query_string, const char *modname)
{
- unsigned int flags = 0, mask = 0;
+ struct flagsettings mods = {};
struct ddebug_query query;
#define MAXWORDS 9
int nwords, nfound;
@@ -494,7 +496,7 @@ static int ddebug_exec_query(char *query_string, const char *modname)
return -EINVAL;
}
/* check flags 1st (last arg) so query is pairs of spec,val */
- if (ddebug_parse_flags(words[nwords-1], &flags, &mask)) {
+ if (ddebug_parse_flags(words[nwords-1], &mods)) {
pr_err("flags parse failed\n");
return -EINVAL;
}
@@ -503,7 +505,7 @@ static int ddebug_exec_query(char *query_string, const char *modname)
return -EINVAL;
}
/* actually go and implement the change */
- nfound = ddebug_change(&query, flags, mask);
+ nfound = ddebug_change(&query, &mods);
vpr_info_dq(&query, nfound ? "applied" : "no-match");
return nfound;
--
2.23.0
Accept these additional query forms:
echo "file $filestr +_" > control
path/to/file.c:100 # as from control, column 1
path/to/file.c:1-100 # or any legal line-range
path/to/file.c:func_A # as from an editor/browser
path/to/file.c:drm_\* # wildcards still work
path/to/file.c:*_foo # lead wildcard too
1st 2 examples are treated as line-ranges, 3,4 are treated as func's
Doc these changes, and sprinkle in a few extra wild-card examples and
trailing # explanation texts.
Signed-off-by: Jim Cromie <[email protected]>
---
.../admin-guide/dynamic-debug-howto.rst | 5 +++++
lib/dynamic_debug.c | 20 ++++++++++++++++++-
2 files changed, 24 insertions(+), 1 deletion(-)
diff --git a/Documentation/admin-guide/dynamic-debug-howto.rst b/Documentation/admin-guide/dynamic-debug-howto.rst
index e011f8907116..689a30316589 100644
--- a/Documentation/admin-guide/dynamic-debug-howto.rst
+++ b/Documentation/admin-guide/dynamic-debug-howto.rst
@@ -156,6 +156,7 @@ func
of each callsite. Example::
func svc_tcp_accept
+ func *recv* # in rfcomm, bluetooth, ping, tcp
file
The given string is compared against either the src-root relative
@@ -164,6 +165,9 @@ file
file svcsock.c
file kernel/freezer.c # ie column 1 of control file
+ file drivers/usb/* # all callsites under it
+ file inode.c:start_* # parse :tail as a func (above)
+ file inode.c:1-100 # parse :tail as a line-range (above)
module
The given string is compared against the module name
@@ -173,6 +177,7 @@ module
module sunrpc
module nfsd
+ module drm* # both drm, drm_kms_helper
format
The given string is searched for in the dynamic debug format
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index f0cf90e672b8..9fa6d4eeae5c 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -322,6 +322,8 @@ static int parse_linerange(struct ddebug_query *query, const char *first)
} else {
query->last_lineno = query->first_lineno;
}
+ vpr_info("parsed line %d-%d\n", query->first_lineno,
+ query->last_lineno);
return 0;
}
@@ -358,6 +360,7 @@ static int ddebug_parse_query(char *words[], int nwords,
{
unsigned int i;
int rc = 0;
+ char *fline;
/* check we have an even number of words */
if (nwords % 2 != 0) {
@@ -374,7 +377,22 @@ static int ddebug_parse_query(char *words[], int nwords,
if (!strcmp(words[i], "func")) {
rc = check_set(&query->function, words[i+1], "func");
} else if (!strcmp(words[i], "file")) {
- rc = check_set(&query->filename, words[i+1], "file");
+ if (check_set(&query->filename, words[i+1], "file"))
+ return -EINVAL;
+
+ /* tail :$info is function or line-range */
+ fline = strchr(query->filename, ':');
+ if (!fline)
+ break;
+ *fline++ = '\0';
+ if (isalpha(*fline) || *fline == '*' || *fline == '?') {
+ /* take as function name */
+ if (check_set(&query->function, fline, "func"))
+ return -EINVAL;
+ } else
+ if (parse_linerange(query, fline))
+ return -EINVAL;
+
} else if (!strcmp(words[i], "module")) {
rc = check_set(&query->module, words[i+1], "module");
} else if (!strcmp(words[i], "format")) {
--
2.23.0
Add a new *filter param to 2 functions, allowing ddebug_parse_flags()
to communicate filter settings to ddebug_change(),
Also, ddebug_change doesn't alter any of its arguments, including its 2
new ones; mods, filter. Say so by adding const modifier to them.
Signed-off-by: Jim Cromie <[email protected]>
---
lib/dynamic_debug.c | 17 ++++++++++++-----
1 file changed, 12 insertions(+), 5 deletions(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 0d1b3dbdec1d..8c62c76badcf 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -147,7 +147,8 @@ static void vpr_info_dq(const struct ddebug_query *query, const char *msg)
* logs the changes. Takes ddebug_lock.
*/
static int ddebug_change(const struct ddebug_query *query,
- struct flagsettings *mods)
+ const struct flagsettings *mods,
+ const struct flagsettings *filter)
{
int i;
struct ddebug_table *dt;
@@ -445,7 +446,10 @@ static int ddebug_read_flags(const char *str, struct flagsettings *f)
* flags fields of matched _ddebug's. Returns 0 on success
* or <0 on error.
*/
-static int ddebug_parse_flags(const char *str, struct flagsettings *mods)
+
+static int ddebug_parse_flags(const char *str,
+ struct flagsettings *mods,
+ struct flagsettings *filter)
{
int op;
@@ -477,7 +481,9 @@ static int ddebug_parse_flags(const char *str, struct flagsettings *mods)
mods->flags = 0;
break;
}
- vpr_info("*flagsp=0x%x *maskp=0x%x\n", mods->flags, mods->mask);
+
+ vpr_info("mods:flags=0x%x,mask=0x%x filter:flags=0x%x,mask=0x%x\n",
+ mods->flags, mods->mask, filter->flags, filter->mask);
return 0;
}
@@ -485,6 +491,7 @@ static int ddebug_parse_flags(const char *str, struct flagsettings *mods)
static int ddebug_exec_query(char *query_string, const char *modname)
{
struct flagsettings mods = {};
+ struct flagsettings filter = {};
struct ddebug_query query;
#define MAXWORDS 9
int nwords, nfound;
@@ -496,7 +503,7 @@ static int ddebug_exec_query(char *query_string, const char *modname)
return -EINVAL;
}
/* check flags 1st (last arg) so query is pairs of spec,val */
- if (ddebug_parse_flags(words[nwords-1], &mods)) {
+ if (ddebug_parse_flags(words[nwords-1], &mods, &filter)) {
pr_err("flags parse failed\n");
return -EINVAL;
}
@@ -505,7 +512,7 @@ static int ddebug_exec_query(char *query_string, const char *modname)
return -EINVAL;
}
/* actually go and implement the change */
- nfound = ddebug_change(&query, &mods);
+ nfound = ddebug_change(&query, &mods, &filter);
vpr_info_dq(&query, nfound ? "applied" : "no-match");
return nfound;
--
2.23.0
1. Add a user-flag [u] which works like original [pfmlt] flags, but
has no effect on callsite behavior; it allows marking of arbitrary
sets of callsites.
2. Add [PFMLTU] flags, which negate their counterparts; P===!p etc.
And in ddebug_read_flags():
current code does: [pfmltu_] -> flags
copy it to: [PFMLTU_] -> mask
also disallow both of a pair: ie no 'pP', no true & false.
3. Add filtering ops into ddebug_change(), right after all the
callsite-property selections are complete. These test the callsite's
current flagstate before applying modflags.
Why ?
The new user flag facilitates batching of changes. By marking
individual callsites with 'u', user can compose an arbitrary set of
changes, then activate them together by selecting on 'u':
#> echo 'file foo.c +u; file bar.c +u' > control
#> echo 'u+p' > control
The user flag isn't strictly needed, but with it you can avoid using
[fmlt] flags for marking, which would alter logging when enabled.
Signed-off-by: Jim Cromie <[email protected]>
---
.../admin-guide/dynamic-debug-howto.rst | 31 ++++++++++++++++---
include/linux/dynamic_debug.h | 1 +
lib/dynamic_debug.c | 31 ++++++++++++++-----
3 files changed, 51 insertions(+), 12 deletions(-)
diff --git a/Documentation/admin-guide/dynamic-debug-howto.rst b/Documentation/admin-guide/dynamic-debug-howto.rst
index cdc45dcb3e0c..9f68062ba316 100644
--- a/Documentation/admin-guide/dynamic-debug-howto.rst
+++ b/Documentation/admin-guide/dynamic-debug-howto.rst
@@ -230,16 +230,39 @@ The flags are::
l Include line number in the printed message
m Include module name in the printed message
t Include thread ID in messages not generated from interrupt context
+ u user flag, to mark callsites into a group
_ No flags are set. (Or'd with others on input)
-For ``print_hex_dump_debug()`` and ``print_hex_dump_bytes()``, only ``p`` flag
-have meaning, other flags ignored.
+Additionally, the flag-chars ``[pflmtu]`` have negating flag-chars
+``[PFMLTU]``, which invert the meanings above. Their use follows.
+
+Using Filters::
+
+Filter-flags specify an optional additional selector on pr_debug
+callsites; with them you can compose an arbitrary set of callsites, by
+iteratively marking them with ``+u``, then enabling them all with
+``u+p``. You can also use ``fmlt`` flags for this, unless the format
+changes are inconvenient.
+
+Filters can also contain the negating flags, like ``UF``, which select
+only callsites with ``u`` and ``f`` cleared.
+
+Flagsets cannot contain ``pP`` etc, a flag cannot be true and false.
+
+modflags containing upper-case flags is reserved/undefined for now.
+inverted-flags are currently ignored, usage gets trickier if given
+``-pXy``, it should leave x set.
+
+Notes::
+
+For ``print_hex_dump_debug()`` and ``print_hex_dump_bytes()``, only
+``p`` flag has meaning, other flags are ignored.
For display, the flags are preceded by ``=``
(mnemonic: what the flags are currently equal to).
-Note the regexp ``^[-+=][flmpt_]+$`` matches a flags specification.
-To clear all flags at once, use ``=_`` or ``-flmpt``.
+Note the regexp ``^[-+=][flmptu_]+$`` matches a flags specification.
+To clear all flags at once, use ``=_`` or ``-flmptu``.
Debug messages during Boot Process
diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index 802480ea8708..a5d76f8f6b40 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -32,6 +32,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_USR (1<<5)
#if defined DEBUG
#define _DPRINTK_FLAGS_DEFAULT _DPRINTK_FLAGS_PRINT
#else
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 26432f88b329..736895efe17d 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -85,13 +85,14 @@ static inline const char *trim_prefix(const char *path)
return path + skip;
}
-static struct { unsigned flag:8; char opt_char; } opt_array[] = {
- { _DPRINTK_FLAGS_PRINT, 'p' },
- { _DPRINTK_FLAGS_INCL_MODNAME, 'm' },
- { _DPRINTK_FLAGS_INCL_FUNCNAME, 'f' },
- { _DPRINTK_FLAGS_INCL_LINENO, 'l' },
- { _DPRINTK_FLAGS_INCL_TID, 't' },
- { _DPRINTK_FLAGS_NONE, '_' },
+static struct { unsigned flag:8; char opt_char, not_char; } opt_array[] = {
+ { _DPRINTK_FLAGS_PRINT, 'p', 'P' },
+ { _DPRINTK_FLAGS_INCL_MODNAME, 'm', 'M' },
+ { _DPRINTK_FLAGS_INCL_FUNCNAME, 'f', 'F' },
+ { _DPRINTK_FLAGS_INCL_LINENO, 'l', 'L' },
+ { _DPRINTK_FLAGS_INCL_TID, 't', 'T' },
+ { _DPRINTK_FLAGS_NONE, '_', '_' },
+ { _DPRINTK_FLAGS_USR, 'u', 'U' },
};
/* format a string into buf[] which describes the _ddebug's flags */
@@ -195,6 +196,13 @@ static int ddebug_change(const struct ddebug_query *query,
dp->lineno > query->last_lineno)
continue;
+ /* filter for required flags */
+ if ((dp->flags & filter->flags) != filter->flags)
+ continue;
+ /* filter on prohibited bits */
+ if ((~dp->flags & filter->mask) != filter->mask)
+ continue;
+
nfound++;
newflags = (dp->flags & mods->mask) | mods->flags;
@@ -428,10 +436,17 @@ static int ddebug_read_flags(const char *str, struct flagsettings *f)
if (*str == opt_array[i].opt_char) {
f->flags |= opt_array[i].flag;
break;
+ } else if (*str == opt_array[i].not_char) {
+ f->mask |= opt_array[i].flag;
+ break;
}
}
if (i < 0) {
- pr_err("unknown flag '%c' in \"%s\"\n", *str, str);
+ pr_err("unknown flag '%c'", *str);
+ return -EINVAL;
+ }
+ if (f->flags & f->mask) {
+ pr_err("flag '%c' conflicts with earlier one\n", *str);
return -EINVAL;
}
}
--
2.23.0
drop memset in ddebug_parse_query, instead initialize the stack
variable in the caller.
Signed-off-by: Jim Cromie <[email protected]>
---
lib/dynamic_debug.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index be8299e119ab..26432f88b329 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -373,7 +373,6 @@ static int ddebug_parse_query(char *words[], int nwords,
pr_err("expecting pairs of match-spec <value>\n");
return -EINVAL;
}
- memset(query, 0, sizeof(*query));
if (modname)
/* support $modname.dyndbg=<multiple queries> */
@@ -494,7 +493,7 @@ static int ddebug_exec_query(char *query_string, const char *modname)
{
struct flagsettings mods = {};
struct flagsettings filter = {};
- struct ddebug_query query;
+ struct ddebug_query query = {};
#define MAXWORDS 9
int nwords, nfound;
char *words[MAXWORDS];
--
2.23.0
given that:
~# cat batch-of-dyndbg-cmd-queries > /sys/kernel/debug/dyndbg/control
works, and since '#' is a legal comment character accepted
by >control, the syntax is much more like bash than c++.
So replace '//' with '#'.
Someone might copy-paste these examples, lets make them more usable
Signed-off-by: Jim Cromie <[email protected]>
---
.../admin-guide/dynamic-debug-howto.rst | 51 ++++++++++---------
1 file changed, 27 insertions(+), 24 deletions(-)
diff --git a/Documentation/admin-guide/dynamic-debug-howto.rst b/Documentation/admin-guide/dynamic-debug-howto.rst
index d91dbb52721d..33eed4713bb8 100644
--- a/Documentation/admin-guide/dynamic-debug-howto.rst
+++ b/Documentation/admin-guide/dynamic-debug-howto.rst
@@ -189,11 +189,11 @@ format
characters (``"``) or single quote characters (``'``).
Examples::
- format svcrdma: // many of the NFS/RDMA server pr_debugs
- format readahead // some pr_debugs in the readahead cache
- format nfsd:\040SETATTR // one way to match a format with whitespace
- format "nfsd: SETATTR" // a neater way to match a format with whitespace
- format 'nfsd: SETATTR' // yet another way to match a format with whitespace
+ format svcrdma: # many of the NFS/RDMA server pr_debugs
+ format readahead # some pr_debugs in the readahead cache
+ format nfsd:\040SETATTR # one way to match a format with whitespace
+ format "nfsd: SETATTR" # a neater way to match a format with whitespace
+ format 'nfsd: SETATTR' # yet another way to match a format with whitespace
line
The given line number or range of line numbers is compared
@@ -204,10 +204,10 @@ line
the first line in the file, an empty last line number means the
last line number in the file. Examples::
- line 1603 // exactly line 1603
- line 1600-1605 // the six lines from line 1600 to line 1605
- line -1605 // the 1605 lines from line 1 to line 1605
- line 1600- // all lines from line 1600 to the end of the file
+ line 1603 # exactly line 1603
+ line 1600-1605 # the six lines from line 1600 to line 1605
+ line -1605 # the 1605 lines from line 1 to line 1605
+ line 1600- # all lines from line 1600 to the end of the file
Flags Specification::
@@ -345,44 +345,47 @@ Examples
::
- // enable the message at line 1603 of file svcsock.c
+ # enable the message at line 1603 of file svcsock.c
nullarbor:~ # echo -n 'file svcsock.c line 1603 +p' >
<debugfs>/dyndbg/control
- // enable all the messages in file svcsock.c
+ # enable all the messages in file svcsock.c
nullarbor:~ # echo -n 'file svcsock.c +p' >
<debugfs>/dyndbg/control
- // enable all the messages in the NFS server module
+ # enable all the messages in the NFS server module
nullarbor:~ # echo -n 'module nfsd +p' >
<debugfs>/dyndbg/control
- // enable all 12 messages in the function svc_process()
+ # enable all 12 messages in the function svc_process()
nullarbor:~ # echo -n 'func svc_process +p' >
<debugfs>/dyndbg/control
- // disable all 12 messages in the function svc_process()
+ # disable all 12 messages in the function svc_process()
nullarbor:~ # echo -n 'func svc_process -p' >
<debugfs>/dyndbg/control
- // enable messages for NFS calls READ, READLINK, READDIR and READDIR+.
+ # enable messages for NFS calls READ, READLINK, READDIR and READDIR+.
nullarbor:~ # echo -n 'format "nfsd: READ" +p' >
<debugfs>/dyndbg/control
- // enable messages in files of which the paths include string "usb"
+ # enable messages in files of which the paths include string "usb"
nullarbor:~ # echo -n '*usb* +p' > <debugfs>/dyndbg/control
- // enable all messages
+ # enable all messages
nullarbor:~ # echo -n '+p' > <debugfs>/dyndbg/control
- // add module, function to all enabled messages
+ # add module, function to all enabled messages
nullarbor:~ # echo -n '+mf' > <debugfs>/dyndbg/control
- // boot-args example, with newlines and comments for readability
- Kernel command line: ...
- // see whats going on in dyndbg=value processing
+ # boot-args example, with newlines and comments for readability
+ # Kernel command line: ...
+
+ # see whats going on in dyndbg=value processing
dyndbg.verbose=1
- // enable pr_debugs in 2 builtins, #cmt is stripped
+
+ # enable pr_debugs in 2 builtins, #cmt is stripped
dyndbg="module params +p #cmt ; module sys +p"
- // enable pr_debugs in 2 functions in a module loaded later
- pc87360.dyndbg="func pc87360_init_device +p; func pc87360_find +p"
+
+ # enable pr_debugs in 2 functions in a module loaded later
+ pc87360.dyndbg="func *_init_device +p; func *_find +p"
--
2.23.0
Extend flags modifications to allow [PFMLTU] inverted flags.
This allows control-queries like:
#> Q () { echo file inode.c $* > control } # to type less
#> Q -P # same as +p
#> Q +U # same as -u
#> Q u-P # same as u+p
This allows flags in a callsite to be simultaneously set and cleared,
while still starting with the current flagstate (with +- ops).
Generally, you chose -p or +p 1st, then set or clear flags
accordingly.
Signed-off-by: Jim Cromie <[email protected]>
---
Documentation/admin-guide/dynamic-debug-howto.rst | 10 ++++++----
lib/dynamic_debug.c | 6 ++++--
2 files changed, 10 insertions(+), 6 deletions(-)
diff --git a/Documentation/admin-guide/dynamic-debug-howto.rst b/Documentation/admin-guide/dynamic-debug-howto.rst
index 9f68062ba316..5c170e49121d 100644
--- a/Documentation/admin-guide/dynamic-debug-howto.rst
+++ b/Documentation/admin-guide/dynamic-debug-howto.rst
@@ -249,9 +249,11 @@ only callsites with ``u`` and ``f`` cleared.
Flagsets cannot contain ``pP`` etc, a flag cannot be true and false.
-modflags containing upper-case flags is reserved/undefined for now.
-inverted-flags are currently ignored, usage gets trickier if given
-``-pXy``, it should leave x set.
+modflags may contain upper-case flags also, using these lets you
+invert the flag setting implied by the OP; '-pU' means disable
+printing, and mark that callsite with the user-flag to create a group,
+for optional further manipulation. Generally, '+p' and '-p' is your
+main choice, and use of negating flags in modflags is rare.
Notes::
@@ -261,7 +263,7 @@ For ``print_hex_dump_debug()`` and ``print_hex_dump_bytes()``, only
For display, the flags are preceded by ``=``
(mnemonic: what the flags are currently equal to).
-Note the regexp ``^[-+=][flmptu_]+$`` matches a flags specification.
+Note the regexp ``/^[-+=][flmptu_]+$/i`` matches a flags specification.
To clear all flags at once, use ``=_`` or ``-flmptu``.
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 736895efe17d..15bb9939df97 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -486,15 +486,17 @@ static int ddebug_parse_flags(const char *str,
/* calculate final mods: flags, mask based upon op */
switch (op) {
+ unsigned int tmp;
case '=':
mods->mask = 0;
break;
case '+':
- mods->mask = ~0U;
+ mods->mask = ~mods->mask;
break;
case '-':
+ tmp = mods->mask;
mods->mask = ~mods->flags;
- mods->flags = 0;
+ mods->flags = tmp;
break;
}
--
2.23.0
Extend flags modifications to allow [PFMLT_XYZ] inverted flags.
This allows control-queries like:
#> Q () { echo file inode.c $* > control } # to type less
#> Q -P # same as +p
#> Q +X # same as -x
#> Q xyz-P # same as xyz+p
This allows flags in a callsite to be simultaneously set and cleared,
while still starting with the current flagstate (with +- ops).
Generally, you chose -p or +p 1st, then set or clear flags
accordingly.
# enable print on callsites with 'xy'; and re-mark with just 'z'
#> Q xy+pXYz
Signed-off-by: Jim Cromie <[email protected]>
---
Documentation/admin-guide/dynamic-debug-howto.rst | 8 +++++---
lib/dynamic_debug.c | 6 ++++--
2 files changed, 9 insertions(+), 5 deletions(-)
diff --git a/Documentation/admin-guide/dynamic-debug-howto.rst b/Documentation/admin-guide/dynamic-debug-howto.rst
index 5404e23eeac8..493e74a14bdd 100644
--- a/Documentation/admin-guide/dynamic-debug-howto.rst
+++ b/Documentation/admin-guide/dynamic-debug-howto.rst
@@ -250,9 +250,11 @@ only callsites with x&y cleared.
Flagsets cannot contain ``xX`` etc, a flag cannot be true and false.
-modflags containing upper-case flags is reserved/undefined for now.
-inverted-flags are currently ignored, usage gets trickier if given
-``-pXy``, it should leave x set.
+modflags may contain upper-case flags also, using these lets you
+invert the flag setting implied by the OP; '-pX' means disable
+printing, and mark that callsite with usr-x flag to create a group,
+for optional further manipulation. Generally, '+p' and '-p' is your
+main choice, and use of inverted flags in modflags is rare.
Notes::
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index b2630df0c3a5..82daf95b8f64 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -488,15 +488,17 @@ static int ddebug_parse_flags(const char *str,
/* calculate final mods: flags, mask based upon op */
switch (op) {
+ unsigned int tmp;
case '=':
mods->mask = 0;
break;
case '+':
- mods->mask = ~0U;
+ mods->mask = ~mods->mask;
break;
case '-':
+ tmp = mods->mask;
mods->mask = ~mods->flags;
- mods->flags = 0;
+ mods->flags = tmp;
break;
}
--
2.23.0
loadable modules are the last in, and are the only modules that could
be removed. ddebug_remove_module() searches from head, but
ddebug_add_module() uses list_add_tail(). Change it to list_add() for
a micro-optimization.
Signed-off-by: Jim Cromie <[email protected]>
---
lib/dynamic_debug.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 15bb9939df97..d056fca96b9d 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -958,7 +958,7 @@ int ddebug_add_module(struct _ddebug *tab, unsigned int n,
dt->ddebugs = tab;
mutex_lock(&ddebug_lock);
- list_add_tail(&dt->link, &ddebug_tables);
+ list_add(&dt->link, &ddebug_tables);
mutex_unlock(&ddebug_lock);
vpr_info("%u debug prints in module %s\n", n, dt->mod_name);
--
2.23.0
change ddebug_parse_flags to accept /^ filterflags? OP modflags /x, as
well as the currently accepted /^ OP modflags /.
Signed-off-by: Jim Cromie <[email protected]>
---
.../admin-guide/dynamic-debug-howto.rst | 18 +++++++----
lib/dynamic_debug.c | 30 ++++++++++---------
2 files changed, 28 insertions(+), 20 deletions(-)
diff --git a/Documentation/admin-guide/dynamic-debug-howto.rst b/Documentation/admin-guide/dynamic-debug-howto.rst
index 689a30316589..cdc45dcb3e0c 100644
--- a/Documentation/admin-guide/dynamic-debug-howto.rst
+++ b/Documentation/admin-guide/dynamic-debug-howto.rst
@@ -209,13 +209,19 @@ 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::
- - remove the given flags
- + add the given flags
- = set the flags to the given flags
+ flagspec ::= filterflags? OP modflags
+ filterflags ::= flagset
+ modflags ::= flagset
+ flagset ::= ([pfmlt_xyz] | [PFMLT_XYZ])+
+ OP ::= [-+=]
+
+OP: modify callsites per following flagset::
+
+ - remove the following flags
+ + add the following flags
+ = set the flags to the following flags
The flags are::
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 8c62c76badcf..be8299e119ab 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -441,34 +441,36 @@ static int ddebug_read_flags(const char *str, struct flagsettings *f)
}
/*
- * 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.
+ * Parse `str' as a flags-spec, ie: [pfmlt_]*[-+=][pfmlt_]+
+ * Fills flagsettings provided. Returns 0 on success or <0 on error.
*/
-
static int ddebug_parse_flags(const char *str,
struct flagsettings *mods,
struct flagsettings *filter)
{
int op;
+ char *opp = strpbrk(str, "-+=");
- switch (*str) {
- case '+':
- case '-':
- case '=':
- op = *str++;
- break;
- default:
- pr_err("bad flag-op %c, at start of %s\n", *str, str);
+ if (!opp) {
+ pr_err("no OP given in %s\n", str);
return -EINVAL;
}
+ op = *opp;
vpr_info("op='%c'\n", op);
+ if (opp != str) {
+ /* filterflags precedes OP, grab it */
+ *opp++ = '\0';
+ if (ddebug_read_flags(str, filter))
+ return -EINVAL;
+ str = opp;
+ } else
+ str++;
+
if (ddebug_read_flags(str, mods))
return -EINVAL;
- /* calculate final flags, mask based upon op */
+ /* calculate final mods: flags, mask based upon op */
switch (op) {
case '=':
mods->mask = 0;
--
2.23.0
1. Add 3 user-flags [xyz] which work like original [pfmlt] flags, but
have no effect on callsite behavior; they allow marking of arbitrary
sets of callsites. Just adding the flag defs themselves is enough,
they inherit the existing flags mechanics.
2. Add [PFMLT],[XYZ] flags, which invert their counterparts; P===!p etc.
And in ddebug_read_flags():
current code does: [pfmlt_xyz] -> flags
copy it to: [PFMLT_XYZ] -> mask
also disallow both of a pair: ie no 'xX', no true & false.
3. Add filtering ops into ddebug_change(), right after all the
callsite-property selections are complete. These test the callsite's
current flagstate before applying modflags.
Why ?
The 3 new/user flags facilitate batching of changes. By marking
individual callsites with 'xyz', user can compose an arbitrary set of
changes, then activate them together by selecting on 'xyz':
#> echo 'file foo.c +xyz; file bar.c +xyz' > control
#> echo 'xyz+p' > control
These user flags aren't strictly needed, but with them you can avoid
using [fmlt] flags for marking, which would alter logging.
Signed-off-by: Jim Cromie <[email protected]>
---
.../admin-guide/dynamic-debug-howto.rst | 28 ++++++++++++++--
include/linux/dynamic_debug.h | 3 ++
lib/dynamic_debug.c | 33 ++++++++++++++-----
3 files changed, 54 insertions(+), 10 deletions(-)
diff --git a/Documentation/admin-guide/dynamic-debug-howto.rst b/Documentation/admin-guide/dynamic-debug-howto.rst
index cdc45dcb3e0c..5404e23eeac8 100644
--- a/Documentation/admin-guide/dynamic-debug-howto.rst
+++ b/Documentation/admin-guide/dynamic-debug-howto.rst
@@ -231,9 +231,33 @@ The flags are::
m Include module name in the printed message
t Include thread ID in messages not generated from interrupt context
_ No flags are set. (Or'd with others on input)
+ x user flag, to mark callsites into a group
+ y user flag, ...
+ z user flag, ...
-For ``print_hex_dump_debug()`` and ``print_hex_dump_bytes()``, only ``p`` flag
-have meaning, other flags ignored.
+Additionally, the flags above have upper-case versions, which invert
+their respective meanings. Their use follows.
+
+Using Filters::
+
+Filter-flags specify an optional additional selector on pr_debug
+callsites; with them you can compose an arbitrary set of callsites, by
+iteratively marking them with ``+xyz``, then enabling them all with
+``xyz+p``.
+
+Filters can also contain upper-case flags, like ``XY``, which select
+only callsites with x&y cleared.
+
+Flagsets cannot contain ``xX`` etc, a flag cannot be true and false.
+
+modflags containing upper-case flags is reserved/undefined for now.
+inverted-flags are currently ignored, usage gets trickier if given
+``-pXy``, it should leave x set.
+
+Notes::
+
+For ``print_hex_dump_debug()`` and ``print_hex_dump_bytes()``, only
+``p`` flag has meaning, other flags are ignored.
For display, the flags are preceded by ``=``
(mnemonic: what the flags are currently equal to).
diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index 802480ea8708..0d7c9a3538b6 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -32,6 +32,9 @@ 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_USR_X (1<<5)
+#define _DPRINTK_FLAGS_USR_Y (1<<6)
+#define _DPRINTK_FLAGS_USR_Z (1<<7)
#if defined DEBUG
#define _DPRINTK_FLAGS_DEFAULT _DPRINTK_FLAGS_PRINT
#else
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 26432f88b329..b2630df0c3a5 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -85,13 +85,16 @@ static inline const char *trim_prefix(const char *path)
return path + skip;
}
-static struct { unsigned flag:8; char opt_char; } opt_array[] = {
- { _DPRINTK_FLAGS_PRINT, 'p' },
- { _DPRINTK_FLAGS_INCL_MODNAME, 'm' },
- { _DPRINTK_FLAGS_INCL_FUNCNAME, 'f' },
- { _DPRINTK_FLAGS_INCL_LINENO, 'l' },
- { _DPRINTK_FLAGS_INCL_TID, 't' },
- { _DPRINTK_FLAGS_NONE, '_' },
+static struct { unsigned flag:8; char opt_char, not_char; } opt_array[] = {
+ { _DPRINTK_FLAGS_PRINT, 'p', 'P' },
+ { _DPRINTK_FLAGS_INCL_MODNAME, 'm', 'M' },
+ { _DPRINTK_FLAGS_INCL_FUNCNAME, 'f', 'F' },
+ { _DPRINTK_FLAGS_INCL_LINENO, 'l', 'L' },
+ { _DPRINTK_FLAGS_INCL_TID, 't', 'T' },
+ { _DPRINTK_FLAGS_NONE, '_', '_' },
+ { _DPRINTK_FLAGS_USR_X, 'x', 'X' },
+ { _DPRINTK_FLAGS_USR_Y, 'y', 'Y' },
+ { _DPRINTK_FLAGS_USR_Z, 'z', 'Z' },
};
/* format a string into buf[] which describes the _ddebug's flags */
@@ -195,6 +198,13 @@ static int ddebug_change(const struct ddebug_query *query,
dp->lineno > query->last_lineno)
continue;
+ /* filter for required flags */
+ if ((dp->flags & filter->flags) != filter->flags)
+ continue;
+ /* filter on prohibited bits */
+ if ((~dp->flags & filter->mask) != filter->mask)
+ continue;
+
nfound++;
newflags = (dp->flags & mods->mask) | mods->flags;
@@ -428,10 +438,17 @@ static int ddebug_read_flags(const char *str, struct flagsettings *f)
if (*str == opt_array[i].opt_char) {
f->flags |= opt_array[i].flag;
break;
+ } else if (*str == opt_array[i].not_char) {
+ f->mask |= opt_array[i].flag;
+ break;
}
}
if (i < 0) {
- pr_err("unknown flag '%c' in \"%s\"\n", *str, str);
+ pr_err("unknown flag '%c'", *str);
+ return -EINVAL;
+ }
+ if (f->flags & f->mask) {
+ pr_err("flag '%c' conflicts with earlier one\n", *str);
return -EINVAL;
}
}
--
2.23.0
This rename fixes a subtle usage wrinkle; the __setup() names didn't
match the fake "dyndbg" module parameter used to enable dynamic-printk
callsites in modules. See the last change in Docs for the effect.
It also shortens the "__FILE__:__func__" prefix in dyndbg.verbose
messages, effectively s/dynamic_debug/dyndbg/
This is a 99.9% rename; trim_prefix and debugfs_create_dir arg excepted.
Nonetheless, it also changes both /sys appearances:
bash-5.0# ls -R /sys/kernel/debug/dyndbg/ /sys/module/dyndbg/parameters/
/sys/kernel/debug/dyndbg/:
control
/sys/module/dyndbg/parameters/:
verbose
Finally, paths in docs are ~= s|/dynamic_debug/|/dyndbg/|,
plus the kernel cmdline example tweak cited above.
Signed-off-by: Jim Cromie <[email protected]>
---
.../admin-guide/dynamic-debug-howto.rst | 50 +++++++++----------
lib/Makefile | 4 +-
lib/{dynamic_debug.c => dyndbg.c} | 4 +-
3 files changed, 29 insertions(+), 29 deletions(-)
rename lib/{dynamic_debug.c => dyndbg.c} (99%)
diff --git a/Documentation/admin-guide/dynamic-debug-howto.rst b/Documentation/admin-guide/dynamic-debug-howto.rst
index 5c170e49121d..d91dbb52721d 100644
--- a/Documentation/admin-guide/dynamic-debug-howto.rst
+++ b/Documentation/admin-guide/dynamic-debug-howto.rst
@@ -31,7 +31,7 @@ Dynamic debug has even more useful features:
- module name
- format string
- * Provides a debugfs control file: ``<debugfs>/dynamic_debug/control``
+ * Provides a debugfs control file: ``<debugfs>/dyndbg/control``
which can be read to display the complete list of known debug
statements, to help guide you
@@ -42,16 +42,16 @@ The behaviour of ``pr_debug()``/``dev_dbg()`` 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
+``<debugfs>/dyndbg/control``. For example, 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
+ <debugfs>/dyndbg/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
+ <debugfs>/dyndbg/control
-bash: echo: write error: Invalid argument
Viewing Dynamic Debug Behaviour
@@ -60,7 +60,7 @@ Viewing Dynamic Debug Behaviour
You can view the currently configured behaviour of all the debug
statements via::
- nullarbor:~ # cat <debugfs>/dynamic_debug/control
+ nullarbor:~ # cat <debugfs>/dyndbg/control
# filename:lineno [module]function flags format
net/sunrpc/svc_rdma.c:323 [svcxprt_rdma]svc_rdma_cleanup =_ "SVCRDMA Module Removed, deregister RPC RDMA transport\012"
net/sunrpc/svc_rdma.c:341 [svcxprt_rdma]svc_rdma_init =_ "\011max_inline : %d\012"
@@ -72,10 +72,10 @@ statements via::
You can also apply standard Unix text manipulation filters to this
data, e.g.::
- nullarbor:~ # grep -i rdma <debugfs>/dynamic_debug/control | wc -l
+ nullarbor:~ # grep -i rdma <debugfs>/dyndbg/control | wc -l
62
- nullarbor:~ # grep -i tcp <debugfs>/dynamic_debug/control | wc -l
+ nullarbor:~ # grep -i tcp <debugfs>/dyndbg/control | wc -l
42
The third column shows the currently enabled flags for each debug
@@ -83,7 +83,7 @@ statement callsite (see below for definitions of the flags). The
default value, with no flags enabled, is ``=_``. So you can view all
the debug statement callsites with any non-default flags::
- nullarbor:~ # awk '$3 != "=_"' <debugfs>/dynamic_debug/control
+ nullarbor:~ # awk '$3 != "=_"' <debugfs>/dyndbg/control
# filename:lineno [module]function flags format
net/sunrpc/svcsock.c:1603 [sunrpc]svc_send p "svc_process: st_sendto returned %d\012"
@@ -94,27 +94,27 @@ At the lexical level, a command comprises a sequence of words separated
by spaces or tabs. So these are all equivalent::
nullarbor:~ # echo -n 'file svcsock.c line 1603 +p' >
- <debugfs>/dynamic_debug/control
+ <debugfs>/dyndbg/control
nullarbor:~ # echo -n ' file svcsock.c line 1603 +p ' >
- <debugfs>/dynamic_debug/control
+ <debugfs>/dyndbg/control
nullarbor:~ # echo -n 'file svcsock.c line 1603 +p' >
- <debugfs>/dynamic_debug/control
+ <debugfs>/dyndbg/control
Command submissions are bounded by a write() system call.
Multiple commands can be written together, separated by ``;`` or ``\n``::
~# echo "func pnpacpi_get_resources +p; func pnp_assign_mem +p" \
- > <debugfs>/dynamic_debug/control
+ > <debugfs>/dyndbg/control
If your query set is big, you can batch them too::
- ~# cat query-batch-file > <debugfs>/dynamic_debug/control
+ ~# cat query-batch-file > <debugfs>/dyndbg/control
Another way is to use wildcards. The match rule supports ``*`` (matches
zero or more characters) and ``?`` (matches exactly one character). For
example, you can match all usb drivers::
- ~# echo "file drivers/usb/* +p" > <debugfs>/dynamic_debug/control
+ ~# echo "file drivers/usb/* +p" > <debugfs>/dyndbg/control
At the syntactical level, a command comprises a sequence of match
specifications, followed by a flags change specification::
@@ -338,7 +338,7 @@ For ``CONFIG_DYNAMIC_DEBUG`` kernels, any settings given at boot-time (or
enabled by ``-DDEBUG`` flag during compilation) can be disabled later via
the debugfs interface if the debug messages are no longer needed::
- echo "module module_name -p" > <debugfs>/dynamic_debug/control
+ echo "module module_name -p" > <debugfs>/dyndbg/control
Examples
========
@@ -347,41 +347,41 @@ Examples
// enable the message at line 1603 of file svcsock.c
nullarbor:~ # echo -n 'file svcsock.c line 1603 +p' >
- <debugfs>/dynamic_debug/control
+ <debugfs>/dyndbg/control
// enable all the messages in file svcsock.c
nullarbor:~ # echo -n 'file svcsock.c +p' >
- <debugfs>/dynamic_debug/control
+ <debugfs>/dyndbg/control
// enable all the messages in the NFS server module
nullarbor:~ # echo -n 'module nfsd +p' >
- <debugfs>/dynamic_debug/control
+ <debugfs>/dyndbg/control
// enable all 12 messages in the function svc_process()
nullarbor:~ # echo -n 'func svc_process +p' >
- <debugfs>/dynamic_debug/control
+ <debugfs>/dyndbg/control
// disable all 12 messages in the function svc_process()
nullarbor:~ # echo -n 'func svc_process -p' >
- <debugfs>/dynamic_debug/control
+ <debugfs>/dyndbg/control
// enable messages for NFS calls READ, READLINK, READDIR and READDIR+.
nullarbor:~ # echo -n 'format "nfsd: READ" +p' >
- <debugfs>/dynamic_debug/control
+ <debugfs>/dyndbg/control
// enable messages in files of which the paths include string "usb"
- nullarbor:~ # echo -n '*usb* +p' > <debugfs>/dynamic_debug/control
+ nullarbor:~ # echo -n '*usb* +p' > <debugfs>/dyndbg/control
// enable all messages
- nullarbor:~ # echo -n '+p' > <debugfs>/dynamic_debug/control
+ nullarbor:~ # echo -n '+p' > <debugfs>/dyndbg/control
// add module, function to all enabled messages
- nullarbor:~ # echo -n '+mf' > <debugfs>/dynamic_debug/control
+ nullarbor:~ # echo -n '+mf' > <debugfs>/dyndbg/control
// boot-args example, with newlines and comments for readability
Kernel command line: ...
// see whats going on in dyndbg=value processing
- dynamic_debug.verbose=1
+ dyndbg.verbose=1
// enable pr_debugs in 2 builtins, #cmt is stripped
dyndbg="module params +p #cmt ; module sys +p"
// enable pr_debugs in 2 functions in a module loaded later
diff --git a/lib/Makefile b/lib/Makefile
index c5892807e06f..ea4028aa7852 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -15,7 +15,7 @@ KCOV_INSTRUMENT_string.o := n
KCOV_INSTRUMENT_rbtree.o := n
KCOV_INSTRUMENT_list_debug.o := n
KCOV_INSTRUMENT_debugobjects.o := n
-KCOV_INSTRUMENT_dynamic_debug.o := n
+KCOV_INSTRUMENT_dyndbg.o := n
# Early boot use of cmdline, don't instrument it
ifdef CONFIG_AMD_MEM_ENCRYPT
@@ -180,7 +180,7 @@ lib-$(CONFIG_GENERIC_BUG) += bug.o
obj-$(CONFIG_HAVE_ARCH_TRACEHOOK) += syscall.o
-obj-$(CONFIG_DYNAMIC_DEBUG) += dynamic_debug.o
+obj-$(CONFIG_DYNAMIC_DEBUG) += dyndbg.o
obj-$(CONFIG_NLATTR) += nlattr.o
diff --git a/lib/dynamic_debug.c b/lib/dyndbg.c
similarity index 99%
rename from lib/dynamic_debug.c
rename to lib/dyndbg.c
index d056fca96b9d..f410d341841a 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dyndbg.c
@@ -77,7 +77,7 @@ module_param(verbose, int, 0644);
/* 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");
+ int skip = strlen(__FILE__) - strlen("lib/dyndbg.c");
if (strncmp(path, __FILE__, skip))
skip = 0; /* prefix mismatch, don't skip */
@@ -1055,7 +1055,7 @@ static int __init dynamic_debug_init_debugfs(void)
if (!ddebug_init_success)
return -ENODEV;
- dir = debugfs_create_dir("dynamic_debug", NULL);
+ dir = debugfs_create_dir("dyndbg", NULL);
debugfs_create_file("control", 0644, dir, NULL, &ddebug_proc_fops);
return 0;
--
2.23.0
during dyndbg init, verbose logging prints its ram overhead. It
counted strlens of struct _ddebug's 4 string members, in all callsite
entries, which would be approximately correct if each had been
mallocd. But they are pointers into shared .rodata; for example, all
10 kobject callsites have identical filename, module values.
Its best not to count that memory at all, since we cannot know they
were linked in because of CONFIG_DYNAMIC_DEBUG=y, and we want to
report a number that reflects what ram is saved by deconfiguring it.
Also fix wording and size under-reporting of the __dyndbg section.
Heres my overhead, on a virtme-run VM on a fedora-31 laptop:
dynamic_debug:dynamic_debug_init: 260 modules, 2479 entries \
and 10400 bytes in ddebug tables, 138824 bytes in __dyndbg section
Signed-off-by: Jim Cromie <[email protected]>
---
lib/dynamic_debug.c | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 0a4588fe342e..b5fb0aa0fbc3 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -1008,7 +1008,6 @@ static int __init dynamic_debug_init(void)
char *cmdline;
int ret = 0;
int n = 0, entries = 0, modct = 0;
- int verbose_bytes = 0;
if (__start___dyndbg == __stop___dyndbg) {
pr_warn("_ddebug table is empty in a CONFIG_DYNAMIC_DEBUG build\n");
@@ -1019,9 +1018,6 @@ static int __init dynamic_debug_init(void)
iter_start = iter;
for (; iter < __stop___dyndbg; iter++) {
entries++;
- verbose_bytes += strlen(iter->modname) + strlen(iter->function)
- + strlen(iter->filename) + strlen(iter->format);
-
if (strcmp(modname, iter->modname)) {
modct++;
ret = ddebug_add_module(iter_start, n, modname);
@@ -1038,9 +1034,9 @@ static int __init dynamic_debug_init(void)
goto out_err;
ddebug_init_success = 1;
- vpr_info("%d modules, %d entries and %d bytes in ddebug tables, %d bytes in (readonly) verbose section\n",
+ vpr_info("%d modules, %d entries and %d bytes in ddebug tables, %d bytes in __dyndbg section\n",
modct, entries, (int)(modct * sizeof(struct ddebug_table)),
- verbose_bytes + (int)(__stop___dyndbg - __start___dyndbg));
+ (int)(entries * sizeof(struct _ddebug)));
/* apply ddebug_query boot param, dont unload tables on err */
if (ddebug_setup_string[0] != '\0') {
--
2.23.0
On Thu, Dec 5, 2019 at 11:54 PM Jim Cromie <[email protected]> wrote:
>
> This rename fixes a subtle usage wrinkle; the __setup() names didn't
> match the fake "dyndbg" module parameter used to enable dynamic-printk
> callsites in modules. See the last change in Docs for the effect.
>
> It also shortens the "__FILE__:__func__" prefix in dyndbg.verbose
> messages, effectively s/dynamic_debug/dyndbg/
>
> This is a 99.9% rename; trim_prefix and debugfs_create_dir arg excepted.
> Nonetheless, it also changes both /sys appearances:
>
> bash-5.0# ls -R /sys/kernel/debug/dyndbg/ /sys/module/dyndbg/parameters/
> /sys/kernel/debug/dyndbg/:
> control
> /sys/module/dyndbg/parameters/:
Isn't this path a part of ABI?
> verbose
>
> Finally, paths in docs are ~= s|/dynamic_debug/|/dyndbg/|,
> plus the kernel cmdline example tweak cited above.
--
With Best Regards,
Andy Shevchenko
On 05/12/2019 23.24, Andy Shevchenko wrote:
> On Thu, Dec 5, 2019 at 11:54 PM Jim Cromie <[email protected]> wrote:
>>
>> This rename fixes a subtle usage wrinkle; the __setup() names didn't
>> match the fake "dyndbg" module parameter used to enable dynamic-printk
>> callsites in modules. See the last change in Docs for the effect.
>>
>> It also shortens the "__FILE__:__func__" prefix in dyndbg.verbose
>> messages, effectively s/dynamic_debug/dyndbg/
>>
>> This is a 99.9% rename; trim_prefix and debugfs_create_dir arg excepted.
>> Nonetheless, it also changes both /sys appearances:
>>
>> bash-5.0# ls -R /sys/kernel/debug/dyndbg/ /sys/module/dyndbg/parameters/
>> /sys/kernel/debug/dyndbg/:
>> control
>
>> /sys/module/dyndbg/parameters/:
>
> Isn't this path a part of ABI?
Yeah, I think this is a somewhat dangerous change, and I don't really
see the point.
Unrelated: Jim, if you want these patches picked up eventually, you have
to put akpm on the recipient list (he is on this one, but AFAICT not on
any of the others).
Rasmus
On Fri, Dec 6, 2019 at 12:49 AM Rasmus Villemoes
<[email protected]> wrote:
>
> On 05/12/2019 23.24, Andy Shevchenko wrote:
> > On Thu, Dec 5, 2019 at 11:54 PM Jim Cromie <[email protected]> wrote:
> >
..
> >> /sys/module/dyndbg/parameters/:
> >
> > Isn't this path a part of ABI?
>
> Yeah, I think this is a somewhat dangerous change, and I don't really
> see the point.
OK, I didnt realize this was ABI.
I withdraw that patch, and will fix the following one.
>
> Unrelated: Jim, if you want these patches picked up eventually, you have
> to put akpm on the recipient list (he is on this one, but AFAICT not on
> any of the others).
>
Oof.
GregKH has in the past picked up my dyndbg stuff, and Jason's too
but that was 7 years ago
> Rasmus
thanks,
JIm