linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 00/18] dynamic-debug cleanups, 2 new features
@ 2019-12-05 21:51 Jim Cromie
  2019-12-05 21:51 ` [PATCH 01/18] dyndbg-docs: eschew file /full/path query in docs Jim Cromie
                   ` (19 more replies)
  0 siblings, 20 replies; 25+ messages in thread
From: Jim Cromie @ 2019-12-05 21:51 UTC (permalink / raw)
  To: jbaron, gregkh, linux-kernel; +Cc: linux, Jim Cromie

changes from v2:
  only claim one user flag, dont really need more
  fix patchset grooming err Reported-by: kbuild test robot <lkp@intel.com>
  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


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH 01/18] dyndbg-docs: eschew file /full/path query in docs
  2019-12-05 21:51 [PATCH v3 00/18] dynamic-debug cleanups, 2 new features Jim Cromie
@ 2019-12-05 21:51 ` Jim Cromie
  2019-12-05 21:51 ` [PATCH 02/18] dyndbg: drop obsolete comment on ddebug_proc_open Jim Cromie
                   ` (18 subsequent siblings)
  19 siblings, 0 replies; 25+ messages in thread
From: Jim Cromie @ 2019-12-05 21:51 UTC (permalink / raw)
  To: jbaron, gregkh, linux-kernel
  Cc: 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 related	[flat|nested] 25+ messages in thread

* [PATCH 02/18] dyndbg: drop obsolete comment on ddebug_proc_open
  2019-12-05 21:51 [PATCH v3 00/18] dynamic-debug cleanups, 2 new features Jim Cromie
  2019-12-05 21:51 ` [PATCH 01/18] dyndbg-docs: eschew file /full/path query in docs Jim Cromie
@ 2019-12-05 21:51 ` Jim Cromie
  2019-12-05 21:51 ` [PATCH 03/18] dyndbg: raise verbosity needed to enable ddebug_proc_* logging Jim Cromie
                   ` (17 subsequent siblings)
  19 siblings, 0 replies; 25+ messages in thread
From: Jim Cromie @ 2019-12-05 21:51 UTC (permalink / raw)
  To: jbaron, gregkh, linux-kernel; +Cc: 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 related	[flat|nested] 25+ messages in thread

* [PATCH 03/18] dyndbg: raise verbosity needed to enable ddebug_proc_* logging
  2019-12-05 21:51 [PATCH v3 00/18] dynamic-debug cleanups, 2 new features Jim Cromie
  2019-12-05 21:51 ` [PATCH 01/18] dyndbg-docs: eschew file /full/path query in docs Jim Cromie
  2019-12-05 21:51 ` [PATCH 02/18] dyndbg: drop obsolete comment on ddebug_proc_open Jim Cromie
@ 2019-12-05 21:51 ` Jim Cromie
  2019-12-05 21:51 ` [PATCH 04/18] dyndbg: rename __verbose section to __dyndbg Jim Cromie
                   ` (16 subsequent siblings)
  19 siblings, 0 replies; 25+ messages in thread
From: Jim Cromie @ 2019-12-05 21:51 UTC (permalink / raw)
  To: jbaron, gregkh, linux-kernel; +Cc: 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 related	[flat|nested] 25+ messages in thread

* [PATCH 04/18] dyndbg: rename __verbose section to __dyndbg
  2019-12-05 21:51 [PATCH v3 00/18] dynamic-debug cleanups, 2 new features Jim Cromie
                   ` (2 preceding siblings ...)
  2019-12-05 21:51 ` [PATCH 03/18] dyndbg: raise verbosity needed to enable ddebug_proc_* logging Jim Cromie
@ 2019-12-05 21:51 ` Jim Cromie
  2019-12-05 21:51 ` [PATCH 05/18] dyndbg: fix overcounting of ram used by dyndbg Jim Cromie
                   ` (15 subsequent siblings)
  19 siblings, 0 replies; 25+ messages in thread
From: Jim Cromie @ 2019-12-05 21:51 UTC (permalink / raw)
  To: jbaron, gregkh, linux-kernel
  Cc: 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 related	[flat|nested] 25+ messages in thread

* [PATCH 05/18] dyndbg: fix overcounting of ram used by dyndbg
  2019-12-05 21:51 [PATCH v3 00/18] dynamic-debug cleanups, 2 new features Jim Cromie
                   ` (3 preceding siblings ...)
  2019-12-05 21:51 ` [PATCH 04/18] dyndbg: rename __verbose section to __dyndbg Jim Cromie
@ 2019-12-05 21:51 ` Jim Cromie
  2019-12-05 21:51 ` [PATCH 06/18] dyndbg: fix a BUG_ON in ddebug_describe_flags Jim Cromie
                   ` (14 subsequent siblings)
  19 siblings, 0 replies; 25+ messages in thread
