Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Jim Cromie <jim.cromie@gmail.com>
To:     jbaron@akamai.com, linux-kernel@vger.kernel.org,
        akpm@linuxfoundation.org, gregkh@linuxfoundation.org
Cc:     linux@rasmusvillemoes.dk, Jim Cromie <jim.cromie@gmail.com>
Subject: [PATCH v2 20/24] dyndbg: WIP towards debug-print-class based callsite controls
Date:   Sat, 13 Jun 2020 09:57:34 -0600
Message-Id: <20200613155738.2249399-21-jim.cromie@gmail.com>
In-Reply-To: <20200613155738.2249399-1-jim.cromie@gmail.com>
References: <20200613155738.2249399-1-jim.cromie@gmail.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

There are *lots* of ad-hoc debug printing solutions in kernel,
this is a 1st attempt at providing a common mechanism for many of them.

Basically, there are 2 styles of debug printing:
- levels, with increasing verbosity, 1-10 forex
- bits/flags, independently controlling separate groups of dprints

This patch does bits/flags only.

proposed API:

Usage model is for a module developer to create N exclusive subsets of
pr_debug()s by changing some of them to pr_debug_n(1,) .. pr_debug_n(N,).
Each callsite must be a single print-class, with 0 default.

No multi-type classification ala pr_debug_M(1|2, ...) is contemplated.

- change pr_debug(...)  -->  pr_debug_n(pr_class=0, ...)
- all existing uses have pr_class=0
- developer creates exclusive types of log messages with pr_class>0
  1, 2, 3 are disjoint groups, for example: hi, mid, low
  0 is reserved for existing uses.

- adds query term: "mflags $arg"
  rename keyword to prcls ?

  Qfoo() { echo module foo $* >/proc/dynamic_debug/control }
  Qfoo +p  		# all groups, including default 0
  Qfoo mflags 1 +p	# only group 1
  Qfoo mflags 12 +p	# TBD[1]: groups 1 or 2
  Qfoo mflags 0 +p	# ignored atm TBD[2]
  Qfoo mflags af +p	# TBD[3]: groups a or f (10 or 15)

so patch does:

- add unsigned int pr_classes into struct ddebug_query
  this is a bit-vector
  bit positions select which print-classes to filter callsites for

- add unsigned int pr_class:5 to struct _ddebug
  picks a single debugflag bit.  No subclass or multitype nonsense.
  nice and dense, packs with other members.
  if adoption is good, kernel will have a lot of struct _ddebugs.

- in ddebug_change()
  IFF query->module is given, and matches dt->mod_name
  print-classes are defined on a module, so we require one.
  this is fooled by "module *"
  simple fix is to exclude any wildcard when mflags given

- in parse_query()
  accept new query term: mflags $arg
  populate query->mflags
  arg-type needs some attention, but basic plumbing is there

WIP: not included:

- pr_debug_n( pr_class=0, ....)
  aka: pr_debug_class() or pr_debug_id()
  bikeshedding welcome.
  the bitpos is 1<<shift, allowing a single type. no ISA relations.
  this covers OP's high,mid,low case, many others

- no way to exersize new code in ddebug_change
  need pr_debug_n() to make a (not-null) typed callsite.
  yet - done in subsequent patches

- mflags arg-parse is primitive, placeholder

- module.debug vars
  I think this can be sanely handled with a callback to handle updates.
  Perhaps several to handle different debug/verbose flavors
  maybe we export ddebug_exec_queries.

Notes:

1- A query ANDs all its query terms together, so Qfoo() above
requires both "module foo" AND all additional query terms given in $*

But since callsite pr_class creates disjoint groups, "mflags 12" is
nonsense if it means groups 1 AND 2.  Here, 1 OR 2 is meaningful, if
its not judged to be too confusing.

2- im not sure what this does atm, or should do
   Qfoo mflags 0 +p 	 # select only untyped ? or no flags check at all ?

