linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 00/19] fix DRM_USE_DYNAMIC_DEBUG regression
@ 2023-01-25 20:37 Jim Cromie
  2023-01-25 20:37 ` [PATCH v3 01/19] test-dyndbg: fixup CLASSMAP usage error Jim Cromie
                   ` (19 more replies)
  0 siblings, 20 replies; 23+ messages in thread
From: Jim Cromie @ 2023-01-25 20:37 UTC (permalink / raw)
  To: linux-kernel, dri-devel, amd-gfx, intel-gvt-dev, intel-gfx
  Cc: jani.nikula, ville.syrjala, daniel.vetter, seanpaul, robdclark,
	Jim Cromie

Hi everyone,

In v6.1 DRM_USE_DYNAMIC_DEBUG=y has a regression enabling drm.debug in
drivers at modprobe.

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

But with DRM_USE_DYNAMIC_DEBUG=y, the runtime test is replaced with a
static_key for each drm_dbg/dyndbg callsite, enabled by dyndbg's
kparam callback on __drm_debug.  So with drm.ko loaded and initialized
before the dependent modules, their debug callsites aren't yet present
to be enabled.

STATUS - v3

not quite ready.
rebased on -rc5, hopefully applies to patchwork head 
still has RFC patch -> CI_ONLY temporary, to avoid panics
boots on my amdgpu box, drm.debug=0x3ff works at boot-time
the "toggled" warning is repeatable with test_dynamic_debug*.ko
it also occurs on amdgpu, so not just artificial.
v2 is https://lore.kernel.org/lkml/20230113193016.749791-1-jim.cromie@gmail.com/

OVERVIEW

As Jani Nikula noted rather more gently, DECLARE_DYNDBG_CLASSMAP is
error-prone enough to call broken: sharing of a common classmap
required identical classmap definitions in all modules using DRM_UT_*,
which is inherently error-prone.  IOW, it muddled the K&R distinction
between a (single) definition, and multiple references.

So patches 10-13 split it into:

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

DYNDBG_CLASSMAP_DEFINE initializes the classmap, stores it into the
(existing) __dyndbg_classes section, and exports the struct var
(unlike DECLARE_DYNDBG_CLASSMAP).

DYNDBG_CLASSMAP_USE initializes a class-ref struct, containing the
user-module-name, and a ref to the exported classmap var.

The distinction allows separate treatment of classmaps and
classmap-refs, the latter getting additional behavior to propagate
parent's kparam settings to USEr. (forex: drm.debug to drm-drivers) 

. lookup the classmap defn being referenced, and its module
. find the module's kernel-params using the classmap
. propagate kparam vals into the prdgs in module being added.

It also makes the weird coordinated-changes-by-identical-classmaps
"feature" unnecessary.

Patch-10 splits the DECLARE macro into DEFINE & USE, and updates uses.

Patch-11 is the core of it; the separate treatment begins in
ddebug_add_module().  It calls ddebug_attach_module_classes(1) to
handle class-defns; this adds ddebug_attach_client_module_classes(2)
to handle class-refs, as they are found while modprobing drm
drivers. (2) calls ddebug_apply_parents_params(3) on each USEr's
referred classmap definition.

(3) scans kernel-params owned by the module DEFINEing the classmap,
either builtin or loadable, calls ddebug_match_apply_kparam(4) on each.

(4) looks for kparams which are wired to dyndbg's param-ops.  Those
params have a struct ddebug_class_param attached, which has a classmap
and a ref to a state-var (__drm_debug for DRM case).  If the kparam's
classmap is the same as from (2), then apply its state-var to the
client module by calling ddebug_apply_class_bitmap().

Patch-12 cleans up DYNDBG_CLASSMAP_USE, dropping now unneeded args.

Patch-13 improves DYNDBG_CLASSMAP_DEFINE, by accepting DRM_UT_*
symbols directly, not "DRM_UT_*" (their strings).  It adds new
include/linux/map.h to support this.

Patches 1-9 are prep, refactor, cleanup, tighten interfaces

Patches 15-18 extend test_dynamic_debug to recreate DRM's multi-module
regression; it builds both test_dynamic_debug.ko and _submod.ko, with
an ifdef to _DEFINE in the main module, and _USE in the submod.  This
gives both modules identical set of prdbgs, which is helpful for
comparing results.

here it is, working properly:

doing class DRM_UT_CORE -p
[ 9904.961750] dyndbg: read 21 bytes from userspace
[ 9904.962286] dyndbg: query 0: "class DRM_UT_CORE -p" mod:*
[ 9904.962848] dyndbg: split into words: "class" "DRM_UT_CORE" "-p"
[ 9904.963444] dyndbg: op='-' flags=0x0 maskp=0xfffffffe
[ 9904.963945] dyndbg: parsed: func="" file="" module="" format="" lineno=0-0 class=DRM_UT_CORE
[ 9904.964781] dyndbg: good-class: drm.DRM_UT_CORE  module:drm nd:302 nc:1 nu:0
[ 9904.966411] dyndbg: class-ref: drm_kms_helper.DRM_UT_CORE  module:drm_kms_helper nd:95 nc:0 nu:1
[ 9904.967265] dyndbg: class-ref: drm_display_helper.DRM_UT_CORE  module:drm_display_helper nd:150 nc:0 nu:1
[ 9904.968349] dyndbg: class-ref: i915.DRM_UT_CORE  module:i915 nd:1659 nc:0 nu:1
[ 9904.969801] dyndbg: class-ref: amdgpu.DRM_UT_CORE  module:amdgpu nd:4425 nc:0 nu:1
[ 9904.977079] dyndbg: class-ref: nouveau.DRM_UT_CORE  module:nouveau nd:103 nc:0 nu:1
[ 9904.977830] dyndbg: processed 1 queries, with 507 matches, 0 errs
doing class DRM_UT_DRIVER +p
[ 9906.151761] dyndbg: read 23 bytes from userspace
[ 9906.152241] dyndbg: query 0: "class DRM_UT_DRIVER +p" mod:*
[ 9906.152793] dyndbg: split into words: "class" "DRM_UT_DRIVER" "+p"
[ 9906.153388] dyndbg: op='+' flags=0x1 maskp=0xffffffff
[ 9906.153896] dyndbg: parsed: func="" file="" module="" format="" lineno=0-0 class=DRM_UT_DRIVER
[ 9906.154746] dyndbg: good-class: drm.DRM_UT_DRIVER  module:drm nd:302 nc:1 nu:0
[ 9906.155433] dyndbg: class-ref: drm_kms_helper.DRM_UT_DRIVER  module:drm_kms_helper nd:95 nc:0 nu:1
[ 9906.156267] dyndbg: class-ref: drm_display_helper.DRM_UT_DRIVER  module:drm_display_helper nd:150 nc:0 nu:1
[ 9906.157365] dyndbg: class-ref: i915.DRM_UT_DRIVER  module:i915 nd:1659 nc:0 nu:1
[ 9906.163848] dyndbg: class-ref: amdgpu.DRM_UT_DRIVER  module:amdgpu nd:4425 nc:0 nu:1
[ 9906.178963] dyndbg: class-ref: nouveau.DRM_UT_DRIVER  module:nouveau nd:103 nc:0 nu:1
[ 9906.179934] dyndbg: processed 1 queries, with 1286 matches, 0 errs


Patch-19 is a *workaround* for a panic: __jump_label_patch can "crash
the box" when the jump-entry is in the wrong state.  The current code
makes no distinction between a well-formed "toggled" state and an
"insane" state.  Not for keeps.

It fixes mis-initialization problems like this:

[ 1594.032504] dyndbg: query 0: "class D2_DRIVER -p" mod:*
[ 1594.032823] dyndbg: split into words: "class" "D2_DRIVER" "-p"
[ 1594.033183] dyndbg: op='-' flags=0x0 maskp=0xfffffffe
[ 1594.033507] dyndbg: parsed: func="" file="" module="" format="" lineno=0-0 class=D2_DRIVER
[ 1594.034014] dyndbg: good-class: test_dynamic_debug.D2_DRIVER  module:test_dynamic_debug nd:32 nc:4 nu:0
[ 1594.034695] dyndbg: changed lib/test_dynamic_debug.c:156 [test_dynamic_debug]do_cats p => _
[ 1594.035304] dyndbg: class-ref: test_dynamic_debug_submod.D2_DRIVER  module:test_dynamic_debug_submod nd:32 nc:0 nu:4
[ 1594.036052] jump_label: found toggled op at do_cats+0x16/0x180 [test_dynamic_debug_submod] [00000000ff2582ac] (0f 1f 44 00 00 != e9 e1 00 00 00)) size:5 type:0
[ 1594.037036] dyndbg: changed lib/test_dynamic_debug.c:156 [test_dynamic_debug_submod]do_cats p => _
[ 1594.037604] dyndbg: processed 1 queries, with 2 matches, 0 errs
[ 1594.037968] dyndbg: bit_1: 2 matches on class: D2_DRIVER -> 0x0

These errors are reliably reproduced by a shell-func which modprobes
(with the right args) the test mod & submod.ko (in the commit message).

So this isnt really ready for inclusion, but Id like to send the whole
set to the CI-gym for a workout.  The RFC/for-TESTING patch will
mitigate panics, and still be detectable.

Besides, Murphys law requires I publish some error before I can make progress.


Jim Cromie (19):
  test-dyndbg: fixup CLASSMAP usage error
  test-dyndbg: show that DEBUG enables prdbgs at compiletime
  dyndbg: replace classmap list with a vector
  dyndbg: make ddebug_apply_class_bitmap more selective
  dyndbg: split param_set_dyndbg_classes to inner/outer fns
  dyndbg: drop NUM_TYPE_ARRAY
  dyndbg: reduce verbose/debug clutter
  dyndbg: tighten ddebug_class_name() 1st arg
  dyndbg: constify ddebug_apply_class_bitmap args
  dyndbg-API: split DECLARE_(DYNDBG_CLASSMAP) to $1(_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
  test-dyndbg: build test_dynamic_debug_submod
  test-dyndbg: rename DD_SYS_WRAP to DYNDBG_CLASSMAP_PARAM
  test-dyndbg: disable WIP dyndbg-trace params
  test-dyndbg: tune sub-module behavior
  jump_label: RFC / temporary for CI - tolerate toggled state

 arch/x86/kernel/jump_label.c            |  24 ++-
 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       |   1 +
 include/drm/drm_print.h                 |   6 +-
 include/linux/dynamic_debug.h           |  57 ++++--
 include/linux/map.h                     |  55 +++++
 kernel/module/main.c                    |   3 +
 lib/Makefile                            |   3 +-
 lib/dynamic_debug.c                     | 258 ++++++++++++++++++------
 lib/test_dynamic_debug.c                | 118 +++++++----
 lib/test_dynamic_debug_submod.c         |  10 +
 16 files changed, 437 insertions(+), 190 deletions(-)
 create mode 100644 include/linux/map.h
 create mode 100644 lib/test_dynamic_debug_submod.c

-- 
2.39.1


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

* [PATCH v3 01/19] test-dyndbg: fixup CLASSMAP usage error
  2023-01-25 20:37 [PATCH v3 00/19] fix DRM_USE_DYNAMIC_DEBUG regression Jim Cromie
@ 2023-01-25 20:37 ` Jim Cromie
  2023-01-25 20:37 ` [PATCH v3 02/19] test-dyndbg: show that DEBUG enables prdbgs at compiletime Jim Cromie
                   ` (18 subsequent siblings)
  19 siblings, 0 replies; 23+ messages in thread
From: Jim Cromie @ 2023-01-25 20:37 UTC (permalink / raw)
  To: linux-kernel, dri-devel, amd-gfx, intel-gvt-dev, intel-gfx
  Cc: jani.nikula, ville.syrjala, daniel.vetter, seanpaul, robdclark,
	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 DECLARE_DYNDBG_CLASSMAP([1]), 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.

[1] name changed later to DYNDBG_CLASSMAP_DEFINE

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.39.1


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

* [PATCH v3 02/19] test-dyndbg: show that DEBUG enables prdbgs at compiletime
  2023-01-25 20:37 [PATCH v3 00/19] fix DRM_USE_DYNAMIC_DEBUG regression Jim Cromie
  2023-01-25 20:37 ` [PATCH v3 01/19] test-dyndbg: fixup CLASSMAP usage error Jim Cromie
@ 2023-01-25 20:37 ` Jim Cromie
  2023-01-25 20:37 ` [PATCH v3 03/19] dyndbg: replace classmap list with a vector Jim Cromie
                   ` (17 subsequent siblings)
  19 siblings, 0 replies; 23+ messages in thread
From: Jim Cromie @ 2023-01-25 20:37 UTC (permalink / raw)
  To: linux-kernel, dri-devel, amd-gfx, intel-gvt-dev, intel-gfx
  Cc: jani.nikula, ville.syrjala, daniel.vetter, seanpaul, robdclark,
	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.  If this sounds wrong, note that the DEBUG is in the
module source file, and is thus privileged.

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..89dd7f285e31 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 /* enable all prdbgs (plain & class'd) at compiletime */
+
 #include <linux/module.h>
 
 /* run tests by reading or writing sysfs node: do_prints */
-- 
2.39.1


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

* [PATCH v3 03/19] dyndbg: replace classmap list with a vector
  2023-01-25 20:37 [PATCH v3 00/19] fix DRM_USE_DYNAMIC_DEBUG regression Jim Cromie
  2023-01-25 20:37 ` [PATCH v3 01/19] test-dyndbg: fixup CLASSMAP usage error Jim Cromie
  2023-01-25 20:37 ` [PATCH v3 02/19] test-dyndbg: show that DEBUG enables prdbgs at compiletime Jim Cromie
@ 2023-01-25 20:37 ` Jim Cromie
  2023-01-25 20:37 ` [PATCH v3 04/19] dyndbg: make ddebug_apply_class_bitmap more selective Jim Cromie
                   ` (16 subsequent siblings)
  19 siblings, 0 replies; 23+ messages in thread
From: Jim Cromie @ 2023-01-25 20:37 UTC (permalink / raw)
  To: linux-kernel, dri-devel, amd-gfx, intel-gvt-dev, intel-gfx
  Cc: jani.nikula, ville.syrjala, daniel.vetter, seanpaul, robdclark,
	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-head link.

struct ddebug_table drops the list-head maps, and gets: classes &
num_classes for the start-addy and num_classes, 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 009f2ead09c1..823190094350 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
@@ -1107,9 +1109,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];
 