From: Jim Cromie @ 2019-12-05 21:51 UTC (permalink / raw)
  To: jbaron, gregkh, linux-kernel; +Cc: 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 related	[flat|nested] 25+ messages in thread

* [PATCH 06/18] dyndbg: fix a BUG_ON in ddebug_describe_flags
  2019-12-05 21:51 [PATCH v3 00/18] dynamic-debug cleanups, 2 new features Jim Cromie
                   ` (4 preceding siblings ...)
  2019-12-05 21:51 ` [PATCH 05/18] dyndbg: fix overcounting of ram used by dyndbg Jim Cromie
@ 2019-12-05 21:51 ` Jim Cromie
  2019-12-05 21:51 ` [PATCH 07/18] dyndbg: refactor parse_linerange out of ddebug_parse_query Jim Cromie
                   ` (13 subsequent siblings)
  19 siblings, 0 replies; 25+ messages in thread
From: Jim Cromie @ 2019-12-05 21:51 UTC (permalink / raw)
  To: jbaron, gregkh, linux-kernel; +Cc: 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 related	[flat|nested] 25+ messages in thread

* [PATCH 07/18] dyndbg: refactor parse_linerange out of ddebug_parse_query
  2019-12-05 21:51 [PATCH v3 00/18] dynamic-debug cleanups, 2 new features Jim Cromie
                   ` (5 preceding siblings ...)
  2019-12-05 21:51 ` [PATCH 06/18] dyndbg: fix a BUG_ON in ddebug_describe_flags Jim Cromie
@ 2019-12-05 21:51 ` Jim Cromie
  2019-12-05 21:51 ` [PATCH 08/18] dyndbg: accept 'file foo.c:func1' and 'file foo.c:10-100' Jim Cromie
                   ` (12 subsequent siblings)
  19 siblings, 0 replies; 25+ messages in thread
From: Jim Cromie @ 2019-12-05 21:51 UTC (permalink / raw)
  To: jbaron, gregkh, linux-kernel; +Cc: 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 related	[flat|nested] 25+ messages in thread

* [PATCH 08/18] dyndbg: accept 'file foo.c:func1' and 'file foo.c:10-100'
  2019-12-05 21:51 [PATCH v3 00/18] dynamic-debug cleanups, 2 new features Jim Cromie
                   ` (6 preceding siblings ...)
  2019-12-05 21:51 ` [PATCH 07/18] dyndbg: refactor parse_linerange out of ddebug_parse_query Jim Cromie
@ 2019-12-05 21:51 ` Jim Cromie
  2019-12-05 21:51 ` [PATCH 09/18] dyndbg: refactor ddebug_read_flags out of ddebug_parse_flags Jim Cromie
                   ` (11 subsequent siblings)
  19 siblings, 0 replies; 25+ messages in thread
From: Jim Cromie @ 2019-12-05 21:51 UTC (permalink / raw)
  To: jbaron, gregkh, linux-kernel
  Cc: 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 related	[flat|nested] 25+ messages in thread

* [PATCH 09/18] dyndbg: refactor ddebug_read_flags out of ddebug_parse_flags
  2019-12-05 21:51 [PATCH v3 00/18] dynamic-debug cleanups, 2 new features Jim Cromie
                   ` (7 preceding siblings ...)
  2019-12-05 21:51 ` [PATCH 08/18] dyndbg: accept 'file foo.c:func1' and 'file foo.c:10-100' Jim Cromie
@ 2019-12-05 21:51 ` Jim Cromie
  2019-12-05 21:51 ` [PATCH 10/18] dyndbg: combine flags & mask into a struct, use that Jim Cromie
                   ` (10 subsequent siblings)
  19 siblings, 0 replies; 25+ messages in thread
From: Jim Cromie @ 2019-12-05 21:51 UTC (permalink / raw)
  To: jbaron, gregkh, linux-kernel; +Cc: 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 related	[flat|nested] 25+ messages in thread