3- pr_class:5 gives 32 print-classes
   we can map [1-9a-w] to select any pr_class with 1 char
   then "12", "af" work as noted.
   it is succinct, but arcane.
   but it does allow mnemonic choices of pr_class
   - l,m,h	low, mid, hi
   - l,r	left right
   it also allows us to treat 1-9 as levels
   - by auto-setting 1-7 when 7 is enabled.
     ie: "mflags 7 +pu" in effect does "mflags 1234567 +pu"
     note that even if this is done,
     individual callsites or sets of them can be undone.
     you can even use 'u' as above to mark them for easier grepping
---
 include/linux/dynamic_debug.h |  1 +
 lib/dynamic_debug.c           | 33 +++++++++++++++++++++++++++++++--
 2 files changed, 32 insertions(+), 2 deletions(-)

diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index 59960a8dd9f9..7ac822d6be87 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -20,6 +20,7 @@ struct _ddebug {
 	const char *function;
 	const char *filename;
 	const char *format;
+	unsigned int pr_class:5;	/* >0 for distinct developer groups */
 	unsigned int lineno:18;
 	/*
 	 * The flags field controls the behaviour at the callsite.
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index cb5c7480e026..0035218d7059 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -55,6 +55,7 @@ struct ddebug_query {
 	const char *function;
 	const char *format;
 	unsigned int first_lineno, last_lineno;
+	unsigned int pr_classes;
 };
 
 struct ddebug_iter {
@@ -132,13 +133,14 @@ static void vpr_info_dq(const struct ddebug_query *query, const char *msg)
 			fmtlen--;
 	}
 
-	vpr_info("%s: func=\"%s\" file=\"%s\" module=\"%s\" format=\"%.*s\" lineno=%u-%u\n",
+	vpr_info("%s: func=\"%s\" file=\"%s\" module=\"%s\" format=\"%.*s\" lineno=%u-%u prcls=0x%x\n",
 		 msg,
 		 maybe(query->function, ""),
 		 maybe(query->filename, ""),
 		 maybe(query->module, ""),
 		 fmtlen, maybe(query->format, ""),
-		 query->first_lineno, query->last_lineno);
+		 query->first_lineno, query->last_lineno,
+		 query->pr_classes);
 }
 
 /*
@@ -203,6 +205,27 @@ static int ddebug_change(const struct ddebug_query *query,
 			if ((~dp->flags & filter->mask) != filter->mask)
 				continue;
 
+			/* filter on non-zero print-classes */
+			if (query->pr_classes) {
+				if (!query->module) {
+					pr_err("using prcls requires module too");
+					return -EINVAL;
+				}
+				/* since print-classes are module
+				 * specific, require a module query
+				 * too.  For 'module kvm* mflags 1'
+				 * >control, this will enable
+				 * pr_class=1 in several matching modules
+				 */
+				if ((query->pr_classes & (1<<(dp->pr_class-1)))
+				    != (1<<(dp->pr_class-1))) {
+					v2pr_info("%s ~ %s mflags:0x%x !~ %d\n",
+						  dt->mod_name, query->module,
+						  query->pr_classes, dp->pr_class);
+					continue;
+				}
+			}
+
 			nfound++;
 
 			newflags = (dp->flags & mods->mask) | mods->flags;
@@ -427,6 +450,12 @@ static int ddebug_parse_query(char *words[], int nwords,
 		} else if (!strcmp(keyword, "line")) {
 			if (parse_linerange(query, arg))
 				return -EINVAL;
+		} else if (!strcmp(keyword, "mflags")) {
+			pr_info("handle mflags arg: %s\n", arg);
+			if (kstrtouint(arg, 4, &query->pr_classes) < 0) {
+				pr_err("bad arg for mflags: %s\n", arg);
+				return -EINVAL;
+			}
 		} else {
 			pr_err("unknown keyword \"%s\"\n", keyword);
 			return -EINVAL;
-- 
2.26.2