@@ -1193,30 +1196,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);
+	}
 }
 
 /*
@@ -1250,10 +1254,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);
@@ -1342,8 +1345,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.39.1


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

* [PATCH v3 04/19] dyndbg: make ddebug_apply_class_bitmap more selective
  2023-01-25 20:37 [PATCH v3 00/19] fix DRM_USE_DYNAMIC_DEBUG regression Jim Cromie
                   ` (2 preceding siblings ...)
  2023-01-25 20:37 ` [PATCH v3 03/19] dyndbg: replace classmap list with a vector Jim Cromie
@ 2023-01-25 20:37 ` Jim Cromie
  2023-01-25 20:37 ` [PATCH v3 05/19] dyndbg: split param_set_dyndbg_classes to inner/outer fns Jim Cromie
                   ` (15 subsequent siblings)
  19 siblings, 0 replies; 23+ messages in thread
From: Jim Cromie @ 2023-01-25 20:37 UTC (permalink / raw)
  To: linux-kernel, dri-devel, amd-gfx, intel-gvt-dev, intel-gfx
  Cc: jani.nikula, ville.syrjala, daniel.vetter, seanpaul, robdclark,
	Jim Cromie

Add query_module param to ddebug_apply_class_bitmap().  This allows
its caller to update just one module, or all (as currently).  We'll
use this later to propagate drm.debug to each USEr as they're
modprobed.

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 | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 823190094350..943e0597ecd4 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];
@@ -608,7 +609,8 @@ 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 for %s\n", *new_bits, *old_bits,
+		  query_modname ?: "");
 
 	for (bi = 0; bi < map->length; bi++) {
 		if (test_bit(bi, new_bits) == test_bit(bi, old_bits))
@@ -617,12 +619,15 @@ 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,
 			  ct, map->class_names[bi], *new_bits);
 	}
+	v2pr_info("applied bitmap: 0x%lx to: 0x%lx for %s\n", *new_bits, *old_bits,
+		  query_modname ?: "");
+
 	return matches;
 }
 
@@ -678,7 +683,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 +693,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 +756,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 +769,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.39.1


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

* [PATCH v3 05/19] dyndbg: split param_set_dyndbg_classes to inner/outer fns
  2023-01-25 20:37 [PATCH v3 00/19] fix DRM_USE_DYNAMIC_DEBUG regression Jim Cromie
                   ` (3 preceding siblings ...)
  2023-01-25 20:37 ` [PATCH v3 04/19] dyndbg: make ddebug_apply_class_bitmap more selective Jim Cromie
@ 2023-01-25 20:37 ` Jim Cromie
  2023-01-25 20:37 ` [PATCH v3 06/19] dyndbg: drop NUM_TYPE_ARRAY Jim Cromie
                   ` (14 subsequent siblings)
  19 siblings, 0 replies; 23+ messages in thread
From: Jim Cromie @ 2023-01-25 20:37 UTC (permalink / raw)
  To: linux-kernel, dri-devel, amd-gfx, intel-gvt-dev, intel-gfx
  Cc: jani.nikula, ville.syrjala, daniel.vetter, seanpaul, robdclark,
	Jim Cromie

Inner fn adds mod_name param, allowing caller to guarantee that only
one module is affected by a prdbgs update.  Outer fn preserves
kernel_param interface, passing NULL to inner fn.

no functional change.

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

diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 943e0597ecd4..0a5efc735b36 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -707,18 +707,9 @@ static int param_set_dyndbg_classnames(const char *instr, const struct kernel_pa
 	return 0;
 }
 
-/**
- * param_set_dyndbg_classes - class FOO >control
- * @instr: string echo>d to sysfs, input depends on map_type
- * @kp:    kp->arg has state: bits/lvl, map, map_type
- *
- * Enable/disable prdbgs by their class, as given in the arguments to
- * DECLARE_DYNDBG_CLASSMAP.  For LEVEL map-types, enforce relative
- * levels by bitpos.
- *
- * Returns: 0 or <0 if error.
- */
-int param_set_dyndbg_classes(const char *instr, const struct kernel_param *kp)
+static int param_set_dyndbg_module_classes(const char *instr,
+					   const struct kernel_param *kp,
+					   const char *modnm)
 {
 	const struct ddebug_class_param *dcp = kp->arg;
 	const struct ddebug_class_map *map = dcp->map;
@@ -755,8 +746,8 @@ int param_set_dyndbg_classes(const char *instr, const struct kernel_param *kp)
 				KP_NAME(kp), inrep, CLASSMAP_BITMASK(map->length));
 			inrep &= CLASSMAP_BITMASK(map->length);
 		}
-		v2pr_info("bits:%lx > %s\n", inrep, KP_NAME(kp));
-		totct += ddebug_apply_class_bitmap(dcp, &inrep, dcp->bits, NULL);
+		v2pr_info("bits:0x%lx > %s.%s\n", inrep, modnm ?: "*", KP_NAME(kp));
+		totct += ddebug_apply_class_bitmap(dcp, &inrep, dcp->bits, modnm);
 		*dcp->bits = inrep;
 		break;
 	case DD_CLASS_TYPE_LEVEL_NUM:
@@ -769,7 +760,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, NULL);
+		totct += ddebug_apply_class_bitmap(dcp, &new_bits, &old_bits, modnm);
 		*dcp->lvl = inrep;
 		break;
 	default:
@@ -778,6 +769,21 @@ int param_set_dyndbg_classes(const char *instr, const struct kernel_param *kp)
 	vpr_info("%s: total matches: %d\n", KP_NAME(kp), totct);
 	return 0;
 }
+/**
+ * param_set_dyndbg_classes - class FOO >control
+ * @instr: string echo>d to sysfs, input depends on map_type
+ * @kp:    kp->arg has state: bits/lvl, map, map_type
+ *
+ * Enable/disable prdbgs by their class, as given in the arguments to
+ * DECLARE_DYNDBG_CLASSMAP.  For LEVEL map-types, enforce relative
+ * levels by bitpos.
+ *
+ * Returns: 0 or <0 if error.
+ */
+int param_set_dyndbg_classes(const char *instr, const struct kernel_param *kp)
+{
+	return param_set_dyndbg_module_classes(instr, kp, NULL);
+}
 EXPORT_SYMBOL(param_set_dyndbg_classes);
 
 /**
-- 
2.39.1


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

* [PATCH v3 06/19] dyndbg: drop NUM_TYPE_ARRAY
  2023-01-25 20:37 [PATCH v3 00/19] fix DRM_USE_DYNAMIC_DEBUG regression Jim Cromie
                   ` (4 preceding siblings ...)
  2023-01-25 20:37 ` [PATCH v3 05/19] dyndbg: split param_set_dyndbg_classes to inner/outer fns Jim Cromie
@ 2023-01-25 20:37 ` Jim Cromie
  2023-01-25 20:37 ` [PATCH v3 07/19] dyndbg: reduce verbose/debug clutter Jim Cromie
                   ` (13 subsequent siblings)
  19 siblings, 0 replies; 23+ messages in thread
From: Jim Cromie @ 2023-01-25 20:37 UTC (permalink / raw)
  To: linux-kernel, dri-devel, amd-gfx, intel-gvt-dev, intel-gfx
  Cc: jani.nikula, ville.syrjala, daniel.vetter, seanpaul, robdclark,
	Jim Cromie

ARRAY_SIZE works here, since array decl is complete.

no functional change

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.39.1


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

* [PATCH v3 07/19] dyndbg: reduce verbose/debug clutter
  2023-01-25 20:37 [PATCH v3 00/19] fix DRM_USE_DYNAMIC_DEBUG regression Jim Cromie
                   ` (5 preceding siblings ...)
  2023-01-25 20:37 ` [PATCH v3 06/19] dyndbg: drop NUM_TYPE_ARRAY Jim Cromie
@ 2023-01-25 20:37 ` Jim Cromie
  2023-01-25 20:37 ` [PATCH v3 08/19] dyndbg: tighten ddebug_class_name() 1st arg Jim Cromie
                   ` (12 subsequent siblings)
  19 siblings, 0 replies; 23+ messages in thread
From: Jim Cromie @ 2023-01-25 20:37 UTC (permalink / raw)
  To: linux-kernel, dri-devel, amd-gfx, intel-gvt-dev, intel-gfx
  Cc: jani.nikula, ville.syrjala, daniel.vetter, seanpaul, robdclark,
	Jim Cromie

currently, for verbose=3, this is logged:

 dyndbg: query 0: "class DRM_UT_CORE +p" mod:*
 dyndbg: split into words: "class" "DRM_UT_CORE" "+p"

 dyndbg: op='+'
 dyndbg: flags=0x1
 dyndbg: *flagsp=0x1 *maskp=0xffffffff

 dyndbg: parsed: func="" file="" module="" format="" lineno=0-0 class=DRM_UT_CORE
 dyndbg: no matches for query
 dyndbg: no-match: func="" file="" module="" format="" lineno=0-0 class=DRM_UT_CORE
 dyndbg: processed 1 queries, with 0 matches, 0 errs

This patch:

shrinks 3 lines of 2nd stanza to single line

drops 2 middle lines of 3rd stanza
 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 | 14 +++-----------
 1 file changed, 3 insertions(+), 11 deletions(-)

diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 0a5efc735b36..2d4640479e5b 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;
 }
 
@@ -496,7 +493,6 @@ static int ddebug_parse_flags(const char *str, struct flag_settings *modifiers)
 		pr_err("bad flag-op %c, at start of %s\n", *str, str);
 		return -EINVAL;
 	}
-	v3pr_info("op='%c'\n", op);
 
 	for (; *str ; ++str) {
 		for (i = ARRAY_SIZE(opt_array) - 1; i >= 0; i--) {
@@ -510,7 +506,6 @@ static int ddebug_parse_flags(const char *str, struct flag_settings *modifiers)
 			return -EINVAL;
 		}
 	}
-	v3pr_info("flags=0x%x\n", modifiers->flags);
 
 	/* calculate final flags, mask based upon op */
 	switch (op) {
@@ -526,7 +521,7 @@ static int ddebug_parse_flags(const char *str, struct flag_settings *modifiers)
 		modifiers->flags = 0;
 		break;
 	}
-	v3pr_info("*flagsp=0x%x *maskp=0x%x\n", modifiers->flags, modifiers->mask);
+	v3pr_info("op='%c' flags=0x%x maskp=0x%x\n", op, modifiers->flags, modifiers->mask);
 
 	return 0;
 }
@@ -536,7 +531,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 +549,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.39.1


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

* [PATCH v3 08/19] dyndbg: tighten ddebug_class_name() 1st arg
  2023-01-25 20:37 [PATCH v3 00/19] fix DRM_USE_DYNAMIC_DEBUG regression Jim Cromie
                   ` (6 preceding siblings ...)
  2023-01-25 20:37 ` [PATCH v3 07/19] dyndbg: reduce verbose/debug clutter Jim Cromie
@ 2023-01-25 20:37 ` Jim Cromie
  2023-01-25 20:37 ` [PATCH v3 09/19] dyndbg: constify ddebug_apply_class_bitmap args Jim Cromie
                   ` (11 subsequent siblings)
  19 siblings, 0 replies; 23+ messages in thread
From: Jim Cromie @ 2023-01-25 20:37 UTC (permalink / raw)
  To: linux-kernel, dri-devel, amd-gfx, intel-gvt-dev, intel-gfx
  Cc: jani.nikula, ville.syrjala, daniel.vetter, seanpaul, robdclark,
	Jim Cromie

Change function's 1st arg-type, by derefing in the caller.
The fn doesn't need any other fields in the struct.

no functional change.

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

diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 2d4640479e5b..10c29bc19901 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -1110,12 +1110,12 @@ static void *ddebug_proc_next(struct seq_file *m, void *p, loff_t *pos)
 #define class_in_range(class_id, map)					\
 	(class_id >= map->base && class_id < map->base + map->length)
 
-static const char *ddebug_class_name(struct ddebug_iter *iter, struct _ddebug *dp)
+static const char *ddebug_class_name(struct ddebug_table *dt, struct _ddebug *dp)
 {
-	struct ddebug_class_map *map = iter->table->classes;
-	int i, nc = iter->table->num_classes;
+	struct ddebug_class_map *map = dt->classes;
+	int i;
 
-	for (i = 0; i < nc; i++, map++)
+	for (i = 0; i < dt->num_classes; i++, map++)
 		if (class_in_range(dp->class_id, map))
 			return map->class_names[dp->class_id - map->base];
 
@@ -1149,7 +1149,7 @@ static int ddebug_proc_show(struct seq_file *m, void *p)
 	seq_puts(m, "\"");
 
 	if (dp->class_id != _DPRINTK_CLASS_DFLT) {
-		class = ddebug_class_name(iter, dp);
+		class = ddebug_class_name(iter->table, dp);
 		if (class)
 			seq_printf(m, " class:%s", class);
 		else
-- 
2.39.1


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

* [PATCH v3 09/19] dyndbg: constify ddebug_apply_class_bitmap args
  2023-01-25 20:37 [PATCH v3 00/19] fix DRM_USE_DYNAMIC_DEBUG regression Jim Cromie
                   ` (7 preceding siblings ...)
  2023-01-25 20:37 ` [PATCH v3 08/19] dyndbg: tighten ddebug_class_name() 1st arg Jim Cromie
@ 2023-01-25 20:37 ` Jim Cromie
  2023-01-25 20:37 ` [PATCH v3 10/19] dyndbg-API: split DECLARE_(DYNDBG_CLASSMAP) to $1(_DEFINE|_USE) Jim Cromie
                   ` (10 subsequent siblings)
  19 siblings, 0 replies; 23+ messages in thread
From: Jim Cromie @ 2023-01-25 20:37 UTC (permalink / raw)
  To: linux-kernel, dri-devel, amd-gfx, intel-gvt-dev, intel-gfx
  Cc: jani.nikula, ville.syrjala, daniel.vetter, seanpaul, robdclark,
	Jim Cromie

ddebug_apply_class_bitmap() does not alter its 2 bitmap args, make
this guarantee in the interface.

NOTE: the bitmap is also available in the dcp arg, but the 2 vars
serve a 2nd purpose; the CLASS_TYPE callers use them to translate
levels into their underlying disjoint representation.