* [PATCH 10/18] dyndbg: combine flags & mask into a struct, use that
  2019-12-05 21:51 [PATCH v3 00/18] dynamic-debug cleanups, 2 new features Jim Cromie
                   ` (8 preceding siblings ...)
  2019-12-05 21:51 ` [PATCH 09/18] dyndbg: refactor ddebug_read_flags out of ddebug_parse_flags Jim Cromie
@ 2019-12-05 21:51 ` Jim Cromie
  2019-12-05 21:51 ` [PATCH 11/18] dyndbg: add filter parameter to ddebug_parse_flags Jim Cromie
                   ` (9 subsequent siblings)
  19 siblings, 0 replies; 25+ messages in thread
From: Jim Cromie @ 2019-12-05 21:51 UTC (permalink / raw)
  To: jbaron, gregkh, linux-kernel; +Cc: 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 related	[flat|nested] 25+ messages in thread

* [PATCH 11/18] dyndbg: add filter parameter to ddebug_parse_flags
  2019-12-05 21:51 [PATCH v3 00/18] dynamic-debug cleanups, 2 new features Jim Cromie
                   ` (9 preceding siblings ...)
  2019-12-05 21:51 ` [PATCH 10/18] dyndbg: combine flags & mask into a struct, use that Jim Cromie
@ 2019-12-05 21:51 ` Jim Cromie
  2019-12-05 21:51 ` [PATCH 12/18] dyndbg: extend ddebug_parse_flags to accept optional filter-flags Jim Cromie
                   ` (8 subsequent siblings)
  19 siblings, 0 replies; 25+ messages in thread
From: Jim Cromie @ 2019-12-05 21:51 UTC (permalink / raw)
  To: jbaron, gregkh, linux-kernel; +Cc: 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 related	[flat|nested] 25+ messages in thread

* [PATCH 12/18] dyndbg: extend ddebug_parse_flags to accept optional filter-flags
  2019-12-05 21:51 [PATCH v3 00/18] dynamic-debug cleanups, 2 new features Jim Cromie
                   ` (10 preceding siblings ...)
  2019-12-05 21:51 ` [PATCH 11/18] dyndbg: add filter parameter to ddebug_parse_flags Jim Cromie
@ 2019-12-05 21:51 ` Jim Cromie
  2019-12-05 21:51 ` [PATCH 13/18] dyndbg: prefer declarative init in caller, to memset in callee Jim Cromie
                   ` (7 subsequent siblings)
  19 siblings, 0 replies; 25+ messages in thread
From: Jim Cromie @ 2019-12-05 21:51 UTC (permalink / raw)
  To: jbaron, gregkh, linux-kernel
  Cc: 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 related	[flat|nested] 25+ messages in thread

* [PATCH 13/18] dyndbg: prefer declarative init in caller, to memset in callee
  2019-12-05 21:51 [PATCH v3 00/18] dynamic-debug cleanups, 2 new features Jim Cromie
                   ` (11 preceding siblings ...)
  2019-12-05 21:51 ` [PATCH 12/18] dyndbg: extend ddebug_parse_flags to accept optional filter-flags Jim Cromie
@ 2019-12-05 21:51 ` Jim Cromie
  2019-12-05 21:51 ` [PATCH 14/16] dyndbg: add inverted-flags, implement filtering on flags Jim Cromie
                   ` (6 subsequent siblings)
  19 siblings, 0 replies; 25+ messages in thread
From: Jim Cromie @ 2019-12-05 21:51 UTC (permalink / raw)
  To: jbaron, gregkh, linux-kernel; +Cc: 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 related	[flat|nested] 25+ messages in thread

* [PATCH 14/16] dyndbg: add inverted-flags, implement filtering on flags
  2019-12-05 21:51 [PATCH v3 00/18] dynamic-debug cleanups, 2 new features Jim Cromie
                   ` (12 preceding siblings ...)
  2019-12-05 21:51 ` [PATCH 13/18] dyndbg: prefer declarative init in caller, to memset in callee Jim Cromie
