* [PATCH v4 01/16] dyndbg-docs: eschew file /full/path query in docs
2019-12-10 2:27 [PATCH v4 00/16] dynamic-debug cleanups, 2 new features Jim Cromie
@ 2019-12-10 2:27 ` Jim Cromie
2019-12-10 2:27 ` [PATCH v4 02/16] dyndbg: drop obsolete comment on ddebug_proc_open Jim Cromie
` (14 subsequent siblings)
15 siblings, 0 replies; 19+ messages in thread
From: Jim Cromie @ 2019-12-10 2:27 UTC (permalink / raw)
To: jbaron, linux-kernel, akpm
Cc: gregkh, linux, Jim Cromie, Jonathan Corbet, linux-doc
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 <jim.cromie@gmail.com>
---
.../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
^ permalink raw reply [flat|nested] 19+ messages in thread* [PATCH v4 02/16] dyndbg: drop obsolete comment on ddebug_proc_open
2019-12-10 2:27 [PATCH v4 00/16] dynamic-debug cleanups, 2 new features Jim Cromie
2019-12-10 2:27 ` [PATCH v4 01/16] dyndbg-docs: eschew file /full/path query in docs Jim Cromie
@ 2019-12-10 2:27 ` Jim Cromie
2019-12-10 2:27 ` [PATCH v4 03/16] dyndbg: raise verbosity needed to enable ddebug_proc_* logging Jim Cromie
` (13 subsequent siblings)
15 siblings, 0 replies; 19+ messages in thread
From: Jim Cromie @ 2019-12-10 2:27 UTC (permalink / raw)
To: jbaron, linux-kernel, akpm; +Cc: gregkh, linux, Jim Cromie
commit 4bad78c55002 ("lib/dynamic_debug.c: use seq_open_private() instead of seq_open()")'
The commit was one of a tree-wide set which replaced open-coded
boilerplate with a single tail-call. It therefore obsoleted the
comment about that boilerplate, clean that up now.
Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
lib/dynamic_debug.c | 7 -------
1 file changed, 7 deletions(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index c60409138e13..6cefceffadcb 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -853,13 +853,6 @@ static const struct seq_operations ddebug_proc_seqops = {
.stop = ddebug_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
- * _ddebugs. Note that we create a seq_file always, even for O_WRONLY
- * files where it's not needed, as doing so simplifies the ->release
- * method.
- */
static int ddebug_proc_open(struct inode *inode, struct file *file)
{
vpr_info("called\n");
--
2.23.0
^ permalink raw reply [flat|nested] 19+ messages in thread* [PATCH v4 03/16] dyndbg: raise verbosity needed to enable ddebug_proc_* logging
2019-12-10 2:27 [PATCH v4 00/16] dynamic-debug cleanups, 2 new features Jim Cromie
2019-12-10 2:27 ` [PATCH v4 01/16] dyndbg-docs: eschew file /full/path query in docs Jim Cromie
2019-12-10 2:27 ` [PATCH v4 02/16] dyndbg: drop obsolete comment on ddebug_proc_open Jim Cromie
@ 2019-12-10 2:27 ` Jim Cromie
2019-12-10 2:27 ` [PATCH v4 04/16] dyndbg: rename __verbose section to __dyndbg Jim Cromie
` (12 subsequent siblings)
15 siblings, 0 replies; 19+ messages in thread
From: Jim Cromie @ 2019-12-10 2:27 UTC (permalink / raw)
To: jbaron, linux-kernel, akpm; +Cc: gregkh, linux, Jim Cromie
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 <jim.cromie@gmail.com>
---
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
^ permalink raw reply [flat|nested] 19+ messages in thread* [PATCH v4 04/16] dyndbg: rename __verbose section to __dyndbg
2019-12-10 2:27 [PATCH v4 00/16] dynamic-debug cleanups, 2 new features Jim Cromie
` (2 preceding siblings ...)
2019-12-10 2:27 ` [PATCH v4 03/16] dyndbg: raise verbosity needed to enable ddebug_proc_* logging Jim Cromie
@ 2019-12-10 2:27 ` Jim Cromie
2019-12-10 2:27 ` [PATCH v4 05/16] dyndbg: fix overcounting of ram used by dyndbg Jim Cromie
` (11 subsequent siblings)
15 siblings, 0 replies; 19+ messages in thread
From: Jim Cromie @ 2019-12-10 2:27 UTC (permalink / raw)
To: jbaron, linux-kernel, akpm
Cc: gregkh, linux, Jim Cromie, Arnd Bergmann, Jessica Yu, linux-arch
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 <jim.cromie@gmail.com>
---
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
^ permalink raw reply [flat|nested] 19+ messages in thread* [PATCH v4 05/16] dyndbg: fix overcounting of ram used by dyndbg
2019-12-10 2:27 [PATCH v4 00/16] dynamic-debug cleanups, 2 new features Jim Cromie
` (3 preceding siblings ...)
2019-12-10 2:27 ` [PATCH v4 04/16] dyndbg: rename __verbose section to __dyndbg Jim Cromie
@ 2019-12-10 2:27 ` Jim Cromie
2019-12-10 2:27 ` [PATCH v4 06/16] dyndbg: fix a BUG_ON in ddebug_describe_flags Jim Cromie
` (10 subsequent siblings)
15 siblings, 0 replies; 19+ messages in thread
From: Jim Cromie @ 2019-12-10 2:27 UTC (permalink / raw)
To: jbaron, linux-kernel, akpm; +Cc: gregkh, linux, Jim Cromie
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 <jim.cromie@gmail.com>
---
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
^ permalink raw reply [flat|nested] 19+ messages in thread* [PATCH v4 06/16] dyndbg: fix a BUG_ON in ddebug_describe_flags
2019-12-10 2:27 [PATCH v4 00/16] dynamic-debug cleanups, 2 new features Jim Cromie
` (4 preceding siblings ...)
2019-12-10 2:27 ` [PATCH v4 05/16] dyndbg: fix overcounting of ram used by dyndbg Jim Cromie
@ 2019-12-10 2:27 ` Jim Cromie
2019-12-10 19:58 ` Jason Baron
2019-12-10 2:27 ` [PATCH v4 07/16] dyndbg: refactor parse_linerange out of ddebug_parse_query Jim Cromie
` (9 subsequent siblings)
15 siblings, 1 reply; 19+ messages in thread
From: Jim Cromie @ 2019-12-10 2:27 UTC (permalink / raw)
To: jbaron, linux-kernel, akpm; +Cc: gregkh, linux, Jim Cromie, kbuild test robot
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 <lkp@intel.com>
Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
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
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH v4 06/16] dyndbg: fix a BUG_ON in ddebug_describe_flags
2019-12-10 2:27 ` [PATCH v4 06/16] dyndbg: fix a BUG_ON in ddebug_describe_flags Jim Cromie
@ 2019-12-10 19:58 ` Jason Baron
2019-12-13 1:35 ` jim.cromie
0 siblings, 1 reply; 19+ messages in thread
From: Jason Baron @ 2019-12-10 19:58 UTC (permalink / raw)
To: Jim Cromie, linux-kernel, akpm; +Cc: gregkh, linux, kbuild test robot
On 12/9/19 9:27 PM, Jim Cromie wrote:
> 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 <lkp@intel.com>
>
> Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
> ---
> 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 */
ARRAY_SIZE(opt_array) + 1
max number of flags + string termination.
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH v4 06/16] dyndbg: fix a BUG_ON in ddebug_describe_flags
2019-12-10 19:58 ` Jason Baron
@ 2019-12-13 1:35 ` jim.cromie
0 siblings, 0 replies; 19+ messages in thread
From: jim.cromie @ 2019-12-13 1:35 UTC (permalink / raw)
To: Jason Baron; +Cc: LKML, akpm, Greg KH, Rasmus Villemoes, kbuild test robot
On Tue, Dec 10, 2019 at 12:58 PM Jason Baron <jbaron@akamai.com> wrote:
> >
> > +struct flagsbuf { char buf[12]; }; /* big enough to hold all the flags */
>
>
> ARRAY_SIZE(opt_array) + 1
>
> max number of flags + string termination.
>
Ok 12 is generous.
v2 had xyz flags, for a total of 9+1 flag chars.
figured a multiple of 4 was sensible, almost went to 16
then didnt think about it anymore.
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v4 07/16] dyndbg: refactor parse_linerange out of ddebug_parse_query
2019-12-10 2:27 [PATCH v4 00/16] dynamic-debug cleanups, 2 new features Jim Cromie
` (5 preceding siblings ...)
2019-12-10 2:27 ` [PATCH v4 06/16] dyndbg: fix a BUG_ON in ddebug_describe_flags Jim Cromie
@ 2019-12-10 2:27 ` Jim Cromie
2019-12-10 2:27 ` [PATCH v4 08/16] dyndbg: accept 'file foo.c:func1' and 'file foo.c:10-100' Jim Cromie
` (8 subsequent siblings)
15 siblings, 0 replies; 19+ messages in thread
From: Jim Cromie @ 2019-12-10 2:27 UTC (permalink / raw)
To: jbaron, linux-kernel, akpm; +Cc: gregkh, linux, Jim Cromie
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 <jim.cromie@gmail.com>
---
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
^ permalink raw reply [flat|nested] 19+ messages in thread* [PATCH v4 08/16] dyndbg: accept 'file foo.c:func1' and 'file foo.c:10-100'
2019-12-10 2:27 [PATCH v4 00/16] dynamic-debug cleanups, 2 new features Jim Cromie
` (6 preceding siblings ...)
2019-12-10 2:27 ` [PATCH v4 07/16] dyndbg: refactor parse_linerange out of ddebug_parse_query Jim Cromie
@ 2019-12-10 2:27 ` Jim Cromie
2019-12-10 2:27 ` [PATCH v4 09/16] dyndbg: refactor ddebug_read_flags out of ddebug_parse_flags Jim Cromie
` (7 subsequent siblings)
15 siblings, 0 replies; 19+ messages in thread
From: Jim Cromie @ 2019-12-10 2:27 UTC (permalink / raw)
To: jbaron, linux-kernel, akpm
Cc: gregkh, linux, Jim Cromie, Jonathan Corbet, linux-doc
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 <jim.cromie@gmail.com>
---
.../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
^ permalink raw reply [flat|nested] 19+ messages in thread* [PATCH v4 09/16] dyndbg: refactor ddebug_read_flags out of ddebug_parse_flags
2019-12-10 2:27 [PATCH v4 00/16] dynamic-debug cleanups, 2 new features Jim Cromie
` (7 preceding siblings ...)
2019-12-10 2:27 ` [PATCH v4 08/16] dyndbg: accept 'file foo.c:func1' and 'file foo.c:10-100' Jim Cromie
@ 2019-12-10 2:27 ` Jim Cromie
2019-12-10 2:27 ` [PATCH v4 10/16] dyndbg: combine flags & mask into a struct, use that Jim Cromie
` (6 subsequent siblings)
15 siblings, 0 replies; 19+ messages in thread
From: Jim Cromie @ 2019-12-10 2:27 UTC (permalink / raw)
To: jbaron, linux-kernel, akpm; +Cc: gregkh, linux, Jim Cromie
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 <jim.cromie@gmail.com>
---
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
^ permalink raw reply [flat|nested] 19+ messages in thread* [PATCH v4 10/16] dyndbg: combine flags & mask into a struct, use that
2019-12-10 2:27 [PATCH v4 00/16] dynamic-debug cleanups, 2 new features Jim Cromie
` (8 preceding siblings ...)
2019-12-10 2:27 ` [PATCH v4 09/16] dyndbg: refactor ddebug_read_flags out of ddebug_parse_flags Jim Cromie
@ 2019-12-10 2:27 ` Jim Cromie
2019-12-10 2:27 ` [PATCH v4 11/16] dyndbg: add filter parameter to ddebug_parse_flags Jim Cromie
` (5 subsequent siblings)
15 siblings, 0 replies; 19+ messages in thread
From: Jim Cromie @ 2019-12-10 2:27 UTC (permalink / raw)
To: jbaron, linux-kernel, akpm; +Cc: gregkh, linux, Jim Cromie
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 <jim.cromie@gmail.com>
---
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
^ permalink raw reply [flat|nested] 19+ messages in thread* [PATCH v4 11/16] dyndbg: add filter parameter to ddebug_parse_flags
2019-12-10 2:27 [PATCH v4 00/16] dynamic-debug cleanups, 2 new features Jim Cromie
` (9 preceding siblings ...)
2019-12-10 2:27 ` [PATCH v4 10/16] dyndbg: combine flags & mask into a struct, use that Jim Cromie
@ 2019-12-10 2:27 ` Jim Cromie
2019-12-10 2:27 ` [PATCH v4 12/16] dyndbg: extend ddebug_parse_flags to accept optional filter-flags Jim Cromie
` (4 subsequent siblings)
15 siblings, 0 replies; 19+ messages in thread
From: Jim Cromie @ 2019-12-10 2:27 UTC (permalink / raw)
To: jbaron, linux-kernel, akpm; +Cc: gregkh, linux, Jim Cromie
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 <jim.cromie@gmail.com>
---
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
^ permalink raw reply [flat|nested] 19+ messages in thread* [PATCH v4 12/16] dyndbg: extend ddebug_parse_flags to accept optional filter-flags
2019-12-10 2:27 [PATCH v4 00/16] dynamic-debug cleanups, 2 new features Jim Cromie
` (10 preceding siblings ...)
2019-12-10 2:27 ` [PATCH v4 11/16] dyndbg: add filter parameter to ddebug_parse_flags Jim Cromie
@ 2019-12-10 2:27 ` Jim Cromie
2019-12-10 2:27 ` [PATCH v4 13/16] dyndbg: prefer declarative init in caller, to memset in callee Jim Cromie
` (3 subsequent siblings)
15 siblings, 0 replies; 19+ messages in thread
From: Jim Cromie @ 2019-12-10 2:27 UTC (permalink / raw)
To: jbaron, linux-kernel, akpm
Cc: gregkh, linux, Jim Cromie, Jonathan Corbet, linux-doc
change ddebug_parse_flags to accept /^ filterflags? OP modflags /x, as
well as the currently accepted /^ OP modflags /.
Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
.../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
^ permalink raw reply [flat|nested] 19+ messages in thread* [PATCH v4 13/16] dyndbg: prefer declarative init in caller, to memset in callee
2019-12-10 2:27 [PATCH v4 00/16] dynamic-debug cleanups, 2 new features Jim Cromie
` (11 preceding siblings ...)
2019-12-10 2:27 ` [PATCH v4 12/16] dyndbg: extend ddebug_parse_flags to accept optional filter-flags Jim Cromie
@ 2019-12-10 2:27 ` Jim Cromie
2019-12-10 2:27 ` [PATCH v4 14/16] dyndbg: add user-flag, negating-flags, and filtering on flags Jim Cromie
` (2 subsequent siblings)
15 siblings, 0 replies; 19+ messages in thread
From: Jim Cromie @ 2019-12-10 2:27 UTC (permalink / raw)
To: jbaron, linux-kernel, akpm; +Cc: gregkh, linux, Jim Cromie
drop memset in ddebug_parse_query, instead initialize the stack
variable in the caller.
Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
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
^ permalink raw reply [flat|nested] 19+ messages in thread* [PATCH v4 14/16] dyndbg: add user-flag, negating-flags, and filtering on flags
2019-12-10 2:27 [PATCH v4 00/16] dynamic-debug cleanups, 2 new features Jim Cromie
` (12 preceding siblings ...)
2019-12-10 2:27 ` [PATCH v4 13/16] dyndbg: prefer declarative init in caller, to memset in callee Jim Cromie
@ 2019-12-10 2:27 ` Jim Cromie
2019-12-10 2:27 ` [PATCH v4 15/16] dyndbg: allow negating flag-chars in modflags Jim Cromie
2019-12-10 2:27 ` [PATCH v4 16/16] dyndbg: make ddebug_tables list LIFO for add/remove_module Jim Cromie
15 siblings, 0 replies; 19+ messages in thread
From: Jim Cromie @ 2019-12-10 2:27 UTC (permalink / raw)
To: jbaron, linux-kernel, akpm
Cc: gregkh, linux, Jim Cromie, Jonathan Corbet, linux-doc
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 <jim.cromie@gmail.com>
---
.../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
^ permalink raw reply [flat|nested] 19+ messages in thread* [PATCH v4 15/16] dyndbg: allow negating flag-chars in modflags
2019-12-10 2:27 [PATCH v4 00/16] dynamic-debug cleanups, 2 new features Jim Cromie
` (13 preceding siblings ...)
2019-12-10 2:27 ` [PATCH v4 14/16] dyndbg: add user-flag, negating-flags, and filtering on flags Jim Cromie
@ 2019-12-10 2:27 ` Jim Cromie
2019-12-10 2:27 ` [PATCH v4 16/16] dyndbg: make ddebug_tables list LIFO for add/remove_module Jim Cromie
15 siblings, 0 replies; 19+ messages in thread
From: Jim Cromie @ 2019-12-10 2:27 UTC (permalink / raw)
To: jbaron, linux-kernel, akpm
Cc: gregkh, linux, Jim Cromie, Jonathan Corbet, linux-doc
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 <jim.cromie@gmail.com>
---
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
^ permalink raw reply [flat|nested] 19+ messages in thread* [PATCH v4 16/16] dyndbg: make ddebug_tables list LIFO for add/remove_module
2019-12-10 2:27 [PATCH v4 00/16] dynamic-debug cleanups, 2 new features Jim Cromie
` (14 preceding siblings ...)
2019-12-10 2:27 ` [PATCH v4 15/16] dyndbg: allow negating flag-chars in modflags Jim Cromie
@ 2019-12-10 2:27 ` Jim Cromie
15 siblings, 0 replies; 19+ messages in thread
From: Jim Cromie @ 2019-12-10 2:27 UTC (permalink / raw)
To: jbaron, linux-kernel, akpm; +Cc: gregkh, linux, Jim Cromie
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 <jim.cromie@gmail.com>
---
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
^ permalink raw reply [flat|nested] 19+ messages in thread