[RFC PATCH 00/17] DRM_USE_DYNAMIC_DEBUG regression

[RFC PATCH 00/17] DRM_USE_DYNAMIC_DEBUG regression

[Jim Cromie]

Hi everyone,

DRM_USE_DYNAMIC_DEBUG=y has a regression on rc-*

Regression is due to a chicken-egg problem loading modules; on
`modprobe i915`, drm is loaded 1st, and drm.debug is set.  When
drm_debug_enabled() tested __drm_debug at runtime, that just worked.

But with DRM_USE_DYNAMIC_DEBUG=y, the runtime test is replaced with a
post-load enablement of drm_dbg/dyndbg callsites (static-keys), via
dyndbg's callback on __drm_debug.  Since all drm-drivers need drm.ko,
it is loaded 1st, then drm.debug=X is applied, then drivers load, but
too late for drm_dbgs to be enabled.

STATUS

For all-loadable drm,i915,amdgpu configs, it almost works, but
propagating drm.debug to dependent modules doesnt actually apply,
though the motions are there.  This is not the problem I want to chase
here.

The more basic trouble is:

For builtin drm + helpers, things are broken pretty early; at the
beginning of dynamic_debug_init().  As the ddebug_sanity() commit-msg
describes in some detail, the records added by _USE fail to reference
the struct ddebug_class_map created and exported by _DEFINE, but get
separate addresses to "other" data that segv's when used as the
expected pointer. FWIW, the pointer val starts with "revi".

OVERVIEW

DECLARE_DYNDBG_CLASSMAP is broken: it is one-size-fits-all-poorly.
It muddles the distinction between a (single) definition, and multiple
references.  Something exported should suffice.

The core of this patchset splits it into:

DYNDBG_CLASSMAP_DEFINE	used once per subsystem to define each classmap
DYNDBG_CLASSMAP_USE	declare dependence on a DEFINEd classmap

This makes the weird coordinated-changes-by-identical-classmaps
"feature" unnecessary; the DEFINE can export the var, and USE refers
to the exported var.

So this patchset adds another section: __dyndbg_class_refs.

It is like __dyndbg_classes; it is scanned under ddebug_add_module(),
and attached to each module's ddebug_table.  Once attached, it can be
used like classes to validate and apply class FOO >control queries.

It also maps the class user -> definer explicitly, so that when the
module is loaded, the section scan can find the kernel-param that is
wired to dyndbg's kparam-callback, and apply its state-var, forex:
__drm_debug to the just loaded helper/driver module.

Theres plenty to address Im sure.

Jim Cromie (17):
  test-dyndbg: fixup CLASSMAP usage error
  test-dyndbg: show that DEBUG enables prdbgs at compiletime
  dyndbg: fix readback value on LEVEL_NAMES interfaces
  dyndbg: replace classmap list with a vector
  dyndbg: make ddebug_apply_class_bitmap more selective
  dyndbg: dynamic_debug_init - use pointer inequality, not strcmp
  dyndbg: drop NUM_TYPE_ARRAY
  dyndbg: reduce verbose/debug clutter
  dyndbg-API: replace DECLARE_DYNDBG_CLASSMAP with
    DYNDBG_CLASSMAP(_DEFINE|_USE)
  dyndbg-API: specialize DYNDBG_CLASSMAP_(DEFINE|USE)
  dyndbg-API: DYNDBG_CLASSMAP_USE drop extra args
  dyndbg-API: DYNDBG_CLASSMAP_DEFINE() improvements
  drm_print: fix stale macro-name in comment
  dyndbg: unwrap __ddebug_add_module inner function NOTYET
  dyndbg: ddebug_sanity()
  dyndbg: mess-w-dep-class
  dyndbg: miss-on HACK

 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c |  14 +-
 drivers/gpu/drm/display/drm_dp_helper.c |  14 +-
 drivers/gpu/drm/drm_crtc_helper.c       |  14 +-
 drivers/gpu/drm/drm_print.c             |  22 +--
 drivers/gpu/drm/i915/i915_params.c      |  14 +-
 drivers/gpu/drm/nouveau/nouveau_drm.c   |  14 +-
 include/asm-generic/vmlinux.lds.h       |   3 +
 include/drm/drm_print.h                 |   6 +-
 include/linux/dynamic_debug.h           |  57 ++++--
 include/linux/map.h                     |  54 ++++++
 kernel/module/main.c                    |   2 +
 lib/dynamic_debug.c                     | 240 +++++++++++++++++++-----
 lib/test_dynamic_debug.c                |  47 ++---
 13 files changed, 344 insertions(+), 157 deletions(-)
 create mode 100644 include/linux/map.h

-- 
2.38.1

[RFC PATCH 01/17] test-dyndbg: fixup CLASSMAP usage error

[Jim Cromie]

more careful reading of test output reveals:

lib/test_dynamic_debug.c:103 [test_dynamic_debug]do_cats =pmf "doing categories\n"
lib/test_dynamic_debug.c:105 [test_dynamic_debug]do_cats =p "LOW msg\n" class:MID
lib/test_dynamic_debug.c:106 [test_dynamic_debug]do_cats =p "MID msg\n" class:HI
lib/test_dynamic_debug.c:107 [test_dynamic_debug]do_cats =_ "HI msg\n" class unknown, _id:13

That last line is wrong, the HI class is declared.

But the enum's 1st val (explicitly initialized) was wrong; it must be
_base, not _base+1 (a DECLARE_DYNDBG_CLASSMAP param).  So the last
enumeration exceeded the range of mapped class-id's, which triggered
the "class unknown" report.  Basically, I coded in an error, and
forgot to verify it and remove it.

RFC:

This patch fixes a bad usage of DEFINE_DYNDBG_CLASSMAP(), showing that
it is too error-prone.  As noted in test-dynamic-debug.c comments:

 * Using the CLASSMAP api:
 * - classmaps must have corresponding enum
 * - enum symbols must match/correlate with class-name strings in the map.
 * - base must equal enum's 1st value
 * - multiple maps must set their base to share the 0-62 class_id space !!
 *   (build-bug-on tips welcome)