no functional change

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 10c29bc19901..b51f4bde6198 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -592,7 +592,7 @@ 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,
+				     const unsigned long *new_bits, const unsigned long *old_bits,
 				     const char *query_modname)
 {
 #define QUERY_SIZE 128
-- 
2.39.1


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

* [PATCH v3 10/19] dyndbg-API: split DECLARE_(DYNDBG_CLASSMAP) to $1(_DEFINE|_USE)
  2023-01-25 20:37 [PATCH v3 00/19] fix DRM_USE_DYNAMIC_DEBUG regression Jim Cromie
                   ` (8 preceding siblings ...)
  2023-01-25 20:37 ` [PATCH v3 09/19] dyndbg: constify ddebug_apply_class_bitmap args Jim Cromie
@ 2023-01-25 20:37 ` Jim Cromie
  2023-01-25 20:37 ` [PATCH v3 11/19] dyndbg-API: specialize DYNDBG_CLASSMAP_(DEFINE|USE) Jim Cromie
                   ` (9 subsequent siblings)
  19 siblings, 0 replies; 23+ messages in thread
From: Jim Cromie @ 2023-01-25 20:37 UTC (permalink / raw)
  To: linux-kernel, dri-devel, amd-gfx, intel-gvt-dev, intel-gfx
  Cc: jani.nikula, ville.syrjala, daniel.vetter, seanpaul, robdclark,
	Jim Cromie

DECLARE_DYNDBG_CLASSMAP's 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 definition vs declaration that K&R gave us, improving 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 cd4caaa29528..a7a3a382c4a6 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -189,7 +189,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",
@@ -200,6 +201,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 a209659a996c..7e6b25446303 100644
--- a/drivers/gpu/drm/drm_crtc_helper.c
+++ b/drivers/gpu/drm/drm_crtc_helper.c
@@ -50,7 +50,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",
@@ -61,6 +62,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 80f154b6adab..e4146b9af357 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 89dd7f285e31..8d384b979e74 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.39.1


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

* [PATCH v3 11/19] dyndbg-API: specialize DYNDBG_CLASSMAP_(DEFINE|USE)
  2023-01-25 20:37 [PATCH v3 00/19] fix DRM_USE_DYNAMIC_DEBUG regression Jim Cromie
                   ` (9 preceding siblings ...)
  2023-01-25 20:37 ` [PATCH v3 10/19] dyndbg-API: split DECLARE_(DYNDBG_CLASSMAP) to $1(_DEFINE|_USE) Jim Cromie
@ 2023-01-25 20:37 ` Jim Cromie
  2023-01-25 20:37 ` [PATCH v3 12/19] dyndbg-API: DYNDBG_CLASSMAP_USE drop extra args Jim Cromie
                   ` (8 subsequent siblings)
  19 siblings, 0 replies; 23+ messages in thread
From: Jim Cromie @ 2023-01-25 20:37 UTC (permalink / raw)
  To: linux-kernel, dri-devel, amd-gfx, intel-gvt-dev, intel-gfx
  Cc: jani.nikula, ville.syrjala, daniel.vetter, seanpaul, robdclark,
	Jim Cromie

Now that the DECLARE_DYNDBG_CLASSMAP macro has been split into
DYNDBG_CLASSMAP_DEFINE and DYNDBG_CLASSMAP_USE, 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 refs/uses of the classmap definitions.
. authorizes dyndbg to >control those class'd prdbgs in ref'g module.
. maps the client module to classmap definitions
  drm-drivers and helpers are clients.
  this allows dyndbg to apply drm.debug to the client module, when added.

The distinction of the 2 roles yields these gains:

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

It allows separate handling of class-refs, to find the classed
kernel-params (if any) using this classmap, and propagate their
settings to the class'd prdbgs in the client module.

It fixes the chicken-egg problem that DRM_USE_DYNAMIC_DEBUG=y has; the
new type allows dyndbg to handle class-refs found while adding the
module.

The new DYNDBG_CLASSMAP_* macros add records to the sections:

DYNDBG_CLASSMAP_DEFINE:
  invoked just once per sub-system.
  for drm, its drm_print, where drm.debug is exposed.
  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.
  has client-name, so dyndbg can recognize loading client modules.

To support this, a few data changes:

. struct ddebug_class_user
  contains: user-module-name, ref to classmap-defn
  encodes drm-driver's use of a classmap, allowing lookup

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.

vmlinux.lds.h: new BOUNDED_SECTION for class-refs, with linker symbols

dynamic_debug.c: changes to:
  setup - in/under ddebug_add_module(), immediately following
  prdbg enables - in/under ddebug_change(), further below

ddebug_attach_module_classes() - largely unchanged:
  called from ddebug_add_module
  finds classmaps whose .mod_name matches module being added.
  attaches them to the module's ddebug_table.
  minor tweaks for code regularity, debug output

ddebug_attach_client_module_classes() - new fn:
. like above, but works class-refs, not classes.
. called from ddebug_add_module, after list-add to ddebug-tables.
  this lets ddebug_change find it to apply the settings
. scans class-refs, for the block "owned" by the module being added.
  for builtins, its N consecutive of many. for loadables, N of N
. calls ddebug_apply_parents_params() for each.

ddebug_apply_parents_params(new fn)

scans module's/builtin kernel-params, calls ddebug_match_attach_kparam
for each to find the params/sysfs-nodes using a classmap.

ddebug_match_apply_kparam(new fn):

1st, it tests the kernel-param.ops is dyndbg's; this guarantees that
the attached arg is a struct ddebug_class_param, which has a ref to
the param's state, and to the classmap.

2nd, it requires that the classmap attached to the kparam is the one
were called for; modules can use many separate classmaps (as
test_dynamic_debug does).

Then apply the "parent" kparam's setting to the client.

The ddebug_change() support:

ddebug_find_valid_class(): This does the search over classmaps,
looking for the class FOO echo'd to >control.  So now it searches over
__dyndbg_class_refs[] after __dyndbg_classes[].

ddebug_apply_class_bitmap(): now quieter when not changing things.

ddebug_class_name(): return class-names for defined AND used classes.

Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
--
v3 - s/BUG_ON/WARN_ON/ in __dyndbg_class_refs handling
     simpler args in callchain
v2 - rebase past merge conflicts
---
 include/asm-generic/vmlinux.lds.h |   1 +
 include/linux/dynamic_debug.h     |  39 ++++---
 kernel/module/main.c              |   3 +
 lib/dynamic_debug.c               | 170 ++++++++++++++++++++++++++----
 4 files changed, 179 insertions(+), 34 deletions(-)

diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index 659bf3b31c91..5beb0321613e 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -373,6 +373,7 @@
 	/* implement dynamic printk debug */				\
 	. = ALIGN(8);							\
 	BOUNDED_SECTION_BY(__dyndbg_classes, ___dyndbg_classes)		\
+	BOUNDED_SECTION_BY(__dyndbg_class_refs, ___dyndbg_class_refs)	\
 	BOUNDED_SECTION_BY(__dyndbg, ___dyndbg)				\
 	LIKELY_PROFILE()		       				\
 	BRANCH_PROFILE()						\
diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index 1cdfd62fd2e4..397ac8294230 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -81,8 +81,8 @@ enum ddebug_class_map_type {
 };
 
 struct ddebug_class_map {
-	struct module *mod;
-	const char *mod_name;	/* needed for builtins */
+	const struct module *mod;		/* NULL for builtins */
+	const char *mod_name;
 	const char **class_names;
 	const int length;
 	const int base;		/* index of 1st .class_id, allows split/shared space */
@@ -99,8 +99,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 +108,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 48568a0f5651..cfccbd06ae2d 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -2113,6 +2113,9 @@ 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 b51f4bde6198..57efd435dda7 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 {
@@ -147,21 +150,36 @@ static void vpr_info_dq(const struct ddebug_query *query, const char *msg)
 		  query->first_lineno, query->last_lineno, query->class_string);
 }
 
+#define vpr_dt_info(dt, _msg, ...)					\
+	v2pr_info(_msg " module:%s nd:%d nc:%d nu:%d\n", ##__VA_ARGS__,	\
+		  dt->mod_name, dt->num_ddebugs, dt->num_classes, dt->num_class_refs)
+
 #define __outvar /* filled by callee */
 static struct ddebug_class_map *ddebug_find_valid_class(struct ddebug_table const *dt,
 							const char *class_string,
 							__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;
+			vpr_dt_info(dt, "good-class: %s.%s ", map->mod_name, class_string);
 			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 + cli->map->base;
+			vpr_dt_info(dt, "class-ref: %s.%s ",
+				    cli->user_mod_name, class_string);
+			return cli->map;
+		}
+	}
 	*class_id = -ENOENT;
 	return NULL;
 }
@@ -590,9 +608,10 @@ static int ddebug_exec_queries(char *query, const char *modname)
 	return nfound;
 }
 
-/* apply a new bitmap to the sys-knob's current bit-state */
+/* apply a new class-param setting */
 static int ddebug_apply_class_bitmap(const struct ddebug_class_param *dcp,
-				     const unsigned long *new_bits, const unsigned long *old_bits,
+				     const unsigned long *new_bits,
+				     const unsigned long *old_bits,
 				     const char *query_modname)
 {
 #define QUERY_SIZE 128
@@ -601,8 +620,9 @@ static int ddebug_apply_class_bitmap(const struct ddebug_class_param *dcp,
 	int matches = 0;
 	int bi, ct;
 
-	v2pr_info("apply bitmap: 0x%lx to: 0x%lx for %s\n", *new_bits, *old_bits,
-		  query_modname ?: "");
+	if (*new_bits != *old_bits)
+		v2pr_info("apply bitmap: 0x%lx to: 0x%lx for %s\n", *new_bits,
+			  *old_bits, query_modname ?: "'*'");
 
 	for (bi = 0; bi < map->length; bi++) {
 		if (test_bit(bi, new_bits) == test_bit(bi, old_bits))
@@ -617,8 +637,9 @@ 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 for %s\n", *new_bits, *old_bits,
-		  query_modname ?: "");
+	if (*new_bits != *old_bits)
+		v2pr_info("applied bitmap: 0x%lx to: 0x%lx for %s\n", *new_bits,
+			  *old_bits, query_modname ?: "'*'");
 
 	return matches;
 }
@@ -707,6 +728,7 @@ static int param_set_dyndbg_module_classes(const char *instr,
 	const struct ddebug_class_map *map = dcp->map;
 	unsigned long inrep, new_bits, old_bits;
 	int rc, totct = 0;
+	char *nl;
 
 	switch (map->map_type) {
 
@@ -720,7 +742,10 @@ static int param_set_dyndbg_module_classes(const char *instr,
 		/* numeric input, accept and fall-thru */
 		rc = kstrtoul(instr, 0, &inrep);
 		if (rc) {
-			pr_err("expecting numeric input: %s > %s\n", instr, KP_NAME(kp));
+			nl = strchr(instr, '\n');
+			if (nl)
+				*nl = '\0';
+			pr_err("expecting numeric input, not: %s > %s\n", instr, KP_NAME(kp));
 			return -EINVAL;
 		}
 		break;
@@ -1113,12 +1138,17 @@ static void *ddebug_proc_next(struct seq_file *m, void *p, loff_t *pos)
 static const char *ddebug_class_name(struct ddebug_table *dt, struct _ddebug *dp)
 {
 	struct ddebug_class_map *map = dt->classes;
+	struct ddebug_class_user *cli = dt->class_refs;
 	int i;
 
 	for (i = 0; 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; i < dt->num_class_refs; i++, cli++)
+		if (class_in_range(dp->class_id, cli->map))
+			return cli->map->class_names[dp->class_id - cli->map->base];
+
 	return NULL;
 }
 
@@ -1199,33 +1229,125 @@ static const struct proc_ops proc_fops = {
 	.proc_write = ddebug_proc_write
 };
 
+
+/*
+ * Find this module's classmaps in a sub/whole-range of the builtin/
+ * modular classmap vector/section.  Save the start and length of the
+ * subrange at its edges.
+ */
 static void ddebug_attach_module_classes(struct ddebug_table *dt, struct _ddebug_info *di)
 {
 	struct ddebug_class_map *cm;
 	int i, nc = 0;
 
-	/*
-	 * 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++) {
+	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[0..]: module:%s base:%d len:%d ty:%d\n",
+					  cm->mod_name, cm->base, cm->length, cm->map_type);
 			}
 			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);
+}
+
+#define vpr_cm_info(cm, _msg, ...)					\
+	v2pr_info(_msg " module:%s base:%d len:%d type:%d\n", ##__VA_ARGS__, \
+		  cm->mod_name, cm->base, cm->length, cm->map_type)
+
+static void ddebug_match_apply_kparam(const struct kernel_param *kp, struct ddebug_class_user *cli)
+{
+	struct ddebug_class_param *dcp;
+	unsigned long new_bits, old_bits = 0;
+	int totct = 0;
+
+	if (kp->ops != &param_ops_dyndbg_classes)
+		return;
+
+	dcp = (struct ddebug_class_param *)kp->arg;
+
+	if (cli->map == dcp->map) {
+		v2pr_info("found kp:%s =0x%lx", kp->name, *dcp->bits);
+		vpr_cm_info(cli->map, "mapped to:");
+		/*
+		 * using apply-bitmap is too low.
+		 * param_set_dyndbg_classes uses map_type to sort
+		 * levels-to-bits.
+		 * param_set_dyndbg_classes is too high, it takes
+		 * string inputs.
+		 * de-union-ing levels/bits might solve it (partly),
+		 * at least simplifying the translation, and possibly
+		 * the apply-bitmap fn/iface
+		 */
+		new_bits = *dcp->bits;
+		totct += ddebug_apply_class_bitmap(dcp, &new_bits, &old_bits,
+						   cli->user_mod_name);
+	}
+}
+
+static void ddebug_apply_parents_params(struct ddebug_class_user *cli)
+{
+	const struct ddebug_class_map *cm = cli->map;
+	const struct kernel_param *kp;
+#if IS_ENABLED(CONFIG_MODULES)
+	int i;
+
+	if (cm->mod) {
+		vpr_cm_info(cm, "loaded class:");
+		for (i = 0, kp = cm->mod->kp; i < cm->mod->num_kp; i++, kp++)
+			ddebug_match_apply_kparam(kp, cli);
+	}
+#endif
+	if (!cm->mod) {
+		vpr_cm_info(cm, "builtin class:");
+		for (kp = __start___param; kp < __stop___param; kp++)
+			ddebug_match_apply_kparam(kp, cli);
 	}
 }
 
+/*
+ * propagates class-params thru their classmaps to class-users.  this
+ * means a query against the dt/module, which means it must be on the
+ * list to be seen by ddebug_change.
+ */
+static void ddebug_attach_client_module_classes(struct ddebug_table *dt, const struct _ddebug_info *di)
+{
+	struct ddebug_class_user *cli;
+	int i, nc = 0;
+
+	/*
+	 * For builtins: scan the array, find start/length of this
+	 * module's refs, save to dt.  For loadables, this is the
+	 * whole array.
+	 */
+	for (i = 0, cli = di->class_refs; i < di->num_class_refs; i++, cli++) {
+
+		if (WARN_ON(!cli || !cli->map || !cli->user_mod_name))
+			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);
+
+			if (!nc++)
+				dt->class_refs = cli;
+		}
+	}
+	dt->num_class_refs = nc;
+
+	/* now iterate dt */
+	for (i = 0, cli = dt->class_refs; i < dt->num_class_refs; i++, cli++)
+		ddebug_apply_parents_params(cli);
+
+	vpr_dt_info(dt, "attach-client-module: ");
+}
+
 /*
  * Allocate a new ddebug_table for the given module
  * and add it to the global list.
@@ -1235,7 +1357,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;
@@ -1258,13 +1381,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;
 }
@@ -1390,8 +1516,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.39.1


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

* [PATCH v3 12/19] dyndbg-API: DYNDBG_CLASSMAP_USE drop extra args
  2023-01-25 20:37 [PATCH v3 00/19] fix DRM_USE_DYNAMIC_DEBUG regression Jim Cromie
                   ` (10 preceding siblings ...)
  2023-01-25 20:37 ` [PATCH v3 11/19] dyndbg-API: specialize DYNDBG_CLASSMAP_(DEFINE|USE) Jim Cromie
@ 2023-01-25 20:37 ` Jim Cromie
  2023-01-25 20:37 ` [PATCH v3 13/19] dyndbg-API: DYNDBG_CLASSMAP_DEFINE() improvements Jim Cromie
                   ` (7 subsequent siblings)
  19 siblings, 0 replies; 23+ messages in thread
From: Jim Cromie @ 2023-01-25 20:37 UTC (permalink / raw)
  To: linux-kernel, dri-devel, amd-gfx, intel-gvt-dev, intel-gfx
  Cc: jani.nikula, ville.syrjala, daniel.vetter, seanpaul, robdclark,
	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 a7a3a382c4a6..6c57e598b7d2 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -190,17 +190,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 7e6b25446303..1780db9de069 100644
--- a/drivers/gpu/drm/drm_crtc_helper.c
+++ b/drivers/gpu/drm/drm_crtc_helper.c
@@ -51,17 +51,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 e4146b9af357..ad341411687f 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 397ac8294230..91015d1a04e0 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -90,13 +90,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__ };		\
@@ -116,16 +118,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.39.1


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

* [PATCH v3 13/19] dyndbg-API: DYNDBG_CLASSMAP_DEFINE() improvements
  2023-01-25 20:37 [PATCH v3 00/19] fix DRM_USE_DYNAMIC_DEBUG regression Jim Cromie
                   ` (11 preceding siblings ...)
  2023-01-25 20:37 ` [PATCH v3 12/19] dyndbg-API: DYNDBG_CLASSMAP_USE drop extra args Jim Cromie
@ 2023-01-25 20:37 ` Jim Cromie
  2023-01-25 20:37 ` [PATCH v3 14/19] drm_print: fix stale macro-name in comment Jim Cromie
                   ` (6 subsequent siblings)
  19 siblings, 0 replies; 23+ messages in thread
From: Jim Cromie @ 2023-01-25 20:37 UTC (permalink / raw)
  To: linux-kernel, dri-devel, amd-gfx, intel-gvt-dev, intel-gfx
  Cc: jani.nikula, ville.syrjala, daniel.vetter, seanpaul, robdclark,
	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- classname args to CLASSMAP macros were given 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 bits in the sysfs-node (like drm.debug)

  - 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           | 55 +++++++++++++++++++++++++++++++++++
 lib/test_dynamic_debug.c      | 43 +++++++++++++--------------
 5 files changed, 96 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 91015d1a04e0..7cdfc4b533ae 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
@@ -90,18 +91,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..206a402648a2
--- /dev/null
+++ b/include/linux/map.h
@@ -0,0 +1,55 @@
+/* SPDX-License-Identifier: GPL-2.0
+ *
+ * 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 8d384b979e74..e678884066bf 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.39.1


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

* [PATCH v3 14/19] drm_print: fix stale macro-name in comment
  2023-01-25 20:37 [PATCH v3 00/19] fix DRM_USE_DYNAMIC_DEBUG regression Jim Cromie
                   ` (12 preceding siblings ...)
  2023-01-25 20:37 ` [PATCH v3 13/19] dyndbg-API: DYNDBG_CLASSMAP_DEFINE() improvements Jim Cromie
@ 2023-01-25 20:37 ` Jim Cromie
  2023-02-11 19:24   ` jim.cromie
  2023-01-25 20:37 ` [PATCH v3 15/19] test-dyndbg: build test_dynamic_debug_submod Jim Cromie
                   ` (5 subsequent siblings)
  19 siblings, 1 reply; 23+ messages in thread
From: Jim Cromie @ 2023-01-25 20:37 UTC (permalink / raw)
  To: linux-kernel, dri-devel, amd-gfx, intel-gvt-dev, intel-gfx
  Cc: jani.nikula, ville.syrjala, daniel.vetter, seanpaul, robdclark,
	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.39.1


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

* [PATCH v3 15/19] test-dyndbg: build test_dynamic_debug_submod
  2023-01-25 20:37 [PATCH v3 00/19] fix DRM_USE_DYNAMIC_DEBUG regression Jim Cromie
                   ` (13 preceding siblings ...)
  2023-01-25 20:37 ` [PATCH v3 14/19] drm_print: fix stale macro-name in comment Jim Cromie
@ 2023-01-25 20:37 ` Jim Cromie
  2023-01-25 20:37 ` [PATCH v3 16/19] test-dyndbg: rename DD_SYS_WRAP to DYNDBG_CLASSMAP_PARAM Jim Cromie
                   ` (4 subsequent siblings)
  19 siblings, 0 replies; 23+ messages in thread
From: Jim Cromie @ 2023-01-25 20:37 UTC (permalink / raw)
  To: linux-kernel, dri-devel, amd-gfx, intel-gvt-dev, intel-gfx
  Cc: jani.nikula, ville.syrjala, daniel.vetter, seanpaul, robdclark,
	Jim Cromie

CONFIG_DRM_USE_DYNAMIC_DEBUG=y has a regression; drm subsystem
modules, which depend upon drm.ko and use the drm.debug API, do not
get enabled when __drm_debug is set by `modprobe drm debug=0x1f`.

With =N, __drm_debug is checked before logging the msg, so the
end-of-modprobe debug=$init affected all later checks.  But with =y,
each run-time check is replaced by a static-key that is set at
end-of-modprobe.

This creates a chicken-egg dependency; i915 must be modprobed before
its drm.debugs are enabled, but drm.ko (and __drm_debug=$init) must be
done before modprobe i915, so its callsites arent there yet to be
enabled.

The fix is to split DECLARE_DYNDBG_CLASSMAP to:

DYNDBG_CLASSMAP_DEFINE - invoked in 'parent'
DYNDBG_CLASSMAP_USE - invoked in dependent, to USE the exported definition

To prove the fix w/o involving DRM, we need 2 modules, one dependent
on the other.  Add test_dynamic_debug_submod.ko, which _USEs the
classmaps _exported by test_dynamic_debug.ko

To keep code to a minimum, test_dynamic_debug.c ifdefs on
TEST_DYNAMIC_DEBUG_SUBMOD to build either parent or dependent, with
either DYNDBG_CLASSMAP_DEFINE or DYNDBG_CLASSMAP_USE invocations.

So test_dynamic_debug_submod.c is just 2 lines: include the .c after
defining SUBMOD.  This also gives the 2 modules identical prdbg
callsites, only differing by enablement/configuration.

Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
 lib/Makefile                    |  3 +-
 lib/test_dynamic_debug.c        | 52 ++++++++++++++++++++++++++++-----
 lib/test_dynamic_debug_submod.c | 10 +++++++
 3 files changed, 57 insertions(+), 8 deletions(-)
 create mode 100644 lib/test_dynamic_debug_submod.c

diff --git a/lib/Makefile b/lib/Makefile
index 4d9461bfea42..7f7e75f44cd7 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -77,7 +77,7 @@ obj-$(CONFIG_TEST_SORT) += test_sort.o
 obj-$(CONFIG_TEST_USER_COPY) += test_user_copy.o
 obj-$(CONFIG_TEST_STATIC_KEYS) += test_static_keys.o
 obj-$(CONFIG_TEST_STATIC_KEYS) += test_static_key_base.o
-obj-$(CONFIG_TEST_DYNAMIC_DEBUG) += test_dynamic_debug.o
+obj-$(CONFIG_TEST_DYNAMIC_DEBUG) += test_dynamic_debug.o test_dynamic_debug_submod.o
 obj-$(CONFIG_TEST_PRINTF) += test_printf.o
 obj-$(CONFIG_TEST_SCANF) += test_scanf.o
 obj-$(CONFIG_TEST_BITMAP) += test_bitmap.o
@@ -98,6 +98,7 @@ obj-$(CONFIG_KPROBES_SANITY_TEST) += test_kprobes.o
 obj-$(CONFIG_TEST_REF_TRACKER) += test_ref_tracker.o
 CFLAGS_test_fprobe.o += $(CC_FLAGS_FTRACE)
 obj-$(CONFIG_FPROBE_SANITY_TEST) += test_fprobe.o
+
 #
 # CFLAGS for compiling floating point code inside the kernel. x86/Makefile turns
 # off the generation of FPU/SSE* instructions for kernel proper but FPU_FLAGS
diff --git a/lib/test_dynamic_debug.c b/lib/test_dynamic_debug.c
index e678884066bf..8c005c17f2db 100644
--- a/lib/test_dynamic_debug.c
+++ b/lib/test_dynamic_debug.c
@@ -6,7 +6,11 @@
  *      Jim Cromie	<jim.cromie@gmail.com>
  */
 
-#define pr_fmt(fmt) "test_dd: " fmt
+#if defined(TEST_DYNAMIC_DEBUG_SUBMOD)
+  #define pr_fmt(fmt) "test_dd_submod: " fmt
+#else
+  #define pr_fmt(fmt) "test_dd: " fmt
+#endif
 
 #define DEBUG /* enable all prdbgs (plain & class'd) at compiletime */
 
@@ -49,6 +53,14 @@ module_param_cb(do_prints, &param_ops_do_prints, NULL, 0600);
 	};								\
 	module_param_cb(_flags##_##_model, &param_ops_dyndbg_classes, &_flags##_model, 0600)
 
+/*
+ * dynamic-debug imitates drm.debug's use of enums (DRM_UT_CORE etc)
+ * to define its classes/categories.  dyndbg allows class-id's 0..62,
+ * reserving 63 for plain old (non-class'd) prdbgs.  A module can
+ * define multiple classmaps, as long as they claim non-overlapping
+ * subranges.
+ */
+
 /* numeric input, independent bits */
 enum cat_disjoint_bits {
 	D2_CORE = 0,
@@ -61,7 +73,36 @@ enum cat_disjoint_bits {
 	D2_LEASE,
 	D2_DP,
 	D2_DRMRES };
+
+/* symbolic input, independent bits */
+enum cat_disjoint_names { LOW = 10, MID, HI };
+
+/* numeric verbosity, V2 > V1 related */
+enum cat_level_num { V0 = 14, V1, V2, V3, V4, V5, V6, V7 };
+
+/* symbolic verbosity */
+enum cat_level_names { L0 = 22, L1, L2, L3, L4, L5, L6, L7 };
+
+#if defined(TEST_DYNAMIC_DEBUG_SUBMOD)
+
+/* use the classmaps defined in 'parent' module below */
+DYNDBG_CLASSMAP_USE(map_disjoint_bits);
+DYNDBG_CLASSMAP_USE(map_disjoint_names);
+DYNDBG_CLASSMAP_USE(map_level_num);
+DYNDBG_CLASSMAP_USE(map_level_names);
+
+#else
+
+/*
+ * parent module, define a classmap of each of 4 types.
+ * enum values are class-ids
+ * enum symbols are stringified, used as classnames
+ * param bits are mapped in order: 0..N
+ * (a straight, obvious, linear map is encouraged)
+ */
+
 DYNDBG_CLASSMAP_DEFINE(map_disjoint_bits, DD_CLASS_TYPE_DISJOINT_BITS,
+		       /* bits 0..N of param are mapped to these class-ids */
 		       D2_CORE,
 		       D2_DRIVER,
 		       D2_KMS,
@@ -75,27 +116,23 @@ DYNDBG_CLASSMAP_DEFINE(map_disjoint_bits, DD_CLASS_TYPE_DISJOINT_BITS,
 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,
 		       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,
 		       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,
 		       L0, L1, L2, L3, L4, L5, L6, L7);
 DD_SYS_WRAP(level_names, p);
 DD_SYS_WRAP(level_names, T);
 
+#endif /* TEST_DYNAMIC_DEBUG_SUBMOD */
+
 /* stand-in for all pr_debug etc */
 #define prdbg(SYM) __pr_debug_cls(SYM, #SYM " msg\n")
 
@@ -142,6 +179,7 @@ static void do_levels(void)
 
 static void do_prints(void)
 {
+	pr_debug("do_prints:\n");
 	do_cats();
 	do_levels();
 }
diff --git a/lib/test_dynamic_debug_submod.c b/lib/test_dynamic_debug_submod.c
new file mode 100644
index 000000000000..9a893402ce1a
--- /dev/null
+++ b/lib/test_dynamic_debug_submod.c
@@ -0,0 +1,10 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Kernel module for testing dynamic_debug
+ *
+ * Authors:
+ *      Jim Cromie	<jim.cromie@gmail.com>
+ */
+
+#define TEST_DYNAMIC_DEBUG_SUBMOD
+#include "test_dynamic_debug.c"
-- 
2.39.1


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

* [PATCH v3 16/19] test-dyndbg: rename DD_SYS_WRAP to DYNDBG_CLASSMAP_PARAM
  2023-01-25 20:37 [PATCH v3 00/19] fix DRM_USE_DYNAMIC_DEBUG regression Jim Cromie
                   ` (14 preceding siblings ...)
  2023-01-25 20:37 ` [PATCH v3 15/19] test-dyndbg: build test_dynamic_debug_submod Jim Cromie
@ 2023-01-25 20:37 ` Jim Cromie
  2023-01-25 20:37 ` [PATCH v3 17/19] test-dyndbg: disable WIP dyndbg-trace params Jim Cromie
                   ` (3 subsequent siblings)
  19 siblings, 0 replies; 23+ messages in thread
From: Jim Cromie @ 2023-01-25 20:37 UTC (permalink / raw)
  To: linux-kernel, dri-devel, amd-gfx, intel-gvt-dev, intel-gfx
  Cc: jani.nikula, ville.syrjala, daniel.vetter, seanpaul, robdclark,
	Jim Cromie

Original name was a punt; but the macro is maybe general enough to put
in the API later.  Rename and improve the macro towards that end.

Also tweak internal name constructed in the macro, to add a '_'
between the name components.  This changes the .i file only.

no functional change.

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

diff --git a/lib/test_dynamic_debug.c b/lib/test_dynamic_debug.c
index 8c005c17f2db..ff1e70ae060e 100644
--- a/lib/test_dynamic_debug.c
+++ b/lib/test_dynamic_debug.c
@@ -44,14 +44,15 @@ module_param_cb(do_prints, &param_ops_do_prints, NULL, 0600);
  * Additionally, here:
  * - tie together sysname, mapname, bitsname, flagsname
  */
-#define DD_SYS_WRAP(_model, _flags)					\
+#define DYNDBG_CLASSMAP_PARAM(_model, _flags)				\
 	static unsigned long bits_##_model;				\
-	static struct ddebug_class_param _flags##_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)
+	module_param_cb(_flags##_##_model, &param_ops_dyndbg_classes,	\
+			&_flags##_##_model, 0600)
 
 /*
  * dynamic-debug imitates drm.debug's use of enums (DRM_UT_CORE etc)
@@ -113,23 +114,23 @@ DYNDBG_CLASSMAP_DEFINE(map_disjoint_bits, DD_CLASS_TYPE_DISJOINT_BITS,
 		       D2_LEASE,
 		       D2_DP,
 		       D2_DRMRES);
-DD_SYS_WRAP(disjoint_bits, p);
-DD_SYS_WRAP(disjoint_bits, T);
+DYNDBG_CLASSMAP_PARAM(disjoint_bits, p);
+DYNDBG_CLASSMAP_PARAM(disjoint_bits, T);
 
 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);
+DYNDBG_CLASSMAP_PARAM(disjoint_names, p);
+DYNDBG_CLASSMAP_PARAM(disjoint_names, T);
 
 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);
+DYNDBG_CLASSMAP_PARAM(level_num, p);
+DYNDBG_CLASSMAP_PARAM(level_num, T);
 
 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);
+DYNDBG_CLASSMAP_PARAM(level_names, p);
+DYNDBG_CLASSMAP_PARAM(level_names, T);
 
 #endif /* TEST_DYNAMIC_DEBUG_SUBMOD */
 
-- 
2.39.1


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

* [PATCH v3 17/19] test-dyndbg: disable WIP dyndbg-trace params
  2023-01-25 20:37 [PATCH v3 00/19] fix DRM_USE_DYNAMIC_DEBUG regression Jim Cromie
                   ` (15 preceding siblings ...)
  2023-01-25 20:37 ` [PATCH v3 16/19] test-dyndbg: rename DD_SYS_WRAP to DYNDBG_CLASSMAP_PARAM Jim Cromie
@ 2023-01-25 20:37 ` Jim Cromie
  2023-01-25 20:37 ` [PATCH v3 18/19] test-dyndbg: tune sub-module behavior Jim Cromie
                   ` (2 subsequent siblings)
  19 siblings, 0 replies; 23+ messages in thread
From: Jim Cromie @ 2023-01-25 20:37 UTC (permalink / raw)
  To: linux-kernel, dri-devel, amd-gfx, intel-gvt-dev, intel-gfx
  Cc: jani.nikula, ville.syrjala, daniel.vetter, seanpaul, robdclark,
	Jim Cromie

The dyndbg-to-trace feature is WIP, and not in mainline, so the
presence of the interface to use/test it is unhelpful/confusing.

So define DYNDBG_CLASSMAP_PARAM_T() as DYNDBG_CLASSMAP_PARAM() or
blank, depending upon ifdef DYDBG_TRACE, and update 4 params
controlling the T-flag to use it.

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

diff --git a/lib/test_dynamic_debug.c b/lib/test_dynamic_debug.c
index ff1e70ae060e..6b7bd35c3e15 100644
--- a/lib/test_dynamic_debug.c
+++ b/lib/test_dynamic_debug.c
@@ -54,6 +54,14 @@ module_param_cb(do_prints, &param_ops_do_prints, NULL, 0600);
 	module_param_cb(_flags##_##_model, &param_ops_dyndbg_classes,	\
 			&_flags##_##_model, 0600)
 
+/* TBD */
+#ifdef DYNDBG_TRACE
+#define DYNDBG_CLASSMAP_PARAM_T(_model, _flags)	\
+	DYNDBG_CLASSMAP_PARAM(_model, _flags)
+#else
+#define DYNDBG_CLASSMAP_PARAM_T(_model, _flags)
+#endif
+
 /*
  * dynamic-debug imitates drm.debug's use of enums (DRM_UT_CORE etc)
  * to define its classes/categories.  dyndbg allows class-id's 0..62,
@@ -115,22 +123,22 @@ DYNDBG_CLASSMAP_DEFINE(map_disjoint_bits, DD_CLASS_TYPE_DISJOINT_BITS,
 		       D2_DP,
 		       D2_DRMRES);
 DYNDBG_CLASSMAP_PARAM(disjoint_bits, p);
-DYNDBG_CLASSMAP_PARAM(disjoint_bits, T);
+DYNDBG_CLASSMAP_PARAM_T(disjoint_bits, T);
 
 DYNDBG_CLASSMAP_DEFINE(map_disjoint_names, DD_CLASS_TYPE_DISJOINT_NAMES,
 		       LOW, MID, HI);
 DYNDBG_CLASSMAP_PARAM(disjoint_names, p);
-DYNDBG_CLASSMAP_PARAM(disjoint_names, T);
+DYNDBG_CLASSMAP_PARAM_T(disjoint_names, T);
 
 DYNDBG_CLASSMAP_DEFINE(map_level_num, DD_CLASS_TYPE_LEVEL_NUM,
 		       V0, V1, V2, V3, V4, V5, V6, V7);
 DYNDBG_CLASSMAP_PARAM(level_num, p);
-DYNDBG_CLASSMAP_PARAM(level_num, T);
+DYNDBG_CLASSMAP_PARAM_T(level_num, T);
 
 DYNDBG_CLASSMAP_DEFINE(map_level_names, DD_CLASS_TYPE_LEVEL_NAMES,
 		       L0, L1, L2, L3, L4, L5, L6, L7);
 DYNDBG_CLASSMAP_PARAM(level_names, p);
-DYNDBG_CLASSMAP_PARAM(level_names, T);
+DYNDBG_CLASSMAP_PARAM_T(level_names, T);
 
 #endif /* TEST_DYNAMIC_DEBUG_SUBMOD */
 
-- 
2.39.1


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

* [PATCH v3 18/19] test-dyndbg: tune sub-module behavior
  2023-01-25 20:37 [PATCH v3 00/19] fix DRM_USE_DYNAMIC_DEBUG regression Jim Cromie
                   ` (16 preceding siblings ...)
  2023-01-25 20:37 ` [PATCH v3 17/19] test-dyndbg: disable WIP dyndbg-trace params Jim Cromie
@ 2023-01-25 20:37 ` Jim Cromie
  2023-01-25 20:37 ` [PATCH v3 19/19] jump_label: RFC / temporary for CI - tolerate toggled state Jim Cromie
  2023-02-03  9:34 ` [PATCH v3 00/19] fix DRM_USE_DYNAMIC_DEBUG regression Jani Nikula
  19 siblings, 0 replies; 23+ messages in thread
From: Jim Cromie @ 2023-01-25 20:37 UTC (permalink / raw)
  To: linux-kernel, dri-devel, amd-gfx, intel-gvt-dev, intel-gfx
  Cc: jani.nikula, ville.syrjala, daniel.vetter, seanpaul, robdclark,
	Jim Cromie

lib/test_dynamic_debug.c is used to build 2 modules:
test_dynamic_debug.ko and test_dynamic_debug_submod.ko

Define DEBUG only in the main module, not in the submod.  Its purpose
is to insure that prdbgs are enabled by default, so that a modprobe
without params actually logs something, showing that compile-time
enablement works.  This doesn't need to be repeated in the submodule.

Rather, the submodule's purpose is to prove that classmaps defined and
exported from a parent module are propagated to submodules, setting
their class'd debugs accordingly.

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

diff --git a/lib/test_dynamic_debug.c b/lib/test_dynamic_debug.c
index 6b7bd35c3e15..70a1e8955ad0 100644
--- a/lib/test_dynamic_debug.c
+++ b/lib/test_dynamic_debug.c
@@ -10,10 +10,9 @@
   #define pr_fmt(fmt) "test_dd_submod: " fmt
 #else
   #define pr_fmt(fmt) "test_dd: " fmt
+  #define DEBUG	/* enable all prdbgs (plain & class'd), to log by default */
 #endif
 
-#define DEBUG /* enable all prdbgs (plain & class'd) at compiletime */
-
 #include <linux/module.h>
 
 /* run tests by reading or writing sysfs node: do_prints */
-- 
2.39.1


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

* [PATCH v3 19/19] jump_label: RFC / temporary for CI - tolerate toggled state
  2023-01-25 20:37 [PATCH v3 00/19] fix DRM_USE_DYNAMIC_DEBUG regression Jim Cromie
                   ` (17 preceding siblings ...)
  2023-01-25 20:37 ` [PATCH v3 18/19] test-dyndbg: tune sub-module behavior Jim Cromie
@ 2023-01-25 20:37 ` Jim Cromie
  2023-02-03  9:34 ` [PATCH v3 00/19] fix DRM_USE_DYNAMIC_DEBUG regression Jani Nikula
  19 siblings, 0 replies; 23+ messages in thread
From: Jim Cromie @ 2023-01-25 20:37 UTC (permalink / raw)
  To: linux-kernel, dri-devel, amd-gfx, intel-gvt-dev, intel-gfx
  Cc: jani.nikula, ville.syrjala, daniel.vetter, seanpaul, robdclark,
	Jim Cromie, Jason Baron, Peter Zijlstra

__jump_label_patch currently will "crash the box" if it finds a
jump_entry not as expected; it makes no allowances for the well-formed
but incorrect "toggled" state.  This patch changes panic-on-toggled
into a warning, allowing to reduce the problem to a repeatable script.

note: this patch is arch/x86 only, so might not help CI at all.

submod () {
    # set drm.debug analogues
    echo  MP test_dynamic_debug p_disjoint_bits=${1:-0} p_level_num=${2:-0}
    modprobe test_dynamic_debug p_disjoint_bits=${1:-0} p_level_num=${2:-0} dyndbg=+p
    # _submod should pick up kparams
    echo  MP test_dynamic_debug_submod dyndbg=+pmf
    modprobe test_dynamic_debug_submod dyndbg=+pmf
}
unmod () {
    rmmod test_dynamic_debug_submod
    rmmod test_dynamic_debug
}
note () {
    echo NOTE: $* >&2
    sleep 0.1
}

submod_probe () {

    echo 4 > /sys/module/dynamic_debug/parameters/verbose
    unmod
    submod $*

    note submod prdbgs are supposedly enabled
    grep test_dynamic_debug /proc/dynamic_debug/control
    cat /sys/module/test_dynamic_debug/parameters/do_prints

    note but they dont print here
    cat /sys/module/test_dynamic_debug_submod/parameters/do_prints

    note if D2_CORE in $1, trigger toggled warning
    note echo class D2_CORE -p
    echo class D2_CORE -p > /proc/dynamic_debug/control
}

heres the repeatable results

[   17.023758] virtme-init: triggering udev coldplug
[   18.285949] virtme-init: waiting for udev to settle
[   22.550866] i2c_piix4: module verification failed: signature and/or required key missing - tainting kernel
[   22.551945] dyndbg: add-module: i2c_piix4 12 sites 0.0
[   22.552277] dyndbg:  12 debug prints in module i2c_piix4
[   22.555099] piix4_smbus 0000:00:01.3: SMBus Host Controller at 0x700, revision 0
[   22.597344] dyndbg: add-module: serio_raw 2 sites 0.0
[   22.597633] dyndbg:   2 debug prints in module serio_raw
[   22.603506] input: PC Speaker as /devices/platform/pcspkr/input/input4
[   23.556657] dyndbg: add-module: intel_rapl_common 12 sites 0.0
[   23.557000] dyndbg:  12 debug prints in module intel_rapl_common
[   23.759499] dyndbg: add-module: intel_rapl_msr 2 sites 0.0
[   23.759928] dyndbg:   2 debug prints in module intel_rapl_msr
[   26.081050] virtme-init: udev is done
virtme-init: console is ttyS0
bash-5.2# . test-funcs.rc
:#> submod_probe 1 0
rmmod: ERROR: Module test_dynamic_debug_submod is not currently loaded
rmmod: ERROR: Module test_dynamic_debug is not currently loaded
MP test_dynamic_debug p_disjoint_bits=1 p_level_num=0 dyndbg=+pm
[   61.712445] dyndbg: add-module: test_dynamic_debug 33 sites 4.0
[   61.712789] dyndbg: classes[0..]: module:test_dynamic_debug base:22 len:8 ty:3
[   61.713144] dyndbg: module:test_dynamic_debug attached 4 classes
[   61.713894] dyndbg:  33 debug prints in module test_dynamic_debug
[   61.715486] dyndbg: bits:0x1 > *.p_disjoint_bits
[   61.715732] dyndbg: apply bitmap: 0x1 to: 0x0 for '*'
[   61.715983] dyndbg: query 0: "class D2_CORE +p" mod:*
[   61.716253] dyndbg: split into words: "class" "D2_CORE" "+p"
[   61.716539] dyndbg: op='+' flags=0x1 maskp=0xffffffff
[   61.716794] dyndbg: parsed: func="" file="" module="" format="" lineno=0-0 class=D2_CORE
[   61.717232] dyndbg: good-class: test_dynamic_debug.D2_CORE  module:test_dynamic_debug nd:33 nc:4 nu:0
[   61.717690] dyndbg: processed 1 queries, with 1 matches, 0 errs
[   61.717982] dyndbg: bit_0: 1 matches on class: D2_CORE -> 0x1
[   61.718283] dyndbg: applied bitmap: 0x1 to: 0x0 for '*'
[   61.718542] dyndbg: p_disjoint_bits: total matches: 1
[   61.718799] dyndbg: lvl:0 bits:0x0 > p_level_num
[   61.719029] dyndbg: p_level_num: total matches: 0
[   61.719279] dyndbg: module: test_dynamic_debug dyndbg="+pm"
[   61.719554] dyndbg: query 0: "+pm" mod:test_dynamic_debug
[   61.719824] dyndbg: split into words: "+pm"
[   61.720034] dyndbg: op='+' flags=0x3 maskp=0xffffffff
[   61.720299] dyndbg: parsed: func="" file="" module="test_dynamic_debug" format="" lineno=0-0 class=(null)
[   61.720786] dyndbg: changed lib/test_dynamic_debug.c:206 [test_dynamic_debug]test_dynamic_debug_exit p => pm
[   61.721289] dyndbg: changed lib/test_dynamic_debug.c:200 [test_dynamic_debug]test_dynamic_debug_init p => pm
[   61.721778] dyndbg: changed lib/test_dynamic_debug.c:198 [test_dynamic_debug]test_dynamic_debug_init p => pm
[   61.722283] dyndbg: changed lib/test_dynamic_debug.c:191 [test_dynamic_debug]do_prints p => pm
[   61.722711] dyndbg: changed lib/test_dynamic_debug.c:170 [test_dynamic_debug]do_levels p => pm
[   61.723128] dyndbg: changed lib/test_dynamic_debug.c:150 [test_dynamic_debug]do_cats p => pm
[   61.723554] dyndbg: processed 1 queries, with 6 matches, 0 errs
[   61.725233] test_dynamic_debug: test_dd: init start
[   61.725487] test_dynamic_debug: test_dd: do_prints:
[   61.725745] test_dynamic_debug: test_dd: doing categories
[   61.726011] test_dd: LOW msg
[   61.726176] test_dd: MID msg
[   61.726328] test_dd: HI msg
[   61.726470] test_dd: D2_CORE msg
[   61.726640] test_dd: D2_DRIVER msg
[   61.726815] test_dd: D2_KMS msg
[   61.726976] test_dd: D2_PRIME msg
[   61.727144] test_dd: D2_ATOMIC msg
[   61.727332] test_dd: D2_VBL msg
[   61.727494] test_dd: D2_STATE msg
[   61.727669] test_dd: D2_LEASE msg
[   61.727839] test_dd: D2_DP msg
[   61.727996] test_dd: D2_DRMRES msg
[   61.728187] test_dynamic_debug: test_dd: doing levels
[   61.728440] test_dd: V1 msg
[   61.728585] test_dd: V2 msg
[   61.728737] test_dd: V3 msg
[   61.728882] test_dd: V4 msg
[   61.729028] test_dd: V5 msg
[   61.729201] test_dd: V6 msg
[   61.729365] test_dd: V7 msg
[   61.729510] test_dd: L1 msg
[   61.729660] test_dd: L2 msg
[   61.729804] test_dd: L3 msg
[   61.729949] test_dd: L4 msg
[   61.730093] test_dd: L5 msg
[   61.730256] test_dd: L6 msg
[   61.730402] test_dd: L7 msg
[   61.730547] test_dynamic_debug: test_dd: init done
MP test_dynamic_debug_submod dyndbg=+pmf
[   61.908067] dyndbg: add-module: test_dynamic_debug_submod 33 sites 0.4
[   61.908669] dyndbg: class_ref[0] test_dynamic_debug_submod -> test_dynamic_debug
[   61.909089] dyndbg: class_ref[1] test_dynamic_debug_submod -> test_dynamic_debug
[   61.909466] dyndbg: class_ref[2] test_dynamic_debug_submod -> test_dynamic_debug
[   61.909841] dyndbg: class_ref[3] test_dynamic_debug_submod -> test_dynamic_debug
[   61.910232] dyndbg: loaded class: module:test_dynamic_debug base:22 len:8 type:3
[   61.910599] dyndbg: found kp:p_level_names =0x0
[   61.910601] dyndbg: mapped to: module:test_dynamic_debug base:22 len:8 type:3
[   61.911275] dyndbg: loaded class: module:test_dynamic_debug base:14 len:8 type:1
[   61.911672] dyndbg: found kp:p_level_num =0x0
[   61.911674] dyndbg: mapped to: module:test_dynamic_debug base:14 len:8 type:1
[   61.912318] dyndbg: loaded class: module:test_dynamic_debug base:10 len:3 type:2
[   61.912739] dyndbg: found kp:p_disjoint_names =0x0
[   61.912741] dyndbg: mapped to: module:test_dynamic_debug base:10 len:3 type:2
[   61.913373] dyndbg: loaded class: module:test_dynamic_debug base:0 len:10 type:0
[   61.913746] dyndbg: found kp:p_disjoint_bits =0x1
[   61.913747] dyndbg: mapped to: module:test_dynamic_debug base:0 len:10 type:0
[   61.914347] dyndbg: apply bitmap: 0x1 to: 0x0 for test_dynamic_debug_submod
[   61.914695] dyndbg: query 0: "class D2_CORE +p" mod:test_dynamic_debug_submod
[   61.915086] dyndbg: split into words: "class" "D2_CORE" "+p"
[   61.915413] dyndbg: op='+' flags=0x1 maskp=0xffffffff
[   61.915692] dyndbg: parsed: func="" file="" module="test_dynamic_debug_submod" format="" lineno=0-0 class=D2_CORE
[   61.916321] dyndbg: class-ref: test_dynamic_debug_submod.D2_CORE  module:test_dynamic_debug_submod nd:33 nc:0 nu:4
[   61.916912] dyndbg: changed lib/test_dynamic_debug.c:156 [test_dynamic_debug_submod]do_cats _ => p
[   61.917391] dyndbg: processed 1 queries, with 1 matches, 0 errs
[   61.917691] dyndbg: bit_0: 1 matches on class: D2_CORE -> 0x1
[   61.917980] dyndbg: applied bitmap: 0x1 to: 0x0 for test_dynamic_debug_submod
[   61.918341] dyndbg: attach-client-module:  module:test_dynamic_debug_submod nd:33 nc:0 nu:4
[   61.918755] dyndbg:  33 debug prints in module test_dynamic_debug_submod
[   61.919314] dyndbg: module: test_dynamic_debug_submod dyndbg="+pmf"
[   61.919668] dyndbg: query 0: "+pmf" mod:test_dynamic_debug_submod
[   61.920028] dyndbg: split into words: "+pmf"
[   61.920282] dyndbg: op='+' flags=0x7 maskp=0xffffffff
[   61.920576] dyndbg: parsed: func="" file="" module="test_dynamic_debug_submod" format="" lineno=0-0 class=(null)
[   61.921419] dyndbg: changed lib/test_dynamic_debug.c:206 [test_dynamic_debug_submod]test_dynamic_debug_exit _ => pmf
[   61.921990] dyndbg: changed lib/test_dynamic_debug.c:200 [test_dynamic_debug_submod]test_dynamic_debug_init _ => pmf
[   61.922559] dyndbg: changed lib/test_dynamic_debug.c:198 [test_dynamic_debug_submod]test_dynamic_debug_init _ => pmf
[   61.923165] dyndbg: changed lib/test_dynamic_debug.c:191 [test_dynamic_debug_submod]do_prints _ => pmf
[   61.923802] dyndbg: changed lib/test_dynamic_debug.c:170 [test_dynamic_debug_submod]do_levels _ => pmf
[   61.924584] dyndbg: changed lib/test_dynamic_debug.c:150 [test_dynamic_debug_submod]do_cats _ => pmf
[   61.925161] dyndbg: processed 1 queries, with 6 matches, 0 errs
[   61.926602] test_dynamic_debug_submod:test_dynamic_debug_init: test_dd_submod: init start
[   61.927092] test_dynamic_debug_submod:do_prints: test_dd_submod: do_prints:
[   61.927557] test_dynamic_debug_submod:do_cats: test_dd_submod: doing categories
[   61.928130] test_dynamic_debug_submod:do_levels: test_dd_submod: doing levels
[   61.928731] test_dynamic_debug_submod:test_dynamic_debug_init: test_dd_submod: init done
NOTE: submod prdbgs are supposedly enabled
lib/test_dynamic_debug.c:150 [test_dynamic_debug]do_cats =pm "doing categories\n"
lib/test_dynamic_debug.c:152 [test_dynamic_debug]do_cats =p "LOW msg\n" class:LOW
lib/test_dynamic_debug.c:153 [test_dynamic_debug]do_cats =p "MID msg\n" class:MID
lib/test_dynamic_debug.c:154 [test_dynamic_debug]do_cats =p "HI msg\n" class:HI
lib/test_dynamic_debug.c:156 [test_dynamic_debug]do_cats =p "D2_CORE msg\n" class:D2_CORE
lib/test_dynamic_debug.c:157 [test_dynamic_debug]do_cats =p "D2_DRIVER msg\n" class:D2_DRIVER
lib/test_dynamic_debug.c:158 [test_dynamic_debug]do_cats =p "D2_KMS msg\n" class:D2_KMS
lib/test_dynamic_debug.c:159 [test_dynamic_debug]do_cats =p "D2_PRIME msg\n" class:D2_PRIME
lib/test_dynamic_debug.c:160 [test_dynamic_debug]do_cats =p "D2_ATOMIC msg\n" class:D2_ATOMIC
lib/test_dynamic_debug.c:161 [test_dynamic_debug]do_cats =p "D2_VBL msg\n" class:D2_VBL
lib/test_dynamic_debug.c:162 [test_dynamic_debug]do_cats =p "D2_STATE msg\n" class:D2_STATE
lib/test_dynamic_debug.c:163 [test_dynamic_debug]do_cats =p "D2_LEASE msg\n" class:D2_LEASE
lib/test_dynamic_debug.c:164 [test_dynamic_debug]do_cats =p "D2_DP msg\n" class:D2_DP
lib/test_dynamic_debug.c:165 [test_dynamic_debug]do_cats =p "D2_DRMRES msg\n" class:D2_DRMRES
lib/test_dynamic_debug.c:170 [test_dynamic_debug]do_levels =pm "doing levels\n"
lib/test_dynamic_debug.c:172 [test_dynamic_debug]do_levels =p "V1 msg\n" class:V1
lib/test_dynamic_debug.c:173 [test_dynamic_debug]do_levels =p "V2 msg\n" class:V2
lib/test_dynamic_debug.c:174 [test_dynamic_debug]do_levels =p "V3 msg\n" class:V3
lib/test_dynamic_debug.c:175 [test_dynamic_debug]do_levels =p "V4 msg\n" class:V4
lib/test_dynamic_debug.c:176 [test_dynamic_debug]do_levels =p "V5 msg\n" class:V5
lib/test_dynamic_debug.c:177 [test_dynamic_debug]do_levels =p "V6 msg\n" class:V6
lib/test_dynamic_debug.c:178 [test_dynamic_debug]do_levels =p "V7 msg\n" class:V7
lib/test_dynamic_debug.c:180 [test_dynamic_debug]do_levels =p "L1 msg\n" class:L1
lib/test_dynamic_debug.c:181 [test_dynamic_debug]do_levels =p "L2 msg\n" class:L2
lib/test_dynamic_debug.c:182 [test_dynamic_debug]do_levels =p "L3 msg\n" class:L3
lib/test_dynamic_debug.c:183 [test_dynamic_debug]do_levels =p "L4 msg\n" class:L4
lib/test_dynamic_debug.c:184 [test_dynamic_debug]do_levels =p "L5 msg\n" class:L5
lib/test_dynamic_debug.c:185 [test_dynamic_debug]do_levels =p "L6 msg\n" class:L6
lib/test_dynamic_debug.c:186 [test_dynamic_debug]do_levels =p "L7 msg\n" class:L7
lib/test_dynamic_debug.c:191 [test_dynamic_debug]do_prints =pm "do_prints:\n"
lib/test_dynamic_debug.c:198 [test_dynamic_debug]test_dynamic_debug_init =pm "init start\n"
lib/test_dynamic_debug.c:200 [test_dynamic_debug]test_dynamic_debug_init =pm "init done\n"
lib/test_dynamic_debug.c:206 [test_dynamic_debug]test_dynamic_debug_exit =pm "exited\n"
lib/test_dynamic_debug.c:150 [test_dynamic_debug_submod]do_cats =pmf "doing categories\n"
lib/test_dynamic_debug.c:152 [test_dynamic_debug_submod]do_cats =_ "LOW msg\n" class:LOW
lib/test_dynamic_debug.c:153 [test_dynamic_debug_submod]do_cats =_ "MID msg\n" class:MID
lib/test_dynamic_debug.c:154 [test_dynamic_debug_submod]do_cats =_ "HI msg\n" class:HI
lib/test_dynamic_debug.c:156 [test_dynamic_debug_submod]do_cats =p "D2_CORE msg\n" class:D2_CORE
lib/test_dynamic_debug.c:157 [test_dynamic_debug_submod]do_cats =_ "D2_DRIVER msg\n" class:D2_DRIVER
lib/test_dynamic_debug.c:158 [test_dynamic_debug_submod]do_cats =_ "D2_KMS msg\n" class:D2_KMS
lib/test_dynamic_debug.c:159 [test_dynamic_debug_submod]do_cats =_ "D2_PRIME msg\n" class:D2_PRIME
lib/test_dynamic_debug.c:160 [test_dynamic_debug_submod]do_cats =_ "D2_ATOMIC msg\n" class:D2_ATOMIC
lib/test_dynamic_debug.c:161 [test_dynamic_debug_submod]do_cats =_ "D2_VBL msg\n" class:D2_VBL
lib/test_dynamic_debug.c:162 [test_dynamic_debug_submod]do_cats =_ "D2_STATE msg\n" class:D2_STATE
lib/test_dynamic_debug.c:163 [test_dynamic_debug_submod]do_cats =_ "D2_LEASE msg\n" class:D2_LEASE
lib/test_dynamic_debug.c:164 [test_dynamic_debug_submod]do_cats =_ "D2_DP msg\n" class:D2_DP
lib/test_dynamic_debug.c:165 [test_dynamic_debug_submod]do_cats =_ "D2_DRMRES msg\n" class:D2_DRMRES
lib/test_dynamic_debug.c:170 [test_dynamic_debug_submod]do_levels =pmf "doing levels\n"
lib/test_dynamic_debug.c:172 [test_dynamic_debug_submod]do_levels =_ "V1 msg\n" class:V1
lib/test_dynamic_debug.c:173 [test_dynamic_debug_submod]do_levels =_ "V2 msg\n" class:V2
lib/test_dynamic_debug.c:174 [test_dynamic_debug_submod]do_levels =_ "V3 msg\n" class:V3
lib/test_dynamic_debug.c:175 [test_dynamic_debug_submod]do_levels =_ "V4 msg\n" class:V4
lib/test_dynamic_debug.c:176 [test_dynamic_debug_submod]do_levels =_ "V5 msg\n" class:V5
lib/test_dynamic_debug.c:177 [test_dynamic_debug_submod]do_levels =_ "V6 msg\n" class:V6
lib/test_dynamic_debug.c:178 [test_dynamic_debug_submod]do_levels =_ "V7 msg\n" class:V7
lib/test_dynamic_debug.c:180 [test_dynamic_debug_submod]do_levels =_ "L1 msg\n" class:L1
lib/test_dynamic_debug.c:181 [test_dynamic_debug_submod]do_levels =_ "L2 msg\n" class:L2
lib/test_dynamic_debug.c:182 [test_dynamic_debug_submod]do_levels =_ "L3 msg\n" class:L3
lib/test_dynamic_debug.c:183 [test_dynamic_debug_submod]do_levels =_ "L4 msg\n" class:L4
lib/test_dynamic_debug.c:184 [test_dynamic_debug_submod]do_levels =_ "L5 msg\n" class:L5
lib/test_dynamic_debug.c:185 [test_dynamic_debug_submod]do_levels =_ "L6 msg\n" class:L6
lib/test_dynamic_debug.c:186 [test_dynamic_debug_submod]do_levels =_ "L7 msg\n" class:L7
lib/test_dynamic_debug.c:191 [test_dynamic_debug_submod]do_prints =pmf "do_prints:\n"
lib/test_dynamic_debug.c:198 [test_dynamic_debug_submod]test_dynamic_debug_init =pmf "init start\n"
lib/test_dynamic_debug.c:200 [test_dynamic_debug_submod]test_dynamic_debug_init =pmf "init done\n"
lib/test_dynamic_debug.c:206 [test_dynamic_debug_submod]test_dynamic_debug_exit =pmf "exited\n"
[   62.186979] test_dynamic_debug: test_dd: do_prints:
[   62.187728] test_dynamic_debug: test_dd: doing categories
[   62.188004] test_dd: LOW msg
[   62.188167] test_dd: MID msg
[   62.188319] test_dd: HI msg
[   62.188463] test_dd: D2_CORE msg
[   62.188632] test_dd: D2_DRIVER msg
[   62.188807] test_dd: D2_KMS msg
[   62.188968] test_dd: D2_PRIME msg
[   62.189138] test_dd: D2_ATOMIC msg
[   62.189321] test_dd: D2_VBL msg
[   62.189487] test_dd: D2_STATE msg
[   62.189661] test_dd: D2_LEASE msg
[   62.189832] test_dd: D2_DP msg
[   62.189991] test_dd: D2_DRMRES msg
[   62.190175] test_dynamic_debug: test_dd: doing levels
[   62.190431] test_dd: V1 msg
[   62.190577] test_dd: V2 msg
[   62.190725] test_dd: V3 msg
[   62.190870] test_dd: V4 msg
[   62.191014] test_dd: V5 msg
[   62.191169] test_dd: V6 msg
[   62.191316] test_dd: V7 msg
[   62.191460] test_dd: L1 msg
[   62.191605] test_dd: L2 msg
[   62.191759] test_dd: L3 msg
[   62.191903] test_dd: L4 msg
[   62.192047] test_dd: L5 msg
[   62.192200] test_dd: L6 msg
[   62.192376] test_dd: L7 msg
did do_prints
NOTE: but they dont print here
[   62.348171] test_dynamic_debug_submod:do_prints: test_dd_submod: do_prints:
[   62.348940] test_dynamic_debug_submod:do_cats: test_dd_submod: doing categories
[   62.349740] test_dynamic_debug_submod:do_levels: test_dd_submod: doing levels
did do_prints
NOTE: if D2_CORE in 1, trigger toggled warning
NOTE: echo class D2_CORE -p
[   62.632634] dyndbg: read 17 bytes from userspace
[   62.633077] dyndbg: query 0: "class D2_CORE -p" mod:*
[   62.633523] dyndbg: split into words: "class" "D2_CORE" "-p"
[   62.634646] dyndbg: op='-' flags=0x0 maskp=0xfffffffe
[   62.635636] dyndbg: parsed: func="" file="" module="" format="" lineno=0-0 class=D2_CORE
[   62.637202] dyndbg: good-class: test_dynamic_debug.D2_CORE  module:test_dynamic_debug nd:33 nc:4 nu:0
[   62.639078] dyndbg: changed lib/test_dynamic_debug.c:156 [test_dynamic_debug]do_cats p => _
[   62.640840] dyndbg: class-ref: test_dynamic_debug_submod.D2_CORE  module:test_dynamic_debug_submod nd:33 nc:0 nu:4
[   62.642777] jump_label: found toggled op at do_cats+0x11/0x180 [test_dynamic_debug_submod] [000000002815e1ff] (0f 1f 44 00 00 != e9 fe 00 00 00)) size:5 type:0
[   62.645454] dyndbg: changed lib/test_dynamic_debug.c:156 [test_dynamic_debug_submod]do_cats p => _
[   62.647335] dyndbg: processed 1 queries, with 2 matches, 0 errs
:#>

The warning is on submod.ko's D2_CORE prdbg, the -p change finds the
callsite already disabled.  That same callsite was supposedly enabled
earlier, but did not actually print when submod.pr_cats was called.
So it begs the question, what did get enabled earlier, that didnt draw
an error but also failed to enable the callsite ?

The initial value dependence is a strong hint, but I havent decoded it yet.

CC: Jason Baron <jbaron@akamai.com>
CC: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
---
 arch/x86/kernel/jump_label.c | 24 ++++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/arch/x86/kernel/jump_label.c b/arch/x86/kernel/jump_label.c
index f5b8ef02d172..6c76bb8aed54 100644
--- a/arch/x86/kernel/jump_label.c
+++ b/arch/x86/kernel/jump_label.c
@@ -35,7 +35,7 @@ struct jump_label_patch {
 static struct jump_label_patch
 __jump_label_patch(struct jump_entry *entry, enum jump_label_type type)
 {
-	const void *expect, *code, *nop;
+	const void *expect, *code, *nop, *toggled;
 	const void *addr, *dest;
 	int size;
 
@@ -57,20 +57,28 @@ __jump_label_patch(struct jump_entry *entry, enum jump_label_type type)
 	default: BUG();
 	}
 
-	if (type == JUMP_LABEL_JMP)
+	if (type == JUMP_LABEL_JMP) {
 		expect = nop;
-	else
+		toggled = code;
+	} else {
 		expect = code;
-
+		toggled = nop;
+	}
 	if (memcmp(addr, expect, size)) {
 		/*
-		 * The location is not an op that we were expecting.
-		 * Something went wrong. Crash the box, as something could be
+		 * The location is not the op that we were expecting.
+		 * If its a well-formed toggled op, then warn,
+		 * otherwise crash the box, as something could be
 		 * corrupting the kernel.
 		 */
-		pr_crit("jump_label: Fatal kernel bug, unexpected op at %pS [%p] (%5ph != %5ph)) size:%d type:%d\n",
+		if (memcmp(addr, toggled, size)) {
+			pr_crit("jump_label: Fatal kernel bug, unexpected op at %pS [%p] (%5ph != %5ph)) size:%d type:%d\n",
+				addr, addr, addr, expect, size, type);
+			BUG();
+		} else {
+			pr_warn("jump_label: found toggled op at %pS [%p] (%5ph != %5ph)) size:%d type:%d\n",
 				addr, addr, addr, expect, size, type);
-		BUG();
+		}
 	}
 
 	if (type == JUMP_LABEL_NOP)
-- 
2.39.1


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

* Re: [PATCH v3 00/19] fix DRM_USE_DYNAMIC_DEBUG regression
  2023-01-25 20:37 [PATCH v3 00/19] fix DRM_USE_DYNAMIC_DEBUG regression Jim Cromie
                   ` (18 preceding siblings ...)
  2023-01-25 20:37 ` [PATCH v3 19/19] jump_label: RFC / temporary for CI - tolerate toggled state Jim Cromie
@ 2023-02-03  9:34 ` Jani Nikula
  2023-02-03 15:08   ` Stanislaw Gruszka
  19 siblings, 1 reply; 23+ messages in thread
From: Jani Nikula @ 2023-02-03  9:34 UTC (permalink / raw)
  To: Jim Cromie, linux-kernel, dri-devel, amd-gfx, intel-gvt-dev, intel-gfx
  Cc: ville.syrjala, daniel.vetter, seanpaul, robdclark, Jim Cromie,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, Dave Airlie,
	Greg KH

On Wed, 25 Jan 2023, Jim Cromie <jim.cromie@gmail.com> wrote:
> Hi everyone,
>
> In v6.1 DRM_USE_DYNAMIC_DEBUG=y has a regression enabling drm.debug in
> drivers at modprobe.

I realize we haven't actually addressed the regression in any way yet,
and any distro enabling DYNAMIC_DEBUG || DYNAMIC_DEBUG_CORE will have
DRM_USE_DYNAMIC_DEBUG=y by default, and we're hitting the issue with
trying to gather logs from users on v6.1 or later. It hampers debugging
pretty badly.

I appreciate the effort in fixing the problem properly here, but we'll
need a fix that we can backport to stable kernels.

Maybe just Ville's idea of

 config DRM_USE_DYNAMIC_DEBUG
        bool "use dynamic debug to implement drm.debug"
-       default y
+       default n
+       depends on BROKEN
        depends on DRM
        depends on DYNAMIC_DEBUG || DYNAMIC_DEBUG_CORE

but we'll need that as a patch and merged and backported ASAP.

In the mean time, is there a workaround that the user could enable, say,
on the kernel command line, to enable drm debugs on driver kernel
modules, all the way from boot?


BR,
Jani.




>
> It is due to a chicken-egg problem loading modules; on `modprobe
> i915`, drm is loaded 1st, and drm/parameters/debug is set.  When
> drm_debug_enabled() tested __drm_debug at runtime, this just worked.
>
> But with DRM_USE_DYNAMIC_DEBUG=y, the runtime test is replaced with a
> static_key for each drm_dbg/dyndbg callsite, enabled by dyndbg's
> kparam callback on __drm_debug.  So with drm.ko loaded and initialized
> before the dependent modules, their debug callsites aren't yet present
> to be enabled.
>
> STATUS - v3
>
> not quite ready.
> rebased on -rc5, hopefully applies to patchwork head 
> still has RFC patch -> CI_ONLY temporary, to avoid panics
> boots on my amdgpu box, drm.debug=0x3ff works at boot-time
> the "toggled" warning is repeatable with test_dynamic_debug*.ko
> it also occurs on amdgpu, so not just artificial.
> v2 is https://lore.kernel.org/lkml/20230113193016.749791-1-jim.cromie@gmail.com/
>
> OVERVIEW
>
> As Jani Nikula noted rather more gently, DECLARE_DYNDBG_CLASSMAP is
> error-prone enough to call broken: sharing of a common classmap
> required identical classmap definitions in all modules using DRM_UT_*,
> which is inherently error-prone.  IOW, it muddled the K&R distinction
> between a (single) definition, and multiple references.
>
> So patches 10-13 split it into:
>
> DYNDBG_CLASSMAP_DEFINE	used once per subsystem to define each classmap.
> DYNDBG_CLASSMAP_USE	declare dependence on a DEFINEd classmap.
>
> DYNDBG_CLASSMAP_DEFINE initializes the classmap, stores it into the
> (existing) __dyndbg_classes section, and exports the struct var
> (unlike DECLARE_DYNDBG_CLASSMAP).
>
> DYNDBG_CLASSMAP_USE initializes a class-ref struct, containing the
> user-module-name, and a ref to the exported classmap var.
>
> The distinction allows separate treatment of classmaps and
> classmap-refs, the latter getting additional behavior to propagate
> parent's kparam settings to USEr. (forex: drm.debug to drm-drivers) 
>
> . lookup the classmap defn being referenced, and its module
> . find the module's kernel-params using the classmap
> . propagate kparam vals into the prdgs in module being added.
>
> It also makes the weird coordinated-changes-by-identical-classmaps
> "feature" unnecessary.
>
> Patch-10 splits the DECLARE macro into DEFINE & USE, and updates uses.
>
> Patch-11 is the core of it; the separate treatment begins in
> ddebug_add_module().  It calls ddebug_attach_module_classes(1) to
> handle class-defns; this adds ddebug_attach_client_module_classes(2)
> to handle class-refs, as they are found while modprobing drm
> drivers. (2) calls ddebug_apply_parents_params(3) on each USEr's
> referred classmap definition.
>
> (3) scans kernel-params owned by the module DEFINEing the classmap,
> either builtin or loadable, calls ddebug_match_apply_kparam(4) on each.
>
> (4) looks for kparams which are wired to dyndbg's param-ops.  Those
> params have a struct ddebug_class_param attached, which has a classmap
> and a ref to a state-var (__drm_debug for DRM case).  If the kparam's
> classmap is the same as from (2), then apply its state-var to the
> client module by calling ddebug_apply_class_bitmap().
>
> Patch-12 cleans up DYNDBG_CLASSMAP_USE, dropping now unneeded args.
>
> Patch-13 improves DYNDBG_CLASSMAP_DEFINE, by accepting DRM_UT_*
> symbols directly, not "DRM_UT_*" (their strings).  It adds new
> include/linux/map.h to support this.
>
> Patches 1-9 are prep, refactor, cleanup, tighten interfaces
>
> Patches 15-18 extend test_dynamic_debug to recreate DRM's multi-module
> regression; it builds both test_dynamic_debug.ko and _submod.ko, with
> an ifdef to _DEFINE in the main module, and _USE in the submod.  This
> gives both modules identical set of prdbgs, which is helpful for
> comparing results.
>
> here it is, working properly:
>
> doing class DRM_UT_CORE -p
> [ 9904.961750] dyndbg: read 21 bytes from userspace
> [ 9904.962286] dyndbg: query 0: "class DRM_UT_CORE -p" mod:*
> [ 9904.962848] dyndbg: split into words: "class" "DRM_UT_CORE" "-p"
> [ 9904.963444] dyndbg: op='-' flags=0x0 maskp=0xfffffffe
> [ 9904.963945] dyndbg: parsed: func="" file="" module="" format="" lineno=0-0 class=DRM_UT_CORE
> [ 9904.964781] dyndbg: good-class: drm.DRM_UT_CORE  module:drm nd:302 nc:1 nu:0
> [ 9904.966411] dyndbg: class-ref: drm_kms_helper.DRM_UT_CORE  module:drm_kms_helper nd:95 nc:0 nu:1
> [ 9904.967265] dyndbg: class-ref: drm_display_helper.DRM_UT_CORE  module:drm_display_helper nd:150 nc:0 nu:1
> [ 9904.968349] dyndbg: class-ref: i915.DRM_UT_CORE  module:i915 nd:1659 nc:0 nu:1
> [ 9904.969801] dyndbg: class-ref: amdgpu.DRM_UT_CORE  module:amdgpu nd:4425 nc:0 nu:1
> [ 9904.977079] dyndbg: class-ref: nouveau.DRM_UT_CORE  module:nouveau nd:103 nc:0 nu:1
> [ 9904.977830] dyndbg: processed 1 queries, with 507 matches, 0 errs
> doing class DRM_UT_DRIVER +p
> [ 9906.151761] dyndbg: read 23 bytes from userspace
> [ 9906.152241] dyndbg: query 0: "class DRM_UT_DRIVER +p" mod:*
> [ 9906.152793] dyndbg: split into words: "class" "DRM_UT_DRIVER" "+p"
> [ 9906.153388] dyndbg: op='+' flags=0x1 maskp=0xffffffff
> [ 9906.153896] dyndbg: parsed: func="" file="" module="" format="" lineno=0-0 class=DRM_UT_DRIVER
> [ 9906.154746] dyndbg: good-class: drm.DRM_UT_DRIVER  module:drm nd:302 nc:1 nu:0
> [ 9906.155433] dyndbg: class-ref: drm_kms_helper.DRM_UT_DRIVER  module:drm_kms_helper nd:95 nc:0 nu:1
> [ 9906.156267] dyndbg: class-ref: drm_display_helper.DRM_UT_DRIVER  module:drm_display_helper nd:150 nc:0 nu:1
> [ 9906.157365] dyndbg: class-ref: i915.DRM_UT_DRIVER  module:i915 nd:1659 nc:0 nu:1
> [ 9906.163848] dyndbg: class-ref: amdgpu.DRM_UT_DRIVER  module:amdgpu nd:4425 nc:0 nu:1
> [ 9906.178963] dyndbg: class-ref: nouveau.DRM_UT_DRIVER  module:nouveau nd:103 nc:0 nu:1
> [ 9906.179934] dyndbg: processed 1 queries, with 1286 matches, 0 errs
>
>
> Patch-19 is a *workaround* for a panic: __jump_label_patch can "crash
> the box" when the jump-entry is in the wrong state.  The current code
> makes no distinction between a well-formed "toggled" state and an
> "insane" state.  Not for keeps.
>
> It fixes mis-initialization problems like this:
>
> [ 1594.032504] dyndbg: query 0: "class D2_DRIVER -p" mod:*
> [ 1594.032823] dyndbg: split into words: "class" "D2_DRIVER" "-p"
> [ 1594.033183] dyndbg: op='-' flags=0x0 maskp=0xfffffffe
> [ 1594.033507] dyndbg: parsed: func="" file="" module="" format="" lineno=0-0 class=D2_DRIVER
> [ 1594.034014] dyndbg: good-class: test_dynamic_debug.D2_DRIVER  module:test_dynamic_debug nd:32 nc:4 nu:0
> [ 1594.034695] dyndbg: changed lib/test_dynamic_debug.c:156 [test_dynamic_debug]do_cats p => _
> [ 1594.035304] dyndbg: class-ref: test_dynamic_debug_submod.D2_DRIVER  module:test_dynamic_debug_submod nd:32 nc:0 nu:4
> [ 1594.036052] jump_label: found toggled op at do_cats+0x16/0x180 [test_dynamic_debug_submod] [00000000ff2582ac] (0f 1f 44 00 00 != e9 e1 00 00 00)) size:5 type:0
> [ 1594.037036] dyndbg: changed lib/test_dynamic_debug.c:156 [test_dynamic_debug_submod]do_cats p => _
> [ 1594.037604] dyndbg: processed 1 queries, with 2 matches, 0 errs
> [ 1594.037968] dyndbg: bit_1: 2 matches on class: D2_DRIVER -> 0x0
>
> These errors are reliably reproduced by a shell-func which modprobes
> (with the right args) the test mod & submod.ko (in the commit message).
>
> So this isnt really ready for inclusion, but Id like to send the whole
> set to the CI-gym for a workout.  The RFC/for-TESTING patch will
> mitigate panics, and still be detectable.
>
> Besides, Murphys law requires I publish some error before I can make progress.
>
>
> Jim Cromie (19):
>   test-dyndbg: fixup CLASSMAP usage error
>   test-dyndbg: show that DEBUG enables prdbgs at compiletime
>   dyndbg: replace classmap list with a vector
>   dyndbg: make ddebug_apply_class_bitmap more selective
>   dyndbg: split param_set_dyndbg_classes to inner/outer fns
>   dyndbg: drop NUM_TYPE_ARRAY
>   dyndbg: reduce verbose/debug clutter
>   dyndbg: tighten ddebug_class_name() 1st arg
>   dyndbg: constify ddebug_apply_class_bitmap args
>   dyndbg-API: split DECLARE_(DYNDBG_CLASSMAP) to $1(_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
>   test-dyndbg: build test_dynamic_debug_submod
>   test-dyndbg: rename DD_SYS_WRAP to DYNDBG_CLASSMAP_PARAM
>   test-dyndbg: disable WIP dyndbg-trace params
>   test-dyndbg: tune sub-module behavior
>   jump_label: RFC / temporary for CI - tolerate toggled state
>
>  arch/x86/kernel/jump_label.c            |  24 ++-
>  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       |   1 +
>  include/drm/drm_print.h                 |   6 +-
>  include/linux/dynamic_debug.h           |  57 ++++--
>  include/linux/map.h                     |  55 +++++
>  kernel/module/main.c                    |   3 +
>  lib/Makefile                            |   3 +-
>  lib/dynamic_debug.c                     | 258 ++++++++++++++++++------
>  lib/test_dynamic_debug.c                | 118 +++++++----
>  lib/test_dynamic_debug_submod.c         |  10 +
>  16 files changed, 437 insertions(+), 190 deletions(-)
>  create mode 100644 include/linux/map.h
>  create mode 100644 lib/test_dynamic_debug_submod.c

-- 
Jani Nikula, Intel Open Source Graphics Center

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

* Re: [PATCH v3 00/19] fix DRM_USE_DYNAMIC_DEBUG regression
  2023-02-03  9:34 ` [PATCH v3 00/19] fix DRM_USE_DYNAMIC_DEBUG regression Jani Nikula
@ 2023-02-03 15:08   ` Stanislaw Gruszka
  0 siblings, 0 replies; 23+ messages in thread
From: Stanislaw Gruszka @ 2023-02-03 15:08 UTC (permalink / raw)
  To: Jani Nikula
  Cc: Jim Cromie, linux-kernel, dri-devel, amd-gfx, intel-gvt-dev,
	intel-gfx, daniel.vetter, seanpaul, Thomas Zimmermann, Greg KH

On Fri, Feb 03, 2023 at 11:34:02AM +0200, Jani Nikula wrote:
> On Wed, 25 Jan 2023, Jim Cromie <jim.cromie@gmail.com> wrote:
> > Hi everyone,
> >
> > In v6.1 DRM_USE_DYNAMIC_DEBUG=y has a regression enabling drm.debug in
> > drivers at modprobe.
> 
> I realize we haven't actually addressed the regression in any way yet,
> and any distro enabling DYNAMIC_DEBUG || DYNAMIC_DEBUG_CORE will have
> DRM_USE_DYNAMIC_DEBUG=y by default, and we're hitting the issue with
> trying to gather logs from users on v6.1 or later. It hampers debugging
> pretty badly.
> 
> I appreciate the effort in fixing the problem properly here, but we'll
> need a fix that we can backport to stable kernels.
> 
> Maybe just Ville's idea of
> 
>  config DRM_USE_DYNAMIC_DEBUG
>         bool "use dynamic debug to implement drm.debug"
> -       default y
> +       default n
> +       depends on BROKEN
>         depends on DRM
>         depends on DYNAMIC_DEBUG || DYNAMIC_DEBUG_CORE
> 
> but we'll need that as a patch and merged and backported ASAP.

+1 for this

Regards
Stanislaw

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

* Re: [PATCH v3 14/19] drm_print: fix stale macro-name in comment
  2023-01-25 20:37 ` [PATCH v3 14/19] drm_print: fix stale macro-name in comment Jim Cromie
@ 2023-02-11 19:24   ` jim.cromie
  0 siblings, 0 replies; 23+ messages in thread
From: jim.cromie @ 2023-02-11 19:24 UTC (permalink / raw)
  To: linux-kernel, dri-devel, amd-gfx, intel-gvt-dev, intel-gfx
  Cc: jani.nikula, ville.syrjala, daniel.vetter, seanpaul, robdclark

On Wed, Jan 25, 2023 at 1:38 PM Jim Cromie <jim.cromie@gmail.com> wrote:
>
> Cited commit uses stale macro name, fix this, and explain better.




So this patch is somehow drawing an 'F' flag from patchwork,
but theres no hint of what went wrong.
(I have seen a merge conflict, probably not that).

https://patchwork.freedesktop.org/series/113361/

https://patchwork.freedesktop.org/patch/520460/?series=113361&rev=1

Without this resolved, I cant see BAT results or the more exhaustive tests.





>
> 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.39.1
>

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

end of thread, other threads:[~2023-02-11 19:24 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-25 20:37 [PATCH v3 00/19] fix DRM_USE_DYNAMIC_DEBUG regression Jim Cromie
2023-01-25 20:37 ` [PATCH v3 01/19] test-dyndbg: fixup CLASSMAP usage error Jim Cromie
2023-01-25 20:37 ` [PATCH v3 02/19] test-dyndbg: show that DEBUG enables prdbgs at compiletime Jim Cromie
2023-01-25 20:37 ` [PATCH v3 03/19] dyndbg: replace classmap list with a vector Jim Cromie
2023-01-25 20:37 ` [PATCH v3 04/19] dyndbg: make ddebug_apply_class_bitmap more selective Jim Cromie
2023-01-25 20:37 ` [PATCH v3 05/19] dyndbg: split param_set_dyndbg_classes to inner/outer fns Jim Cromie
2023-01-25 20:37 ` [PATCH v3 06/19] dyndbg: drop NUM_TYPE_ARRAY Jim Cromie
2023-01-25 20:37 ` [PATCH v3 07/19] dyndbg: reduce verbose/debug clutter Jim Cromie
2023-01-25 20:37 ` [PATCH v3 08/19] dyndbg: tighten ddebug_class_name() 1st arg Jim Cromie
2023-01-25 20:37 ` [PATCH v3 09/19] dyndbg: constify ddebug_apply_class_bitmap args Jim Cromie
2023-01-25 20:37 ` [PATCH v3 10/19] dyndbg-API: split DECLARE_(DYNDBG_CLASSMAP) to $1(_DEFINE|_USE) Jim Cromie
2023-01-25 20:37 ` [PATCH v3 11/19] dyndbg-API: specialize DYNDBG_CLASSMAP_(DEFINE|USE) Jim Cromie
2023-01-25 20:37 ` [PATCH v3 12/19] dyndbg-API: DYNDBG_CLASSMAP_USE drop extra args Jim Cromie
2023-01-25 20:37 ` [PATCH v3 13/19] dyndbg-API: DYNDBG_CLASSMAP_DEFINE() improvements Jim Cromie
2023-01-25 20:37 ` [PATCH v3 14/19] drm_print: fix stale macro-name in comment Jim Cromie
2023-02-11 19:24   ` jim.cromie
2023-01-25 20:37 ` [PATCH v3 15/19] test-dyndbg: build test_dynamic_debug_submod Jim Cromie
2023-01-25 20:37 ` [PATCH v3 16/19] test-dyndbg: rename DD_SYS_WRAP to DYNDBG_CLASSMAP_PARAM Jim Cromie
2023-01-25 20:37 ` [PATCH v3 17/19] test-dyndbg: disable WIP dyndbg-trace params Jim Cromie
2023-01-25 20:37 ` [PATCH v3 18/19] test-dyndbg: tune sub-module behavior Jim Cromie
2023-01-25 20:37 ` [PATCH v3 19/19] jump_label: RFC / temporary for CI - tolerate toggled state Jim Cromie
2023-02-03  9:34 ` [PATCH v3 00/19] fix DRM_USE_DYNAMIC_DEBUG regression Jani Nikula
2023-02-03 15:08   ` Stanislaw Gruszka

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