@ 2019-12-05 21:51 ` Jim Cromie
  2019-12-05 21:51 ` [PATCH 14/18] dyndbg: add user-flag, negating-flags, and " Jim Cromie
                   ` (5 subsequent siblings)
  19 siblings, 0 replies; 25+ messages in thread
From: Jim Cromie @ 2019-12-05 21:51 UTC (permalink / raw)
  To: jbaron, gregkh, linux-kernel
  Cc: linux, Jim Cromie, Jonathan Corbet, linux-doc

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 <jim.cromie@gmail.com>
---
 .../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


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 14/18] dyndbg: add user-flag, negating-flags, and filtering on flags
  2019-12-05 21:51 [PATCH v3 00/18] dynamic-debug cleanups, 2 new features Jim Cromie
                   ` (13 preceding siblings ...)
  2019-12-05 21:51 ` [PATCH 14/16] dyndbg: add inverted-flags, implement filtering on flags Jim Cromie
@ 2019-12-05 21:51 ` Jim Cromie
  2019-12-05 21:51 ` [PATCH 15/16] dyndbg: allow inverted-flag-chars in modflags Jim Cromie
                   ` (4 subsequent siblings)
  19 siblings, 0 replies; 25+ messages in thread
From: Jim Cromie @ 2019-12-05 21:51 UTC (permalink / raw)
  To: jbaron, gregkh, linux-kernel
  Cc: 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 related	[flat|nested] 25+ messages in thread

* [PATCH 15/16] dyndbg: allow inverted-flag-chars in modflags
  2019-12-05 21:51 [PATCH v3 00/18] dynamic-debug cleanups, 2 new features Jim Cromie
                   ` (14 preceding siblings ...)
  2019-12-05 21:51 ` [PATCH 14/18] dyndbg: add user-flag, negating-flags, and " Jim Cromie
@ 2019-12-05 21:51 ` Jim Cromie
  2019-12-05 21:51 ` [PATCH 15/18] dyndbg: allow negating flag-chars " Jim Cromie
                   ` (3 subsequent siblings)
  19 siblings, 0 replies; 25+ messages in thread
From: Jim Cromie @ 2019-12-05 21:51 UTC (permalink / raw)
  To: jbaron, gregkh, linux-kernel
  Cc: linux, Jim Cromie, Jonathan Corbet, linux-doc

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 <jim.cromie@gmail.com>
---
 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


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 15/18] dyndbg: allow negating flag-chars in modflags
  2019-12-05 21:51 [PATCH v3 00/18] dynamic-debug cleanups, 2 new features Jim Cromie
                   ` (15 preceding siblings ...)
  2019-12-05 21:51 ` [PATCH 15/16] dyndbg: allow inverted-flag-chars in modflags Jim Cromie
@ 2019-12-05 21:51 ` Jim Cromie
  2019-12-05 21:51 ` [PATCH 16/18] dyndbg: make ddebug_tables list LIFO for add/remove_module Jim Cromie
                   ` (2 subsequent siblings)
  19 siblings, 0 replies; 25+ messages in thread
From: Jim Cromie @ 2019-12-05 21:51 UTC (permalink / raw)
  To: jbaron, gregkh, linux-kernel
  Cc: 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 related	[flat|nested] 25+ messages in thread

* [PATCH 16/18] dyndbg: make ddebug_tables list LIFO for add/remove_module
  2019-12-05 21:51 [PATCH v3 00/18] dynamic-debug cleanups, 2 new features Jim Cromie
                   ` (16 preceding siblings ...)
  2019-12-05 21:51 ` [PATCH 15/18] dyndbg: allow negating flag-chars " Jim Cromie
@ 2019-12-05 21:51 ` Jim Cromie
  2019-12-05 21:51 ` [PATCH 17/18] dyndbg: rename dynamic_debug to dyndbg Jim Cromie
  2019-12-05 21:51 ` [PATCH 18/18] dyndbg-docs: normalize comments in examples Jim Cromie
  19 siblings, 0 replies; 25+ messages in thread
From: Jim Cromie @ 2019-12-05 21:51 UTC (permalink / raw)
  To: jbaron, gregkh, linux-kernel; +Cc: 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 related	[flat|nested] 25+ messages in thread

* [PATCH 17/18] dyndbg: rename dynamic_debug to dyndbg
  2019-12-05 21:51 [PATCH v3 00/18] dynamic-debug cleanups, 2 new features Jim Cromie
                   ` (17 preceding siblings ...)
  2019-12-05 21:51 ` [PATCH 16/18] dyndbg: make ddebug_tables list LIFO for add/remove_module Jim Cromie
@ 2019-12-05 21:51 ` Jim Cromie
  2019-12-05 22:24   ` Andy Shevchenko
  2019-12-05 21:51 ` [PATCH 18/18] dyndbg-docs: normalize comments in examples Jim Cromie
  19 siblings, 1 reply; 25+ messages in thread