Those shortcomings could largely be fixed with a __stringify_list
(which doesn't exist) used in DEFINE_DYNAMIC_DEBUG_CLASSMAP(), on
__VA_ARGS__ a 2nd time.  Then, DRM would pass DRM_UT_* ; all the
categories, in order, and not their stringifications, which created
all the usage complications above.

Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
 lib/test_dynamic_debug.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/test_dynamic_debug.c b/lib/test_dynamic_debug.c
index 8dd250ad022b..a01f0193a419 100644
--- a/lib/test_dynamic_debug.c
+++ b/lib/test_dynamic_debug.c
@@ -75,7 +75,7 @@ DD_SYS_WRAP(disjoint_bits, p);
 DD_SYS_WRAP(disjoint_bits, T);
 
 /* symbolic input, independent bits */
-enum cat_disjoint_names { LOW = 11, MID, HI };
+enum cat_disjoint_names { LOW = 10, MID, HI };
 DECLARE_DYNDBG_CLASSMAP(map_disjoint_names, DD_CLASS_TYPE_DISJOINT_NAMES, 10,
 			"LOW", "MID", "HI");
 DD_SYS_WRAP(disjoint_names, p);
-- 
2.38.1

[RFC PATCH 02/17] test-dyndbg: show that DEBUG enables prdbgs at compiletime

[Jim Cromie]

Dyndbg is required to enable prdbgs at compile-time if DEBUG is
defined.  Show this works; add the defn to test_dynamic_debug.c,
and manually inspect/verify its effect at module load:

[   15.292810] dyndbg: module:test_dynamic_debug attached 4 classes
[   15.293189] dyndbg:  32 debug prints in module test_dynamic_debug
[   15.293715] test_dd: init start
[   15.293716] test_dd: doing categories
[   15.293716] test_dd: LOW msg
...
[   15.293733] test_dd: L6 msg
[   15.293733] test_dd: L7 msg
[   15.293733] test_dd: init done

NOTES:

As is observable above, define DEBUG enables all prdbgs, including
those in mod_init-fn, and more notably, the "class"d ones (callsites
with non-default class_ids).

This differs from the >control interface, which in order to properly
protect a client's class'd prdbgs, requires a "class FOO" in queries
to change them.  Note that the DEBUG is in the module source file.

This yields an occaisional surprise; the following disables all the
compile-time enabled plain prdbgs, but leaves the class'd ones
enabled.

 :#> modprobe test_dynamic_debug dyndbg==_

Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
 lib/test_dynamic_debug.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lib/test_dynamic_debug.c b/lib/test_dynamic_debug.c
index a01f0193a419..9d48689dc0ab 100644
--- a/lib/test_dynamic_debug.c
+++ b/lib/test_dynamic_debug.c
@@ -8,6 +8,8 @@
 
 #define pr_fmt(fmt) "test_dd: " fmt
 
+#define DEBUG
+
 #include <linux/module.h>
 
 /* run tests by reading or writing sysfs node: do_prints */
-- 
2.38.1

[RFC PATCH 03/17] dyndbg: fix readback value on LEVEL_NAMES interfaces

[Jim Cromie]

Since sysfs knobs should generally read-back what was just written
(unless theres a good reason to do otherwise), this result (using
test_dynamic_debug.ko) is suboptimal:

  echo L3 > p_level_names
  cat p_level_names
  4

Fix this with a -1 offset in LEVEL_NAMES readback.

NOTE:

Calling this a BUG is debatable, and the above is slightly inaccurate
wrt "read-back"; whats written is a LEVEL_NAME (a string).  Whats read
back is an integer, giving the 'edge' of the 'low-pass-filter'

The actual test looks like:

RTT: L4 -> p_level_names : 4 :: DOING: levels 4-1
[   17.509594] dyndbg: "L4" > p_level_names:0x4
[   17.510115] dyndbg: apply: 0x1f to: 0xf
[   17.510506] dyndbg: query 0: "class L4 +p" mod:*
[   17.510992] dyndbg: split into words: "class" "L4" "+p"
[   17.511521] dyndbg: op='+'
[   17.511811] dyndbg: flags=0x1
[   17.512127] dyndbg: *flagsp=0x1 *maskp=0xffffffff
[   17.512604] dyndbg: parsed: func="" file="" module="" format="" lineno=0-0 class=L4
[   17.513414] dyndbg: applied: func="" file="" module="" format="" lineno=0-0 class=L4
[   17.514204] dyndbg: processed 1 queries, with 1 matches, 0 errs
[   17.514809] dyndbg: bit_4: 1 matches on class: L4 -> 0x1f
[   17.515355] dyndbg: p_level_names: changed bit-4: "L4" f->1f
[   17.515933] dyndbg: total matches: 1
crap [[ 5 != 4 ]]

This -1 adjustment just reports the edge consistently with its
input-mapping.

Fixes: b9400852c080 (dyndbg: add drm.debug style (drm/parameters/debug) bitmap support)

Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
 lib/dynamic_debug.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 009f2ead09c1..48ca1387a409 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -794,6 +794,8 @@ int param_get_dyndbg_classes(char *buffer, const struct kernel_param *kp)
 		return scnprintf(buffer, PAGE_SIZE, "0x%lx\n", *dcp->bits);
 
 	case DD_CLASS_TYPE_LEVEL_NAMES:
+		return scnprintf(buffer, PAGE_SIZE, "%d\n", *dcp->lvl - 1);
+
 	case DD_CLASS_TYPE_LEVEL_NUM:
 		return scnprintf(buffer, PAGE_SIZE, "%d\n", *dcp->lvl);
 	default:
-- 
2.38.1

[RFC PATCH 04/17] dyndbg: replace classmap list with a vector

[Jim Cromie]

Classmaps are stored/linked in a section/array, but are each added to
the module's ddebug_table.maps list-head.

This is unnecessary; even when ddebug_attach_classmap() is handling
the builtin section (with classmaps for multiple builtin modules), its
contents are ordered, so a module's possibly multiple classmaps will
be consecutive in the section, and could be treated as a vector/block,
since both start-addy and subrange length are in the ddebug_info arg.

So this changes:

struct ddebug_class_map drops list link.

struct ddebug_table drops the maps list-head, and gets: classes &
num_classes for the start-addy, and length. num_classes is placed to
improve struct packing.

The loading: in ddebug_attach_module_classes(), replace the
for-the-modname list-add loop, with a forloop that finds the module's
subrange (start,length) of matching classmaps within the possibly
builtin classmaps vector, and saves those to the ddebug_table.

The reading/using: change list-foreach loops in ddebug_class_name() &
ddebug_find_valid_class() to walk the array from start to length.

Also:
Move #define __outvar up, above an added use in a fn-prototype.
Simplify ddebug_attach_module_classes args, ref has both addy,len.

This isn't technically a bugfix, but IMO simplifies later fixes for
the chicken-egg post-init enablement regression.

Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
 include/linux/dynamic_debug.h |  1 -
 lib/dynamic_debug.c           | 61 ++++++++++++++++++-----------------
 2 files changed, 32 insertions(+), 30 deletions(-)

diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index 41682278d2e8..bf47bcfad8e6 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -81,7 +81,6 @@ enum class_map_type {
 };
 
 struct ddebug_class_map {
-	struct list_head link;
 	struct module *mod;
 	const char *mod_name;	/* needed for builtins */
 	const char **class_names;
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 48ca1387a409..fd5296dbb40f 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -45,10 +45,11 @@ extern struct ddebug_class_map __start___dyndbg_classes[];
 extern struct ddebug_class_map __stop___dyndbg_classes[];
 
 struct ddebug_table {
-	struct list_head link, maps;
+	struct list_head link;
 	const char *mod_name;
-	unsigned int num_ddebugs;
 	struct _ddebug *ddebugs;
+	struct ddebug_class_map *classes;
+	unsigned int num_ddebugs, num_classes;
 };
 
 struct ddebug_query {
@@ -146,13 +147,15 @@ static void vpr_info_dq(const struct ddebug_query *query, const char *msg)
 		  query->first_lineno, query->last_lineno, query->class_string);
 }
 
+#define __outvar /* filled by callee */
 static struct ddebug_class_map *ddebug_find_valid_class(struct ddebug_table const *dt,
-							  const char *class_string, int *class_id)
+							const char *class_string,
+							__outvar int *class_id)
 {
 	struct ddebug_class_map *map;
-	int idx;
+	int i, idx;
 
-	list_for_each_entry(map, &dt->maps, link) {
+	for (map = dt->classes, i = 0; i < dt->num_classes; i++, map++) {
 		idx = match_string(map->class_names, map->length, class_string);
 		if (idx >= 0) {
 			*class_id = idx + map->base;
@@ -163,7 +166,6 @@ static struct ddebug_class_map *ddebug_find_valid_class(struct ddebug_table cons
 	return NULL;
 }
 
-#define __outvar /* filled by callee */
 /*
  * Search the tables for _ddebug's which match the given `query' and
  * apply the `flags' and `mask' to them.  Returns number of matching
@@ -1109,9 +1111,10 @@ static void *ddebug_proc_next(struct seq_file *m, void *p, loff_t *pos)
 
 static const char *ddebug_class_name(struct ddebug_iter *iter, struct _ddebug *dp)
 {
-	struct ddebug_class_map *map;
+	struct ddebug_class_map *map = iter->table->classes;
+	int i, nc = iter->table->num_classes;
 
-	list_for_each_entry(map, &iter->table->maps, link)
+	for (i = 0; i < nc; i++, map++)
 		if (class_in_range(dp->class_id, map))
 			return map->class_names[dp->class_id - map->base];
 
@@ -1195,30 +1198,31 @@ static const struct proc_ops proc_fops = {
 	.proc_write = ddebug_proc_write
 };
 
-static void ddebug_attach_module_classes(struct ddebug_table *dt,
-					 struct ddebug_class_map *classes,
-					 int num_classes)
+static void ddebug_attach_module_classes(struct ddebug_table *dt, struct _ddebug_info *di)
 {
 	struct ddebug_class_map *cm;
-	int i, j, ct = 0;
+	int i, nc = 0;
 
-	for (cm = classes, i = 0; i < num_classes; i++, cm++) {
+	/*
+	 * Find this module's classmaps in a subrange/wholerange of
+	 * the builtin/modular classmap vector/section.  Save the start
+	 * and length of the subrange at its edges.
+	 */
+	for (cm = di->classes, i = 0; i < di->num_classes; i++, cm++) {
 
 		if (!strcmp(cm->mod_name, dt->mod_name)) {
-
-			v2pr_info("class[%d]: module:%s base:%d len:%d ty:%d\n", i,
-				  cm->mod_name, cm->base, cm->length, cm->map_type);
-
-			for (j = 0; j < cm->length; j++)
-				v3pr_info(" %d: %d %s\n", j + cm->base, j,
-					  cm->class_names[j]);
-
-			list_add(&cm->link, &dt->maps);
-			ct++;
+			if (!nc) {
+				v2pr_info("start subrange, class[%d]: module:%s base:%d len:%d ty:%d\n",
+					  i, cm->mod_name, cm->base, cm->length, cm->map_type);
+				dt->classes = cm;
+			}
+			nc++;
 		}
 	}
-	if (ct)
-		vpr_info("module:%s attached %d classes\n", dt->mod_name, ct);
+	if (nc) {
+		dt->num_classes = nc;
+		vpr_info("module:%s attached %d classes\n", dt->mod_name, nc);
+	}
 }
 
 /*
@@ -1252,10 +1256,9 @@ static int __ddebug_add_module(struct _ddebug_info *di, unsigned int base,
 	dt->num_ddebugs = di->num_descs;
 
 	INIT_LIST_HEAD(&dt->link);
-	INIT_LIST_HEAD(&dt->maps);
 
 	if (di->classes && di->num_classes)
-		ddebug_attach_module_classes(dt, di->classes, di->num_classes);
+		ddebug_attach_module_classes(dt, di);
 
 	mutex_lock(&ddebug_lock);
 	list_add_tail(&dt->link, &ddebug_tables);
@@ -1344,8 +1347,8 @@ static void ddebug_remove_all_tables(void)
 	mutex_lock(&ddebug_lock);
 	while (!list_empty(&ddebug_tables)) {
 		struct ddebug_table *dt = list_entry(ddebug_tables.next,
-						      struct ddebug_table,
-						      link);
+						     struct ddebug_table,
+						     link);
 		ddebug_table_free(dt);
 	}
 	mutex_unlock(&ddebug_lock);
-- 
2.38.1

[RFC PATCH 05/17] dyndbg: make ddebug_apply_class_bitmap more selective

[Jim Cromie]

ddebug_apply_class_bitmap() currently applies the class settings to
all modules, make it more selective, by adding query_module param.

The fn now calls ddebug_exec_queries(query, NULL), where NULL is
query_modname wildcard ("*" does the same).  So just expose the
parameter, and alter users to explicitly pass the wildcard value.

This allows its more selective use later; for propagating drm.debug
settings to dependent modules when/just-after they load.  Doing this
propagation with "*" is fine, but would match with all previously
loaded modules, creating more dynamic_debug.verbose=3 logging
activity.

No functional change.

Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---

after `modprobe i915`, heres the module dependencies,
though not all on drm.debug.

bash-5.2# lsmod
Module                  Size  Used by
i915                 3133440  0
drm_buddy              20480  1 i915
ttm                    90112  1 i915
i2c_algo_bit           16384  1 i915
video                  61440  1 i915
wmi                    32768  1 video
drm_display_helper    200704  1 i915
drm_kms_helper        208896  2 drm_display_helper,i915
drm                   606208  5 drm_kms_helper,drm_display_helper,drm_buddy,i915,ttm
cec                    57344  2 drm_display_helper,i915
---
 lib/dynamic_debug.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index fd5296dbb40f..5d609ff0d559 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -600,7 +600,8 @@ static int ddebug_exec_queries(char *query, const char *modname)
 
 /* apply a new bitmap to the sys-knob's current bit-state */
 static int ddebug_apply_class_bitmap(const struct ddebug_class_param *dcp,
-				     unsigned long *new_bits, unsigned long *old_bits)
+				     unsigned long *new_bits, unsigned long *old_bits,
+				     const char *query_modname)
 {
 #define QUERY_SIZE 128
 	char query[QUERY_SIZE];
@@ -617,7 +618,7 @@ static int ddebug_apply_class_bitmap(const struct ddebug_class_param *dcp,
 		snprintf(query, QUERY_SIZE, "class %s %c%s", map->class_names[bi],
 			 test_bit(bi, new_bits) ? '+' : '-', dcp->flags);
 
-		ct = ddebug_exec_queries(query, NULL);
+		ct = ddebug_exec_queries(query, query_modname);
 		matches += ct;
 
 		v2pr_info("bit_%d: %d matches on class: %s -> 0x%lx\n", bi,
@@ -678,7 +679,7 @@ static int param_set_dyndbg_classnames(const char *instr, const struct kernel_pa
 				continue;
 			}
 			curr_bits ^= BIT(cls_id);
-			totct += ddebug_apply_class_bitmap(dcp, &curr_bits, dcp->bits);
+			totct += ddebug_apply_class_bitmap(dcp, &curr_bits, dcp->bits, NULL);
 			*dcp->bits = curr_bits;
 			v2pr_info("%s: changed bit %d:%s\n", KP_NAME(kp), cls_id,
 				  map->class_names[cls_id]);
@@ -688,7 +689,7 @@ static int param_set_dyndbg_classnames(const char *instr, const struct kernel_pa
 			old_bits = CLASSMAP_BITMASK(*dcp->lvl);
 			curr_bits = CLASSMAP_BITMASK(cls_id + (wanted ? 1 : 0 ));
 
-			totct += ddebug_apply_class_bitmap(dcp, &curr_bits, &old_bits);
+			totct += ddebug_apply_class_bitmap(dcp, &curr_bits, &old_bits, NULL);
 			*dcp->lvl = (cls_id + (wanted ? 1 : 0));
 			v2pr_info("%s: changed bit-%d: \"%s\" %lx->%lx\n", KP_NAME(kp), cls_id,
 				  map->class_names[cls_id], old_bits, curr_bits);
@@ -751,7 +752,7 @@ int param_set_dyndbg_classes(const char *instr, const struct kernel_param *kp)
 			inrep &= CLASSMAP_BITMASK(map->length);
 		}
 		v2pr_info("bits:%lx > %s\n", inrep, KP_NAME(kp));
-		totct += ddebug_apply_class_bitmap(dcp, &inrep, dcp->bits);
+		totct += ddebug_apply_class_bitmap(dcp, &inrep, dcp->bits, NULL);
 		*dcp->bits = inrep;
 		break;
 	case DD_CLASS_TYPE_LEVEL_NUM:
@@ -764,7 +765,7 @@ int param_set_dyndbg_classes(const char *instr, const struct kernel_param *kp)
 		old_bits = CLASSMAP_BITMASK(*dcp->lvl);
 		new_bits = CLASSMAP_BITMASK(inrep);
 		v2pr_info("lvl:%ld bits:0x%lx > %s\n", inrep, new_bits, KP_NAME(kp));
-		totct += ddebug_apply_class_bitmap(dcp, &new_bits, &old_bits);
+		totct += ddebug_apply_class_bitmap(dcp, &new_bits, &old_bits, NULL);
 		*dcp->lvl = inrep;
 		break;
 	default:
-- 
2.38.1

[RFC PATCH 06/17] dyndbg: dynamic_debug_init - use pointer inequality, not strcmp

[Jim Cromie]

dynamic_debug_init() currently uses strcmp to find the module
boundaries in the builtin _ddebug[] table.

The table is filled by the linker; for its content, pointer inequality
works, is faster, and communicates the data properties more tightly.

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 5d609ff0d559..a0dc681cd215 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -1410,7 +1410,7 @@ static int __init dynamic_debug_init(void)
 
 	for (; iter < __stop___dyndbg; iter++, i++, mod_sites++) {
 
-		if (strcmp(modname, iter->modname)) {
+		if (modname != iter->modname) {
 			mod_ct++;
 			di.num_descs = mod_sites;
 			di.descs = iter_mod_start;
-- 
2.38.1

[RFC PATCH 07/17] dyndbg: drop NUM_TYPE_ARRAY

[Jim Cromie]

ARRAY_SIZE works here, since array decl is complete.

Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
 include/linux/dynamic_debug.h | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index bf47bcfad8e6..81b643ab7f6e 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -104,11 +104,9 @@ struct ddebug_class_map {
 		.mod_name = KBUILD_MODNAME,				\
 		.base = _base,						\
 		.map_type = _maptype,					\
-		.length = NUM_TYPE_ARGS(char*, __VA_ARGS__),		\
+		.length = ARRAY_SIZE(_var##_classnames),		\
 		.class_names = _var##_classnames,			\
 	}
-#define NUM_TYPE_ARGS(eltype, ...)				\
-        (sizeof((eltype[]){__VA_ARGS__}) / sizeof(eltype))
 
 /* encapsulate linker provided built-in (or module) dyndbg data */
 struct _ddebug_info {
-- 
2.38.1

[RFC PATCH 08/17] dyndbg: reduce verbose/debug clutter

[Jim Cromie]

currently, for verbose=3, this is logged:

[ 3832.333424] dyndbg: parsed: func="" file="" module="amdgpu" format="" lineno=0-0 class=DRM_UT_PRIME
[ 3832.333888] dyndbg: no matches for query
[ 3832.334093] dyndbg: no-match: func="" file="" module="amdgpu" format="" lineno=0-0 class=DRM_UT_PRIME
[ 3832.334570] dyndbg: processed 1 queries, with 0 matches, 0 errs

This patch removes 2nd & 3rd lines;
 3 differs from 1 only by status
 2 is just status, retold in 4, with more info.

Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
 lib/dynamic_debug.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index a0dc681cd215..445f25ef2461 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -265,9 +265,6 @@ static int ddebug_change(const struct ddebug_query *query,
 	}
 	mutex_unlock(&ddebug_lock);
 
-	if (!nfound && verbose)
-		pr_info("no matches for query\n");
-
 	return nfound;
 }
 
@@ -536,7 +533,7 @@ static int ddebug_exec_query(char *query_string, const char *modname)
 	struct flag_settings modifiers = {};
 	struct ddebug_query query = {};
 #define MAXWORDS 9
-	int nwords, nfound;
+	int nwords;
 	char *words[MAXWORDS];
 
 	nwords = ddebug_tokenize(query_string, words, MAXWORDS);
@@ -554,10 +551,7 @@ static int ddebug_exec_query(char *query_string, const char *modname)
 		return -EINVAL;
 	}
 	/* actually go and implement the change */
-	nfound = ddebug_change(&query, &modifiers);
-	vpr_info_dq(&query, nfound ? "applied" : "no-match");
-
-	return nfound;
+	return ddebug_change(&query, &modifiers);
 }
 
 /* handle multiple queries in query string, continue on error, return
-- 
2.38.1

[RFC PATCH 09/17] dyndbg-API: replace DECLARE_DYNDBG_CLASSMAP with DYNDBG_CLASSMAP(_DEFINE|_USE)

[Jim Cromie]

DECLARE_DYNDBG_CLASSMAPs job was to allow modules to declare the debug
classes/categories they want dyndbg to >control on their behalf.  Its
args give the class-names, their mapping to class_ids, and the sysfs
interface style (usually a class-bitmap).  Modules wanting a drm.debug
style knob need to create the kparam, and call module_param_cb() to
wire the sysfs node to the classmap.  DRM does this is in drm_print.c

In DRM, multiple modules declare identical DRM_UT_* classmaps, so that
the class'd prdbgs are modified across those modules in a coordinated
way across the subsystem, by either explicit class DRM_UT_* queries to
>control, or by writes to /sys/module/drm/parameters/debug (drm.debug)

This coordination-by-identical-declarations is weird, so this patch
splits the macro into _DEFINE and _USE flavors.  This distinction
follows the "define-once, declare-many-uses" principle, so it improves
the api; _DEFINE is used once to specify the classmap, and multiple
users _USE the single definition explicitly.

Currently the latter just reuses the former, and still needs all the
same args, but that can be tuned later; the _DEFINE can initialize an
(extern/global) struct classmap, and _USE can, well use/reference
that struct.

Also wrap DYNDBG_CLASSMAP_USEs with ifdef DRM_USE_DYNAMIC_DEBUG to
balance with the one around drm_print's use of DYNDBG_CLASSMAP_DEFINE.

Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c |  4 +++-
 drivers/gpu/drm/display/drm_dp_helper.c |  4 +++-
 drivers/gpu/drm/drm_crtc_helper.c       |  4 +++-
 drivers/gpu/drm/drm_print.c             |  2 +-
 drivers/gpu/drm/i915/i915_params.c      |  4 +++-
 drivers/gpu/drm/nouveau/nouveau_drm.c   |  4 +++-
 include/linux/dynamic_debug.h           | 20 ++++++++++++----
 lib/test_dynamic_debug.c                | 32 ++++++++++++-------------
 8 files changed, 48 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index bf2d50c8c92a..0075184b5d93 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -188,7 +188,8 @@ int amdgpu_vcnfw_log;
 
 static void amdgpu_drv_delayed_reset_work_handler(struct work_struct *work);
 
-DECLARE_DYNDBG_CLASSMAP(drm_debug_classes, DD_CLASS_TYPE_DISJOINT_BITS, 0,
+#if defined(CONFIG_DRM_USE_DYNAMIC_DEBUG)
+DYNDBG_CLASSMAP_USE(drm_debug_classes, DD_CLASS_TYPE_DISJOINT_BITS, 0,
 			"DRM_UT_CORE",
 			"DRM_UT_DRIVER",
 			"DRM_UT_KMS",
@@ -199,6 +200,7 @@ DECLARE_DYNDBG_CLASSMAP(drm_debug_classes, DD_CLASS_TYPE_DISJOINT_BITS, 0,
 			"DRM_UT_LEASE",
 			"DRM_UT_DP",
 			"DRM_UT_DRMRES");
+#endif
 
 struct amdgpu_mgpu_info mgpu_info = {
 	.mutex = __MUTEX_INITIALIZER(mgpu_info.mutex),
diff --git a/drivers/gpu/drm/display/drm_dp_helper.c b/drivers/gpu/drm/display/drm_dp_helper.c
index 16565a0a5da6..8fa7a88299e7 100644
--- a/drivers/gpu/drm/display/drm_dp_helper.c
+++ b/drivers/gpu/drm/display/drm_dp_helper.c
@@ -41,7 +41,8 @@
 
 #include "drm_dp_helper_internal.h"
 
-DECLARE_DYNDBG_CLASSMAP(drm_debug_classes, DD_CLASS_TYPE_DISJOINT_BITS, 0,
+#if defined(CONFIG_DRM_USE_DYNAMIC_DEBUG)
+DYNDBG_CLASSMAP_USE(drm_debug_classes, DD_CLASS_TYPE_DISJOINT_BITS, 0,
 			"DRM_UT_CORE",
 			"DRM_UT_DRIVER",
 			"DRM_UT_KMS",
@@ -52,6 +53,7 @@ DECLARE_DYNDBG_CLASSMAP(drm_debug_classes, DD_CLASS_TYPE_DISJOINT_BITS, 0,
 			"DRM_UT_LEASE",
 			"DRM_UT_DP",
 			"DRM_UT_DRMRES");
+#endif
 
 struct dp_aux_backlight {
 	struct backlight_device *base;
diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c
index 7d86020b5244..2f747c9c8f60 100644
--- a/drivers/gpu/drm/drm_crtc_helper.c
+++ b/drivers/gpu/drm/drm_crtc_helper.c
@@ -51,7 +51,8 @@
 
 #include "drm_crtc_helper_internal.h"
 
-DECLARE_DYNDBG_CLASSMAP(drm_debug_classes, DD_CLASS_TYPE_DISJOINT_BITS, 0,
+#if defined(CONFIG_DRM_USE_DYNAMIC_DEBUG)
+DYNDBG_CLASSMAP_USE(drm_debug_classes, DD_CLASS_TYPE_DISJOINT_BITS, 0,
 			"DRM_UT_CORE",
 			"DRM_UT_DRIVER",
 			"DRM_UT_KMS",
@@ -62,6 +63,7 @@ DECLARE_DYNDBG_CLASSMAP(drm_debug_classes, DD_CLASS_TYPE_DISJOINT_BITS, 0,
 			"DRM_UT_LEASE",
 			"DRM_UT_DP",
 			"DRM_UT_DRMRES");
+#endif
 
 /**
  * DOC: overview
diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c
index 5b93c11895bb..4b697e18238d 100644
--- a/drivers/gpu/drm/drm_print.c
+++ b/drivers/gpu/drm/drm_print.c
@@ -56,7 +56,7 @@ MODULE_PARM_DESC(debug, "Enable debug output, where each bit enables a debug cat
 module_param_named(debug, __drm_debug, ulong, 0600);
 #else
 /* classnames must match vals of enum drm_debug_category */
-DECLARE_DYNDBG_CLASSMAP(drm_debug_classes, DD_CLASS_TYPE_DISJOINT_BITS, 0,
+DYNDBG_CLASSMAP_DEFINE(drm_debug_classes, DD_CLASS_TYPE_DISJOINT_BITS, 0,
 			"DRM_UT_CORE",
 			"DRM_UT_DRIVER",
 			"DRM_UT_KMS",
diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
index d1e4d528cb17..b5b2542ae364 100644
--- a/drivers/gpu/drm/i915/i915_params.c
+++ b/drivers/gpu/drm/i915/i915_params.c
@@ -29,7 +29,8 @@
 #include "i915_params.h"
 #include "i915_drv.h"
 
-DECLARE_DYNDBG_CLASSMAP(drm_debug_classes, DD_CLASS_TYPE_DISJOINT_BITS, 0,
+#if defined(CONFIG_DRM_USE_DYNAMIC_DEBUG)
+DYNDBG_CLASSMAP_USE(drm_debug_classes, DD_CLASS_TYPE_DISJOINT_BITS, 0,
 			"DRM_UT_CORE",
 			"DRM_UT_DRIVER",
 			"DRM_UT_KMS",
@@ -40,6 +41,7 @@ DECLARE_DYNDBG_CLASSMAP(drm_debug_classes, DD_CLASS_TYPE_DISJOINT_BITS, 0,
 			"DRM_UT_LEASE",
 			"DRM_UT_DP",
 			"DRM_UT_DRMRES");
+#endif
 
 #define i915_param_named(name, T, perm, desc) \
 	module_param_named(name, i915_modparams.name, T, perm); \
diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c b/drivers/gpu/drm/nouveau/nouveau_drm.c
index fd99ec0f4257..2963cf5b0807 100644
--- a/drivers/gpu/drm/nouveau/nouveau_drm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
@@ -71,7 +71,8 @@
 #include "nouveau_svm.h"
 #include "nouveau_dmem.h"
 
-DECLARE_DYNDBG_CLASSMAP(drm_debug_classes, DD_CLASS_TYPE_DISJOINT_BITS, 0,
+#if defined(CONFIG_DRM_USE_DYNAMIC_DEBUG)
+DYNDBG_CLASSMAP_USE(drm_debug_classes, DD_CLASS_TYPE_DISJOINT_BITS, 0,
 			"DRM_UT_CORE",
 			"DRM_UT_DRIVER",
 			"DRM_UT_KMS",
@@ -82,6 +83,7 @@ DECLARE_DYNDBG_CLASSMAP(drm_debug_classes, DD_CLASS_TYPE_DISJOINT_BITS, 0,
 			"DRM_UT_LEASE",
 			"DRM_UT_DP",
 			"DRM_UT_DRMRES");
+#endif
 
 MODULE_PARM_DESC(config, "option string to pass to driver core");
 static char *nouveau_config;
diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index 81b643ab7f6e..1cdfd62fd2e4 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -56,7 +56,7 @@ struct _ddebug {
 #endif
 } __attribute__((aligned(8)));
 
-enum class_map_type {
+enum ddebug_class_map_type {
 	DD_CLASS_TYPE_DISJOINT_BITS,
 	/**
 	 * DD_CLASS_TYPE_DISJOINT_BITS: classes are independent, one per bit.
@@ -86,17 +86,19 @@ struct ddebug_class_map {
 	const char **class_names;
 	const int length;
 	const int base;		/* index of 1st .class_id, allows split/shared space */
-	enum class_map_type map_type;
+	enum ddebug_class_map_type map_type;
 };
 
 /**
- * DECLARE_DYNDBG_CLASSMAP - declare classnames known by a module
+ * DYNDBG_CLASSMAP_DEFINE - define the class_map that names the
+ * debug classes used in this module.  This tells dyndbg the authorized
+ * classnames it should manipulate.
  * @_var:   a struct ddebug_class_map, passed to module_param_cb
  * @_type:  enum class_map_type, chooses bits/verbose, numeric/symbolic
  * @_base:  offset of 1st class-name. splits .class_id space
  * @classes: class-names used to control class'd prdbgs
  */
-#define DECLARE_DYNDBG_CLASSMAP(_var, _maptype, _base, ...)		\
+#define DYNDBG_CLASSMAP_DEFINE(_var, _maptype, _base, ...)		\
 	static const char *_var##_classnames[] = { __VA_ARGS__ };	\
 	static struct ddebug_class_map __aligned(8) __used		\
 		__section("__dyndbg_classes") _var = {			\
@@ -108,6 +110,16 @@ struct ddebug_class_map {
 		.class_names = _var##_classnames,			\
 	}
 
+/*
+ * refer to the classmap instantiated once, by the macro above.  This
+ * distinguishes the multiple users of drm.debug from the single
+ * definition, allowing them to specialize.  ATM its a pass-thru, but
+ * it should help regularize the admittedly wierd sharing by identical
+ * definitions.
+ */
+#define DYNDBG_CLASSMAP_USE(_var, _maptype, _base, ...)		\
+	DYNDBG_CLASSMAP_DEFINE(_var, _maptype, _base, __VA_ARGS__)
+
 /* encapsulate linker provided built-in (or module) dyndbg data */
 struct _ddebug_info {
 	struct _ddebug *descs;
diff --git a/lib/test_dynamic_debug.c b/lib/test_dynamic_debug.c
index 9d48689dc0ab..4ae01f7fa920 100644
--- a/lib/test_dynamic_debug.c
+++ b/lib/test_dynamic_debug.c
@@ -62,38 +62,38 @@ enum cat_disjoint_bits {
 	D2_LEASE,
 	D2_DP,
 	D2_DRMRES };
-DECLARE_DYNDBG_CLASSMAP(map_disjoint_bits, DD_CLASS_TYPE_DISJOINT_BITS, 0,
-			"D2_CORE",
-			"D2_DRIVER",
-			"D2_KMS",
-			"D2_PRIME",
-			"D2_ATOMIC",
-			"D2_VBL",
-			"D2_STATE",
-			"D2_LEASE",
-			"D2_DP",
-			"D2_DRMRES");
+DYNDBG_CLASSMAP_DEFINE(map_disjoint_bits, DD_CLASS_TYPE_DISJOINT_BITS, 0,
+		       "D2_CORE",
+		       "D2_DRIVER",
+		       "D2_KMS",
+		       "D2_PRIME",
+		       "D2_ATOMIC",
+		       "D2_VBL",
+		       "D2_STATE",
+		       "D2_LEASE",
+		       "D2_DP",
+		       "D2_DRMRES");
 DD_SYS_WRAP(disjoint_bits, p);
 DD_SYS_WRAP(disjoint_bits, T);
 
 /* symbolic input, independent bits */
 enum cat_disjoint_names { LOW = 10, MID, HI };
-DECLARE_DYNDBG_CLASSMAP(map_disjoint_names, DD_CLASS_TYPE_DISJOINT_NAMES, 10,
-			"LOW", "MID", "HI");
+DYNDBG_CLASSMAP_DEFINE(map_disjoint_names, DD_CLASS_TYPE_DISJOINT_NAMES, 10,
+		       "LOW", "MID", "HI");
 DD_SYS_WRAP(disjoint_names, p);
 DD_SYS_WRAP(disjoint_names, T);
 
 /* numeric verbosity, V2 > V1 related */
 enum cat_level_num { V0 = 14, V1, V2, V3, V4, V5, V6, V7 };
-DECLARE_DYNDBG_CLASSMAP(map_level_num, DD_CLASS_TYPE_LEVEL_NUM, 14,
+DYNDBG_CLASSMAP_DEFINE(map_level_num, DD_CLASS_TYPE_LEVEL_NUM, 14,
 		       "V0", "V1", "V2", "V3", "V4", "V5", "V6", "V7");
 DD_SYS_WRAP(level_num, p);
 DD_SYS_WRAP(level_num, T);
 
 /* symbolic verbosity */
 enum cat_level_names { L0 = 22, L1, L2, L3, L4, L5, L6, L7 };
-DECLARE_DYNDBG_CLASSMAP(map_level_names, DD_CLASS_TYPE_LEVEL_NAMES, 22,
-			"L0", "L1", "L2", "L3", "L4", "L5", "L6", "L7");
+DYNDBG_CLASSMAP_DEFINE(map_level_names, DD_CLASS_TYPE_LEVEL_NAMES, 22,
+		       "L0", "L1", "L2", "L3", "L4", "L5", "L6", "L7");
 DD_SYS_WRAP(level_names, p);
 DD_SYS_WRAP(level_names, T);
 
-- 
2.38.1

[RFC PATCH 10/17] dyndbg-API: specialize DYNDBG_CLASSMAP_(DEFINE|USE)

[Jim Cromie]

Now that the DECLARE_DYNDBG_CLASSMAP macro has been split into
DYNDBG_CLASSMAP_DEFINE and DYNDBG_CLASSMAP_USE variants, lets
differentiate them according to their separate jobs.

Dyndbg's existing __dyndbg_classes[] section does:

. catalogs the classmaps defined by the module (or builtin modules)
. authorizes dyndbg to >control those class'd prdbgs for the module.

This patch adds __dyndbg_class_refs[] section:

. catalogs references/uses of the above classmap definitions.
. authorizes dyndbg to >control those class'd prdbgs in ref'g module.
. maps the client module to classmap definitions
  this allows dyndbg to propagate drm.debug to the client module.

The distinction of the 2 roles yields 2 gains:

It follows the define-once-declare-elsewhere pattern that K&R gave us,
dumping the weird coordinated-changes-by-identical-classmaps API.

It should help solve the chicken-egg problem that drm.debug-on-dyndbg
has; the _USEr module must propagate the drm.debug setting once the
using module has loaded.

The new DYNDBG_CLASSMAP macros add records to the sections:

DYNDBG_CLASSMAP_DEFINE:
  invoked by drm_print, just once per sub-system.
  defines the classmap, names "DRM_UT_*", maps to class_id's
  authorizes dyndbg to exert >control
  populates __dyndbg_classes[] "section", __used.
  exports the classmap.

DYNDBG_CLASSMAP_USE:
  invoked by modules using classmaps defined & exported elsewhere
  populates __dyndbg_class_refs[] "section", __used.
  maps client-module name to the extern'd classmap.
  no client-name yet, but will need it.

also:

struct ddebug_info gets 2 new fields to encapsulate the new section:
  class_refs, num_class_refs.
  set by dynamic_debug_init() for builtins.
  or by kernel/module/main:load_info() for loadable modules.

. struct ddebug_class_user
  contains: user-module-name, ref to classmap-defn
  dyndbg finds drm-driver's use of a classmap, gets/applies its settings

. struct ddebug_class_map gets .knob ptr to ddebug_class_param.
  compiled null, init'd under ddebug_add_module()
  allows finding drm.debug setting.

. vmlinux.lds.h additions: linker symbols, KEEP for new section

dynamic_debug.c:

ddebug_attach_module_classes():
  as before
  foreach __dyndbg_classes: ddebug_find_kparam(classmap*)

ddebug_attach_client_module_classes():
  foreach __dyndbg_class_refs: ddebug_param_load_dependent_class(classmap*)
  called after list-add to ddebug-tables.

ddebug_load_dependent_class():

This applies >controls to the module, it needs the module to be
present in the ddebug-tables list so that ddebug_change can apply the
changes to it.  So in ddebug_add_module, call 2nd fn *after* adding
the ddebug_table to the list.

ddebug_find_kparam(classmap*):

Finds the kernel-param / sysfs-node that controls the classmap, by
verifying that dyndbg's kparam-ops are used by the kparam.  The found
kparams arg is our ddebug_class_param, and has a ref to the state-var
under the sysfs-node.

ddebug_match_attach_kparam():

tests that kparam.ops == dyndbg's, then casts arg to
ddebug_class_param, and tests modnames for equality.

ddebug_param_load_dependent_class(struct ddebug_class_user*):

Called on class_refs entries, these classmaps have already been seen
by ddebug_find_kparam() which has wired the classmap.knob to the kparam.
So this fn just calls ddebug_apply_class_bitmap() on the state-var.

ddebug_find_valid_class():

This helps ddebug_change(), doing the search over classmaps, looking
for the class given to >control.  So now it searches over
__dyndbg_class_refs[] after __dyndbg_classes[].

Thats the theory anyway.  things are still broken (differently) for
both builtins and loadables.  For loadables, the >control is applied,
but doesnt alter any callsites.

Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
 include/asm-generic/vmlinux.lds.h |   3 +
 include/linux/dynamic_debug.h     |  41 ++++++---
 kernel/module/main.c              |   2 +
 lib/dynamic_debug.c               | 137 +++++++++++++++++++++++++++---
 4 files changed, 157 insertions(+), 26 deletions(-)

diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index 3dc5824141cd..7100701fb68c 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -363,6 +363,9 @@
 	__start___dyndbg_classes = .;					\
 	KEEP(*(__dyndbg_classes))					\
 	__stop___dyndbg_classes = .;					\
+	__start___dyndbg_class_refs = .;				\
+	KEEP(*(__dyndbg_class_refs))					\
+	__stop___dyndbg_class_refs = .;					\
 	__start___dyndbg = .;						\
 	KEEP(*(__dyndbg))						\
 	__stop___dyndbg = .;						\
diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index 1cdfd62fd2e4..dabbe1a9180c 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -81,8 +81,10 @@ enum ddebug_class_map_type {
 };
 
 struct ddebug_class_map {
-	struct module *mod;
-	const char *mod_name;	/* needed for builtins */
+	struct list_head link;
+	struct module *mod;			/* NULL for builtins */
+	const char *mod_name;
+	struct ddebug_class_param *dc_parm;	/* controlling sysfs node */
 	const char **class_names;
 	const int length;
 	const int base;		/* index of 1st .class_id, allows split/shared space */
@@ -99,8 +101,8 @@ struct ddebug_class_map {
  * @classes: class-names used to control class'd prdbgs
  */
 #define DYNDBG_CLASSMAP_DEFINE(_var, _maptype, _base, ...)		\
-	static const char *_var##_classnames[] = { __VA_ARGS__ };	\
-	static struct ddebug_class_map __aligned(8) __used		\
+	const char *_var##_classnames[] = { __VA_ARGS__ };		\
+	struct ddebug_class_map __aligned(8) __used			\
 		__section("__dyndbg_classes") _var = {			\
 		.mod = THIS_MODULE,					\
 		.mod_name = KBUILD_MODNAME,				\
@@ -108,24 +110,37 @@ struct ddebug_class_map {
 		.map_type = _maptype,					\
 		.length = ARRAY_SIZE(_var##_classnames),		\
 		.class_names = _var##_classnames,			\
-	}
+	};								\
+	EXPORT_SYMBOL(_var)
 
-/*
- * refer to the classmap instantiated once, by the macro above.  This
- * distinguishes the multiple users of drm.debug from the single
- * definition, allowing them to specialize.  ATM its a pass-thru, but
- * it should help regularize the admittedly wierd sharing by identical
- * definitions.
+struct ddebug_class_user {
+	char *user_mod_name;
+	struct ddebug_class_map *map;
+};
+/**
+ * DYNDBG_CLASSMAP_USE - Use a classmap DEFINEd in another module.
+ * This lets dyndbg initialize the dependent module's prdbgs from the
+ * other module's controlling sysfs node.
  */
-#define DYNDBG_CLASSMAP_USE(_var, _maptype, _base, ...)		\
-	DYNDBG_CLASSMAP_DEFINE(_var, _maptype, _base, __VA_ARGS__)
+#define DYNDBG_CLASSMAP_USE(_var, ...)					\
+	DYNDBG_CLASSMAP_USE_(_var, __UNIQUE_ID(ddebug_class_user),	\
+			     __VA_ARGS__)
+#define DYNDBG_CLASSMAP_USE_(_var, _uname, ...)				\
+	extern struct ddebug_class_map _var[];				\
+	static struct ddebug_class_user __used				\
+	__section("__dyndbg_class_refs") _uname = {			\
+		.user_mod_name = KBUILD_MODNAME,			\
+		.map = _var,						\
+	}
 
 /* encapsulate linker provided built-in (or module) dyndbg data */
 struct _ddebug_info {
 	struct _ddebug *descs;
 	struct ddebug_class_map *classes;
+	struct ddebug_class_user *class_refs;
 	unsigned int num_descs;
 	unsigned int num_classes;
+	unsigned int num_class_refs;
 };
 
 struct ddebug_class_param {
diff --git a/kernel/module/main.c b/kernel/module/main.c
index d02d39c7174e..ee4f85a3b8f0 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -2111,6 +2111,8 @@ static int find_module_sections(struct module *mod, struct load_info *info)
 					sizeof(*info->dyndbg.descs), &info->dyndbg.num_descs);
 	info->dyndbg.classes = section_objs(info, "__dyndbg_classes",
 					sizeof(*info->dyndbg.classes), &info->dyndbg.num_classes);
+	info->dyndbg.class_refs = section_objs(info, "__dyndbg_class_refs",
+					sizeof(*info->dyndbg.class_refs), &info->dyndbg.num_class_refs);
 
 	return 0;
 }
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 445f25ef2461..45b8414fa130 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -43,13 +43,16 @@ extern struct _ddebug __start___dyndbg[];
 extern struct _ddebug __stop___dyndbg[];
 extern struct ddebug_class_map __start___dyndbg_classes[];
 extern struct ddebug_class_map __stop___dyndbg_classes[];
+extern struct ddebug_class_user __start___dyndbg_class_refs[];
+extern struct ddebug_class_user __stop___dyndbg_class_refs[];
 
 struct ddebug_table {
 	struct list_head link;
 	const char *mod_name;
 	struct _ddebug *ddebugs;
 	struct ddebug_class_map *classes;
-	unsigned int num_ddebugs, num_classes;
+	struct ddebug_class_user *class_refs;
+	unsigned int num_ddebugs, num_classes, num_class_refs;
 };
 
 struct ddebug_query {
@@ -153,15 +156,23 @@ static struct ddebug_class_map *ddebug_find_valid_class(struct ddebug_table cons
 							__outvar int *class_id)
 {
 	struct ddebug_class_map *map;
+	struct ddebug_class_user *cli;
 	int i, idx;
 
-	for (map = dt->classes, i = 0; i < dt->num_classes; i++, map++) {
+	for (i = 0, map = dt->classes; i < dt->num_classes; i++, map++) {
 		idx = match_string(map->class_names, map->length, class_string);
 		if (idx >= 0) {
 			*class_id = idx + map->base;
 			return map;
 		}
 	}
+	for (i = 0, cli = dt->class_refs; i < dt->num_class_refs; i++, cli++) {
+		idx = match_string(cli->map->class_names, cli->map->length, class_string);
+		if (idx >= 0) {
+			*class_id = idx + map->base;
+			return map;
+		}
+	}
 	*class_id = -ENOENT;
 	return NULL;
 }
@@ -603,7 +614,7 @@ static int ddebug_apply_class_bitmap(const struct ddebug_class_param *dcp,
 	int matches = 0;
 	int bi, ct;
 
-	v2pr_info("apply: 0x%lx to: 0x%lx\n", *new_bits, *old_bits);
+	v2pr_info("apply bitmap: 0x%lx to: 0x%lx\n", *new_bits, *old_bits);
 
 	for (bi = 0; bi < map->length; bi++) {
 		if (test_bit(bi, new_bits) == test_bit(bi, old_bits))
@@ -1106,13 +1117,19 @@ static void *ddebug_proc_next(struct seq_file *m, void *p, loff_t *pos)
 
 static const char *ddebug_class_name(struct ddebug_iter *iter, struct _ddebug *dp)
 {
-	struct ddebug_class_map *map = iter->table->classes;
-	int i, nc = iter->table->num_classes;
+	struct ddebug_table *dt = iter->table;
+	struct ddebug_class_map *map;
+	struct ddebug_class_user *cli;
+	int i;
 
-	for (i = 0; i < nc; i++, map++)
+	for (i = 0, map = dt->classes; i < dt->num_classes; i++, map++)
 		if (class_in_range(dp->class_id, map))
 			return map->class_names[dp->class_id - map->base];
 
+	for (i = 0, cli = dt->class_refs; i < dt->num_class_refs; i++, cli++)
+		if (class_in_range(dp->class_id, cli->map))
+			return cli->map->class_names[dp->class_id - map->base];
+
 	return NULL;
 }
 
@@ -1193,6 +1210,54 @@ static const struct proc_ops proc_fops = {
 	.proc_write = ddebug_proc_write
 };
 
+static void ddebug_match_attach_kparam(const struct kernel_param *kp,
+				       struct ddebug_class_map *cm)
+{
+	struct ddebug_class_param *dcp;
+
+	if (kp->ops != &param_ops_dyndbg_classes)
+		return;
+	dcp = (struct ddebug_class_param *)kp->arg;
+
+	if (!strncmp(cm->mod_name, dcp->map->mod_name, strlen(cm->mod_name))) {
+		cm->dc_parm = dcp;
+		v2pr_info("controlling kp: %s.%s\n", cm->mod_name, kp->name);
+	} else
+		v2pr_info("not this: %s %s\n", cm->mod_name, kp->name);
+}
+
+static void ddebug_find_kparam(struct ddebug_class_map *cm)
+{
+	const struct kernel_param *kp;
+	int i;
+
+	if (cm->mod) {
+		v2pr_info("loaded class: module:%s base:%d len:%d ty:%d\n",
+			  cm->mod_name, cm->base, cm->length, cm->map_type);
+
+		for (i = 0, kp = cm->mod->kp; i < cm->mod->num_kp; i++, kp++)
+			ddebug_match_attach_kparam(kp, cm);
+	} else {
+		v2pr_info("builtin class: module:%s base:%d len:%d ty:%d\n",
+			  cm->mod_name, cm->base, cm->length, cm->map_type);
+
+		for (kp = __start___param; kp < __stop___param; kp++)
+			ddebug_match_attach_kparam(kp, cm);
+	}
+}
+
+static void ddebug_param_load_dependent_class(const struct ddebug_class_user *cli)
+{
+	unsigned long new_bits, old_bits = 0;
+
+	new_bits = *cli->map->dc_parm->bits;
+
+	vpr_info("%s needs %s, 0x%lx\n", cli->user_mod_name,
+		 cli->map->mod_name, new_bits);
+
+	ddebug_apply_class_bitmap(cli->map->dc_parm, &new_bits, &old_bits, cli->user_mod_name);
+}
+
 static void ddebug_attach_module_classes(struct ddebug_table *dt, struct _ddebug_info *di)
 {
 	struct ddebug_class_map *cm;
@@ -1203,20 +1268,60 @@ static void ddebug_attach_module_classes(struct ddebug_table *dt, struct _ddebug
 	 * the builtin/modular classmap vector/section.  Save the start
 	 * and length of the subrange at its edges.
 	 */
-	for (cm = di->classes, i = 0; i < di->num_classes; i++, cm++) {
+	for (i = 0, cm = di->classes; i < di->num_classes; i++, cm++) {
 
 		if (!strcmp(cm->mod_name, dt->mod_name)) {
 			if (!nc) {
-				v2pr_info("start subrange, class[%d]: module:%s base:%d len:%d ty:%d\n",
-					  i, cm->mod_name, cm->base, cm->length, cm->map_type);
 				dt->classes = cm;
+				v2pr_info("classes start: class[%d]: module:%s base:%d len:%d ty:%d\n",
+					  i, cm->mod_name, cm->base, cm->length, cm->map_type);
 			}
+			ddebug_find_kparam(cm);
 			nc++;
 		}
 	}
-	if (nc) {
-		dt->num_classes = nc;
+	dt->num_classes = nc;
+	if (nc)
 		vpr_info("module:%s attached %d classes\n", dt->mod_name, nc);
+}
+
+static void ddebug_attach_client_module_classes(struct ddebug_table *dt, struct _ddebug_info *di)
+{
+	struct ddebug_class_user *cli;
+	int i;
+
+	for (i = 0, cli = di->class_refs; i < di->num_class_refs; i++, cli++) {
+
+		if (!cli) {
+			v2pr_info("NO CLI\n");
+			continue;
+		}
+		if (!cli->map) {
+			v2pr_info("NO CLI map\n");
+			continue;
+		}
+		if (!cli->user_mod_name) {
+			v2pr_info("NO CLI name\n");
+			continue;
+		}
+
+		if (!strcmp(cli->user_mod_name, dt->mod_name)) {
+
+			v2pr_info("class_ref[%d] %s -> %s\n", i,
+				  cli->user_mod_name, cli->map->mod_name);
+
+			ddebug_param_load_dependent_class(cli);
+
+			dt->class_refs = cli;
+			/*
+			 * allow only 1 ref right now, see if that
+			 * works.  dont want to deal with vector,len
+			 * now, maybe inadequate anyway.
+			 */
+			v2pr_info("break on %d/%d\n", i, di->num_class_refs);
+			dt->num_class_refs = 1;
+			break;
+		}
 	}
 }
 
@@ -1229,7 +1334,8 @@ static int __ddebug_add_module(struct _ddebug_info *di, unsigned int base,
 {
 	struct ddebug_table *dt;
 
-	v3pr_info("add-module: %s.%d sites\n", modname, di->num_descs);
+	v3pr_info("add-module: %s %d sites %d.%d\n", modname, di->num_descs,
+		  di->num_classes, di->num_class_refs);
 	if (!di->num_descs) {
 		v3pr_info(" skip %s\n", modname);
 		return 0;
@@ -1252,13 +1358,16 @@ static int __ddebug_add_module(struct _ddebug_info *di, unsigned int base,
 
 	INIT_LIST_HEAD(&dt->link);
 
-	if (di->classes && di->num_classes)
+	if (di->num_classes)
 		ddebug_attach_module_classes(dt, di);
 
 	mutex_lock(&ddebug_lock);
 	list_add_tail(&dt->link, &ddebug_tables);
 	mutex_unlock(&ddebug_lock);
 
+	if (di->num_class_refs)
+		ddebug_attach_client_module_classes(dt, di);
+
 	vpr_info("%3u debug prints in module %s\n", di->num_descs, modname);
 	return 0;
 }
@@ -1384,8 +1493,10 @@ static int __init dynamic_debug_init(void)
 	struct _ddebug_info di = {
 		.descs = __start___dyndbg,
 		.classes = __start___dyndbg_classes,
+		.class_refs = __start___dyndbg_class_refs,
 		.num_descs = __stop___dyndbg - __start___dyndbg,
 		.num_classes = __stop___dyndbg_classes - __start___dyndbg_classes,
+		.num_class_refs = __stop___dyndbg_class_refs - __start___dyndbg_class_refs,
 	};
 
 	if (&__start___dyndbg == &__stop___dyndbg) {
-- 
2.38.1

[RFC PATCH 11/17] dyndbg-API: DYNDBG_CLASSMAP_USE drop extra args

[Jim Cromie]

Drop macro args after _var.  Since DYNDBG_CLASSMAP_USE no longer
forwards to DYNDBG_CLASSMAP_DEFINE, it doesn't need those args to
forward.  Keep only the _var arg, which is the extern'd struct
classmap with all the class info.

Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 12 +---------
 drivers/gpu/drm/display/drm_dp_helper.c | 12 +---------
 drivers/gpu/drm/drm_crtc_helper.c       | 12 +---------
 drivers/gpu/drm/i915/i915_params.c      | 12 +---------
 drivers/gpu/drm/nouveau/nouveau_drm.c   | 12 +---------
 include/linux/dynamic_debug.h           | 30 ++++++++++++++-----------
 6 files changed, 22 insertions(+), 68 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 0075184b5d93..7bcc22ef5d49 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -189,17 +189,7 @@ int amdgpu_vcnfw_log;
 static void amdgpu_drv_delayed_reset_work_handler(struct work_struct *work);
 
 #if defined(CONFIG_DRM_USE_DYNAMIC_DEBUG)
-DYNDBG_CLASSMAP_USE(drm_debug_classes, DD_CLASS_TYPE_DISJOINT_BITS, 0,
-			"DRM_UT_CORE",
-			"DRM_UT_DRIVER",
-			"DRM_UT_KMS",
-			"DRM_UT_PRIME",
-			"DRM_UT_ATOMIC",
-			"DRM_UT_VBL",
-			"DRM_UT_STATE",
-			"DRM_UT_LEASE",
-			"DRM_UT_DP",
-			"DRM_UT_DRMRES");
+DYNDBG_CLASSMAP_USE(drm_debug_classes);
 #endif
 
 struct amdgpu_mgpu_info mgpu_info = {
diff --git a/drivers/gpu/drm/display/drm_dp_helper.c b/drivers/gpu/drm/display/drm_dp_helper.c
index 8fa7a88299e7..3bc188cb1116 100644
--- a/drivers/gpu/drm/display/drm_dp_helper.c
+++ b/drivers/gpu/drm/display/drm_dp_helper.c
@@ -42,17 +42,7 @@
 #include "drm_dp_helper_internal.h"
 
 #if defined(CONFIG_DRM_USE_DYNAMIC_DEBUG)
-DYNDBG_CLASSMAP_USE(drm_debug_classes, DD_CLASS_TYPE_DISJOINT_BITS, 0,
-			"DRM_UT_CORE",
-			"DRM_UT_DRIVER",
-			"DRM_UT_KMS",
-			"DRM_UT_PRIME",
-			"DRM_UT_ATOMIC",
-			"DRM_UT_VBL",
-			"DRM_UT_STATE",
-			"DRM_UT_LEASE",
-			"DRM_UT_DP",
-			"DRM_UT_DRMRES");
+DYNDBG_CLASSMAP_USE(drm_debug_classes);
 #endif
 
 struct dp_aux_backlight {
diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c
index 2f747c9c8f60..5fb83336b015 100644
--- a/drivers/gpu/drm/drm_crtc_helper.c
+++ b/drivers/gpu/drm/drm_crtc_helper.c
@@ -52,17 +52,7 @@
 #include "drm_crtc_helper_internal.h"
 
 #if defined(CONFIG_DRM_USE_DYNAMIC_DEBUG)
-DYNDBG_CLASSMAP_USE(drm_debug_classes, DD_CLASS_TYPE_DISJOINT_BITS, 0,
-			"DRM_UT_CORE",
-			"DRM_UT_DRIVER",
-			"DRM_UT_KMS",
-			"DRM_UT_PRIME",
-			"DRM_UT_ATOMIC",
-			"DRM_UT_VBL",
-			"DRM_UT_STATE",
-			"DRM_UT_LEASE",
-			"DRM_UT_DP",
-			"DRM_UT_DRMRES");
+DYNDBG_CLASSMAP_USE(drm_debug_classes);
 #endif
 
 /**
diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
index b5b2542ae364..e959d0384ead 100644
--- a/drivers/gpu/drm/i915/i915_params.c
+++ b/drivers/gpu/drm/i915/i915_params.c
@@ -30,17 +30,7 @@
 #include "i915_drv.h"
 
 #if defined(CONFIG_DRM_USE_DYNAMIC_DEBUG)
-DYNDBG_CLASSMAP_USE(drm_debug_classes, DD_CLASS_TYPE_DISJOINT_BITS, 0,
-			"DRM_UT_CORE",
-			"DRM_UT_DRIVER",
-			"DRM_UT_KMS",
-			"DRM_UT_PRIME",
-			"DRM_UT_ATOMIC",
-			"DRM_UT_VBL",
-			"DRM_UT_STATE",
-			"DRM_UT_LEASE",
-			"DRM_UT_DP",
-			"DRM_UT_DRMRES");
+DYNDBG_CLASSMAP_USE(drm_debug_classes);
 #endif
 
 #define i915_param_named(name, T, perm, desc) \
diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c b/drivers/gpu/drm/nouveau/nouveau_drm.c
index 2963cf5b0807..609edeb2a117 100644
--- a/drivers/gpu/drm/nouveau/nouveau_drm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
@@ -72,17 +72,7 @@
 #include "nouveau_dmem.h"
 
 #if defined(CONFIG_DRM_USE_DYNAMIC_DEBUG)
-DYNDBG_CLASSMAP_USE(drm_debug_classes, DD_CLASS_TYPE_DISJOINT_BITS, 0,
-			"DRM_UT_CORE",
-			"DRM_UT_DRIVER",
-			"DRM_UT_KMS",
-			"DRM_UT_PRIME",
-			"DRM_UT_ATOMIC",
-			"DRM_UT_VBL",
-			"DRM_UT_STATE",
-			"DRM_UT_LEASE",
-			"DRM_UT_DP",
-			"DRM_UT_DRMRES");
+DYNDBG_CLASSMAP_USE(drm_debug_classes);
 #endif
 
 MODULE_PARM_DESC(config, "option string to pass to driver core");
diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index dabbe1a9180c..0088fc354c98 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -92,13 +92,15 @@ struct ddebug_class_map {
 };
 
 /**
- * DYNDBG_CLASSMAP_DEFINE - define the class_map that names the
- * debug classes used in this module.  This tells dyndbg the authorized
- * classnames it should manipulate.
- * @_var:   a struct ddebug_class_map, passed to module_param_cb
+ * DYNDBG_CLASSMAP_DEFINE - define debug-classes used by a module.
+ * @_var:   name of the classmap, exported for other modules coordinated use.
  * @_type:  enum class_map_type, chooses bits/verbose, numeric/symbolic
  * @_base:  offset of 1st class-name. splits .class_id space
- * @classes: class-names used to control class'd prdbgs
+ * @classes: enum-map - symbol names are "classnames", vals are .class_ids
+ *
+ * @classes vals are _ddebug.class_ids used in the module, the symbol
+ * names are stringified; they authorize "class FOO" to >control.
+ * Connection to a kernel-param is done separately.
  */
 #define DYNDBG_CLASSMAP_DEFINE(_var, _maptype, _base, ...)		\
 	const char *_var##_classnames[] = { __VA_ARGS__ };		\
@@ -118,16 +120,18 @@ struct ddebug_class_user {
 	struct ddebug_class_map *map;
 };
 /**
- * DYNDBG_CLASSMAP_USE - Use a classmap DEFINEd in another module.
- * This lets dyndbg initialize the dependent module's prdbgs from the
- * other module's controlling sysfs node.
+ * DYNDBG_CLASSMAP_USE - refer to a classmap, DEFINEd elsewhere.
+ * @_var: name of the exported classmap
+ *
+ * This registers the module's use of another module's classmap defn,
+ * allowing dyndbg to find the controlling kparam, and propagate its
+ * settings to the dependent module being loaded.
  */
-#define DYNDBG_CLASSMAP_USE(_var, ...)					\
-	DYNDBG_CLASSMAP_USE_(_var, __UNIQUE_ID(ddebug_class_user),	\
-			     __VA_ARGS__)
-#define DYNDBG_CLASSMAP_USE_(_var, _uname, ...)				\
+#define DYNDBG_CLASSMAP_USE(_var)					\
+	DYNDBG_CLASSMAP_USE_(_var, __UNIQUE_ID(ddebug_class_user))
+#define DYNDBG_CLASSMAP_USE_(_var, _uname)				\
 	extern struct ddebug_class_map _var[];				\
-	static struct ddebug_class_user __used				\
+	struct ddebug_class_user __used					\
 	__section("__dyndbg_class_refs") _uname = {			\
 		.user_mod_name = KBUILD_MODNAME,			\
 		.map = _var,						\
-- 
2.38.1

[RFC PATCH 12/17] dyndbg-API: DYNDBG_CLASSMAP_DEFINE() improvements

[Jim Cromie]

patch 1 in this series fixed a CLASSMAP usage error, this improves the
api so that misuse is less likely.

changes here:

0- Add William Swanson's public domain map macro:
   https://github.com/swansontec/map-macro/blob/master/map.h
   this makes 1 possible.

1- classnames were formerly specified as strings: "DRM_UT_CORE"
   now they are the actual enum const symbols:     DRM_UT_CORE
   direct use of symbols is tighter, more comprehensible by tools, grep

2- drop _base arg.
   _base was the value of the 1st classname
   that is now available due to 1, no need to require it 2x

So take _base out of the API/kdoc.  Note that the macro impl keeps the
_base arg so that it can be used to set classmap.base, but reuses it
in the MAP-stringify _base, __VA_ARGS__ expression.

Also cleanup the API usage comment in test_dynamic_debug.c, and since
comments in test-code might not be noticed, restate that here.

Using the CLASSMAP api:

  - class-specifications are enum consts/symbols,
    like DRM_UT_CORE, DRM_UT_KMS, etc.
    their values define bit positions to drm.debug (as before)

  - they are stringified and accepted at >control
    echo class DRM_UT_CORE +p >control

  - multiple class-maps must share the per-module: 0-62 class_id space
    (by setting initial enum values to non-overlapping subranges)

todo: fixup the 'i' prefix, a quick/dirty avoidance of MAP.

NOTE: test_dynamic_debug.c also has this helper macro to wire a
classmap to a drm.debug style parameter; its easier to just use it as
a model/template as needed, rather than try to make it general enough
to be an official API helper.

 define DD_SYS_WRAP(_model, _flags)					\
	static unsigned long bits_##_model;				\
	static struct ddebug_class_param _flags##_model = {		\
		.bits = &bits_##_model,					\
		.flags = #_flags,					\
		.map = &map_##_model,					\
	};								\
	module_param_cb(_flags##_##_model, &param_ops_dyndbg_classes, &_flags##_model, 0600)

Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
 drivers/gpu/drm/drm_print.c   | 22 +++++++-------
 include/drm/drm_print.h       |  1 +
 include/linux/dynamic_debug.h | 17 ++++++-----
 include/linux/map.h           | 54 +++++++++++++++++++++++++++++++++++
 lib/test_dynamic_debug.c      | 43 ++++++++++++++--------------
 5 files changed, 95 insertions(+), 42 deletions(-)
 create mode 100644 include/linux/map.h

diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c
index 4b697e18238d..07c25241e8cc 100644
--- a/drivers/gpu/drm/drm_print.c
+++ b/drivers/gpu/drm/drm_print.c
@@ -56,17 +56,17 @@ MODULE_PARM_DESC(debug, "Enable debug output, where each bit enables a debug cat
 module_param_named(debug, __drm_debug, ulong, 0600);
 #else
 /* classnames must match vals of enum drm_debug_category */
-DYNDBG_CLASSMAP_DEFINE(drm_debug_classes, DD_CLASS_TYPE_DISJOINT_BITS, 0,
-			"DRM_UT_CORE",
-			"DRM_UT_DRIVER",
-			"DRM_UT_KMS",
-			"DRM_UT_PRIME",
-			"DRM_UT_ATOMIC",
-			"DRM_UT_VBL",
-			"DRM_UT_STATE",
-			"DRM_UT_LEASE",
-			"DRM_UT_DP",
-			"DRM_UT_DRMRES");
+DYNDBG_CLASSMAP_DEFINE(drm_debug_classes, DD_CLASS_TYPE_DISJOINT_BITS,
+		       DRM_UT_CORE,
+		       DRM_UT_DRIVER,
+		       DRM_UT_KMS,
+		       DRM_UT_PRIME,
+		       DRM_UT_ATOMIC,
+		       DRM_UT_VBL,
+		       DRM_UT_STATE,
+		       DRM_UT_LEASE,
+		       DRM_UT_DP,
+		       DRM_UT_DRMRES);
 
 static struct ddebug_class_param drm_debug_bitmap = {
 	.bits = &__drm_debug,
diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
index a44fb7ef257f..6a27e8f26770 100644
--- a/include/drm/drm_print.h
+++ b/include/drm/drm_print.h
@@ -333,6 +333,7 @@ static inline bool drm_debug_enabled_raw(enum drm_debug_category category)
 	})
 
 #if defined(CONFIG_DRM_USE_DYNAMIC_DEBUG)
+//extern struct ddebug_class_map drm_debug_classes[];
 /*
  * the drm.debug API uses dyndbg, so each drm_*dbg macro/callsite gets
  * a descriptor, and only enabled callsites are reachable.  They use
diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index 0088fc354c98..6f53a687cb32 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -7,6 +7,7 @@
 #endif
 
 #include <linux/build_bug.h>
+#include <linux/map.h>
 
 /*
  * An instance of this structure is created in a special
@@ -92,18 +93,16 @@ struct ddebug_class_map {
 };
 
 /**
- * DYNDBG_CLASSMAP_DEFINE - define debug-classes used by a module.
- * @_var:   name of the classmap, exported for other modules coordinated use.
- * @_type:  enum class_map_type, chooses bits/verbose, numeric/symbolic
- * @_base:  offset of 1st class-name. splits .class_id space
- * @classes: enum-map - symbol names are "classnames", vals are .class_ids
+ * DYNDBG_CLASSMAP_DEFINE - define the debug classes used in this module.
+ * This tells dyndbg what debug classes it should control for the client.
  *
- * @classes vals are _ddebug.class_ids used in the module, the symbol
- * names are stringified; they authorize "class FOO" to >control.
- * Connection to a kernel-param is done separately.
+ * @_var:    struct ddebug_class_map, as passed to module_param_cb
+ * @_type:   enum ddebug_class_map_type, chooses bits/verbose, numeric/symbolic
+ * @classes: enum class values used in module, such as: DRM_UT_*
  */
 #define DYNDBG_CLASSMAP_DEFINE(_var, _maptype, _base, ...)		\
-	const char *_var##_classnames[] = { __VA_ARGS__ };		\
+	const char *_var##_classnames[] = {				\
+		iMAP_LIST(__stringify, _base, __VA_ARGS__) };		\
 	struct ddebug_class_map __aligned(8) __used			\
 		__section("__dyndbg_classes") _var = {			\
 		.mod = THIS_MODULE,					\
diff --git a/include/linux/map.h b/include/linux/map.h
new file mode 100644
index 000000000000..4348313a596f
--- /dev/null
+++ b/include/linux/map.h
@@ -0,0 +1,54 @@
+/*
+ * Created by William Swanson in 2012.
+ *
+ * I, William Swanson, dedicate this work to the public domain.
+ * I waive all rights to the work worldwide under copyright law,
+ * including all related and neighboring rights,
+ * to the extent allowed by law.
+ *
+ * You can copy, modify, distribute and perform the work,
+ * even for commercial purposes, all without asking permission.
+ */
+
+#ifndef MAP_H_INCLUDED
+#define MAP_H_INCLUDED
+
+#define iEVAL0(...) __VA_ARGS__
+#define iEVAL1(...) iEVAL0(iEVAL0(iEVAL0(__VA_ARGS__)))
+#define iEVAL2(...) iEVAL1(iEVAL1(iEVAL1(__VA_ARGS__)))
+#define iEVAL3(...) iEVAL2(iEVAL2(iEVAL2(__VA_ARGS__)))
+#define iEVAL4(...) iEVAL3(iEVAL3(iEVAL3(__VA_ARGS__)))
+#define iEVAL(...)  iEVAL4(iEVAL4(iEVAL4(__VA_ARGS__)))
+
+#define iMAP_END(...)
+#define iMAP_OUT
+#define iMAP_COMMA ,
+
+#define iMAP_GET_END2() 0, iMAP_END
+#define iMAP_GET_END1(...) iMAP_GET_END2
+#define iMAP_GET_END(...) iMAP_GET_END1
+#define iMAP_NEXT0(test, next, ...) next iMAP_OUT
+#define iMAP_NEXT1(test, next) iMAP_NEXT0(test, next, 0)
+#define iMAP_NEXT(test, next)  iMAP_NEXT1(iMAP_GET_END test, next)
+
+#define iMAP0(f, x, peek, ...) f(x) iMAP_NEXT(peek, iMAP1)(f, peek, __VA_ARGS__)
+#define iMAP1(f, x, peek, ...) f(x) iMAP_NEXT(peek, iMAP0)(f, peek, __VA_ARGS__)
+
+#define iMAP_LIST_NEXT1(test, next) iMAP_NEXT0(test, iMAP_COMMA next, 0)
+#define iMAP_LIST_NEXT(test, next)  iMAP_LIST_NEXT1(iMAP_GET_END test, next)
+
+#define iMAP_LIST0(f, x, peek, ...) f(x) iMAP_LIST_NEXT(peek, iMAP_LIST1)(f, peek, __VA_ARGS__)
+#define iMAP_LIST1(f, x, peek, ...) f(x) iMAP_LIST_NEXT(peek, iMAP_LIST0)(f, peek, __VA_ARGS__)
+
+/**
+ * Applies the function macro `f` to each of the remaining parameters.
+ */
+#define iMAP(f, ...) iEVAL(iMAP1(f, __VA_ARGS__, ()()(), ()()(), ()()(), 0))
+
+/**
+ * Applies the function macro `f` to each of the remaining parameters and
+ * inserts commas between the results.
+ */
+#define iMAP_LIST(f, ...) iEVAL(iMAP_LIST1(f, __VA_ARGS__, ()()(), ()()(), ()()(), 0))
+
+#endif
diff --git a/lib/test_dynamic_debug.c b/lib/test_dynamic_debug.c
index 4ae01f7fa920..f471737fdfc3 100644
--- a/lib/test_dynamic_debug.c
+++ b/lib/test_dynamic_debug.c
@@ -33,11 +33,10 @@ module_param_cb(do_prints, &param_ops_do_prints, NULL, 0600);
 
 /*
  * Using the CLASSMAP api:
- * - classmaps must have corresponding enum
- * - enum symbols must match/correlate with class-name strings in the map.
- * - base must equal enum's 1st value
- * - multiple maps must set their base to share the 0-30 class_id space !!
- *   (build-bug-on tips welcome)
+ * - class-names are enum consts/symbols, like DRM_UT_CORE, DRM_UT_KMS, etc
+ * - those names are accepted at >control interface
+ * - multiple class-maps must share the per-module: 0-62 class_id space
+ *   (by setting initial enum values to non-overlapping subranges)
  * Additionally, here:
  * - tie together sysname, mapname, bitsname, flagsname
  */
@@ -62,38 +61,38 @@ enum cat_disjoint_bits {
 	D2_LEASE,
 	D2_DP,
 	D2_DRMRES };
-DYNDBG_CLASSMAP_DEFINE(map_disjoint_bits, DD_CLASS_TYPE_DISJOINT_BITS, 0,
-		       "D2_CORE",
-		       "D2_DRIVER",
-		       "D2_KMS",
-		       "D2_PRIME",
-		       "D2_ATOMIC",
-		       "D2_VBL",
-		       "D2_STATE",
-		       "D2_LEASE",
-		       "D2_DP",
-		       "D2_DRMRES");
+DYNDBG_CLASSMAP_DEFINE(map_disjoint_bits, DD_CLASS_TYPE_DISJOINT_BITS,
+		       D2_CORE,
+		       D2_DRIVER,
+		       D2_KMS,
+		       D2_PRIME,
+		       D2_ATOMIC,
+		       D2_VBL,
+		       D2_STATE,
+		       D2_LEASE,
+		       D2_DP,
+		       D2_DRMRES);
 DD_SYS_WRAP(disjoint_bits, p);
 DD_SYS_WRAP(disjoint_bits, T);
 
 /* symbolic input, independent bits */
 enum cat_disjoint_names { LOW = 10, MID, HI };
-DYNDBG_CLASSMAP_DEFINE(map_disjoint_names, DD_CLASS_TYPE_DISJOINT_NAMES, 10,
-		       "LOW", "MID", "HI");
+DYNDBG_CLASSMAP_DEFINE(map_disjoint_names, DD_CLASS_TYPE_DISJOINT_NAMES,
+		       LOW, MID, HI);
 DD_SYS_WRAP(disjoint_names, p);
 DD_SYS_WRAP(disjoint_names, T);
 
 /* numeric verbosity, V2 > V1 related */
 enum cat_level_num { V0 = 14, V1, V2, V3, V4, V5, V6, V7 };
-DYNDBG_CLASSMAP_DEFINE(map_level_num, DD_CLASS_TYPE_LEVEL_NUM, 14,
-		       "V0", "V1", "V2", "V3", "V4", "V5", "V6", "V7");
+DYNDBG_CLASSMAP_DEFINE(map_level_num, DD_CLASS_TYPE_LEVEL_NUM,
+		       V0, V1, V2, V3, V4, V5, V6, V7);
 DD_SYS_WRAP(level_num, p);
 DD_SYS_WRAP(level_num, T);
 
 /* symbolic verbosity */
 enum cat_level_names { L0 = 22, L1, L2, L3, L4, L5, L6, L7 };
-DYNDBG_CLASSMAP_DEFINE(map_level_names, DD_CLASS_TYPE_LEVEL_NAMES, 22,
-		       "L0", "L1", "L2", "L3", "L4", "L5", "L6", "L7");
+DYNDBG_CLASSMAP_DEFINE(map_level_names, DD_CLASS_TYPE_LEVEL_NAMES,
+		       L0, L1, L2, L3, L4, L5, L6, L7);
 DD_SYS_WRAP(level_names, p);
 DD_SYS_WRAP(level_names, T);
 
-- 
2.38.1

[RFC PATCH 13/17] drm_print: fix stale macro-name in comment

[Jim Cromie]

Cited commit uses stale macro name, fix this, and explain better.

When DRM_USE_DYNAMIC_DEBUG=y, DYNDBG_CLASSMAP_DEFINE() maps DRM_UT_*
onto BITs in drm.debug.  This still uses enum drm_debug_category, but
it is somewhat indirect, with the ordered set of DRM_UT_* enum-vals.
This requires that the macro args: DRM_UT_* list must be kept in sync
and in order.

Fixes: f158936b60a7 ("drm: POC drm on dyndbg - use in core, 2 helpers, 3 drivers.")
Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
. emphasize ABI non-change despite enum val change - Jani Nikula
. reorder to back of patchset to follow API name changes.
---
 include/drm/drm_print.h | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
index 6a27e8f26770..7695ba31b3a4 100644
--- a/include/drm/drm_print.h
+++ b/include/drm/drm_print.h
@@ -276,7 +276,10 @@ static inline struct drm_printer drm_err_printer(const char *prefix)
  *
  */
 enum drm_debug_category {
-	/* These names must match those in DYNAMIC_DEBUG_CLASSBITS */
+	/*
+	 * Keep DYNDBG_CLASSMAP_DEFINE args in sync with changes here,
+	 * the enum-values define BIT()s in drm.debug, so are ABI.
+	 */
 	/**
 	 * @DRM_UT_CORE: Used in the generic drm code: drm_ioctl.c, drm_mm.c,
 	 * drm_memory.c, ...
-- 
2.38.1

[RFC PATCH 14/17] dyndbg: unwrap __ddebug_add_module inner function NOTYET

[Jim Cromie]

The inner func has a base arg which is unused in the body, so theres
no point in having the inner fn.  Its one-time purpose was subsumed
into the ddebug_info record, which is used in dynamic_debug_init() as
a cursor to load the builtin _ddebug[] table.

TODO: cited commit gives another reason for base, to provide an index
in support of de-duplicating column data.  Refresh that patchset to
see if its still true.

Fixes: 6afa31514977 ("dyndbg: unwrap __ddebug_add_module inner function")

Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
 lib/dynamic_debug.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 45b8414fa130..02f36c553835 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -1329,8 +1329,7 @@ static void ddebug_attach_client_module_classes(struct ddebug_table *dt, struct
  * Allocate a new ddebug_table for the given module
  * and add it to the global list.
  */
-static int __ddebug_add_module(struct _ddebug_info *di, unsigned int base,
-			       const char *modname)
+int ddebug_add_module(struct _ddebug_info *di, const char *modname)
 {
 	struct ddebug_table *dt;
 
@@ -1372,11 +1371,6 @@ static int __ddebug_add_module(struct _ddebug_info *di, unsigned int base,
 	return 0;
 }
 
-int ddebug_add_module(struct _ddebug_info *di, const char *modname)
-{
-	return __ddebug_add_module(di, 0, modname);
-}
-
 /* helper for ddebug_dyndbg_(boot|module)_param_cb */
 static int ddebug_dyndbg_param_cb(char *param, char *val,
 				const char *modname, int on_err)
@@ -1519,7 +1513,7 @@ static int __init dynamic_debug_init(void)
 			mod_ct++;
 			di.num_descs = mod_sites;
 			di.descs = iter_mod_start;
-			ret = __ddebug_add_module(&di, i - mod_sites, modname);
+			ret = ddebug_add_module(&di, modname);
 			if (ret)
 				goto out_err;
 
@@ -1530,7 +1524,7 @@ static int __init dynamic_debug_init(void)
 	}
 	di.num_descs = mod_sites;
 	di.descs = iter_mod_start;
-	ret = __ddebug_add_module(&di, i - mod_sites, modname);
+	ret = ddebug_add_module(&di, modname);
 	if (ret)
 		goto out_err;
 
-- 
2.38.1

[RFC PATCH 15/17] dyndbg: ddebug_sanity()

[Jim Cromie]

It appears that, at least for builtin drm,i915 loadable amdgpu, data
in the class_refs section is not properly linked, this works partway
towards isolating the problem.

The "NO CLI" name test is failing.
 This version of the report fails with a non-canonical address:
 // v2pr_info("NO CLI name on: %s\n", cli->map->mod_name);

[    0.109299] dyndbg: add-module: main 6 sites 1.3
[    0.109595] general protection fault, probably for non-canonical address 0x7265766972640000: 0000

fwiw:
  $ perl -e ' $a = pack "H8", "7265766972640000"; print "a:<$a>\n"'
  a:<revi>

These records are added to __dyndbg_class_refs section by
DYNDBG_CLASSMAP_USE

This patch adds ddebug_sanity(), and calls it 3 times to characterize
what goes wrong and when.  It turns out that its contents are wrong
immediately, in 1st step of dynamic_debug_init().

[    0.107327] dyndbg: classes
[    0.107537] dyndbg: class[0]: module:drm base:0 len:10 ty:0 cm:ffffffff834fc4c8
[    0.107592] dyndbg: class-refs
[    0.107823] dyndbg: class-ref[0]: cli:ffffffff834fc508 map:ffffffff8293e35e nm:0000000000000000 nm:(null)
[    0.108554] dyndbg: class-ref[1]: cli:ffffffff834fc518 map:ffffffff8293fe86 nm:ffffffff834fc4c8 nm:
[    0.108590] dyndbg: class-ref[2]: cli:ffffffff834fc528 map:ffffffff829785aa nm:ffffffff834fc4c8 nm:

Those maps are wrong:

class-refs should all ref the same map, ie class[0]: module:drm.
the nm:s should also show module names of 3 builtin clients of drm.
So things must end poorly.

modprobing the loadable module does better:

bash-5.2# modprobe amdgpu
[ 6645.212706] AMD-Vi: AMD IOMMUv2 functionality not available on this system - This is not a bug.
[ 6645.653124] dyndbg: add-module: amdgpu 4425 sites 0.1
[ 6645.653582] dyndbg: classes
[ 6645.653830] dyndbg: class-refs
[ 6645.654124] dyndbg: class-ref[0]: cli:ffffffffc0a31b90 map:ffffffff834fc4c8 nm:ffffffffc08bc176 nm:amdgpu
[ 6645.654936] dyndbg: classes
[ 6645.655188] dyndbg: class-refs
[ 6645.655450] dyndbg: class-ref[0]: cli:ffffffffc0a31b90 map:ffffffff834fc4c8 nm:ffffffffc08bc176 nm:amdgpu
[ 6645.656246] dyndbg: class_ref[0] amdgpu -> drm
[ 6645.656613] dyndbg: amdgpu needs drm, 0x0
[ 6645.656953] dyndbg: apply bitmap: 0x0 to: 0x0
[ 6645.657322] dyndbg: break on 0/1
[ 6645.657592] dyndbg: 4425 debug prints in module amdgpu

Here, the maps are correct; they ref the class[0] module:drm above.
That said, apply bitmap is wrong.

Signed-off-by: Jim Cromie <jim.cromie@gmail.com>

---

[    1.210094] dyndbg: add-module: drm 302 sites 1.3
[    1.210496] dyndbg: classes
[    1.210674] dyndbg: class[0]: module:drm base:0 len:10 ty:0 cm:ffffffff834fc4c8
[    1.211290] dyndbg: class-refs
[    1.211548] dyndbg: class-ref[0]: cli:ffffffff834fc508 map:ffffffff8293e376 nm:0000000000000000 nm:(null)
[    1.211674] dyndbg: class-ref[1]: cli:ffffffff834fc518 map:ffffffff8293fe9e nm:ffffffff834fc4c8 nm:
[    1.212464] dyndbg: class-ref[2]: cli:ffffffff834fc528 map:ffffffff829785c2 nm:ffffffff834fc4c8 nm:
[    1.212675] dyndbg: classes start: class[0]: module:drm base:0 len:10 ty:0
[    1.213257] dyndbg: builtin class: module:drm base:0 len:10 ty:0
[    1.213675] dyndbg: controlling kp: drm.drm.debug
[    1.214087] dyndbg: module:drm attached 1 classes
[    1.214490] dyndbg: classes
[    1.214674] dyndbg: class[0]: module:drm base:0 len:10 ty:0 cm:ffffffff834fc4c8
[    1.215291] dyndbg: class-refs
[    1.215552] dyndbg: class-ref[0]: cli:ffffffff834fc508 map:ffffffff8293e376 nm:0000000000000000 nm:(null)
[    1.215674] dyndbg: class-ref[1]: cli:ffffffff834fc518 map:ffffffff8293fe9e nm:ffffffff834fc4c8 nm:
[    1.216430] dyndbg: class-ref[2]: cli:ffffffff834fc528 map:ffffffff829785c2 nm:ffffffff834fc4c8 nm:
[    1.216674] dyndbg: NO CLI ffffffff8293e376 7265766972640000
[    1.217149] dyndbg: 302 debug prints in module drm
[    1.217549] dyndbg: add-module: drm_kms_helper 95 sites 1.3
[    1.217674] dyndbg: classes
[    1.217913] dyndbg: class[0]: module:drm base:0 len:10 ty:0 cm:ffffffff834fc4c8
[    1.218525] dyndbg: class-refs
[    1.218674] dyndbg: class-ref[0]: cli:ffffffff834fc508 map:ffffffff8293e376 nm:0000000000000000 nm:(null)
[    1.219486] dyndbg: class-ref[1]: cli:ffffffff834fc518 map:ffffffff8293fe9e nm:ffffffff834fc4c8 nm:
[    1.219674] dyndbg: class-ref[2]: cli:ffffffff834fc528 map:ffffffff829785c2 nm:ffffffff834fc4c8 nm:
[    1.220469] dyndbg: classes
[    1.220674] dyndbg: class[0]: module:drm base:0 len:10 ty:0 cm:ffffffff834fc4c8
[    1.221324] dyndbg: class-refs
[    1.221606] dyndbg: class-ref[0]: cli:ffffffff834fc508 map:ffffffff8293e376 nm:0000000000000000 nm:(null)
[    1.221674] dyndbg: class-ref[1]: cli:ffffffff834fc518 map:ffffffff8293fe9e nm:ffffffff834fc4c8 nm:
[    1.222472] dyndbg: class-ref[2]: cli:ffffffff834fc528 map:ffffffff829785c2 nm:ffffffff834fc4c8 nm:
[    1.222674] dyndbg: NO CLI ffffffff8293e376 7265766972640000
[    1.223173] dyndbg:  95 debug prints in module drm_kms_helper
[    1.223675] dyndbg: add-module: drm_display_helper 150 sites 1.3
[    1.224223] dyndbg: classes
[    1.224484] dyndbg: class[0]: module:drm base:0 len:10 ty:0 cm:ffffffff834fc4c8
[    1.224676] dyndbg: class-refs
[    1.224954] dyndbg: class-ref[0]: cli:ffffffff834fc508 map:ffffffff8293e376 nm:0000000000000000 nm:(null)
[    1.225674] dyndbg: class-ref[1]: cli:ffffffff834fc518 map:ffffffff8293fe9e nm:ffffffff834fc4c8 nm:
[    1.226498] dyndbg: class-ref[2]: cli:ffffffff834fc528 map:ffffffff829785c2 nm:ffffffff834fc4c8 nm:
[    1.226674] dyndbg: classes
[    1.226931] dyndbg: class[0]: module:drm base:0 len:10 ty:0 cm:ffffffff834fc4c8
[    1.227577] dyndbg: class-refs
[    1.227674] dyndbg: class-ref[0]: cli:ffffffff834fc508 map:ffffffff8293e376 nm:0000000000000000 nm:(null)
[    1.228501] dyndbg: class-ref[1]: cli:ffffffff834fc518 map:ffffffff8293fe9e nm:ffffffff834fc4c8 nm:
[    1.228674] dyndbg: class-ref[2]: cli:ffffffff834fc528 map:ffffffff829785c2 nm:ffffffff834fc4c8 nm:
[    1.229443] dyndbg: NO CLI ffffffff8293e376 7265766972640000
[    1.229674] dyndbg: 150 debug prints in module drm_display_helper
[    1.230195] dyndbg: add-module: ttm 2 sites 1.3
[    1.230581] dyndbg: classes
[    1.230674] dyndbg: class[0]: module:drm base:0 len:10 ty:0 cm:ffffffff834fc4c8
[    1.231291] dyndbg: class-refs
[    1.231559] dyndbg: class-ref[0]: cli:ffffffff834fc508 map:ffffffff8293e376 nm:0000000000000000 nm:(null)
[    1.231674] dyndbg: class-ref[1]: cli:ffffffff834fc518 map:ffffffff8293fe9e nm:ffffffff834fc4c8 nm:
[    1.232439] dyndbg: class-ref[2]: cli:ffffffff834fc528 map:ffffffff829785c2 nm:ffffffff834fc4c8 nm:
[    1.232674] dyndbg: classes
[    1.232915] dyndbg: class[0]: module:drm base:0 len:10 ty:0 cm:ffffffff834fc4c8
[    1.233535] dyndbg: class-refs
[    1.233674] dyndbg: class-ref[0]: cli:ffffffff834fc508 map:ffffffff8293e376 nm:0000000000000000 nm:(null)
[    1.234470] dyndbg: class-ref[1]: cli:ffffffff834fc518 map:ffffffff8293fe9e nm:ffffffff834fc4c8 nm:
[    1.234674] dyndbg: class-ref[2]: cli:ffffffff834fc528 map:ffffffff829785c2 nm:ffffffff834fc4c8 nm:
[    1.235427] dyndbg: NO CLI ffffffff8293e376 7265766972640000
[    1.235674] dyndbg:   2 debug prints in module ttm
[    1.236079] dyndbg: add-module: i915 1657 sites 1.3
[    1.236490] dyndbg: classes
[    1.236674] dyndbg: class[0]: module:drm base:0 len:10 ty:0 cm:ffffffff834fc4c8
[    1.237283] dyndbg: class-refs
[    1.237545] dyndbg: class-ref[0]: cli:ffffffff834fc508 map:ffffffff8293e376 nm:0000000000000000 nm:(null)
[    1.237674] dyndbg: class-ref[1]: cli:ffffffff834fc518 map:ffffffff8293fe9e nm:ffffffff834fc4c8 nm:
[    1.238431] dyndbg: class-ref[2]: cli:ffffffff834fc528 map:ffffffff829785c2 nm:ffffffff834fc4c8 nm:
[    1.238674] dyndbg: classes
[    1.238911] dyndbg: class[0]: module:drm base:0 len:10 ty:0 cm:ffffffff834fc4c8
[    1.239519] dyndbg: class-refs
[    1.239674] dyndbg: class-ref[0]: cli:ffffffff834fc508 map:ffffffff8293e376 nm:0000000000000000 nm:(null)
[    1.240467] dyndbg: class-ref[1]: cli:ffffffff834fc518 map:ffffffff8293fe9e nm:ffffffff834fc4c8 nm:
[    1.240674] dyndbg: class-ref[2]: cli:ffffffff834fc528 map:ffffffff829785c2 nm:ffffffff834fc4c8 nm:
[    1.241478] dyndbg: NO CLI ffffffff8293e376 7265766972640000
[    1.241674] dyndbg: 1657 debug prints in module i915
---
 include/linux/dynamic_debug.h |  4 ++--
 lib/dynamic_debug.c           | 32 +++++++++++++++++++++++++++++++-
 2 files changed, 33 insertions(+), 3 deletions(-)

diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index 6f53a687cb32..2a1199aadab6 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -115,8 +115,8 @@ struct ddebug_class_map {
 	EXPORT_SYMBOL(_var)
 
 struct ddebug_class_user {
-	char *user_mod_name;
-	struct ddebug_class_map *map;
+	const char *user_mod_name;
+	const struct ddebug_class_map *map;
 };
 /**
  * DYNDBG_CLASSMAP_USE - refer to a classmap, DEFINEd elsewhere.
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 02f36c553835..46684aa7284d 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -1301,7 +1301,8 @@ static void ddebug_attach_client_module_classes(struct ddebug_table *dt, struct
 			continue;
 		}
 		if (!cli->user_mod_name) {
-			v2pr_info("NO CLI name\n");
+			v2pr_info("NO CLI name %px %px\n", cli->map, cli->map->mod_name);
+			// v2pr_info("NO CLI name on: %s\n", cli->map->mod_name);
 			continue;
 		}
 
@@ -1325,6 +1326,29 @@ static void ddebug_attach_client_module_classes(struct ddebug_table *dt, struct
 	}
 }
 
+static void ddebug_sanity(struct _ddebug_info *di)
+{
+	struct ddebug_class_map *cm;
+	struct ddebug_class_user *cli;
+	int i;
+
+	if (di->num_classes)
+		v2pr_info("classes\n");
+
+	for (i = 0, cm = di->classes; i < di->num_classes; i++, cm++) {
+		v2pr_info("class[%d]: module:%s base:%d len:%d ty:%d cm:%px\n",
+			  i, cm->mod_name, cm->base, cm->length, cm->map_type, cm);
+	}
+	if (di->num_class_refs)
+		v2pr_info("class-refs\n");
+
+	for (i = 0, cli = di->class_refs; i < di->num_class_refs; i++, cli++) {
+		// cli->map->mod_name will segv
+		v2pr_info("class-ref[%d]: cli:%px map:%px nm:%px nm:%s\n", i, cli,
+			  cli->map, cli->user_mod_name, cli->user_mod_name);
+	}
+}
+
 /*
  * Allocate a new ddebug_table for the given module
  * and add it to the global list.
@@ -1357,6 +1381,8 @@ int ddebug_add_module(struct _ddebug_info *di, const char *modname)
 
 	INIT_LIST_HEAD(&dt->link);
 
+	ddebug_sanity(di);
+
 	if (di->num_classes)
 		ddebug_attach_module_classes(dt, di);
 
@@ -1364,6 +1390,8 @@ int ddebug_add_module(struct _ddebug_info *di, const char *modname)
 	list_add_tail(&dt->link, &ddebug_tables);
 	mutex_unlock(&ddebug_lock);
 
+	ddebug_sanity(di);
+
 	if (di->num_class_refs)
 		ddebug_attach_client_module_classes(dt, di);
 
@@ -1493,6 +1521,8 @@ static int __init dynamic_debug_init(void)
 		.num_class_refs = __stop___dyndbg_class_refs - __start___dyndbg_class_refs,
 	};
 
+	ddebug_sanity(&di);
+
 	if (&__start___dyndbg == &__stop___dyndbg) {
 		if (IS_ENABLED(CONFIG_DYNAMIC_DEBUG)) {
 			pr_warn("_ddebug table is empty in a CONFIG_DYNAMIC_DEBUG build\n");
-- 
2.38.1

[RFC PATCH 16/17] dyndbg: mess-w-dep-class

[Jim Cromie]

for loadable drm, helpers, and drivers, dependent-load is failing to
apply changes, needs more investigation.
---
 lib/dynamic_debug.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 46684aa7284d..3ef1c0a1f0cd 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -1248,14 +1248,14 @@ static void ddebug_find_kparam(struct ddebug_class_map *cm)
 
 static void ddebug_param_load_dependent_class(const struct ddebug_class_user *cli)
 {
-	unsigned long new_bits, old_bits = 0;
+	unsigned long *new_bits, old_bits = 0;
 
-	new_bits = *cli->map->dc_parm->bits;
+	new_bits = cli->map->dc_parm->bits;
 
 	vpr_info("%s needs %s, 0x%lx\n", cli->user_mod_name,
-		 cli->map->mod_name, new_bits);
+		 cli->map->mod_name, *new_bits);
 
-	ddebug_apply_class_bitmap(cli->map->dc_parm, &new_bits, &old_bits, cli->user_mod_name);
+	ddebug_apply_class_bitmap(cli->map->dc_parm, new_bits, &old_bits, cli->user_mod_name);
 }
 
 static void ddebug_attach_module_classes(struct ddebug_table *dt, struct _ddebug_info *di)
-- 
2.38.1

[RFC PATCH 17/17] dyndbg: miss-on HACK

[Jim Cromie]

dont break the loop, to see multiple clients.  the 3 client records
are differently wrong.
---
 lib/dynamic_debug.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 3ef1c0a1f0cd..a26eaa348731 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -629,6 +629,7 @@ static int ddebug_apply_class_bitmap(const struct ddebug_class_param *dcp,
 		v2pr_info("bit_%d: %d matches on class: %s -> 0x%lx\n", bi,
 			  ct, map->class_names[bi], *new_bits);
 	}
+	v2pr_info("applied bitmap: 0x%lx to: 0x%lx\n", *new_bits, *old_bits);
 	return matches;
 }
 
@@ -1321,8 +1322,8 @@ static void ddebug_attach_client_module_classes(struct ddebug_table *dt, struct
 			 */
 			v2pr_info("break on %d/%d\n", i, di->num_class_refs);
 			dt->num_class_refs = 1;
-			break;
-		}
+		} else
+			v2pr_info("miss on %d/%d\n", i, di->num_class_refs);
 	}
 }
 
-- 
2.38.1

Re: [RFC PATCH 13/17] drm_print: fix stale macro-name in comment

[Daniel Vetter]

On Mon, Dec 05, 2022 at 05:34:20PM -0700, Jim Cromie wrote:
> Cited commit uses stale macro name, fix this, and explain better.
> 
> When DRM_USE_DYNAMIC_DEBUG=y, DYNDBG_CLASSMAP_DEFINE() maps DRM_UT_*
> onto BITs in drm.debug.  This still uses enum drm_debug_category, but
> it is somewhat indirect, with the ordered set of DRM_UT_* enum-vals.
> This requires that the macro args: DRM_UT_* list must be kept in sync
> and in order.
> 
> Fixes: f158936b60a7 ("drm: POC drm on dyndbg - use in core, 2 helpers, 3 drivers.")
> Signed-off-by: Jim Cromie <jim.cromie@gmail.com>

Should I land this already?
-Daniel

> ---
> . emphasize ABI non-change despite enum val change - Jani Nikula
> . reorder to back of patchset to follow API name changes.
> ---
>  include/drm/drm_print.h | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
> index 6a27e8f26770..7695ba31b3a4 100644
> --- a/include/drm/drm_print.h
> +++ b/include/drm/drm_print.h
> @@ -276,7 +276,10 @@ static inline struct drm_printer drm_err_printer(const char *prefix)
>   *
>   */
>  enum drm_debug_category {
> -	/* These names must match those in DYNAMIC_DEBUG_CLASSBITS */
> +	/*
> +	 * Keep DYNDBG_CLASSMAP_DEFINE args in sync with changes here,
> +	 * the enum-values define BIT()s in drm.debug, so are ABI.
> +	 */
>  	/**
>  	 * @DRM_UT_CORE: Used in the generic drm code: drm_ioctl.c, drm_mm.c,
>  	 * drm_memory.c, ...
> -- 
> 2.38.1
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [RFC PATCH 00/17] DRM_USE_DYNAMIC_DEBUG regression

[Daniel Vetter]

On Mon, Dec 05, 2022 at 05:34:07PM -0700, Jim Cromie wrote:
> Hi everyone,
> 
> DRM_USE_DYNAMIC_DEBUG=y has a regression on rc-*
> 
> Regression is due to a chicken-egg problem loading modules; on
> `modprobe i915`, drm is loaded 1st, and drm.debug is set.  When
> drm_debug_enabled() tested __drm_debug at runtime, that just worked.
> 
> But with DRM_USE_DYNAMIC_DEBUG=y, the runtime test is replaced with a
> post-load enablement of drm_dbg/dyndbg callsites (static-keys), via
> dyndbg's callback on __drm_debug.  Since all drm-drivers need drm.ko,
> it is loaded 1st, then drm.debug=X is applied, then drivers load, but
> too late for drm_dbgs to be enabled.
> 
> STATUS
> 
> For all-loadable drm,i915,amdgpu configs, it almost works, but
> propagating drm.debug to dependent modules doesnt actually apply,
> though the motions are there.  This is not the problem I want to chase
> here.
> 
> The more basic trouble is:
> 
> For builtin drm + helpers, things are broken pretty early; at the
> beginning of dynamic_debug_init().  As the ddebug_sanity() commit-msg
> describes in some detail, the records added by _USE fail to reference
> the struct ddebug_class_map created and exported by _DEFINE, but get
> separate addresses to "other" data that segv's when used as the
> expected pointer. FWIW, the pointer val starts with "revi".

So I honestly have no idea here, linker stuff is way beyond where I have
clue. So what's the way forward here?

The DEFINE/USE split does like the right thing to do at least from the
"how it's used in drivers" pov. But if we're just running circles not
quite getting there I dunno :-/
-Daniel

> 
> OVERVIEW
> 
> DECLARE_DYNDBG_CLASSMAP is broken: it is one-size-fits-all-poorly.
> It muddles the distinction between a (single) definition, and multiple
> references.  Something exported should suffice.
> 
> The core of this patchset splits it into:
> 
> DYNDBG_CLASSMAP_DEFINE	used once per subsystem to define each classmap
> DYNDBG_CLASSMAP_USE	declare dependence on a DEFINEd classmap
> 
> This makes the weird coordinated-changes-by-identical-classmaps
> "feature" unnecessary; the DEFINE can export the var, and USE refers
> to the exported var.
> 
> So this patchset adds another section: __dyndbg_class_refs.
> 
> It is like __dyndbg_classes; it is scanned under ddebug_add_module(),
> and attached to each module's ddebug_table.  Once attached, it can be
> used like classes to validate and apply class FOO >control queries.
> 
> It also maps the class user -> definer explicitly, so that when the
> module is loaded, the section scan can find the kernel-param that is
> wired to dyndbg's kparam-callback, and apply its state-var, forex:
> __drm_debug to the just loaded helper/driver module.
> 
> Theres plenty to address Im sure.
> 
> Jim Cromie (17):
>   test-dyndbg: fixup CLASSMAP usage error
>   test-dyndbg: show that DEBUG enables prdbgs at compiletime
>   dyndbg: fix readback value on LEVEL_NAMES interfaces
>   dyndbg: replace classmap list with a vector
>   dyndbg: make ddebug_apply_class_bitmap more selective
>   dyndbg: dynamic_debug_init - use pointer inequality, not strcmp
>   dyndbg: drop NUM_TYPE_ARRAY
>   dyndbg: reduce verbose/debug clutter
>   dyndbg-API: replace DECLARE_DYNDBG_CLASSMAP with
>     DYNDBG_CLASSMAP(_DEFINE|_USE)
>   dyndbg-API: specialize DYNDBG_CLASSMAP_(DEFINE|USE)
>   dyndbg-API: DYNDBG_CLASSMAP_USE drop extra args
>   dyndbg-API: DYNDBG_CLASSMAP_DEFINE() improvements
>   drm_print: fix stale macro-name in comment
>   dyndbg: unwrap __ddebug_add_module inner function NOTYET
>   dyndbg: ddebug_sanity()
>   dyndbg: mess-w-dep-class
>   dyndbg: miss-on HACK
> 
>  drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c |  14 +-
>  drivers/gpu/drm/display/drm_dp_helper.c |  14 +-
>  drivers/gpu/drm/drm_crtc_helper.c       |  14 +-
>  drivers/gpu/drm/drm_print.c             |  22 +--
>  drivers/gpu/drm/i915/i915_params.c      |  14 +-
>  drivers/gpu/drm/nouveau/nouveau_drm.c   |  14 +-
>  include/asm-generic/vmlinux.lds.h       |   3 +
>  include/drm/drm_print.h                 |   6 +-
>  include/linux/dynamic_debug.h           |  57 ++++--
>  include/linux/map.h                     |  54 ++++++
>  kernel/module/main.c                    |   2 +
>  lib/dynamic_debug.c                     | 240 +++++++++++++++++++-----
>  lib/test_dynamic_debug.c                |  47 ++---
>  13 files changed, 344 insertions(+), 157 deletions(-)
>  create mode 100644 include/linux/map.h
> 
> -- 
> 2.38.1
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [RFC PATCH 00/17] DRM_USE_DYNAMIC_DEBUG regression

[jim.cromie]

On Wed, Jan 11, 2023 at 4:09 PM Daniel Vetter <daniel@ffwll.ch> wrote:
>
> On Mon, Dec 05, 2022 at 05:34:07PM -0700, Jim Cromie wrote:
> > Hi everyone,
> >
> > DRM_USE_DYNAMIC_DEBUG=y has a regression on rc-*
> >
> > Regression is due to a chicken-egg problem loading modules; on
> > `modprobe i915`, drm is loaded 1st, and drm.debug is set.  When
> > drm_debug_enabled() tested __drm_debug at runtime, that just worked.
> >
> > But with DRM_USE_DYNAMIC_DEBUG=y, the runtime test is replaced with a
> > post-load enablement of drm_dbg/dyndbg callsites (static-keys), via
> > dyndbg's callback on __drm_debug.  Since all drm-drivers need drm.ko,
> > it is loaded 1st, then drm.debug=X is applied, then drivers load, but
> > too late for drm_dbgs to be enabled.
> >
> > STATUS
> >
> > For all-loadable drm,i915,amdgpu configs, it almost works, but
> > propagating drm.debug to dependent modules doesnt actually apply,
> > though the motions are there.  This is not the problem I want to chase
> > here.
> >
> > The more basic trouble is:
> >
> > For builtin drm + helpers, things are broken pretty early; at the
> > beginning of dynamic_debug_init().  As the ddebug_sanity() commit-msg
> > describes in some detail, the records added by _USE fail to reference
> > the struct ddebug_class_map created and exported by _DEFINE, but get
> > separate addresses to "other" data that segv's when used as the
> > expected pointer. FWIW, the pointer val starts with "revi".
>
> So I honestly have no idea here, linker stuff is way beyond where I have
> clue. So what's the way forward here?
>

Ive fixed this aspect.
Unsurprisingly, it wasnt the linker :-}

> The DEFINE/USE split does like the right thing to do at least from the
> "how it's used in drivers" pov. But if we're just running circles not
> quite getting there I dunno :-/
> -Daniel
>

Sending new rev next.
I think its getting close.

Re: [RFC PATCH 00/17] DRM_USE_DYNAMIC_DEBUG regression

[Daniel Vetter]

On Fri, Jan 13, 2023 at 11:29:57AM -0700, jim.cromie@gmail.com wrote:
> On Wed, Jan 11, 2023 at 4:09 PM Daniel Vetter <daniel@ffwll.ch> wrote:
> >
> > On Mon, Dec 05, 2022 at 05:34:07PM -0700, Jim Cromie wrote:
> > > Hi everyone,
> > >
> > > DRM_USE_DYNAMIC_DEBUG=y has a regression on rc-*
> > >
> > > Regression is due to a chicken-egg problem loading modules; on
> > > `modprobe i915`, drm is loaded 1st, and drm.debug is set.  When
> > > drm_debug_enabled() tested __drm_debug at runtime, that just worked.
> > >
> > > But with DRM_USE_DYNAMIC_DEBUG=y, the runtime test is replaced with a
> > > post-load enablement of drm_dbg/dyndbg callsites (static-keys), via
> > > dyndbg's callback on __drm_debug.  Since all drm-drivers need drm.ko,
> > > it is loaded 1st, then drm.debug=X is applied, then drivers load, but
> > > too late for drm_dbgs to be enabled.
> > >
> > > STATUS
> > >
> > > For all-loadable drm,i915,amdgpu configs, it almost works, but
> > > propagating drm.debug to dependent modules doesnt actually apply,
> > > though the motions are there.  This is not the problem I want to chase
> > > here.
> > >
> > > The more basic trouble is:
> > >
> > > For builtin drm + helpers, things are broken pretty early; at the
> > > beginning of dynamic_debug_init().  As the ddebug_sanity() commit-msg
> > > describes in some detail, the records added by _USE fail to reference
> > > the struct ddebug_class_map created and exported by _DEFINE, but get
> > > separate addresses to "other" data that segv's when used as the
> > > expected pointer. FWIW, the pointer val starts with "revi".
> >
> > So I honestly have no idea here, linker stuff is way beyond where I have
> > clue. So what's the way forward here?
> >
> 
> Ive fixed this aspect.
> Unsurprisingly, it wasnt the linker :-}

Awesome!

> > The DEFINE/USE split does like the right thing to do at least from the
> > "how it's used in drivers" pov. But if we're just running circles not
> > quite getting there I dunno :-/
> > -Daniel
> >
> 
> Sending new rev next.
> I think its getting close.

Thanks a lot for keeping on pushing this.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch