* [RFC PATCH 01/17] test-dyndbg: fixup CLASSMAP usage error
2022-12-06 0:34 [RFC PATCH 00/17] DRM_USE_DYNAMIC_DEBUG regression Jim Cromie
@ 2022-12-06 0:34 ` Jim Cromie
2022-12-06 0:34 ` [RFC PATCH 02/17] test-dyndbg: show that DEBUG enables prdbgs at compiletime Jim Cromie
` (16 subsequent siblings)
17 siblings, 0 replies; 22+ messages in thread
From: Jim Cromie @ 2022-12-06 0:34 UTC (permalink / raw)
To: linux-kernel, dri-devel, amd-gfx, intel-gvt-dev, intel-gfx
Cc: jani.nikula, ville.syrjala, daniel.vetter, seanpaul, robdclark,
jbaron, gregkh, Jim Cromie
more careful reading of test output reveals:
lib/test_dynamic_debug.c:103 [test_dynamic_debug]do_cats =pmf "doing categories\n"
lib/test_dynamic_debug.c:105 [test_dynamic_debug]do_cats =p "LOW msg\n" class:MID
lib/test_dynamic_debug.c:106 [test_dynamic_debug]do_cats =p "MID msg\n" class:HI
lib/test_dynamic_debug.c:107 [test_dynamic_debug]do_cats =_ "HI msg\n" class unknown, _id:13
That last line is wrong, the HI class is declared.
But the enum's 1st val (explicitly initialized) was wrong; it must be
_base, not _base+1 (a DECLARE_DYNDBG_CLASSMAP param). So the last
enumeration exceeded the range of mapped class-id's, which triggered
the "class unknown" report. Basically, I coded in an error, and
forgot to verify it and remove it.
RFC:
This patch fixes a bad usage of DEFINE_DYNDBG_CLASSMAP(), showing that
it is too error-prone. As noted in test-dynamic-debug.c comments:
* Using the CLASSMAP api:
* - classmaps must have corresponding enum
* - enum symbols must match/correlate with class-name strings in the map.
* - base must equal enum's 1st value
* - multiple maps must set their base to share the 0-62 class_id space !!
* (build-bug-on tips welcome)
Those shortcomings could largely be fixed with a __stringify_list
(which doesn't exist) used in DEFINE_DYNAMIC_DEBUG_CLASSMAP(), on
__VA_ARGS__ a 2nd time. Then, DRM would pass DRM_UT_* ; all the
categories, in order, and not their stringifications, which created
all the usage complications above.
Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
lib/test_dynamic_debug.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/test_dynamic_debug.c b/lib/test_dynamic_debug.c
index 8dd250ad022b..a01f0193a419 100644
--- a/lib/test_dynamic_debug.c
+++ b/lib/test_dynamic_debug.c
@@ -75,7 +75,7 @@ DD_SYS_WRAP(disjoint_bits, p);
DD_SYS_WRAP(disjoint_bits, T);
/* symbolic input, independent bits */
-enum cat_disjoint_names { LOW = 11, MID, HI };
+enum cat_disjoint_names { LOW = 10, MID, HI };
DECLARE_DYNDBG_CLASSMAP(map_disjoint_names, DD_CLASS_TYPE_DISJOINT_NAMES, 10,
"LOW", "MID", "HI");
DD_SYS_WRAP(disjoint_names, p);
--
2.38.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* [RFC PATCH 02/17] test-dyndbg: show that DEBUG enables prdbgs at compiletime
2022-12-06 0:34 [RFC PATCH 00/17] DRM_USE_DYNAMIC_DEBUG regression Jim Cromie
2022-12-06 0:34 ` [RFC PATCH 01/17] test-dyndbg: fixup CLASSMAP usage error Jim Cromie
@ 2022-12-06 0:34 ` Jim Cromie
2022-12-06 0:34 ` [RFC PATCH 03/17] dyndbg: fix readback value on LEVEL_NAMES interfaces Jim Cromie
` (15 subsequent siblings)
17 siblings, 0 replies; 22+ messages in thread
From: Jim Cromie @ 2022-12-06 0:34 UTC (permalink / raw)
To: linux-kernel, dri-devel, amd-gfx, intel-gvt-dev, intel-gfx
Cc: jani.nikula, ville.syrjala, daniel.vetter, seanpaul, robdclark,
jbaron, gregkh, Jim Cromie
Dyndbg is required to enable prdbgs at compile-time if DEBUG is
defined. Show this works; add the defn to test_dynamic_debug.c,
and manually inspect/verify its effect at module load:
[ 15.292810] dyndbg: module:test_dynamic_debug attached 4 classes
[ 15.293189] dyndbg: 32 debug prints in module test_dynamic_debug
[ 15.293715] test_dd: init start
[ 15.293716] test_dd: doing categories
[ 15.293716] test_dd: LOW msg
...
[ 15.293733] test_dd: L6 msg
[ 15.293733] test_dd: L7 msg
[ 15.293733] test_dd: init done
NOTES:
As is observable above, define DEBUG enables all prdbgs, including
those in mod_init-fn, and more notably, the "class"d ones (callsites
with non-default class_ids).
This differs from the >control interface, which in order to properly
protect a client's class'd prdbgs, requires a "class FOO" in queries
to change them. Note that the DEBUG is in the module source file.
This yields an occaisional surprise; the following disables all the
compile-time enabled plain prdbgs, but leaves the class'd ones
enabled.
:#> modprobe test_dynamic_debug dyndbg==_
Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
lib/test_dynamic_debug.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/lib/test_dynamic_debug.c b/lib/test_dynamic_debug.c
index a01f0193a419..9d48689dc0ab 100644
--- a/lib/test_dynamic_debug.c
+++ b/lib/test_dynamic_debug.c
@@ -8,6 +8,8 @@
#define pr_fmt(fmt) "test_dd: " fmt
+#define DEBUG
+
#include <linux/module.h>
/* run tests by reading or writing sysfs node: do_prints */
--
2.38.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* [RFC PATCH 03/17] dyndbg: fix readback value on LEVEL_NAMES interfaces
2022-12-06 0:34 [RFC PATCH 00/17] DRM_USE_DYNAMIC_DEBUG regression Jim Cromie
2022-12-06 0:34 ` [RFC PATCH 01/17] test-dyndbg: fixup CLASSMAP usage error Jim Cromie
2022-12-06 0:34 ` [RFC PATCH 02/17] test-dyndbg: show that DEBUG enables prdbgs at compiletime Jim Cromie
@ 2022-12-06 0:34 ` Jim Cromie
2022-12-06 0:34 ` [RFC PATCH 04/17] dyndbg: replace classmap list with a vector Jim Cromie
` (14 subsequent siblings)
17 siblings, 0 replies; 22+ messages in thread
From: Jim Cromie @ 2022-12-06 0:34 UTC (permalink / raw)
To: linux-kernel, dri-devel, amd-gfx, intel-gvt-dev, intel-gfx
Cc: jani.nikula, ville.syrjala, daniel.vetter, seanpaul, robdclark,
jbaron, gregkh, Jim Cromie
Since sysfs knobs should generally read-back what was just written
(unless theres a good reason to do otherwise), this result (using
test_dynamic_debug.ko) is suboptimal:
echo L3 > p_level_names
cat p_level_names
4
Fix this with a -1 offset in LEVEL_NAMES readback.
NOTE:
Calling this a BUG is debatable, and the above is slightly inaccurate
wrt "read-back"; whats written is a LEVEL_NAME (a string). Whats read
back is an integer, giving the 'edge' of the 'low-pass-filter'
The actual test looks like:
RTT: L4 -> p_level_names : 4 :: DOING: levels 4-1
[ 17.509594] dyndbg: "L4" > p_level_names:0x4
[ 17.510115] dyndbg: apply: 0x1f to: 0xf
[ 17.510506] dyndbg: query 0: "class L4 +p" mod:*
[ 17.510992] dyndbg: split into words: "class" "L4" "+p"
[ 17.511521] dyndbg: op='+'
[ 17.511811] dyndbg: flags=0x1
[ 17.512127] dyndbg: *flagsp=0x1 *maskp=0xffffffff
[ 17.512604] dyndbg: parsed: func="" file="" module="" format="" lineno=0-0 class=L4
[ 17.513414] dyndbg: applied: func="" file="" module="" format="" lineno=0-0 class=L4
[ 17.514204] dyndbg: processed 1 queries, with 1 matches, 0 errs
[ 17.514809] dyndbg: bit_4: 1 matches on class: L4 -> 0x1f
[ 17.515355] dyndbg: p_level_names: changed bit-4: "L4" f->1f
[ 17.515933] dyndbg: total matches: 1
crap [[ 5 != 4 ]]
This -1 adjustment just reports the edge consistently with its
input-mapping.
Fixes: b9400852c080 (dyndbg: add drm.debug style (drm/parameters/debug) bitmap support)
Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
lib/dynamic_debug.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 009f2ead09c1..48ca1387a409 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -794,6 +794,8 @@ int param_get_dyndbg_classes(char *buffer, const struct kernel_param *kp)
return scnprintf(buffer, PAGE_SIZE, "0x%lx\n", *dcp->bits);
case DD_CLASS_TYPE_LEVEL_NAMES:
+ return scnprintf(buffer, PAGE_SIZE, "%d\n", *dcp->lvl - 1);
+
case DD_CLASS_TYPE_LEVEL_NUM:
return scnprintf(buffer, PAGE_SIZE, "%d\n", *dcp->lvl);
default:
--
2.38.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* [RFC PATCH 04/17] dyndbg: replace classmap list with a vector
2022-12-06 0:34 [RFC PATCH 00/17] DRM_USE_DYNAMIC_DEBUG regression Jim Cromie
` (2 preceding siblings ...)
2022-12-06 0:34 ` [RFC PATCH 03/17] dyndbg: fix readback value on LEVEL_NAMES interfaces Jim Cromie
@ 2022-12-06 0:34 ` Jim Cromie
2022-12-06 0:34 ` [RFC PATCH 05/17] dyndbg: make ddebug_apply_class_bitmap more selective Jim Cromie
` (13 subsequent siblings)
17 siblings, 0 replies; 22+ messages in thread
From: Jim Cromie @ 2022-12-06 0:34 UTC (permalink / raw)
To: linux-kernel, dri-devel, amd-gfx, intel-gvt-dev, intel-gfx
Cc: jani.nikula, ville.syrjala, daniel.vetter, seanpaul, robdclark,
jbaron, gregkh, Jim Cromie
Classmaps are stored/linked in a section/array, but are each added to
the module's ddebug_table.maps list-head.
This is unnecessary; even when ddebug_attach_classmap() is handling
the builtin section (with classmaps for multiple builtin modules), its
contents are ordered, so a module's possibly multiple classmaps will
be consecutive in the section, and could be treated as a vector/block,
since both start-addy and subrange length are in the ddebug_info arg.
So this changes:
struct ddebug_class_map drops list link.
struct ddebug_table drops the maps list-head, and gets: classes &
num_classes for the start-addy, and length. num_classes is placed to
improve struct packing.
The loading: in ddebug_attach_module_classes(), replace the
for-the-modname list-add loop, with a forloop that finds the module's
subrange (start,length) of matching classmaps within the possibly
builtin classmaps vector, and saves those to the ddebug_table.
The reading/using: change list-foreach loops in ddebug_class_name() &
ddebug_find_valid_class() to walk the array from start to length.
Also:
Move #define __outvar up, above an added use in a fn-prototype.
Simplify ddebug_attach_module_classes args, ref has both addy,len.
This isn't technically a bugfix, but IMO simplifies later fixes for
the chicken-egg post-init enablement regression.
Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
include/linux/dynamic_debug.h | 1 -
lib/dynamic_debug.c | 61 ++++++++++++++++++-----------------
2 files changed, 32 insertions(+), 30 deletions(-)
diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index 41682278d2e8..bf47bcfad8e6 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -81,7 +81,6 @@ enum class_map_type {
};
struct ddebug_class_map {
- struct list_head link;
struct module *mod;
const char *mod_name; /* needed for builtins */
const char **class_names;
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 48ca1387a409..fd5296dbb40f 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -45,10 +45,11 @@ extern struct ddebug_class_map __start___dyndbg_classes[];
extern struct ddebug_class_map __stop___dyndbg_classes[];
struct ddebug_table {
- struct list_head link, maps;
+ struct list_head link;
const char *mod_name;
- unsigned int num_ddebugs;
struct _ddebug *ddebugs;
+ struct ddebug_class_map *classes;
+ unsigned int num_ddebugs, num_classes;
};
struct ddebug_query {
@@ -146,13 +147,15 @@ static void vpr_info_dq(const struct ddebug_query *query, const char *msg)
query->first_lineno, query->last_lineno, query->class_string);
}
+#define __outvar /* filled by callee */
static struct ddebug_class_map *ddebug_find_valid_class(struct ddebug_table const *dt,
- const char *class_string, int *class_id)
+ const char *class_string,
+ __outvar int *class_id)
{
struct ddebug_class_map *map;
- int idx;
+ int i, idx;
- list_for_each_entry(map, &dt->maps, link) {
+ for (map = dt->classes, i = 0; i < dt->num_classes; i++, map++) {
idx = match_string(map->class_names, map->length, class_string);
if (idx >= 0) {
*class_id = idx + map->base;
@@ -163,7 +166,6 @@ static struct ddebug_class_map *ddebug_find_valid_class(struct ddebug_table cons
return NULL;
}
-#define __outvar /* filled by callee */
/*
* Search the tables for _ddebug's which match the given `query' and
* apply the `flags' and `mask' to them. Returns number of matching
@@ -1109,9 +1111,10 @@ static void *ddebug_proc_next(struct seq_file *m, void *p, loff_t *pos)
static const char *ddebug_class_name(struct ddebug_iter *iter, struct _ddebug *dp)
{
- struct ddebug_class_map *map;
+ struct ddebug_class_map *map = iter->table->classes;
+ int i, nc = iter->table->num_classes;
- list_for_each_entry(map, &iter->table->maps, link)
+ for (i = 0; i < nc; i++, map++)
if (class_in_range(dp->class_id, map))
return map->class_names[dp->class_id - map->base];
@@ -1195,30 +1198,31 @@ static const struct proc_ops proc_fops = {
.proc_write = ddebug_proc_write
};
-static void ddebug_attach_module_classes(struct ddebug_table *dt,
- struct ddebug_class_map *classes,
- int num_classes)
+static void ddebug_attach_module_classes(struct ddebug_table *dt, struct _ddebug_info *di)
{
struct ddebug_class_map *cm;
- int i, j, ct = 0;
+ int i, nc = 0;
- for (cm = classes, i = 0; i < num_classes; i++, cm++) {
+ /*
+ * Find this module's classmaps in a subrange/wholerange of
+ * the builtin/modular classmap vector/section. Save the start
+ * and length of the subrange at its edges.
+ */
+ for (cm = di->classes, i = 0; i < di->num_classes; i++, cm++) {
if (!strcmp(cm->mod_name, dt->mod_name)) {
-
- v2pr_info("class[%d]: module:%s base:%d len:%d ty:%d\n", i,
- cm->mod_name, cm->base, cm->length, cm->map_type);
-
- for (j = 0; j < cm->length; j++)
- v3pr_info(" %d: %d %s\n", j + cm->base, j,
- cm->class_names[j]);
-
- list_add(&cm->link, &dt->maps);
- ct++;
+ if (!nc) {
+ v2pr_info("start subrange, class[%d]: module:%s base:%d len:%d ty:%d\n",
+ i, cm->mod_name, cm->base, cm->length, cm->map_type);
+ dt->classes = cm;
+ }
+ nc++;
}
}
- if (ct)
- vpr_info("module:%s attached %d classes\n", dt->mod_name, ct);
+ if (nc) {
+ dt->num_classes = nc;
+ vpr_info("module:%s attached %d classes\n", dt->mod_name, nc);
+ }
}
/*
@@ -1252,10 +1256,9 @@ static int __ddebug_add_module(struct _ddebug_info *di, unsigned int base,
dt->num_ddebugs = di->num_descs;
INIT_LIST_HEAD(&dt->link);
- INIT_LIST_HEAD(&dt->maps);
if (di->classes && di->num_classes)
- ddebug_attach_module_classes(dt, di->classes, di->num_classes);
+ ddebug_attach_module_classes(dt, di);
mutex_lock(&ddebug_lock);
list_add_tail(&dt->link, &ddebug_tables);
@@ -1344,8 +1347,8 @@ static void ddebug_remove_all_tables(void)
mutex_lock(&ddebug_lock);
while (!list_empty(&ddebug_tables)) {
struct ddebug_table *dt = list_entry(ddebug_tables.next,
- struct ddebug_table,
- link);
+ struct ddebug_table,
+ link);
ddebug_table_free(dt);
}
mutex_unlock(&ddebug_lock);
--
2.38.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* [RFC PATCH 05/17] dyndbg: make ddebug_apply_class_bitmap more selective
2022-12-06 0:34 [RFC PATCH 00/17] DRM_USE_DYNAMIC_DEBUG regression Jim Cromie
` (3 preceding siblings ...)
2022-12-06 0:34 ` [RFC PATCH 04/17] dyndbg: replace classmap list with a vector Jim Cromie
@ 2022-12-06 0:34 ` Jim Cromie
2022-12-06 0:34 ` [RFC PATCH 06/17] dyndbg: dynamic_debug_init - use pointer inequality, not strcmp Jim Cromie
` (12 subsequent siblings)
17 siblings, 0 replies; 22+ messages in thread
From: Jim Cromie @ 2022-12-06 0:34 UTC (permalink / raw)
To: linux-kernel, dri-devel, amd-gfx, intel-gvt-dev, intel-gfx
Cc: jani.nikula, ville.syrjala, daniel.vetter, seanpaul, robdclark,
jbaron, gregkh, Jim Cromie
ddebug_apply_class_bitmap() currently applies the class settings to
all modules, make it more selective, by adding query_module param.
The fn now calls ddebug_exec_queries(query, NULL), where NULL is
query_modname wildcard ("*" does the same). So just expose the
parameter, and alter users to explicitly pass the wildcard value.
This allows its more selective use later; for propagating drm.debug
settings to dependent modules when/just-after they load. Doing this
propagation with "*" is fine, but would match with all previously
loaded modules, creating more dynamic_debug.verbose=3 logging
activity.
No functional change.
Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
after `modprobe i915`, heres the module dependencies,
though not all on drm.debug.
bash-5.2# lsmod
Module Size Used by
i915 3133440 0
drm_buddy 20480 1 i915
ttm 90112 1 i915
i2c_algo_bit 16384 1 i915
video 61440 1 i915
wmi 32768 1 video
drm_display_helper 200704 1 i915
drm_kms_helper 208896 2 drm_display_helper,i915
drm 606208 5 drm_kms_helper,drm_display_helper,drm_buddy,i915,ttm
cec 57344 2 drm_display_helper,i915
---
lib/dynamic_debug.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index fd5296dbb40f..5d609ff0d559 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -600,7 +600,8 @@ static int ddebug_exec_queries(char *query, const char *modname)
/* apply a new bitmap to the sys-knob's current bit-state */
static int ddebug_apply_class_bitmap(const struct ddebug_class_param *dcp,
- unsigned long *new_bits, unsigned long *old_bits)
+ unsigned long *new_bits, unsigned long *old_bits,
+ const char *query_modname)
{
#define QUERY_SIZE 128
char query[QUERY_SIZE];
@@ -617,7 +618,7 @@ static int ddebug_apply_class_bitmap(const struct ddebug_class_param *dcp,
snprintf(query, QUERY_SIZE, "class %s %c%s", map->class_names[bi],
test_bit(bi, new_bits) ? '+' : '-', dcp->flags);
- ct = ddebug_exec_queries(query, NULL);
+ ct = ddebug_exec_queries(query, query_modname);
matches += ct;
v2pr_info("bit_%d: %d matches on class: %s -> 0x%lx\n", bi,
@@ -678,7 +679,7 @@ static int param_set_dyndbg_classnames(const char *instr, const struct kernel_pa
continue;
}
curr_bits ^= BIT(cls_id);
- totct += ddebug_apply_class_bitmap(dcp, &curr_bits, dcp->bits);
+ totct += ddebug_apply_class_bitmap(dcp, &curr_bits, dcp->bits, NULL);
*dcp->bits = curr_bits;
v2pr_info("%s: changed bit %d:%s\n", KP_NAME(kp), cls_id,
map->class_names[cls_id]);
@@ -688,7 +689,7 @@ static int param_set_dyndbg_classnames(const char *instr, const struct kernel_pa
old_bits = CLASSMAP_BITMASK(*dcp->lvl);
curr_bits = CLASSMAP_BITMASK(cls_id + (wanted ? 1 : 0 ));
- totct += ddebug_apply_class_bitmap(dcp, &curr_bits, &old_bits);
+ totct += ddebug_apply_class_bitmap(dcp, &curr_bits, &old_bits, NULL);
*dcp->lvl = (cls_id + (wanted ? 1 : 0));
v2pr_info("%s: changed bit-%d: \"%s\" %lx->%lx\n", KP_NAME(kp), cls_id,
map->class_names[cls_id], old_bits, curr_bits);
@@ -751,7 +752,7 @@ int param_set_dyndbg_classes(const char *instr, const struct kernel_param *kp)
inrep &= CLASSMAP_BITMASK(map->length);
}
v2pr_info("bits:%lx > %s\n", inrep, KP_NAME(kp));
- totct += ddebug_apply_class_bitmap(dcp, &inrep, dcp->bits);
+ totct += ddebug_apply_class_bitmap(dcp, &inrep, dcp->bits, NULL);
*dcp->bits = inrep;
break;
case DD_CLASS_TYPE_LEVEL_NUM:
@@ -764,7 +765,7 @@ int param_set_dyndbg_classes(const char *instr, const struct kernel_param *kp)
old_bits = CLASSMAP_BITMASK(*dcp->lvl);
new_bits = CLASSMAP_BITMASK(inrep);
v2pr_info("lvl:%ld bits:0x%lx > %s\n", inrep, new_bits, KP_NAME(kp));
- totct += ddebug_apply_class_bitmap(dcp, &new_bits, &old_bits);
+ totct += ddebug_apply_class_bitmap(dcp, &new_bits, &old_bits, NULL);
*dcp->lvl = inrep;
break;
default:
--
2.38.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* [RFC PATCH 06/17] dyndbg: dynamic_debug_init - use pointer inequality, not strcmp
2022-12-06 0:34 [RFC PATCH 00/17] DRM_USE_DYNAMIC_DEBUG regression Jim Cromie
` (4 preceding siblings ...)
2022-12-06 0:34 ` [RFC PATCH 05/17] dyndbg: make ddebug_apply_class_bitmap more selective Jim Cromie
@ 2022-12-06 0:34 ` Jim Cromie
2022-12-06 0:34 ` [RFC PATCH 07/17] dyndbg: drop NUM_TYPE_ARRAY Jim Cromie
` (11 subsequent siblings)
17 siblings, 0 replies; 22+ messages in thread
From: Jim Cromie @ 2022-12-06 0:34 UTC (permalink / raw)
To: linux-kernel, dri-devel, amd-gfx, intel-gvt-dev, intel-gfx
Cc: jani.nikula, ville.syrjala, daniel.vetter, seanpaul, robdclark,
jbaron, gregkh, Jim Cromie
dynamic_debug_init() currently uses strcmp to find the module
boundaries in the builtin _ddebug[] table.
The table is filled by the linker; for its content, pointer inequality
works, is faster, and communicates the data properties more tightly.
Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
lib/dynamic_debug.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 5d609ff0d559..a0dc681cd215 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -1410,7 +1410,7 @@ static int __init dynamic_debug_init(void)
for (; iter < __stop___dyndbg; iter++, i++, mod_sites++) {
- if (strcmp(modname, iter->modname)) {
+ if (modname != iter->modname) {
mod_ct++;
di.num_descs = mod_sites;
di.descs = iter_mod_start;
--
2.38.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* [RFC PATCH 07/17] dyndbg: drop NUM_TYPE_ARRAY
2022-12-06 0:34 [RFC PATCH 00/17] DRM_USE_DYNAMIC_DEBUG regression Jim Cromie
` (5 preceding siblings ...)
2022-12-06 0:34 ` [RFC PATCH 06/17] dyndbg: dynamic_debug_init - use pointer inequality, not strcmp Jim Cromie
@ 2022-12-06 0:34 ` Jim Cromie
2022-12-06 0:34 ` [RFC PATCH 08/17] dyndbg: reduce verbose/debug clutter Jim Cromie
` (10 subsequent siblings)
17 siblings, 0 replies; 22+ messages in thread
From: Jim Cromie @ 2022-12-06 0:34 UTC (permalink / raw)
To: linux-kernel, dri-devel, amd-gfx, intel-gvt-dev, intel-gfx
Cc: jani.nikula, ville.syrjala, daniel.vetter, seanpaul, robdclark,
jbaron, gregkh, Jim Cromie
ARRAY_SIZE works here, since array decl is complete.
Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
include/linux/dynamic_debug.h | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index bf47bcfad8e6..81b643ab7f6e 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -104,11 +104,9 @@ struct ddebug_class_map {
.mod_name = KBUILD_MODNAME, \
.base = _base, \
.map_type = _maptype, \
- .length = NUM_TYPE_ARGS(char*, __VA_ARGS__), \
+ .length = ARRAY_SIZE(_var##_classnames), \
.class_names = _var##_classnames, \
}
-#define NUM_TYPE_ARGS(eltype, ...) \
- (sizeof((eltype[]){__VA_ARGS__}) / sizeof(eltype))
/* encapsulate linker provided built-in (or module) dyndbg data */
struct _ddebug_info {
--
2.38.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* [RFC PATCH 08/17] dyndbg: reduce verbose/debug clutter
2022-12-06 0:34 [RFC PATCH 00/17] DRM_USE_DYNAMIC_DEBUG regression Jim Cromie
` (6 preceding siblings ...)
2022-12-06 0:34 ` [RFC PATCH 07/17] dyndbg: drop NUM_TYPE_ARRAY Jim Cromie
@ 2022-12-06 0:34 ` Jim Cromie
2022-12-06 0:34 ` [RFC PATCH 09/17] dyndbg-API: replace DECLARE_DYNDBG_CLASSMAP with DYNDBG_CLASSMAP(_DEFINE|_USE) Jim Cromie
` (9 subsequent siblings)
17 siblings, 0 replies; 22+ messages in thread
From: Jim Cromie @ 2022-12-06 0:34 UTC (permalink / raw)
To: linux-kernel, dri-devel, amd-gfx, intel-gvt-dev, intel-gfx
Cc: jani.nikula, ville.syrjala, daniel.vetter, seanpaul, robdclark,
jbaron, gregkh, Jim Cromie
currently, for verbose=3, this is logged:
[ 3832.333424] dyndbg: parsed: func="" file="" module="amdgpu" format="" lineno=0-0 class=DRM_UT_PRIME
[ 3832.333888] dyndbg: no matches for query
[ 3832.334093] dyndbg: no-match: func="" file="" module="amdgpu" format="" lineno=0-0 class=DRM_UT_PRIME
[ 3832.334570] dyndbg: processed 1 queries, with 0 matches, 0 errs
This patch removes 2nd & 3rd lines;
3 differs from 1 only by status
2 is just status, retold in 4, with more info.
Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
lib/dynamic_debug.c | 10 ++--------
1 file changed, 2 insertions(+), 8 deletions(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index a0dc681cd215..445f25ef2461 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -265,9 +265,6 @@ static int ddebug_change(const struct ddebug_query *query,
}
mutex_unlock(&ddebug_lock);
- if (!nfound && verbose)
- pr_info("no matches for query\n");
-
return nfound;
}
@@ -536,7 +533,7 @@ static int ddebug_exec_query(char *query_string, const char *modname)
struct flag_settings modifiers = {};
struct ddebug_query query = {};
#define MAXWORDS 9
- int nwords, nfound;
+ int nwords;
char *words[MAXWORDS];
nwords = ddebug_tokenize(query_string, words, MAXWORDS);
@@ -554,10 +551,7 @@ static int ddebug_exec_query(char *query_string, const char *modname)
return -EINVAL;
}
/* actually go and implement the change */
- nfound = ddebug_change(&query, &modifiers);
- vpr_info_dq(&query, nfound ? "applied" : "no-match");
-
- return nfound;
+ return ddebug_change(&query, &modifiers);
}
/* handle multiple queries in query string, continue on error, return
--
2.38.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* [RFC PATCH 09/17] dyndbg-API: replace DECLARE_DYNDBG_CLASSMAP with DYNDBG_CLASSMAP(_DEFINE|_USE)
2022-12-06 0:34 [RFC PATCH 00/17] DRM_USE_DYNAMIC_DEBUG regression Jim Cromie
` (7 preceding siblings ...)
2022-12-06 0:34 ` [RFC PATCH 08/17] dyndbg: reduce verbose/debug clutter Jim Cromie
@ 2022-12-06 0:34 ` Jim Cromie
2022-12-06 0:34 ` [RFC PATCH 10/17] dyndbg-API: specialize DYNDBG_CLASSMAP_(DEFINE|USE) Jim Cromie
` (8 subsequent siblings)
17 siblings, 0 replies; 22+ messages in thread
From: Jim Cromie @ 2022-12-06 0:34 UTC (permalink / raw)
To: linux-kernel, dri-devel, amd-gfx, intel-gvt-dev, intel-gfx
Cc: jani.nikula, ville.syrjala, daniel.vetter, seanpaul, robdclark,
jbaron, gregkh, Jim Cromie
DECLARE_DYNDBG_CLASSMAPs job was to allow modules to declare the debug
classes/categories they want dyndbg to >control on their behalf. Its
args give the class-names, their mapping to class_ids, and the sysfs
interface style (usually a class-bitmap). Modules wanting a drm.debug
style knob need to create the kparam, and call module_param_cb() to
wire the sysfs node to the classmap. DRM does this is in drm_print.c
In DRM, multiple modules declare identical DRM_UT_* classmaps, so that
the class'd prdbgs are modified across those modules in a coordinated
way across the subsystem, by either explicit class DRM_UT_* queries to
>control, or by writes to /sys/module/drm/parameters/debug (drm.debug)
This coordination-by-identical-declarations is weird, so this patch
splits the macro into _DEFINE and _USE flavors. This distinction
follows the "define-once, declare-many-uses" principle, so it improves
the api; _DEFINE is used once to specify the classmap, and multiple
users _USE the single definition explicitly.
Currently the latter just reuses the former, and still needs all the
same args, but that can be tuned later; the _DEFINE can initialize an
(extern/global) struct classmap, and _USE can, well use/reference
that struct.
Also wrap DYNDBG_CLASSMAP_USEs with ifdef DRM_USE_DYNAMIC_DEBUG to
balance with the one around drm_print's use of DYNDBG_CLASSMAP_DEFINE.
Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 4 +++-
drivers/gpu/drm/display/drm_dp_helper.c | 4 +++-
drivers/gpu/drm/drm_crtc_helper.c | 4 +++-
drivers/gpu/drm/drm_print.c | 2 +-
drivers/gpu/drm/i915/i915_params.c | 4 +++-
drivers/gpu/drm/nouveau/nouveau_drm.c | 4 +++-
include/linux/dynamic_debug.h | 20 ++++++++++++----
lib/test_dynamic_debug.c | 32 ++++++++++++-------------
8 files changed, 48 insertions(+), 26 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index bf2d50c8c92a..0075184b5d93 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -188,7 +188,8 @@ int amdgpu_vcnfw_log;
static void amdgpu_drv_delayed_reset_work_handler(struct work_struct *work);
-DECLARE_DYNDBG_CLASSMAP(drm_debug_classes, DD_CLASS_TYPE_DISJOINT_BITS, 0,
+#if defined(CONFIG_DRM_USE_DYNAMIC_DEBUG)
+DYNDBG_CLASSMAP_USE(drm_debug_classes, DD_CLASS_TYPE_DISJOINT_BITS, 0,
"DRM_UT_CORE",
"DRM_UT_DRIVER",
"DRM_UT_KMS",
@@ -199,6 +200,7 @@ DECLARE_DYNDBG_CLASSMAP(drm_debug_classes, DD_CLASS_TYPE_DISJOINT_BITS, 0,
"DRM_UT_LEASE",
"DRM_UT_DP",
"DRM_UT_DRMRES");
+#endif
struct amdgpu_mgpu_info mgpu_info = {
.mutex = __MUTEX_INITIALIZER(mgpu_info.mutex),
diff --git a/drivers/gpu/drm/display/drm_dp_helper.c b/drivers/gpu/drm/display/drm_dp_helper.c
index 16565a0a5da6..8fa7a88299e7 100644
--- a/drivers/gpu/drm/display/drm_dp_helper.c
+++ b/drivers/gpu/drm/display/drm_dp_helper.c
@@ -41,7 +41,8 @@
#include "drm_dp_helper_internal.h"
-DECLARE_DYNDBG_CLASSMAP(drm_debug_classes, DD_CLASS_TYPE_DISJOINT_BITS, 0,
+#if defined(CONFIG_DRM_USE_DYNAMIC_DEBUG)
+DYNDBG_CLASSMAP_USE(drm_debug_classes, DD_CLASS_TYPE_DISJOINT_BITS, 0,
"DRM_UT_CORE",
"DRM_UT_DRIVER",
"DRM_UT_KMS",
@@ -52,6 +53,7 @@ DECLARE_DYNDBG_CLASSMAP(drm_debug_classes, DD_CLASS_TYPE_DISJOINT_BITS, 0,
"DRM_UT_LEASE",
"DRM_UT_DP",
"DRM_UT_DRMRES");
+#endif
struct dp_aux_backlight {
struct backlight_device *base;
diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c
index 7d86020b5244..2f747c9c8f60 100644
--- a/drivers/gpu/drm/drm_crtc_helper.c
+++ b/drivers/gpu/drm/drm_crtc_helper.c
@@ -51,7 +51,8 @@
#include "drm_crtc_helper_internal.h"
-DECLARE_DYNDBG_CLASSMAP(drm_debug_classes, DD_CLASS_TYPE_DISJOINT_BITS, 0,
+#if defined(CONFIG_DRM_USE_DYNAMIC_DEBUG)
+DYNDBG_CLASSMAP_USE(drm_debug_classes, DD_CLASS_TYPE_DISJOINT_BITS, 0,
"DRM_UT_CORE",
"DRM_UT_DRIVER",
"DRM_UT_KMS",
@@ -62,6 +63,7 @@ DECLARE_DYNDBG_CLASSMAP(drm_debug_classes, DD_CLASS_TYPE_DISJOINT_BITS, 0,
"DRM_UT_LEASE",
"DRM_UT_DP",
"DRM_UT_DRMRES");
+#endif
/**
* DOC: overview
diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c
index 5b93c11895bb..4b697e18238d 100644
--- a/drivers/gpu/drm/drm_print.c
+++ b/drivers/gpu/drm/drm_print.c
@@ -56,7 +56,7 @@ MODULE_PARM_DESC(debug, "Enable debug output, where each bit enables a debug cat
module_param_named(debug, __drm_debug, ulong, 0600);
#else
/* classnames must match vals of enum drm_debug_category */
-DECLARE_DYNDBG_CLASSMAP(drm_debug_classes, DD_CLASS_TYPE_DISJOINT_BITS, 0,
+DYNDBG_CLASSMAP_DEFINE(drm_debug_classes, DD_CLASS_TYPE_DISJOINT_BITS, 0,
"DRM_UT_CORE",
"DRM_UT_DRIVER",
"DRM_UT_KMS",
diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
index d1e4d528cb17..b5b2542ae364 100644
--- a/drivers/gpu/drm/i915/i915_params.c
+++ b/drivers/gpu/drm/i915/i915_params.c
@@ -29,7 +29,8 @@
#include "i915_params.h"
#include "i915_drv.h"
-DECLARE_DYNDBG_CLASSMAP(drm_debug_classes, DD_CLASS_TYPE_DISJOINT_BITS, 0,
+#if defined(CONFIG_DRM_USE_DYNAMIC_DEBUG)
+DYNDBG_CLASSMAP_USE(drm_debug_classes, DD_CLASS_TYPE_DISJOINT_BITS, 0,
"DRM_UT_CORE",
"DRM_UT_DRIVER",
"DRM_UT_KMS",
@@ -40,6 +41,7 @@ DECLARE_DYNDBG_CLASSMAP(drm_debug_classes, DD_CLASS_TYPE_DISJOINT_BITS, 0,
"DRM_UT_LEASE",
"DRM_UT_DP",
"DRM_UT_DRMRES");
+#endif
#define i915_param_named(name, T, perm, desc) \
module_param_named(name, i915_modparams.name, T, perm); \
diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c b/drivers/gpu/drm/nouveau/nouveau_drm.c
index fd99ec0f4257..2963cf5b0807 100644
--- a/drivers/gpu/drm/nouveau/nouveau_drm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
@@ -71,7 +71,8 @@
#include "nouveau_svm.h"
#include "nouveau_dmem.h"
-DECLARE_DYNDBG_CLASSMAP(drm_debug_classes, DD_CLASS_TYPE_DISJOINT_BITS, 0,
+#if defined(CONFIG_DRM_USE_DYNAMIC_DEBUG)
+DYNDBG_CLASSMAP_USE(drm_debug_classes, DD_CLASS_TYPE_DISJOINT_BITS, 0,
"DRM_UT_CORE",
"DRM_UT_DRIVER",
"DRM_UT_KMS",
@@ -82,6 +83,7 @@ DECLARE_DYNDBG_CLASSMAP(drm_debug_classes, DD_CLASS_TYPE_DISJOINT_BITS, 0,
"DRM_UT_LEASE",
"DRM_UT_DP",
"DRM_UT_DRMRES");
+#endif
MODULE_PARM_DESC(config, "option string to pass to driver core");
static char *nouveau_config;
diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index 81b643ab7f6e..1cdfd62fd2e4 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -56,7 +56,7 @@ struct _ddebug {
#endif
} __attribute__((aligned(8)));
-enum class_map_type {
+enum ddebug_class_map_type {
DD_CLASS_TYPE_DISJOINT_BITS,
/**
* DD_CLASS_TYPE_DISJOINT_BITS: classes are independent, one per bit.
@@ -86,17 +86,19 @@ struct ddebug_class_map {
const char **class_names;
const int length;
const int base; /* index of 1st .class_id, allows split/shared space */
- enum class_map_type map_type;
+ enum ddebug_class_map_type map_type;
};
/**
- * DECLARE_DYNDBG_CLASSMAP - declare classnames known by a module
+ * DYNDBG_CLASSMAP_DEFINE - define the class_map that names the
+ * debug classes used in this module. This tells dyndbg the authorized
+ * classnames it should manipulate.
* @_var: a struct ddebug_class_map, passed to module_param_cb
* @_type: enum class_map_type, chooses bits/verbose, numeric/symbolic
* @_base: offset of 1st class-name. splits .class_id space
* @classes: class-names used to control class'd prdbgs
*/
-#define DECLARE_DYNDBG_CLASSMAP(_var, _maptype, _base, ...) \
+#define DYNDBG_CLASSMAP_DEFINE(_var, _maptype, _base, ...) \
static const char *_var##_classnames[] = { __VA_ARGS__ }; \
static struct ddebug_class_map __aligned(8) __used \
__section("__dyndbg_classes") _var = { \
@@ -108,6 +110,16 @@ struct ddebug_class_map {
.class_names = _var##_classnames, \
}
+/*
+ * refer to the classmap instantiated once, by the macro above. This
+ * distinguishes the multiple users of drm.debug from the single
+ * definition, allowing them to specialize. ATM its a pass-thru, but
+ * it should help regularize the admittedly wierd sharing by identical
+ * definitions.
+ */
+#define DYNDBG_CLASSMAP_USE(_var, _maptype, _base, ...) \
+ DYNDBG_CLASSMAP_DEFINE(_var, _maptype, _base, __VA_ARGS__)
+
/* encapsulate linker provided built-in (or module) dyndbg data */
struct _ddebug_info {
struct _ddebug *descs;
diff --git a/lib/test_dynamic_debug.c b/lib/test_dynamic_debug.c
index 9d48689dc0ab..4ae01f7fa920 100644
--- a/lib/test_dynamic_debug.c
+++ b/lib/test_dynamic_debug.c
@@ -62,38 +62,38 @@ enum cat_disjoint_bits {
D2_LEASE,
D2_DP,
D2_DRMRES };
-DECLARE_DYNDBG_CLASSMAP(map_disjoint_bits, DD_CLASS_TYPE_DISJOINT_BITS, 0,
- "D2_CORE",
- "D2_DRIVER",
- "D2_KMS",
- "D2_PRIME",
- "D2_ATOMIC",
- "D2_VBL",
- "D2_STATE",
- "D2_LEASE",
- "D2_DP",
- "D2_DRMRES");
+DYNDBG_CLASSMAP_DEFINE(map_disjoint_bits, DD_CLASS_TYPE_DISJOINT_BITS, 0,
+ "D2_CORE",
+ "D2_DRIVER",
+ "D2_KMS",
+ "D2_PRIME",
+ "D2_ATOMIC",
+ "D2_VBL",
+ "D2_STATE",
+ "D2_LEASE",
+ "D2_DP",
+ "D2_DRMRES");
DD_SYS_WRAP(disjoint_bits, p);
DD_SYS_WRAP(disjoint_bits, T);
/* symbolic input, independent bits */
enum cat_disjoint_names { LOW = 10, MID, HI };
-DECLARE_DYNDBG_CLASSMAP(map_disjoint_names, DD_CLASS_TYPE_DISJOINT_NAMES, 10,
- "LOW", "MID", "HI");
+DYNDBG_CLASSMAP_DEFINE(map_disjoint_names, DD_CLASS_TYPE_DISJOINT_NAMES, 10,
+ "LOW", "MID", "HI");
DD_SYS_WRAP(disjoint_names, p);
DD_SYS_WRAP(disjoint_names, T);
/* numeric verbosity, V2 > V1 related */
enum cat_level_num { V0 = 14, V1, V2, V3, V4, V5, V6, V7 };
-DECLARE_DYNDBG_CLASSMAP(map_level_num, DD_CLASS_TYPE_LEVEL_NUM, 14,
+DYNDBG_CLASSMAP_DEFINE(map_level_num, DD_CLASS_TYPE_LEVEL_NUM, 14,
"V0", "V1", "V2", "V3", "V4", "V5", "V6", "V7");
DD_SYS_WRAP(level_num, p);
DD_SYS_WRAP(level_num, T);
/* symbolic verbosity */
enum cat_level_names { L0 = 22, L1, L2, L3, L4, L5, L6, L7 };
-DECLARE_DYNDBG_CLASSMAP(map_level_names, DD_CLASS_TYPE_LEVEL_NAMES, 22,
- "L0", "L1", "L2", "L3", "L4", "L5", "L6", "L7");
+DYNDBG_CLASSMAP_DEFINE(map_level_names, DD_CLASS_TYPE_LEVEL_NAMES, 22,
+ "L0", "L1", "L2", "L3", "L4", "L5", "L6", "L7");
DD_SYS_WRAP(level_names, p);
DD_SYS_WRAP(level_names, T);
--
2.38.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* [RFC PATCH 10/17] dyndbg-API: specialize DYNDBG_CLASSMAP_(DEFINE|USE)
2022-12-06 0:34 [RFC PATCH 00/17] DRM_USE_DYNAMIC_DEBUG regression Jim Cromie
` (8 preceding siblings ...)
2022-12-06 0:34 ` [RFC PATCH 09/17] dyndbg-API: replace DECLARE_DYNDBG_CLASSMAP with DYNDBG_CLASSMAP(_DEFINE|_USE) Jim Cromie
@ 2022-12-06 0:34 ` Jim Cromie
2022-12-06 0:34 ` [RFC PATCH 11/17] dyndbg-API: DYNDBG_CLASSMAP_USE drop extra args Jim Cromie
` (7 subsequent siblings)
17 siblings, 0 replies; 22+ messages in thread
From: Jim Cromie @ 2022-12-06 0:34 UTC (permalink / raw)
To: linux-kernel, dri-devel, amd-gfx, intel-gvt-dev, intel-gfx
Cc: jani.nikula, ville.syrjala, daniel.vetter, seanpaul, robdclark,
jbaron, gregkh, Jim Cromie
Now that the DECLARE_DYNDBG_CLASSMAP macro has been split into
DYNDBG_CLASSMAP_DEFINE and DYNDBG_CLASSMAP_USE variants, lets
differentiate them according to their separate jobs.
Dyndbg's existing __dyndbg_classes[] section does:
. catalogs the classmaps defined by the module (or builtin modules)
. authorizes dyndbg to >control those class'd prdbgs for the module.
This patch adds __dyndbg_class_refs[] section:
. catalogs references/uses of the above classmap definitions.
. authorizes dyndbg to >control those class'd prdbgs in ref'g module.
. maps the client module to classmap definitions
this allows dyndbg to propagate drm.debug to the client module.
The distinction of the 2 roles yields 2 gains:
It follows the define-once-declare-elsewhere pattern that K&R gave us,
dumping the weird coordinated-changes-by-identical-classmaps API.
It should help solve the chicken-egg problem that drm.debug-on-dyndbg
has; the _USEr module must propagate the drm.debug setting once the
using module has loaded.
The new DYNDBG_CLASSMAP macros add records to the sections:
DYNDBG_CLASSMAP_DEFINE:
invoked by drm_print, just once per sub-system.
defines the classmap, names "DRM_UT_*", maps to class_id's
authorizes dyndbg to exert >control
populates __dyndbg_classes[] "section", __used.
exports the classmap.
DYNDBG_CLASSMAP_USE:
invoked by modules using classmaps defined & exported elsewhere
populates __dyndbg_class_refs[] "section", __used.
maps client-module name to the extern'd classmap.
no client-name yet, but will need it.
also:
struct ddebug_info gets 2 new fields to encapsulate the new section:
class_refs, num_class_refs.
set by dynamic_debug_init() for builtins.
or by kernel/module/main:load_info() for loadable modules.
. struct ddebug_class_user
contains: user-module-name, ref to classmap-defn
dyndbg finds drm-driver's use of a classmap, gets/applies its settings
. struct ddebug_class_map gets .knob ptr to ddebug_class_param.
compiled null, init'd under ddebug_add_module()
allows finding drm.debug setting.
. vmlinux.lds.h additions: linker symbols, KEEP for new section
dynamic_debug.c:
ddebug_attach_module_classes():
as before
foreach __dyndbg_classes: ddebug_find_kparam(classmap*)
ddebug_attach_client_module_classes():
foreach __dyndbg_class_refs: ddebug_param_load_dependent_class(classmap*)
called after list-add to ddebug-tables.
ddebug_load_dependent_class():
This applies >controls to the module, it needs the module to be
present in the ddebug-tables list so that ddebug_change can apply the
changes to it. So in ddebug_add_module, call 2nd fn *after* adding
the ddebug_table to the list.
ddebug_find_kparam(classmap*):
Finds the kernel-param / sysfs-node that controls the classmap, by
verifying that dyndbg's kparam-ops are used by the kparam. The found
kparams arg is our ddebug_class_param, and has a ref to the state-var
under the sysfs-node.
ddebug_match_attach_kparam():
tests that kparam.ops == dyndbg's, then casts arg to
ddebug_class_param, and tests modnames for equality.
ddebug_param_load_dependent_class(struct ddebug_class_user*):
Called on class_refs entries, these classmaps have already been seen
by ddebug_find_kparam() which has wired the classmap.knob to the kparam.
So this fn just calls ddebug_apply_class_bitmap() on the state-var.
ddebug_find_valid_class():
This helps ddebug_change(), doing the search over classmaps, looking
for the class given to >control. So now it searches over
__dyndbg_class_refs[] after __dyndbg_classes[].
Thats the theory anyway. things are still broken (differently) for
both builtins and loadables. For loadables, the >control is applied,
but doesnt alter any callsites.
Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
include/asm-generic/vmlinux.lds.h | 3 +
include/linux/dynamic_debug.h | 41 ++++++---
kernel/module/main.c | 2 +
lib/dynamic_debug.c | 137 +++++++++++++++++++++++++++---
4 files changed, 157 insertions(+), 26 deletions(-)
diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index 3dc5824141cd..7100701fb68c 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -363,6 +363,9 @@
__start___dyndbg_classes = .; \
KEEP(*(__dyndbg_classes)) \
__stop___dyndbg_classes = .; \
+ __start___dyndbg_class_refs = .; \
+ KEEP(*(__dyndbg_class_refs)) \
+ __stop___dyndbg_class_refs = .; \
__start___dyndbg = .; \
KEEP(*(__dyndbg)) \
__stop___dyndbg = .; \
diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index 1cdfd62fd2e4..dabbe1a9180c 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -81,8 +81,10 @@ enum ddebug_class_map_type {
};
struct ddebug_class_map {
- struct module *mod;
- const char *mod_name; /* needed for builtins */
+ struct list_head link;
+ struct module *mod; /* NULL for builtins */
+ const char *mod_name;
+ struct ddebug_class_param *dc_parm; /* controlling sysfs node */
const char **class_names;
const int length;
const int base; /* index of 1st .class_id, allows split/shared space */
@@ -99,8 +101,8 @@ struct ddebug_class_map {
* @classes: class-names used to control class'd prdbgs
*/
#define DYNDBG_CLASSMAP_DEFINE(_var, _maptype, _base, ...) \
- static const char *_var##_classnames[] = { __VA_ARGS__ }; \
- static struct ddebug_class_map __aligned(8) __used \
+ const char *_var##_classnames[] = { __VA_ARGS__ }; \
+ struct ddebug_class_map __aligned(8) __used \
__section("__dyndbg_classes") _var = { \
.mod = THIS_MODULE, \
.mod_name = KBUILD_MODNAME, \
@@ -108,24 +110,37 @@ struct ddebug_class_map {
.map_type = _maptype, \
.length = ARRAY_SIZE(_var##_classnames), \
.class_names = _var##_classnames, \
- }
+ }; \
+ EXPORT_SYMBOL(_var)
-/*
- * refer to the classmap instantiated once, by the macro above. This
- * distinguishes the multiple users of drm.debug from the single
- * definition, allowing them to specialize. ATM its a pass-thru, but
- * it should help regularize the admittedly wierd sharing by identical
- * definitions.
+struct ddebug_class_user {
+ char *user_mod_name;
+ struct ddebug_class_map *map;
+};
+/**
+ * DYNDBG_CLASSMAP_USE - Use a classmap DEFINEd in another module.
+ * This lets dyndbg initialize the dependent module's prdbgs from the
+ * other module's controlling sysfs node.
*/
-#define DYNDBG_CLASSMAP_USE(_var, _maptype, _base, ...) \
- DYNDBG_CLASSMAP_DEFINE(_var, _maptype, _base, __VA_ARGS__)
+#define DYNDBG_CLASSMAP_USE(_var, ...) \
+ DYNDBG_CLASSMAP_USE_(_var, __UNIQUE_ID(ddebug_class_user), \
+ __VA_ARGS__)
+#define DYNDBG_CLASSMAP_USE_(_var, _uname, ...) \
+ extern struct ddebug_class_map _var[]; \
+ static struct ddebug_class_user __used \
+ __section("__dyndbg_class_refs") _uname = { \
+ .user_mod_name = KBUILD_MODNAME, \
+ .map = _var, \
+ }
/* encapsulate linker provided built-in (or module) dyndbg data */
struct _ddebug_info {
struct _ddebug *descs;
struct ddebug_class_map *classes;
+ struct ddebug_class_user *class_refs;
unsigned int num_descs;
unsigned int num_classes;
+ unsigned int num_class_refs;
};
struct ddebug_class_param {
diff --git a/kernel/module/main.c b/kernel/module/main.c
index d02d39c7174e..ee4f85a3b8f0 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -2111,6 +2111,8 @@ static int find_module_sections(struct module *mod, struct load_info *info)
sizeof(*info->dyndbg.descs), &info->dyndbg.num_descs);
info->dyndbg.classes = section_objs(info, "__dyndbg_classes",
sizeof(*info->dyndbg.classes), &info->dyndbg.num_classes);
+ info->dyndbg.class_refs = section_objs(info, "__dyndbg_class_refs",
+ sizeof(*info->dyndbg.class_refs), &info->dyndbg.num_class_refs);
return 0;
}
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 445f25ef2461..45b8414fa130 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -43,13 +43,16 @@ extern struct _ddebug __start___dyndbg[];
extern struct _ddebug __stop___dyndbg[];
extern struct ddebug_class_map __start___dyndbg_classes[];
extern struct ddebug_class_map __stop___dyndbg_classes[];
+extern struct ddebug_class_user __start___dyndbg_class_refs[];
+extern struct ddebug_class_user __stop___dyndbg_class_refs[];
struct ddebug_table {
struct list_head link;
const char *mod_name;
struct _ddebug *ddebugs;
struct ddebug_class_map *classes;
- unsigned int num_ddebugs, num_classes;
+ struct ddebug_class_user *class_refs;
+ unsigned int num_ddebugs, num_classes, num_class_refs;
};
struct ddebug_query {
@@ -153,15 +156,23 @@ static struct ddebug_class_map *ddebug_find_valid_class(struct ddebug_table cons
__outvar int *class_id)
{
struct ddebug_class_map *map;
+ struct ddebug_class_user *cli;
int i, idx;
- for (map = dt->classes, i = 0; i < dt->num_classes; i++, map++) {
+ for (i = 0, map = dt->classes; i < dt->num_classes; i++, map++) {
idx = match_string(map->class_names, map->length, class_string);
if (idx >= 0) {
*class_id = idx + map->base;
return map;
}
}
+ for (i = 0, cli = dt->class_refs; i < dt->num_class_refs; i++, cli++) {
+ idx = match_string(cli->map->class_names, cli->map->length, class_string);
+ if (idx >= 0) {
+ *class_id = idx + map->base;
+ return map;
+ }
+ }
*class_id = -ENOENT;
return NULL;
}
@@ -603,7 +614,7 @@ static int ddebug_apply_class_bitmap(const struct ddebug_class_param *dcp,
int matches = 0;
int bi, ct;
- v2pr_info("apply: 0x%lx to: 0x%lx\n", *new_bits, *old_bits);
+ v2pr_info("apply bitmap: 0x%lx to: 0x%lx\n", *new_bits, *old_bits);
for (bi = 0; bi < map->length; bi++) {
if (test_bit(bi, new_bits) == test_bit(bi, old_bits))
@@ -1106,13 +1117,19 @@ static void *ddebug_proc_next(struct seq_file *m, void *p, loff_t *pos)
static const char *ddebug_class_name(struct ddebug_iter *iter, struct _ddebug *dp)
{
- struct ddebug_class_map *map = iter->table->classes;
- int i, nc = iter->table->num_classes;
+ struct ddebug_table *dt = iter->table;
+ struct ddebug_class_map *map;
+ struct ddebug_class_user *cli;
+ int i;
- for (i = 0; i < nc; i++, map++)
+ for (i = 0, map = dt->classes; i < dt->num_classes; i++, map++)
if (class_in_range(dp->class_id, map))
return map->class_names[dp->class_id - map->base];
+ for (i = 0, cli = dt->class_refs; i < dt->num_class_refs; i++, cli++)
+ if (class_in_range(dp->class_id, cli->map))
+ return cli->map->class_names[dp->class_id - map->base];
+
return NULL;
}
@@ -1193,6 +1210,54 @@ static const struct proc_ops proc_fops = {
.proc_write = ddebug_proc_write
};
+static void ddebug_match_attach_kparam(const struct kernel_param *kp,
+ struct ddebug_class_map *cm)
+{
+ struct ddebug_class_param *dcp;
+
+ if (kp->ops != ¶m_ops_dyndbg_classes)
+ return;
+ dcp = (struct ddebug_class_param *)kp->arg;
+
+ if (!strncmp(cm->mod_name, dcp->map->mod_name, strlen(cm->mod_name))) {
+ cm->dc_parm = dcp;
+ v2pr_info("controlling kp: %s.%s\n", cm->mod_name, kp->name);
+ } else
+ v2pr_info("not this: %s %s\n", cm->mod_name, kp->name);
+}
+
+static void ddebug_find_kparam(struct ddebug_class_map *cm)
+{
+ const struct kernel_param *kp;
+ int i;
+
+ if (cm->mod) {
+ v2pr_info("loaded class: module:%s base:%d len:%d ty:%d\n",
+ cm->mod_name, cm->base, cm->length, cm->map_type);
+
+ for (i = 0, kp = cm->mod->kp; i < cm->mod->num_kp; i++, kp++)
+ ddebug_match_attach_kparam(kp, cm);
+ } else {
+ v2pr_info("builtin class: module:%s base:%d len:%d ty:%d\n",
+ cm->mod_name, cm->base, cm->length, cm->map_type);
+
+ for (kp = __start___param; kp < __stop___param; kp++)
+ ddebug_match_attach_kparam(kp, cm);
+ }
+}
+
+static void ddebug_param_load_dependent_class(const struct ddebug_class_user *cli)
+{
+ unsigned long new_bits, old_bits = 0;
+
+ new_bits = *cli->map->dc_parm->bits;
+
+ vpr_info("%s needs %s, 0x%lx\n", cli->user_mod_name,
+ cli->map->mod_name, new_bits);
+
+ ddebug_apply_class_bitmap(cli->map->dc_parm, &new_bits, &old_bits, cli->user_mod_name);
+}
+
static void ddebug_attach_module_classes(struct ddebug_table *dt, struct _ddebug_info *di)
{
struct ddebug_class_map *cm;
@@ -1203,20 +1268,60 @@ static void ddebug_attach_module_classes(struct ddebug_table *dt, struct _ddebug
* the builtin/modular classmap vector/section. Save the start
* and length of the subrange at its edges.
*/
- for (cm = di->classes, i = 0; i < di->num_classes; i++, cm++) {
+ for (i = 0, cm = di->classes; i < di->num_classes; i++, cm++) {
if (!strcmp(cm->mod_name, dt->mod_name)) {
if (!nc) {
- v2pr_info("start subrange, class[%d]: module:%s base:%d len:%d ty:%d\n",
- i, cm->mod_name, cm->base, cm->length, cm->map_type);
dt->classes = cm;
+ v2pr_info("classes start: class[%d]: module:%s base:%d len:%d ty:%d\n",
+ i, cm->mod_name, cm->base, cm->length, cm->map_type);
}
+ ddebug_find_kparam(cm);
nc++;
}
}
- if (nc) {
- dt->num_classes = nc;
+ dt->num_classes = nc;
+ if (nc)
vpr_info("module:%s attached %d classes\n", dt->mod_name, nc);
+}
+
+static void ddebug_attach_client_module_classes(struct ddebug_table *dt, struct _ddebug_info *di)
+{
+ struct ddebug_class_user *cli;
+ int i;
+
+ for (i = 0, cli = di->class_refs; i < di->num_class_refs; i++, cli++) {
+
+ if (!cli) {
+ v2pr_info("NO CLI\n");
+ continue;
+ }
+ if (!cli->map) {
+ v2pr_info("NO CLI map\n");
+ continue;
+ }
+ if (!cli->user_mod_name) {
+ v2pr_info("NO CLI name\n");
+ continue;
+ }
+
+ if (!strcmp(cli->user_mod_name, dt->mod_name)) {
+
+ v2pr_info("class_ref[%d] %s -> %s\n", i,
+ cli->user_mod_name, cli->map->mod_name);
+
+ ddebug_param_load_dependent_class(cli);
+
+ dt->class_refs = cli;
+ /*
+ * allow only 1 ref right now, see if that
+ * works. dont want to deal with vector,len
+ * now, maybe inadequate anyway.
+ */
+ v2pr_info("break on %d/%d\n", i, di->num_class_refs);
+ dt->num_class_refs = 1;
+ break;
+ }
}
}
@@ -1229,7 +1334,8 @@ static int __ddebug_add_module(struct _ddebug_info *di, unsigned int base,
{
struct ddebug_table *dt;
- v3pr_info("add-module: %s.%d sites\n", modname, di->num_descs);
+ v3pr_info("add-module: %s %d sites %d.%d\n", modname, di->num_descs,
+ di->num_classes, di->num_class_refs);
if (!di->num_descs) {
v3pr_info(" skip %s\n", modname);
return 0;
@@ -1252,13 +1358,16 @@ static int __ddebug_add_module(struct _ddebug_info *di, unsigned int base,
INIT_LIST_HEAD(&dt->link);
- if (di->classes && di->num_classes)
+ if (di->num_classes)
ddebug_attach_module_classes(dt, di);
mutex_lock(&ddebug_lock);
list_add_tail(&dt->link, &ddebug_tables);
mutex_unlock(&ddebug_lock);
+ if (di->num_class_refs)
+ ddebug_attach_client_module_classes(dt, di);
+
vpr_info("%3u debug prints in module %s\n", di->num_descs, modname);
return 0;
}
@@ -1384,8 +1493,10 @@ static int __init dynamic_debug_init(void)
struct _ddebug_info di = {
.descs = __start___dyndbg,
.classes = __start___dyndbg_classes,
+ .class_refs = __start___dyndbg_class_refs,
.num_descs = __stop___dyndbg - __start___dyndbg,
.num_classes = __stop___dyndbg_classes - __start___dyndbg_classes,
+ .num_class_refs = __stop___dyndbg_class_refs - __start___dyndbg_class_refs,
};
if (&__start___dyndbg == &__stop___dyndbg) {
--
2.38.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* [RFC PATCH 11/17] dyndbg-API: DYNDBG_CLASSMAP_USE drop extra args
2022-12-06 0:34 [RFC PATCH 00/17] DRM_USE_DYNAMIC_DEBUG regression Jim Cromie
` (9 preceding siblings ...)
2022-12-06 0:34 ` [RFC PATCH 10/17] dyndbg-API: specialize DYNDBG_CLASSMAP_(DEFINE|USE) Jim Cromie
@ 2022-12-06 0:34 ` Jim Cromie
2022-12-06 0:34 ` [RFC PATCH 12/17] dyndbg-API: DYNDBG_CLASSMAP_DEFINE() improvements Jim Cromie
` (6 subsequent siblings)
17 siblings, 0 replies; 22+ messages in thread
From: Jim Cromie @ 2022-12-06 0:34 UTC (permalink / raw)
To: linux-kernel, dri-devel, amd-gfx, intel-gvt-dev, intel-gfx
Cc: jani.nikula, ville.syrjala, daniel.vetter, seanpaul, robdclark,
jbaron, gregkh, Jim Cromie
Drop macro args after _var. Since DYNDBG_CLASSMAP_USE no longer
forwards to DYNDBG_CLASSMAP_DEFINE, it doesn't need those args to
forward. Keep only the _var arg, which is the extern'd struct
classmap with all the class info.
Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 12 +---------
drivers/gpu/drm/display/drm_dp_helper.c | 12 +---------
drivers/gpu/drm/drm_crtc_helper.c | 12 +---------
drivers/gpu/drm/i915/i915_params.c | 12 +---------
drivers/gpu/drm/nouveau/nouveau_drm.c | 12 +---------
include/linux/dynamic_debug.h | 30 ++++++++++++++-----------
6 files changed, 22 insertions(+), 68 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 0075184b5d93..7bcc22ef5d49 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -189,17 +189,7 @@ int amdgpu_vcnfw_log;
static void amdgpu_drv_delayed_reset_work_handler(struct work_struct *work);
#if defined(CONFIG_DRM_USE_DYNAMIC_DEBUG)
-DYNDBG_CLASSMAP_USE(drm_debug_classes, DD_CLASS_TYPE_DISJOINT_BITS, 0,
- "DRM_UT_CORE",
- "DRM_UT_DRIVER",
- "DRM_UT_KMS",
- "DRM_UT_PRIME",
- "DRM_UT_ATOMIC",
- "DRM_UT_VBL",
- "DRM_UT_STATE",
- "DRM_UT_LEASE",
- "DRM_UT_DP",
- "DRM_UT_DRMRES");
+DYNDBG_CLASSMAP_USE(drm_debug_classes);
#endif
struct amdgpu_mgpu_info mgpu_info = {
diff --git a/drivers/gpu/drm/display/drm_dp_helper.c b/drivers/gpu/drm/display/drm_dp_helper.c
index 8fa7a88299e7..3bc188cb1116 100644
--- a/drivers/gpu/drm/display/drm_dp_helper.c
+++ b/drivers/gpu/drm/display/drm_dp_helper.c
@@ -42,17 +42,7 @@
#include "drm_dp_helper_internal.h"
#if defined(CONFIG_DRM_USE_DYNAMIC_DEBUG)
-DYNDBG_CLASSMAP_USE(drm_debug_classes, DD_CLASS_TYPE_DISJOINT_BITS, 0,
- "DRM_UT_CORE",
- "DRM_UT_DRIVER",
- "DRM_UT_KMS",
- "DRM_UT_PRIME",
- "DRM_UT_ATOMIC",
- "DRM_UT_VBL",
- "DRM_UT_STATE",
- "DRM_UT_LEASE",
- "DRM_UT_DP",
- "DRM_UT_DRMRES");
+DYNDBG_CLASSMAP_USE(drm_debug_classes);
#endif
struct dp_aux_backlight {
diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c
index 2f747c9c8f60..5fb83336b015 100644
--- a/drivers/gpu/drm/drm_crtc_helper.c
+++ b/drivers/gpu/drm/drm_crtc_helper.c
@@ -52,17 +52,7 @@
#include "drm_crtc_helper_internal.h"
#if defined(CONFIG_DRM_USE_DYNAMIC_DEBUG)
-DYNDBG_CLASSMAP_USE(drm_debug_classes, DD_CLASS_TYPE_DISJOINT_BITS, 0,
- "DRM_UT_CORE",
- "DRM_UT_DRIVER",
- "DRM_UT_KMS",
- "DRM_UT_PRIME",
- "DRM_UT_ATOMIC",
- "DRM_UT_VBL",
- "DRM_UT_STATE",
- "DRM_UT_LEASE",
- "DRM_UT_DP",
- "DRM_UT_DRMRES");
+DYNDBG_CLASSMAP_USE(drm_debug_classes);
#endif
/**
diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
index b5b2542ae364..e959d0384ead 100644
--- a/drivers/gpu/drm/i915/i915_params.c
+++ b/drivers/gpu/drm/i915/i915_params.c
@@ -30,17 +30,7 @@
#include "i915_drv.h"
#if defined(CONFIG_DRM_USE_DYNAMIC_DEBUG)
-DYNDBG_CLASSMAP_USE(drm_debug_classes, DD_CLASS_TYPE_DISJOINT_BITS, 0,
- "DRM_UT_CORE",
- "DRM_UT_DRIVER",
- "DRM_UT_KMS",
- "DRM_UT_PRIME",
- "DRM_UT_ATOMIC",
- "DRM_UT_VBL",
- "DRM_UT_STATE",
- "DRM_UT_LEASE",
- "DRM_UT_DP",
- "DRM_UT_DRMRES");
+DYNDBG_CLASSMAP_USE(drm_debug_classes);
#endif
#define i915_param_named(name, T, perm, desc) \
diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c b/drivers/gpu/drm/nouveau/nouveau_drm.c
index 2963cf5b0807..609edeb2a117 100644
--- a/drivers/gpu/drm/nouveau/nouveau_drm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
@@ -72,17 +72,7 @@
#include "nouveau_dmem.h"
#if defined(CONFIG_DRM_USE_DYNAMIC_DEBUG)
-DYNDBG_CLASSMAP_USE(drm_debug_classes, DD_CLASS_TYPE_DISJOINT_BITS, 0,
- "DRM_UT_CORE",
- "DRM_UT_DRIVER",
- "DRM_UT_KMS",
- "DRM_UT_PRIME",
- "DRM_UT_ATOMIC",
- "DRM_UT_VBL",
- "DRM_UT_STATE",
- "DRM_UT_LEASE",
- "DRM_UT_DP",
- "DRM_UT_DRMRES");
+DYNDBG_CLASSMAP_USE(drm_debug_classes);
#endif
MODULE_PARM_DESC(config, "option string to pass to driver core");
diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index dabbe1a9180c..0088fc354c98 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -92,13 +92,15 @@ struct ddebug_class_map {
};
/**
- * DYNDBG_CLASSMAP_DEFINE - define the class_map that names the
- * debug classes used in this module. This tells dyndbg the authorized
- * classnames it should manipulate.
- * @_var: a struct ddebug_class_map, passed to module_param_cb
+ * DYNDBG_CLASSMAP_DEFINE - define debug-classes used by a module.
+ * @_var: name of the classmap, exported for other modules coordinated use.
* @_type: enum class_map_type, chooses bits/verbose, numeric/symbolic
* @_base: offset of 1st class-name. splits .class_id space
- * @classes: class-names used to control class'd prdbgs
+ * @classes: enum-map - symbol names are "classnames", vals are .class_ids
+ *
+ * @classes vals are _ddebug.class_ids used in the module, the symbol
+ * names are stringified; they authorize "class FOO" to >control.
+ * Connection to a kernel-param is done separately.
*/
#define DYNDBG_CLASSMAP_DEFINE(_var, _maptype, _base, ...) \
const char *_var##_classnames[] = { __VA_ARGS__ }; \
@@ -118,16 +120,18 @@ struct ddebug_class_user {
struct ddebug_class_map *map;
};
/**
- * DYNDBG_CLASSMAP_USE - Use a classmap DEFINEd in another module.
- * This lets dyndbg initialize the dependent module's prdbgs from the
- * other module's controlling sysfs node.
+ * DYNDBG_CLASSMAP_USE - refer to a classmap, DEFINEd elsewhere.
+ * @_var: name of the exported classmap
+ *
+ * This registers the module's use of another module's classmap defn,
+ * allowing dyndbg to find the controlling kparam, and propagate its
+ * settings to the dependent module being loaded.
*/
-#define DYNDBG_CLASSMAP_USE(_var, ...) \
- DYNDBG_CLASSMAP_USE_(_var, __UNIQUE_ID(ddebug_class_user), \
- __VA_ARGS__)
-#define DYNDBG_CLASSMAP_USE_(_var, _uname, ...) \
+#define DYNDBG_CLASSMAP_USE(_var) \
+ DYNDBG_CLASSMAP_USE_(_var, __UNIQUE_ID(ddebug_class_user))
+#define DYNDBG_CLASSMAP_USE_(_var, _uname) \
extern struct ddebug_class_map _var[]; \
- static struct ddebug_class_user __used \
+ struct ddebug_class_user __used \
__section("__dyndbg_class_refs") _uname = { \
.user_mod_name = KBUILD_MODNAME, \
.map = _var, \
--
2.38.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* [RFC PATCH 12/17] dyndbg-API: DYNDBG_CLASSMAP_DEFINE() improvements
2022-12-06 0:34 [RFC PATCH 00/17] DRM_USE_DYNAMIC_DEBUG regression Jim Cromie
` (10 preceding siblings ...)
2022-12-06 0:34 ` [RFC PATCH 11/17] dyndbg-API: DYNDBG_CLASSMAP_USE drop extra args Jim Cromie
@ 2022-12-06 0:34 ` Jim Cromie
2022-12-06 0:34 ` [RFC PATCH 13/17] drm_print: fix stale macro-name in comment Jim Cromie
` (5 subsequent siblings)
17 siblings, 0 replies; 22+ messages in thread
From: Jim Cromie @ 2022-12-06 0:34 UTC (permalink / raw)
To: linux-kernel, dri-devel, amd-gfx, intel-gvt-dev, intel-gfx
Cc: jani.nikula, ville.syrjala, daniel.vetter, seanpaul, robdclark,
jbaron, gregkh, Jim Cromie
patch 1 in this series fixed a CLASSMAP usage error, this improves the
api so that misuse is less likely.
changes here:
0- Add William Swanson's public domain map macro:
https://github.com/swansontec/map-macro/blob/master/map.h
this makes 1 possible.
1- classnames were formerly specified as strings: "DRM_UT_CORE"
now they are the actual enum const symbols: DRM_UT_CORE
direct use of symbols is tighter, more comprehensible by tools, grep
2- drop _base arg.
_base was the value of the 1st classname
that is now available due to 1, no need to require it 2x
So take _base out of the API/kdoc. Note that the macro impl keeps the
_base arg so that it can be used to set classmap.base, but reuses it
in the MAP-stringify _base, __VA_ARGS__ expression.
Also cleanup the API usage comment in test_dynamic_debug.c, and since
comments in test-code might not be noticed, restate that here.
Using the CLASSMAP api:
- class-specifications are enum consts/symbols,
like DRM_UT_CORE, DRM_UT_KMS, etc.
their values define bit positions to drm.debug (as before)
- they are stringified and accepted at >control
echo class DRM_UT_CORE +p >control
- multiple class-maps must share the per-module: 0-62 class_id space
(by setting initial enum values to non-overlapping subranges)
todo: fixup the 'i' prefix, a quick/dirty avoidance of MAP.
NOTE: test_dynamic_debug.c also has this helper macro to wire a
classmap to a drm.debug style parameter; its easier to just use it as
a model/template as needed, rather than try to make it general enough
to be an official API helper.
define DD_SYS_WRAP(_model, _flags) \
static unsigned long bits_##_model; \
static struct ddebug_class_param _flags##_model = { \
.bits = &bits_##_model, \
.flags = #_flags, \
.map = &map_##_model, \
}; \
module_param_cb(_flags##_##_model, ¶m_ops_dyndbg_classes, &_flags##_model, 0600)
Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
drivers/gpu/drm/drm_print.c | 22 +++++++-------
include/drm/drm_print.h | 1 +
include/linux/dynamic_debug.h | 17 ++++++-----
include/linux/map.h | 54 +++++++++++++++++++++++++++++++++++
lib/test_dynamic_debug.c | 43 ++++++++++++++--------------
5 files changed, 95 insertions(+), 42 deletions(-)
create mode 100644 include/linux/map.h
diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c
index 4b697e18238d..07c25241e8cc 100644
--- a/drivers/gpu/drm/drm_print.c
+++ b/drivers/gpu/drm/drm_print.c
@@ -56,17 +56,17 @@ MODULE_PARM_DESC(debug, "Enable debug output, where each bit enables a debug cat
module_param_named(debug, __drm_debug, ulong, 0600);
#else
/* classnames must match vals of enum drm_debug_category */
-DYNDBG_CLASSMAP_DEFINE(drm_debug_classes, DD_CLASS_TYPE_DISJOINT_BITS, 0,
- "DRM_UT_CORE",
- "DRM_UT_DRIVER",
- "DRM_UT_KMS",
- "DRM_UT_PRIME",
- "DRM_UT_ATOMIC",
- "DRM_UT_VBL",
- "DRM_UT_STATE",
- "DRM_UT_LEASE",
- "DRM_UT_DP",
- "DRM_UT_DRMRES");
+DYNDBG_CLASSMAP_DEFINE(drm_debug_classes, DD_CLASS_TYPE_DISJOINT_BITS,
+ DRM_UT_CORE,
+ DRM_UT_DRIVER,
+ DRM_UT_KMS,
+ DRM_UT_PRIME,
+ DRM_UT_ATOMIC,
+ DRM_UT_VBL,
+ DRM_UT_STATE,
+ DRM_UT_LEASE,
+ DRM_UT_DP,
+ DRM_UT_DRMRES);
static struct ddebug_class_param drm_debug_bitmap = {
.bits = &__drm_debug,
diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
index a44fb7ef257f..6a27e8f26770 100644
--- a/include/drm/drm_print.h
+++ b/include/drm/drm_print.h
@@ -333,6 +333,7 @@ static inline bool drm_debug_enabled_raw(enum drm_debug_category category)
})
#if defined(CONFIG_DRM_USE_DYNAMIC_DEBUG)
+//extern struct ddebug_class_map drm_debug_classes[];
/*
* the drm.debug API uses dyndbg, so each drm_*dbg macro/callsite gets
* a descriptor, and only enabled callsites are reachable. They use
diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index 0088fc354c98..6f53a687cb32 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -7,6 +7,7 @@
#endif
#include <linux/build_bug.h>
+#include <linux/map.h>
/*
* An instance of this structure is created in a special
@@ -92,18 +93,16 @@ struct ddebug_class_map {
};
/**
- * DYNDBG_CLASSMAP_DEFINE - define debug-classes used by a module.
- * @_var: name of the classmap, exported for other modules coordinated use.
- * @_type: enum class_map_type, chooses bits/verbose, numeric/symbolic
- * @_base: offset of 1st class-name. splits .class_id space
- * @classes: enum-map - symbol names are "classnames", vals are .class_ids
+ * DYNDBG_CLASSMAP_DEFINE - define the debug classes used in this module.
+ * This tells dyndbg what debug classes it should control for the client.
*
- * @classes vals are _ddebug.class_ids used in the module, the symbol
- * names are stringified; they authorize "class FOO" to >control.
- * Connection to a kernel-param is done separately.
+ * @_var: struct ddebug_class_map, as passed to module_param_cb
+ * @_type: enum ddebug_class_map_type, chooses bits/verbose, numeric/symbolic
+ * @classes: enum class values used in module, such as: DRM_UT_*
*/
#define DYNDBG_CLASSMAP_DEFINE(_var, _maptype, _base, ...) \
- const char *_var##_classnames[] = { __VA_ARGS__ }; \
+ const char *_var##_classnames[] = { \
+ iMAP_LIST(__stringify, _base, __VA_ARGS__) }; \
struct ddebug_class_map __aligned(8) __used \
__section("__dyndbg_classes") _var = { \
.mod = THIS_MODULE, \
diff --git a/include/linux/map.h b/include/linux/map.h
new file mode 100644
index 000000000000..4348313a596f
--- /dev/null
+++ b/include/linux/map.h
@@ -0,0 +1,54 @@
+/*
+ * Created by William Swanson in 2012.
+ *
+ * I, William Swanson, dedicate this work to the public domain.
+ * I waive all rights to the work worldwide under copyright law,
+ * including all related and neighboring rights,
+ * to the extent allowed by law.
+ *
+ * You can copy, modify, distribute and perform the work,
+ * even for commercial purposes, all without asking permission.
+ */
+
+#ifndef MAP_H_INCLUDED
+#define MAP_H_INCLUDED
+
+#define iEVAL0(...) __VA_ARGS__
+#define iEVAL1(...) iEVAL0(iEVAL0(iEVAL0(__VA_ARGS__)))
+#define iEVAL2(...) iEVAL1(iEVAL1(iEVAL1(__VA_ARGS__)))
+#define iEVAL3(...) iEVAL2(iEVAL2(iEVAL2(__VA_ARGS__)))
+#define iEVAL4(...) iEVAL3(iEVAL3(iEVAL3(__VA_ARGS__)))
+#define iEVAL(...) iEVAL4(iEVAL4(iEVAL4(__VA_ARGS__)))
+
+#define iMAP_END(...)
+#define iMAP_OUT
+#define iMAP_COMMA ,
+
+#define iMAP_GET_END2() 0, iMAP_END
+#define iMAP_GET_END1(...) iMAP_GET_END2
+#define iMAP_GET_END(...) iMAP_GET_END1
+#define iMAP_NEXT0(test, next, ...) next iMAP_OUT
+#define iMAP_NEXT1(test, next) iMAP_NEXT0(test, next, 0)
+#define iMAP_NEXT(test, next) iMAP_NEXT1(iMAP_GET_END test, next)
+
+#define iMAP0(f, x, peek, ...) f(x) iMAP_NEXT(peek, iMAP1)(f, peek, __VA_ARGS__)
+#define iMAP1(f, x, peek, ...) f(x) iMAP_NEXT(peek, iMAP0)(f, peek, __VA_ARGS__)
+
+#define iMAP_LIST_NEXT1(test, next) iMAP_NEXT0(test, iMAP_COMMA next, 0)
+#define iMAP_LIST_NEXT(test, next) iMAP_LIST_NEXT1(iMAP_GET_END test, next)
+
+#define iMAP_LIST0(f, x, peek, ...) f(x) iMAP_LIST_NEXT(peek, iMAP_LIST1)(f, peek, __VA_ARGS__)
+#define iMAP_LIST1(f, x, peek, ...) f(x) iMAP_LIST_NEXT(peek, iMAP_LIST0)(f, peek, __VA_ARGS__)
+
+/**
+ * Applies the function macro `f` to each of the remaining parameters.
+ */
+#define iMAP(f, ...) iEVAL(iMAP1(f, __VA_ARGS__, ()()(), ()()(), ()()(), 0))
+
+/**
+ * Applies the function macro `f` to each of the remaining parameters and
+ * inserts commas between the results.
+ */
+#define iMAP_LIST(f, ...) iEVAL(iMAP_LIST1(f, __VA_ARGS__, ()()(), ()()(), ()()(), 0))
+
+#endif
diff --git a/lib/test_dynamic_debug.c b/lib/test_dynamic_debug.c
index 4ae01f7fa920..f471737fdfc3 100644
--- a/lib/test_dynamic_debug.c
+++ b/lib/test_dynamic_debug.c
@@ -33,11 +33,10 @@ module_param_cb(do_prints, ¶m_ops_do_prints, NULL, 0600);
/*
* Using the CLASSMAP api:
- * - classmaps must have corresponding enum
- * - enum symbols must match/correlate with class-name strings in the map.
- * - base must equal enum's 1st value
- * - multiple maps must set their base to share the 0-30 class_id space !!
- * (build-bug-on tips welcome)
+ * - class-names are enum consts/symbols, like DRM_UT_CORE, DRM_UT_KMS, etc
+ * - those names are accepted at >control interface
+ * - multiple class-maps must share the per-module: 0-62 class_id space
+ * (by setting initial enum values to non-overlapping subranges)
* Additionally, here:
* - tie together sysname, mapname, bitsname, flagsname
*/
@@ -62,38 +61,38 @@ enum cat_disjoint_bits {
D2_LEASE,
D2_DP,
D2_DRMRES };
-DYNDBG_CLASSMAP_DEFINE(map_disjoint_bits, DD_CLASS_TYPE_DISJOINT_BITS, 0,
- "D2_CORE",
- "D2_DRIVER",
- "D2_KMS",
- "D2_PRIME",
- "D2_ATOMIC",
- "D2_VBL",
- "D2_STATE",
- "D2_LEASE",
- "D2_DP",
- "D2_DRMRES");
+DYNDBG_CLASSMAP_DEFINE(map_disjoint_bits, DD_CLASS_TYPE_DISJOINT_BITS,
+ D2_CORE,
+ D2_DRIVER,
+ D2_KMS,
+ D2_PRIME,
+ D2_ATOMIC,
+ D2_VBL,
+ D2_STATE,
+ D2_LEASE,
+ D2_DP,
+ D2_DRMRES);
DD_SYS_WRAP(disjoint_bits, p);
DD_SYS_WRAP(disjoint_bits, T);
/* symbolic input, independent bits */
enum cat_disjoint_names { LOW = 10, MID, HI };
-DYNDBG_CLASSMAP_DEFINE(map_disjoint_names, DD_CLASS_TYPE_DISJOINT_NAMES, 10,
- "LOW", "MID", "HI");
+DYNDBG_CLASSMAP_DEFINE(map_disjoint_names, DD_CLASS_TYPE_DISJOINT_NAMES,
+ LOW, MID, HI);
DD_SYS_WRAP(disjoint_names, p);
DD_SYS_WRAP(disjoint_names, T);
/* numeric verbosity, V2 > V1 related */
enum cat_level_num { V0 = 14, V1, V2, V3, V4, V5, V6, V7 };
-DYNDBG_CLASSMAP_DEFINE(map_level_num, DD_CLASS_TYPE_LEVEL_NUM, 14,
- "V0", "V1", "V2", "V3", "V4", "V5", "V6", "V7");
+DYNDBG_CLASSMAP_DEFINE(map_level_num, DD_CLASS_TYPE_LEVEL_NUM,
+ V0, V1, V2, V3, V4, V5, V6, V7);
DD_SYS_WRAP(level_num, p);
DD_SYS_WRAP(level_num, T);
/* symbolic verbosity */
enum cat_level_names { L0 = 22, L1, L2, L3, L4, L5, L6, L7 };
-DYNDBG_CLASSMAP_DEFINE(map_level_names, DD_CLASS_TYPE_LEVEL_NAMES, 22,
- "L0", "L1", "L2", "L3", "L4", "L5", "L6", "L7");
+DYNDBG_CLASSMAP_DEFINE(map_level_names, DD_CLASS_TYPE_LEVEL_NAMES,
+ L0, L1, L2, L3, L4, L5, L6, L7);
DD_SYS_WRAP(level_names, p);
DD_SYS_WRAP(level_names, T);
--
2.38.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* [RFC PATCH 13/17] drm_print: fix stale macro-name in comment
2022-12-06 0:34 [RFC PATCH 00/17] DRM_USE_DYNAMIC_DEBUG regression Jim Cromie
` (11 preceding siblings ...)
2022-12-06 0:34 ` [RFC PATCH 12/17] dyndbg-API: DYNDBG_CLASSMAP_DEFINE() improvements Jim Cromie
@ 2022-12-06 0:34 ` Jim Cromie
2023-01-11 23:02 ` Daniel Vetter
2022-12-06 0:34 ` [RFC PATCH 14/17] dyndbg: unwrap __ddebug_add_module inner function NOTYET Jim Cromie
` (4 subsequent siblings)
17 siblings, 1 reply; 22+ messages in thread
From: Jim Cromie @ 2022-12-06 0:34 UTC (permalink / raw)
To: linux-kernel, dri-devel, amd-gfx, intel-gvt-dev, intel-gfx
Cc: jani.nikula, ville.syrjala, daniel.vetter, seanpaul, robdclark,
jbaron, gregkh, Jim Cromie
Cited commit uses stale macro name, fix this, and explain better.
When DRM_USE_DYNAMIC_DEBUG=y, DYNDBG_CLASSMAP_DEFINE() maps DRM_UT_*
onto BITs in drm.debug. This still uses enum drm_debug_category, but
it is somewhat indirect, with the ordered set of DRM_UT_* enum-vals.
This requires that the macro args: DRM_UT_* list must be kept in sync
and in order.
Fixes: f158936b60a7 ("drm: POC drm on dyndbg - use in core, 2 helpers, 3 drivers.")
Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
. emphasize ABI non-change despite enum val change - Jani Nikula
. reorder to back of patchset to follow API name changes.
---
include/drm/drm_print.h | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
index 6a27e8f26770..7695ba31b3a4 100644
--- a/include/drm/drm_print.h
+++ b/include/drm/drm_print.h
@@ -276,7 +276,10 @@ static inline struct drm_printer drm_err_printer(const char *prefix)
*
*/
enum drm_debug_category {
- /* These names must match those in DYNAMIC_DEBUG_CLASSBITS */
+ /*
+ * Keep DYNDBG_CLASSMAP_DEFINE args in sync with changes here,
+ * the enum-values define BIT()s in drm.debug, so are ABI.
+ */
/**
* @DRM_UT_CORE: Used in the generic drm code: drm_ioctl.c, drm_mm.c,
* drm_memory.c, ...
--
2.38.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [RFC PATCH 13/17] drm_print: fix stale macro-name in comment
2022-12-06 0:34 ` [RFC PATCH 13/17] drm_print: fix stale macro-name in comment Jim Cromie
@ 2023-01-11 23:02 ` Daniel Vetter
0 siblings, 0 replies; 22+ messages in thread
From: Daniel Vetter @ 2023-01-11 23:02 UTC (permalink / raw)
To: Jim Cromie
Cc: linux-kernel, dri-devel, amd-gfx, intel-gvt-dev, intel-gfx,
jani.nikula, ville.syrjala, daniel.vetter, seanpaul, robdclark,
jbaron, gregkh
On Mon, Dec 05, 2022 at 05:34:20PM -0700, Jim Cromie wrote:
> Cited commit uses stale macro name, fix this, and explain better.
>
> When DRM_USE_DYNAMIC_DEBUG=y, DYNDBG_CLASSMAP_DEFINE() maps DRM_UT_*
> onto BITs in drm.debug. This still uses enum drm_debug_category, but
> it is somewhat indirect, with the ordered set of DRM_UT_* enum-vals.
> This requires that the macro args: DRM_UT_* list must be kept in sync
> and in order.
>
> Fixes: f158936b60a7 ("drm: POC drm on dyndbg - use in core, 2 helpers, 3 drivers.")
> Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
Should I land this already?
-Daniel
> ---
> . emphasize ABI non-change despite enum val change - Jani Nikula
> . reorder to back of patchset to follow API name changes.
> ---
> include/drm/drm_print.h | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
> index 6a27e8f26770..7695ba31b3a4 100644
> --- a/include/drm/drm_print.h
> +++ b/include/drm/drm_print.h
> @@ -276,7 +276,10 @@ static inline struct drm_printer drm_err_printer(const char *prefix)
> *
> */
> enum drm_debug_category {
> - /* These names must match those in DYNAMIC_DEBUG_CLASSBITS */
> + /*
> + * Keep DYNDBG_CLASSMAP_DEFINE args in sync with changes here,
> + * the enum-values define BIT()s in drm.debug, so are ABI.
> + */
> /**
> * @DRM_UT_CORE: Used in the generic drm code: drm_ioctl.c, drm_mm.c,
> * drm_memory.c, ...
> --
> 2.38.1
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 22+ messages in thread
* [RFC PATCH 14/17] dyndbg: unwrap __ddebug_add_module inner function NOTYET
2022-12-06 0:34 [RFC PATCH 00/17] DRM_USE_DYNAMIC_DEBUG regression Jim Cromie
` (12 preceding siblings ...)
2022-12-06 0:34 ` [RFC PATCH 13/17] drm_print: fix stale macro-name in comment Jim Cromie
@ 2022-12-06 0:34 ` Jim Cromie
2022-12-06 0:34 ` [RFC PATCH 15/17] dyndbg: ddebug_sanity() Jim Cromie
` (3 subsequent siblings)
17 siblings, 0 replies; 22+ messages in thread
From: Jim Cromie @ 2022-12-06 0:34 UTC (permalink / raw)
To: linux-kernel, dri-devel, amd-gfx, intel-gvt-dev, intel-gfx
Cc: jani.nikula, ville.syrjala, daniel.vetter, seanpaul, robdclark,
jbaron, gregkh, Jim Cromie
The inner func has a base arg which is unused in the body, so theres
no point in having the inner fn. Its one-time purpose was subsumed
into the ddebug_info record, which is used in dynamic_debug_init() as
a cursor to load the builtin _ddebug[] table.
TODO: cited commit gives another reason for base, to provide an index
in support of de-duplicating column data. Refresh that patchset to
see if its still true.
Fixes: 6afa31514977 ("dyndbg: unwrap __ddebug_add_module inner function")
Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
lib/dynamic_debug.c | 12 +++---------
1 file changed, 3 insertions(+), 9 deletions(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 45b8414fa130..02f36c553835 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -1329,8 +1329,7 @@ static void ddebug_attach_client_module_classes(struct ddebug_table *dt, struct
* Allocate a new ddebug_table for the given module
* and add it to the global list.
*/
-static int __ddebug_add_module(struct _ddebug_info *di, unsigned int base,
- const char *modname)
+int ddebug_add_module(struct _ddebug_info *di, const char *modname)
{
struct ddebug_table *dt;
@@ -1372,11 +1371,6 @@ static int __ddebug_add_module(struct _ddebug_info *di, unsigned int base,
return 0;
}
-int ddebug_add_module(struct _ddebug_info *di, const char *modname)
-{
- return __ddebug_add_module(di, 0, modname);
-}
-
/* helper for ddebug_dyndbg_(boot|module)_param_cb */
static int ddebug_dyndbg_param_cb(char *param, char *val,
const char *modname, int on_err)
@@ -1519,7 +1513,7 @@ static int __init dynamic_debug_init(void)
mod_ct++;
di.num_descs = mod_sites;
di.descs = iter_mod_start;
- ret = __ddebug_add_module(&di, i - mod_sites, modname);
+ ret = ddebug_add_module(&di, modname);
if (ret)
goto out_err;
@@ -1530,7 +1524,7 @@ static int __init dynamic_debug_init(void)
}
di.num_descs = mod_sites;
di.descs = iter_mod_start;
- ret = __ddebug_add_module(&di, i - mod_sites, modname);
+ ret = ddebug_add_module(&di, modname);
if (ret)
goto out_err;
--
2.38.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* [RFC PATCH 15/17] dyndbg: ddebug_sanity()
2022-12-06 0:34 [RFC PATCH 00/17] DRM_USE_DYNAMIC_DEBUG regression Jim Cromie
` (13 preceding siblings ...)
2022-12-06 0:34 ` [RFC PATCH 14/17] dyndbg: unwrap __ddebug_add_module inner function NOTYET Jim Cromie
@ 2022-12-06 0:34 ` Jim Cromie
2022-12-06 0:34 ` [RFC PATCH 16/17] dyndbg: mess-w-dep-class Jim Cromie
` (2 subsequent siblings)
17 siblings, 0 replies; 22+ messages in thread
From: Jim Cromie @ 2022-12-06 0:34 UTC (permalink / raw)
To: linux-kernel, dri-devel, amd-gfx, intel-gvt-dev, intel-gfx
Cc: jani.nikula, ville.syrjala, daniel.vetter, seanpaul, robdclark,
jbaron, gregkh, Jim Cromie
It appears that, at least for builtin drm,i915 loadable amdgpu, data
in the class_refs section is not properly linked, this works partway
towards isolating the problem.
The "NO CLI" name test is failing.
This version of the report fails with a non-canonical address:
// v2pr_info("NO CLI name on: %s\n", cli->map->mod_name);
[ 0.109299] dyndbg: add-module: main 6 sites 1.3
[ 0.109595] general protection fault, probably for non-canonical address 0x7265766972640000: 0000
fwiw:
$ perl -e ' $a = pack "H8", "7265766972640000"; print "a:<$a>\n"'
a:<revi>
These records are added to __dyndbg_class_refs section by
DYNDBG_CLASSMAP_USE
This patch adds ddebug_sanity(), and calls it 3 times to characterize
what goes wrong and when. It turns out that its contents are wrong
immediately, in 1st step of dynamic_debug_init().
[ 0.107327] dyndbg: classes
[ 0.107537] dyndbg: class[0]: module:drm base:0 len:10 ty:0 cm:ffffffff834fc4c8
[ 0.107592] dyndbg: class-refs
[ 0.107823] dyndbg: class-ref[0]: cli:ffffffff834fc508 map:ffffffff8293e35e nm:0000000000000000 nm:(null)
[ 0.108554] dyndbg: class-ref[1]: cli:ffffffff834fc518 map:ffffffff8293fe86 nm:ffffffff834fc4c8 nm:
[ 0.108590] dyndbg: class-ref[2]: cli:ffffffff834fc528 map:ffffffff829785aa nm:ffffffff834fc4c8 nm:
Those maps are wrong:
class-refs should all ref the same map, ie class[0]: module:drm.
the nm:s should also show module names of 3 builtin clients of drm.
So things must end poorly.
modprobing the loadable module does better:
bash-5.2# modprobe amdgpu
[ 6645.212706] AMD-Vi: AMD IOMMUv2 functionality not available on this system - This is not a bug.
[ 6645.653124] dyndbg: add-module: amdgpu 4425 sites 0.1
[ 6645.653582] dyndbg: classes
[ 6645.653830] dyndbg: class-refs
[ 6645.654124] dyndbg: class-ref[0]: cli:ffffffffc0a31b90 map:ffffffff834fc4c8 nm:ffffffffc08bc176 nm:amdgpu
[ 6645.654936] dyndbg: classes
[ 6645.655188] dyndbg: class-refs
[ 6645.655450] dyndbg: class-ref[0]: cli:ffffffffc0a31b90 map:ffffffff834fc4c8 nm:ffffffffc08bc176 nm:amdgpu
[ 6645.656246] dyndbg: class_ref[0] amdgpu -> drm
[ 6645.656613] dyndbg: amdgpu needs drm, 0x0
[ 6645.656953] dyndbg: apply bitmap: 0x0 to: 0x0
[ 6645.657322] dyndbg: break on 0/1
[ 6645.657592] dyndbg: 4425 debug prints in module amdgpu
Here, the maps are correct; they ref the class[0] module:drm above.
That said, apply bitmap is wrong.
Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
[ 1.210094] dyndbg: add-module: drm 302 sites 1.3
[ 1.210496] dyndbg: classes
[ 1.210674] dyndbg: class[0]: module:drm base:0 len:10 ty:0 cm:ffffffff834fc4c8
[ 1.211290] dyndbg: class-refs
[ 1.211548] dyndbg: class-ref[0]: cli:ffffffff834fc508 map:ffffffff8293e376 nm:0000000000000000 nm:(null)
[ 1.211674] dyndbg: class-ref[1]: cli:ffffffff834fc518 map:ffffffff8293fe9e nm:ffffffff834fc4c8 nm:
[ 1.212464] dyndbg: class-ref[2]: cli:ffffffff834fc528 map:ffffffff829785c2 nm:ffffffff834fc4c8 nm:
[ 1.212675] dyndbg: classes start: class[0]: module:drm base:0 len:10 ty:0
[ 1.213257] dyndbg: builtin class: module:drm base:0 len:10 ty:0
[ 1.213675] dyndbg: controlling kp: drm.drm.debug
[ 1.214087] dyndbg: module:drm attached 1 classes
[ 1.214490] dyndbg: classes
[ 1.214674] dyndbg: class[0]: module:drm base:0 len:10 ty:0 cm:ffffffff834fc4c8
[ 1.215291] dyndbg: class-refs
[ 1.215552] dyndbg: class-ref[0]: cli:ffffffff834fc508 map:ffffffff8293e376 nm:0000000000000000 nm:(null)
[ 1.215674] dyndbg: class-ref[1]: cli:ffffffff834fc518 map:ffffffff8293fe9e nm:ffffffff834fc4c8 nm:
[ 1.216430] dyndbg: class-ref[2]: cli:ffffffff834fc528 map:ffffffff829785c2 nm:ffffffff834fc4c8 nm:
[ 1.216674] dyndbg: NO CLI ffffffff8293e376 7265766972640000
[ 1.217149] dyndbg: 302 debug prints in module drm
[ 1.217549] dyndbg: add-module: drm_kms_helper 95 sites 1.3
[ 1.217674] dyndbg: classes
[ 1.217913] dyndbg: class[0]: module:drm base:0 len:10 ty:0 cm:ffffffff834fc4c8
[ 1.218525] dyndbg: class-refs
[ 1.218674] dyndbg: class-ref[0]: cli:ffffffff834fc508 map:ffffffff8293e376 nm:0000000000000000 nm:(null)
[ 1.219486] dyndbg: class-ref[1]: cli:ffffffff834fc518 map:ffffffff8293fe9e nm:ffffffff834fc4c8 nm:
[ 1.219674] dyndbg: class-ref[2]: cli:ffffffff834fc528 map:ffffffff829785c2 nm:ffffffff834fc4c8 nm:
[ 1.220469] dyndbg: classes
[ 1.220674] dyndbg: class[0]: module:drm base:0 len:10 ty:0 cm:ffffffff834fc4c8
[ 1.221324] dyndbg: class-refs
[ 1.221606] dyndbg: class-ref[0]: cli:ffffffff834fc508 map:ffffffff8293e376 nm:0000000000000000 nm:(null)
[ 1.221674] dyndbg: class-ref[1]: cli:ffffffff834fc518 map:ffffffff8293fe9e nm:ffffffff834fc4c8 nm:
[ 1.222472] dyndbg: class-ref[2]: cli:ffffffff834fc528 map:ffffffff829785c2 nm:ffffffff834fc4c8 nm:
[ 1.222674] dyndbg: NO CLI ffffffff8293e376 7265766972640000
[ 1.223173] dyndbg: 95 debug prints in module drm_kms_helper
[ 1.223675] dyndbg: add-module: drm_display_helper 150 sites 1.3
[ 1.224223] dyndbg: classes
[ 1.224484] dyndbg: class[0]: module:drm base:0 len:10 ty:0 cm:ffffffff834fc4c8
[ 1.224676] dyndbg: class-refs
[ 1.224954] dyndbg: class-ref[0]: cli:ffffffff834fc508 map:ffffffff8293e376 nm:0000000000000000 nm:(null)
[ 1.225674] dyndbg: class-ref[1]: cli:ffffffff834fc518 map:ffffffff8293fe9e nm:ffffffff834fc4c8 nm:
[ 1.226498] dyndbg: class-ref[2]: cli:ffffffff834fc528 map:ffffffff829785c2 nm:ffffffff834fc4c8 nm:
[ 1.226674] dyndbg: classes
[ 1.226931] dyndbg: class[0]: module:drm base:0 len:10 ty:0 cm:ffffffff834fc4c8
[ 1.227577] dyndbg: class-refs
[ 1.227674] dyndbg: class-ref[0]: cli:ffffffff834fc508 map:ffffffff8293e376 nm:0000000000000000 nm:(null)
[ 1.228501] dyndbg: class-ref[1]: cli:ffffffff834fc518 map:ffffffff8293fe9e nm:ffffffff834fc4c8 nm:
[ 1.228674] dyndbg: class-ref[2]: cli:ffffffff834fc528 map:ffffffff829785c2 nm:ffffffff834fc4c8 nm:
[ 1.229443] dyndbg: NO CLI ffffffff8293e376 7265766972640000
[ 1.229674] dyndbg: 150 debug prints in module drm_display_helper
[ 1.230195] dyndbg: add-module: ttm 2 sites 1.3
[ 1.230581] dyndbg: classes
[ 1.230674] dyndbg: class[0]: module:drm base:0 len:10 ty:0 cm:ffffffff834fc4c8
[ 1.231291] dyndbg: class-refs
[ 1.231559] dyndbg: class-ref[0]: cli:ffffffff834fc508 map:ffffffff8293e376 nm:0000000000000000 nm:(null)
[ 1.231674] dyndbg: class-ref[1]: cli:ffffffff834fc518 map:ffffffff8293fe9e nm:ffffffff834fc4c8 nm:
[ 1.232439] dyndbg: class-ref[2]: cli:ffffffff834fc528 map:ffffffff829785c2 nm:ffffffff834fc4c8 nm:
[ 1.232674] dyndbg: classes
[ 1.232915] dyndbg: class[0]: module:drm base:0 len:10 ty:0 cm:ffffffff834fc4c8
[ 1.233535] dyndbg: class-refs
[ 1.233674] dyndbg: class-ref[0]: cli:ffffffff834fc508 map:ffffffff8293e376 nm:0000000000000000 nm:(null)
[ 1.234470] dyndbg: class-ref[1]: cli:ffffffff834fc518 map:ffffffff8293fe9e nm:ffffffff834fc4c8 nm:
[ 1.234674] dyndbg: class-ref[2]: cli:ffffffff834fc528 map:ffffffff829785c2 nm:ffffffff834fc4c8 nm:
[ 1.235427] dyndbg: NO CLI ffffffff8293e376 7265766972640000
[ 1.235674] dyndbg: 2 debug prints in module ttm
[ 1.236079] dyndbg: add-module: i915 1657 sites 1.3
[ 1.236490] dyndbg: classes
[ 1.236674] dyndbg: class[0]: module:drm base:0 len:10 ty:0 cm:ffffffff834fc4c8
[ 1.237283] dyndbg: class-refs
[ 1.237545] dyndbg: class-ref[0]: cli:ffffffff834fc508 map:ffffffff8293e376 nm:0000000000000000 nm:(null)
[ 1.237674] dyndbg: class-ref[1]: cli:ffffffff834fc518 map:ffffffff8293fe9e nm:ffffffff834fc4c8 nm:
[ 1.238431] dyndbg: class-ref[2]: cli:ffffffff834fc528 map:ffffffff829785c2 nm:ffffffff834fc4c8 nm:
[ 1.238674] dyndbg: classes
[ 1.238911] dyndbg: class[0]: module:drm base:0 len:10 ty:0 cm:ffffffff834fc4c8
[ 1.239519] dyndbg: class-refs
[ 1.239674] dyndbg: class-ref[0]: cli:ffffffff834fc508 map:ffffffff8293e376 nm:0000000000000000 nm:(null)
[ 1.240467] dyndbg: class-ref[1]: cli:ffffffff834fc518 map:ffffffff8293fe9e nm:ffffffff834fc4c8 nm:
[ 1.240674] dyndbg: class-ref[2]: cli:ffffffff834fc528 map:ffffffff829785c2 nm:ffffffff834fc4c8 nm:
[ 1.241478] dyndbg: NO CLI ffffffff8293e376 7265766972640000
[ 1.241674] dyndbg: 1657 debug prints in module i915
---
include/linux/dynamic_debug.h | 4 ++--
lib/dynamic_debug.c | 32 +++++++++++++++++++++++++++++++-
2 files changed, 33 insertions(+), 3 deletions(-)
diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index 6f53a687cb32..2a1199aadab6 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -115,8 +115,8 @@ struct ddebug_class_map {
EXPORT_SYMBOL(_var)
struct ddebug_class_user {
- char *user_mod_name;
- struct ddebug_class_map *map;
+ const char *user_mod_name;
+ const struct ddebug_class_map *map;
};
/**
* DYNDBG_CLASSMAP_USE - refer to a classmap, DEFINEd elsewhere.
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 02f36c553835..46684aa7284d 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -1301,7 +1301,8 @@ static void ddebug_attach_client_module_classes(struct ddebug_table *dt, struct
continue;
}
if (!cli->user_mod_name) {
- v2pr_info("NO CLI name\n");
+ v2pr_info("NO CLI name %px %px\n", cli->map, cli->map->mod_name);
+ // v2pr_info("NO CLI name on: %s\n", cli->map->mod_name);
continue;
}
@@ -1325,6 +1326,29 @@ static void ddebug_attach_client_module_classes(struct ddebug_table *dt, struct
}
}
+static void ddebug_sanity(struct _ddebug_info *di)
+{
+ struct ddebug_class_map *cm;
+ struct ddebug_class_user *cli;
+ int i;
+
+ if (di->num_classes)
+ v2pr_info("classes\n");
+
+ for (i = 0, cm = di->classes; i < di->num_classes; i++, cm++) {
+ v2pr_info("class[%d]: module:%s base:%d len:%d ty:%d cm:%px\n",
+ i, cm->mod_name, cm->base, cm->length, cm->map_type, cm);
+ }
+ if (di->num_class_refs)
+ v2pr_info("class-refs\n");
+
+ for (i = 0, cli = di->class_refs; i < di->num_class_refs; i++, cli++) {
+ // cli->map->mod_name will segv
+ v2pr_info("class-ref[%d]: cli:%px map:%px nm:%px nm:%s\n", i, cli,
+ cli->map, cli->user_mod_name, cli->user_mod_name);
+ }
+}
+
/*
* Allocate a new ddebug_table for the given module
* and add it to the global list.
@@ -1357,6 +1381,8 @@ int ddebug_add_module(struct _ddebug_info *di, const char *modname)
INIT_LIST_HEAD(&dt->link);
+ ddebug_sanity(di);
+
if (di->num_classes)
ddebug_attach_module_classes(dt, di);
@@ -1364,6 +1390,8 @@ int ddebug_add_module(struct _ddebug_info *di, const char *modname)
list_add_tail(&dt->link, &ddebug_tables);
mutex_unlock(&ddebug_lock);
+ ddebug_sanity(di);
+
if (di->num_class_refs)
ddebug_attach_client_module_classes(dt, di);
@@ -1493,6 +1521,8 @@ static int __init dynamic_debug_init(void)
.num_class_refs = __stop___dyndbg_class_refs - __start___dyndbg_class_refs,
};
+ ddebug_sanity(&di);
+
if (&__start___dyndbg == &__stop___dyndbg) {
if (IS_ENABLED(CONFIG_DYNAMIC_DEBUG)) {
pr_warn("_ddebug table is empty in a CONFIG_DYNAMIC_DEBUG build\n");
--
2.38.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* [RFC PATCH 16/17] dyndbg: mess-w-dep-class
2022-12-06 0:34 [RFC PATCH 00/17] DRM_USE_DYNAMIC_DEBUG regression Jim Cromie
` (14 preceding siblings ...)
2022-12-06 0:34 ` [RFC PATCH 15/17] dyndbg: ddebug_sanity() Jim Cromie
@ 2022-12-06 0:34 ` Jim Cromie
2022-12-06 0:34 ` [RFC PATCH 17/17] dyndbg: miss-on HACK Jim Cromie
2023-01-11 23:09 ` [RFC PATCH 00/17] DRM_USE_DYNAMIC_DEBUG regression Daniel Vetter
17 siblings, 0 replies; 22+ messages in thread
From: Jim Cromie @ 2022-12-06 0:34 UTC (permalink / raw)
To: linux-kernel, dri-devel, amd-gfx, intel-gvt-dev, intel-gfx
Cc: jani.nikula, ville.syrjala, daniel.vetter, seanpaul, robdclark,
jbaron, gregkh, Jim Cromie
for loadable drm, helpers, and drivers, dependent-load is failing to
apply changes, needs more investigation.
---
lib/dynamic_debug.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 46684aa7284d..3ef1c0a1f0cd 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -1248,14 +1248,14 @@ static void ddebug_find_kparam(struct ddebug_class_map *cm)
static void ddebug_param_load_dependent_class(const struct ddebug_class_user *cli)
{
- unsigned long new_bits, old_bits = 0;
+ unsigned long *new_bits, old_bits = 0;
- new_bits = *cli->map->dc_parm->bits;
+ new_bits = cli->map->dc_parm->bits;
vpr_info("%s needs %s, 0x%lx\n", cli->user_mod_name,
- cli->map->mod_name, new_bits);
+ cli->map->mod_name, *new_bits);
- ddebug_apply_class_bitmap(cli->map->dc_parm, &new_bits, &old_bits, cli->user_mod_name);
+ ddebug_apply_class_bitmap(cli->map->dc_parm, new_bits, &old_bits, cli->user_mod_name);
}
static void ddebug_attach_module_classes(struct ddebug_table *dt, struct _ddebug_info *di)
--
2.38.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* [RFC PATCH 17/17] dyndbg: miss-on HACK
2022-12-06 0:34 [RFC PATCH 00/17] DRM_USE_DYNAMIC_DEBUG regression Jim Cromie
` (15 preceding siblings ...)
2022-12-06 0:34 ` [RFC PATCH 16/17] dyndbg: mess-w-dep-class Jim Cromie
@ 2022-12-06 0:34 ` Jim Cromie
2023-01-11 23:09 ` [RFC PATCH 00/17] DRM_USE_DYNAMIC_DEBUG regression Daniel Vetter
17 siblings, 0 replies; 22+ messages in thread
From: Jim Cromie @ 2022-12-06 0:34 UTC (permalink / raw)
To: linux-kernel, dri-devel, amd-gfx, intel-gvt-dev, intel-gfx
Cc: jani.nikula, ville.syrjala, daniel.vetter, seanpaul, robdclark,
jbaron, gregkh, Jim Cromie
dont break the loop, to see multiple clients. the 3 client records
are differently wrong.
---
lib/dynamic_debug.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 3ef1c0a1f0cd..a26eaa348731 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -629,6 +629,7 @@ static int ddebug_apply_class_bitmap(const struct ddebug_class_param *dcp,
v2pr_info("bit_%d: %d matches on class: %s -> 0x%lx\n", bi,
ct, map->class_names[bi], *new_bits);
}
+ v2pr_info("applied bitmap: 0x%lx to: 0x%lx\n", *new_bits, *old_bits);
return matches;
}
@@ -1321,8 +1322,8 @@ static void ddebug_attach_client_module_classes(struct ddebug_table *dt, struct
*/
v2pr_info("break on %d/%d\n", i, di->num_class_refs);
dt->num_class_refs = 1;
- break;
- }
+ } else
+ v2pr_info("miss on %d/%d\n", i, di->num_class_refs);
}
}
--
2.38.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [RFC PATCH 00/17] DRM_USE_DYNAMIC_DEBUG regression
2022-12-06 0:34 [RFC PATCH 00/17] DRM_USE_DYNAMIC_DEBUG regression Jim Cromie
` (16 preceding siblings ...)
2022-12-06 0:34 ` [RFC PATCH 17/17] dyndbg: miss-on HACK Jim Cromie
@ 2023-01-11 23:09 ` Daniel Vetter
2023-01-13 18:29 ` jim.cromie
17 siblings, 1 reply; 22+ messages in thread
From: Daniel Vetter @ 2023-01-11 23:09 UTC (permalink / raw)
To: Jim Cromie
Cc: linux-kernel, dri-devel, amd-gfx, intel-gvt-dev, intel-gfx,
jani.nikula, ville.syrjala, daniel.vetter, seanpaul, robdclark,
jbaron, gregkh
On Mon, Dec 05, 2022 at 05:34:07PM -0700, Jim Cromie wrote:
> Hi everyone,
>
> DRM_USE_DYNAMIC_DEBUG=y has a regression on rc-*
>
> Regression is due to a chicken-egg problem loading modules; on
> `modprobe i915`, drm is loaded 1st, and drm.debug is set. When
> drm_debug_enabled() tested __drm_debug at runtime, that just worked.
>
> But with DRM_USE_DYNAMIC_DEBUG=y, the runtime test is replaced with a
> post-load enablement of drm_dbg/dyndbg callsites (static-keys), via
> dyndbg's callback on __drm_debug. Since all drm-drivers need drm.ko,
> it is loaded 1st, then drm.debug=X is applied, then drivers load, but
> too late for drm_dbgs to be enabled.
>
> STATUS
>
> For all-loadable drm,i915,amdgpu configs, it almost works, but
> propagating drm.debug to dependent modules doesnt actually apply,
> though the motions are there. This is not the problem I want to chase
> here.
>
> The more basic trouble is:
>
> For builtin drm + helpers, things are broken pretty early; at the
> beginning of dynamic_debug_init(). As the ddebug_sanity() commit-msg
> describes in some detail, the records added by _USE fail to reference
> the struct ddebug_class_map created and exported by _DEFINE, but get
> separate addresses to "other" data that segv's when used as the
> expected pointer. FWIW, the pointer val starts with "revi".
So I honestly have no idea here, linker stuff is way beyond where I have
clue. So what's the way forward here?
The DEFINE/USE split does like the right thing to do at least from the
"how it's used in drivers" pov. But if we're just running circles not
quite getting there I dunno :-/
-Daniel
>
> OVERVIEW
>
> DECLARE_DYNDBG_CLASSMAP is broken: it is one-size-fits-all-poorly.
> It muddles the distinction between a (single) definition, and multiple
> references. Something exported should suffice.
>
> The core of this patchset splits it into:
>
> DYNDBG_CLASSMAP_DEFINE used once per subsystem to define each classmap
> DYNDBG_CLASSMAP_USE declare dependence on a DEFINEd classmap
>
> This makes the weird coordinated-changes-by-identical-classmaps
> "feature" unnecessary; the DEFINE can export the var, and USE refers
> to the exported var.
>
> So this patchset adds another section: __dyndbg_class_refs.
>
> It is like __dyndbg_classes; it is scanned under ddebug_add_module(),
> and attached to each module's ddebug_table. Once attached, it can be
> used like classes to validate and apply class FOO >control queries.
>
> It also maps the class user -> definer explicitly, so that when the
> module is loaded, the section scan can find the kernel-param that is
> wired to dyndbg's kparam-callback, and apply its state-var, forex:
> __drm_debug to the just loaded helper/driver module.
>
> Theres plenty to address Im sure.
>
> Jim Cromie (17):
> test-dyndbg: fixup CLASSMAP usage error
> test-dyndbg: show that DEBUG enables prdbgs at compiletime
> dyndbg: fix readback value on LEVEL_NAMES interfaces
> dyndbg: replace classmap list with a vector
> dyndbg: make ddebug_apply_class_bitmap more selective
> dyndbg: dynamic_debug_init - use pointer inequality, not strcmp
> dyndbg: drop NUM_TYPE_ARRAY
> dyndbg: reduce verbose/debug clutter
> dyndbg-API: replace DECLARE_DYNDBG_CLASSMAP with
> DYNDBG_CLASSMAP(_DEFINE|_USE)
> dyndbg-API: specialize DYNDBG_CLASSMAP_(DEFINE|USE)
> dyndbg-API: DYNDBG_CLASSMAP_USE drop extra args
> dyndbg-API: DYNDBG_CLASSMAP_DEFINE() improvements
> drm_print: fix stale macro-name in comment
> dyndbg: unwrap __ddebug_add_module inner function NOTYET
> dyndbg: ddebug_sanity()
> dyndbg: mess-w-dep-class
> dyndbg: miss-on HACK
>
> drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 14 +-
> drivers/gpu/drm/display/drm_dp_helper.c | 14 +-
> drivers/gpu/drm/drm_crtc_helper.c | 14 +-
> drivers/gpu/drm/drm_print.c | 22 +--
> drivers/gpu/drm/i915/i915_params.c | 14 +-
> drivers/gpu/drm/nouveau/nouveau_drm.c | 14 +-
> include/asm-generic/vmlinux.lds.h | 3 +
> include/drm/drm_print.h | 6 +-
> include/linux/dynamic_debug.h | 57 ++++--
> include/linux/map.h | 54 ++++++
> kernel/module/main.c | 2 +
> lib/dynamic_debug.c | 240 +++++++++++++++++++-----
> lib/test_dynamic_debug.c | 47 ++---
> 13 files changed, 344 insertions(+), 157 deletions(-)
> create mode 100644 include/linux/map.h
>
> --
> 2.38.1
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [RFC PATCH 00/17] DRM_USE_DYNAMIC_DEBUG regression
2023-01-11 23:09 ` [RFC PATCH 00/17] DRM_USE_DYNAMIC_DEBUG regression Daniel Vetter
@ 2023-01-13 18:29 ` jim.cromie
2023-01-13 18:48 ` Daniel Vetter
0 siblings, 1 reply; 22+ messages in thread
From: jim.cromie @ 2023-01-13 18:29 UTC (permalink / raw)
To: Jim Cromie, linux-kernel, dri-devel, amd-gfx, intel-gvt-dev,
intel-gfx, jani.nikula, ville.syrjala, seanpaul, robdclark,
jbaron, gregkh
Cc: daniel.vetter
On Wed, Jan 11, 2023 at 4:09 PM Daniel Vetter <daniel@ffwll.ch> wrote:
>
> On Mon, Dec 05, 2022 at 05:34:07PM -0700, Jim Cromie wrote:
> > Hi everyone,
> >
> > DRM_USE_DYNAMIC_DEBUG=y has a regression on rc-*
> >
> > Regression is due to a chicken-egg problem loading modules; on
> > `modprobe i915`, drm is loaded 1st, and drm.debug is set. When
> > drm_debug_enabled() tested __drm_debug at runtime, that just worked.
> >
> > But with DRM_USE_DYNAMIC_DEBUG=y, the runtime test is replaced with a
> > post-load enablement of drm_dbg/dyndbg callsites (static-keys), via
> > dyndbg's callback on __drm_debug. Since all drm-drivers need drm.ko,
> > it is loaded 1st, then drm.debug=X is applied, then drivers load, but
> > too late for drm_dbgs to be enabled.
> >
> > STATUS
> >
> > For all-loadable drm,i915,amdgpu configs, it almost works, but
> > propagating drm.debug to dependent modules doesnt actually apply,
> > though the motions are there. This is not the problem I want to chase
> > here.
> >
> > The more basic trouble is:
> >
> > For builtin drm + helpers, things are broken pretty early; at the
> > beginning of dynamic_debug_init(). As the ddebug_sanity() commit-msg
> > describes in some detail, the records added by _USE fail to reference
> > the struct ddebug_class_map created and exported by _DEFINE, but get
> > separate addresses to "other" data that segv's when used as the
> > expected pointer. FWIW, the pointer val starts with "revi".
>
> So I honestly have no idea here, linker stuff is way beyond where I have
> clue. So what's the way forward here?
>
Ive fixed this aspect.
Unsurprisingly, it wasnt the linker :-}
> The DEFINE/USE split does like the right thing to do at least from the
> "how it's used in drivers" pov. But if we're just running circles not
> quite getting there I dunno :-/
> -Daniel
>
Sending new rev next.
I think its getting close.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [RFC PATCH 00/17] DRM_USE_DYNAMIC_DEBUG regression
2023-01-13 18:29 ` jim.cromie
@ 2023-01-13 18:48 ` Daniel Vetter
0 siblings, 0 replies; 22+ messages in thread
From: Daniel Vetter @ 2023-01-13 18:48 UTC (permalink / raw)
To: jim.cromie
Cc: linux-kernel, dri-devel, amd-gfx, intel-gvt-dev, intel-gfx,
jani.nikula, ville.syrjala, seanpaul, robdclark, jbaron, gregkh,
daniel.vetter
On Fri, Jan 13, 2023 at 11:29:57AM -0700, jim.cromie@gmail.com wrote:
> On Wed, Jan 11, 2023 at 4:09 PM Daniel Vetter <daniel@ffwll.ch> wrote:
> >
> > On Mon, Dec 05, 2022 at 05:34:07PM -0700, Jim Cromie wrote:
> > > Hi everyone,
> > >
> > > DRM_USE_DYNAMIC_DEBUG=y has a regression on rc-*
> > >
> > > Regression is due to a chicken-egg problem loading modules; on
> > > `modprobe i915`, drm is loaded 1st, and drm.debug is set. When
> > > drm_debug_enabled() tested __drm_debug at runtime, that just worked.
> > >
> > > But with DRM_USE_DYNAMIC_DEBUG=y, the runtime test is replaced with a
> > > post-load enablement of drm_dbg/dyndbg callsites (static-keys), via
> > > dyndbg's callback on __drm_debug. Since all drm-drivers need drm.ko,
> > > it is loaded 1st, then drm.debug=X is applied, then drivers load, but
> > > too late for drm_dbgs to be enabled.
> > >
> > > STATUS
> > >
> > > For all-loadable drm,i915,amdgpu configs, it almost works, but
> > > propagating drm.debug to dependent modules doesnt actually apply,
> > > though the motions are there. This is not the problem I want to chase
> > > here.
> > >
> > > The more basic trouble is:
> > >
> > > For builtin drm + helpers, things are broken pretty early; at the
> > > beginning of dynamic_debug_init(). As the ddebug_sanity() commit-msg
> > > describes in some detail, the records added by _USE fail to reference
> > > the struct ddebug_class_map created and exported by _DEFINE, but get
> > > separate addresses to "other" data that segv's when used as the
> > > expected pointer. FWIW, the pointer val starts with "revi".
> >
> > So I honestly have no idea here, linker stuff is way beyond where I have
> > clue. So what's the way forward here?
> >
>
> Ive fixed this aspect.
> Unsurprisingly, it wasnt the linker :-}
Awesome!
> > The DEFINE/USE split does like the right thing to do at least from the
> > "how it's used in drivers" pov. But if we're just running circles not
> > quite getting there I dunno :-/
> > -Daniel
> >
>
> Sending new rev next.
> I think its getting close.
Thanks a lot for keeping on pushing this.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 22+ messages in thread