From: Jim Cromie @ 2019-12-05 21:51 UTC (permalink / raw)
  To: jbaron, gregkh, linux-kernel
  Cc: linux, Jim Cromie, Jonathan Corbet, Andrew Morton, Kees Cook,
	Masahiro Yamada, David S. Miller, Joe Lawrence, Kent Overstreet,
	Uladzislau Rezki (Sony),
	Andy Shevchenko, Gary Hook, Arnd Bergmann, Mark Rutland,
	linux-doc

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 <jim.cromie@gmail.com>
---
 .../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


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 18/18] dyndbg-docs: normalize comments in examples
  2019-12-05 21:51 [PATCH v3 00/18] dynamic-debug cleanups, 2 new features Jim Cromie
                   ` (18 preceding siblings ...)
  2019-12-05 21:51 ` [PATCH 17/18] dyndbg: rename dynamic_debug to dyndbg Jim Cromie
@ 2019-12-05 21:51 ` Jim Cromie
  19 siblings, 0 replies; 25+ messages in thread
From: Jim Cromie @ 2019-12-05 21:51 UTC (permalink / raw)
  To: jbaron, gregkh, linux-kernel
  Cc: linux, Jim Cromie, Jonathan Corbet, linux-doc

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 <jim.cromie@gmail.com>
---
 .../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


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* Re: [PATCH 17/18] dyndbg: rename dynamic_debug to dyndbg
  2019-12-05 21:51 ` [PATCH 17/18] dyndbg: rename dynamic_debug to dyndbg Jim Cromie
@ 2019-12-05 22:24   ` Andy Shevchenko
  2019-12-06  7:49     ` Rasmus Villemoes
  0 siblings, 1 reply; 25+ messages in thread
From: Andy Shevchenko @ 2019-12-05 22:24 UTC (permalink / raw)
  To: Jim Cromie
  Cc: jbaron, Greg Kroah-Hartman, Linux Kernel Mailing List,
	Rasmus Villemoes, Jonathan Corbet, Andrew Morton, Kees Cook,
	Masahiro Yamada, David S. Miller, Joe Lawrence, Kent Overstreet,
	Uladzislau Rezki (Sony),
	Andy Shevchenko, Gary Hook, Arnd Bergmann, Mark Rutland,
	Linux Documentation List

On Thu, Dec 5, 2019 at 11:54 PM Jim Cromie <jim.cromie@gmail.com> 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

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 17/18] dyndbg: rename dynamic_debug to dyndbg
  2019-12-05 22:24   ` Andy Shevchenko
@ 2019-12-06  7:49     ` Rasmus Villemoes
  2019-12-06 17:33       ` jim.cromie
  0 siblings, 1 reply; 25+ messages in thread
From: Rasmus Villemoes @ 2019-12-06  7:49 UTC (permalink / raw)
  To: Andy Shevchenko, Jim Cromie
  Cc: jbaron, Greg Kroah-Hartman, Linux Kernel Mailing List,
	Jonathan Corbet, Andrew Morton, Kees Cook, Masahiro Yamada,
	David S. Miller, Joe Lawrence, Kent Overstreet,
	Uladzislau Rezki (Sony),
	Andy Shevchenko, Gary Hook, Arnd Bergmann, Mark Rutland,
	Linux Documentation List

On 05/12/2019 23.24, Andy Shevchenko wrote:
> On Thu, Dec 5, 2019 at 11:54 PM Jim Cromie <jim.cromie@gmail.com> 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

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 17/18] dyndbg: rename dynamic_debug to dyndbg
  2019-12-06  7:49     ` Rasmus Villemoes
@ 2019-12-06 17:33       ` jim.cromie
  0 siblings, 0 replies; 25+ messages in thread
From: jim.cromie @ 2019-12-06 17:33 UTC (permalink / raw)
  To: Rasmus Villemoes
  Cc: Andy Shevchenko, Jason Baron, Greg Kroah-Hartman,
	Linux Kernel Mailing List, Jonathan Corbet, Andrew Morton,
	Kees Cook, Masahiro Yamada, David S. Miller, Joe Lawrence,
	Kent Overstreet, Uladzislau Rezki (Sony),
	Andy Shevchenko, Gary Hook, Arnd Bergmann, Mark Rutland,
	Linux Documentation List

On Fri, Dec 6, 2019 at 12:49 AM Rasmus Villemoes
<linux@rasmusvillemoes.dk> wrote:
>
> On 05/12/2019 23.24, Andy Shevchenko wrote:
> > On Thu, Dec 5, 2019 at 11:54 PM Jim Cromie <jim.cromie@gmail.com> 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

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH 14/16] dyndbg: add inverted-flags, implement filtering on flags
@ 2019-11-27 17:51 Jim Cromie
  0 siblings, 0 replies; 25+ messages in thread
From: Jim Cromie @ 2019-11-27 17:51 UTC (permalink / raw)
  To: jbaron, linux-kernel; +Cc: linux, greg, Jim Cromie, Jonathan Corbet, linux-doc

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 <jim.cromie@gmail.com>
---
 .../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


^ permalink raw reply related	[flat|nested] 25+ messages in thread

end of thread, other threads:[~2019-12-06 17:33 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-05 21:51 [PATCH v3 00/18] dynamic-debug cleanups, 2 new features Jim Cromie
2019-12-05 21:51 ` [PATCH 01/18] dyndbg-docs: eschew file /full/path query in docs Jim Cromie
2019-12-05 21:51 ` [PATCH 02/18] dyndbg: drop obsolete comment on ddebug_proc_open Jim Cromie
2019-12-05 21:51 ` [PATCH 03/18] dyndbg: raise verbosity needed to enable ddebug_proc_* logging Jim Cromie
2019-12-05 21:51 ` [PATCH 04/18] dyndbg: rename __verbose section to __dyndbg Jim Cromie
2019-12-05 21:51 ` [PATCH 05/18] dyndbg: fix overcounting of ram used by dyndbg Jim Cromie
2019-12-05 21:51 ` [PATCH 06/18] dyndbg: fix a BUG_ON in ddebug_describe_flags Jim Cromie
2019-12-05 21:51 ` [PATCH 07/18] dyndbg: refactor parse_linerange out of ddebug_parse_query Jim Cromie
2019-12-05 21:51 ` [PATCH 08/18] dyndbg: accept 'file foo.c:func1' and 'file foo.c:10-100' Jim Cromie
2019-12-05 21:51 ` [PATCH 09/18] dyndbg: refactor ddebug_read_flags out of ddebug_parse_flags Jim Cromie
2019-12-05 21:51 ` [PATCH 10/18] dyndbg: combine flags & mask into a struct, use that Jim Cromie
2019-12-05 21:51 ` [PATCH 11/18] dyndbg: add filter parameter to ddebug_parse_flags Jim Cromie
2019-12-05 21:51 ` [PATCH 12/18] dyndbg: extend ddebug_parse_flags to accept optional filter-flags Jim Cromie
2019-12-05 21:51 ` [PATCH 13/18] dyndbg: prefer declarative init in caller, to memset in callee Jim Cromie
2019-12-05 21:51 ` [PATCH 14/16] dyndbg: add inverted-flags, implement filtering on flags Jim Cromie
2019-12-05 21:51 ` [PATCH 14/18] dyndbg: add user-flag, negating-flags, and " Jim Cromie
2019-12-05 21:51 ` [PATCH 15/16] dyndbg: allow inverted-flag-chars in modflags Jim Cromie
2019-12-05 21:51 ` [PATCH 15/18] dyndbg: allow negating flag-chars " Jim Cromie
2019-12-05 21:51 ` [PATCH 16/18] dyndbg: make ddebug_tables list LIFO for add/remove_module Jim Cromie
2019-12-05 21:51 ` [PATCH 17/18] dyndbg: rename dynamic_debug to dyndbg Jim Cromie
2019-12-05 22:24   ` Andy Shevchenko
2019-12-06  7:49     ` Rasmus Villemoes
2019-12-06 17:33       ` jim.cromie
2019-12-05 21:51 ` [PATCH 18/18] dyndbg-docs: normalize comments in examples Jim Cromie
  -- strict thread matches above, loose matches on Subject: below --
2019-11-27 17:51 [PATCH 14/16] dyndbg: add inverted-flags, implement filtering on flags Jim Cromie

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).