* [PATCH v8 01/35] dyndbg: fix old BUG_ON in >control parser
2024-04-29 19:31 [PATCH v8 00/35] fix CONFIG_DRM_USE_DYNAMIC_DEBUG=y regression Jim Cromie
@ 2024-04-29 19:31 ` Jim Cromie
2024-04-29 19:31 ` [PATCH v8 02/35] docs/dyndbg: update examples \012 to \n Jim Cromie
` (21 subsequent siblings)
22 siblings, 0 replies; 24+ messages in thread
From: Jim Cromie @ 2024-04-29 19:31 UTC (permalink / raw)
To: jbaron, gregkh, linux-kernel, dri-devel, amd-gfx, intel-gvt-dev,
intel-gfx
Cc: ukaszb, linux-doc, daniel.vetter, tvrtko.ursulin, jani.nikula,
ville.syrjala, seanpaul, robdclark, groeck, yanivt, bleung,
Jim Cromie, stable
Fix a BUG_ON from 2009. Even if it looks "unreachable" (I didn't
really look), lets make sure by removing it, doing pr_err and return
-EINVAL instead.
cc: stable@vger.kernel.org
Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
lib/dynamic_debug.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index c78f335fa981..f2c5e7910bb1 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -302,7 +302,11 @@ static int ddebug_tokenize(char *buf, char *words[], int maxwords)
} else {
for (end = buf; *end && !isspace(*end); end++)
;
- BUG_ON(end == buf);
+ if (end == buf) {
+ pr_err("parse err after word:%d=%s\n", nwords,
+ nwords ? words[nwords - 1] : "<none>");
+ return -EINVAL;
+ }
}
/* `buf' is start of word, `end' is one past its end */
--
2.44.0
^ permalink raw reply [flat|nested] 24+ messages in thread* [PATCH v8 02/35] docs/dyndbg: update examples \012 to \n
2024-04-29 19:31 [PATCH v8 00/35] fix CONFIG_DRM_USE_DYNAMIC_DEBUG=y regression Jim Cromie
2024-04-29 19:31 ` [PATCH v8 01/35] dyndbg: fix old BUG_ON in >control parser Jim Cromie
@ 2024-04-29 19:31 ` Jim Cromie
2024-04-29 19:31 ` [PATCH v8 03/35] test-dyndbg: fixup CLASSMAP usage error Jim Cromie
` (20 subsequent siblings)
22 siblings, 0 replies; 24+ messages in thread
From: Jim Cromie @ 2024-04-29 19:31 UTC (permalink / raw)
To: jbaron, gregkh, linux-kernel, dri-devel, amd-gfx, intel-gvt-dev,
intel-gfx
Cc: ukaszb, linux-doc, daniel.vetter, tvrtko.ursulin, jani.nikula,
ville.syrjala, seanpaul, robdclark, groeck, yanivt, bleung,
Jim Cromie
commit 47ea6f99d06e ("dyndbg: use ESCAPE_SPACE for cat control")
changed the control-file to display format strings with "\n" rather
than "\012". Update the docs to match the new reality.
Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
Documentation/admin-guide/dynamic-debug-howto.rst | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/Documentation/admin-guide/dynamic-debug-howto.rst b/Documentation/admin-guide/dynamic-debug-howto.rst
index 0e9b48daf690..6a8ce5a34382 100644
--- a/Documentation/admin-guide/dynamic-debug-howto.rst
+++ b/Documentation/admin-guide/dynamic-debug-howto.rst
@@ -52,12 +52,12 @@ query/commands to the control file. Example::
# grease the interface
:#> alias ddcmd='echo $* > /proc/dynamic_debug/control'
- :#> ddcmd '-p; module main func run* +p'
+ :#> ddcmd '-p; module main func run* +p' # disable all, then enable main
:#> grep =p /proc/dynamic_debug/control
- init/main.c:1424 [main]run_init_process =p " with arguments:\012"
- init/main.c:1426 [main]run_init_process =p " %s\012"
- init/main.c:1427 [main]run_init_process =p " with environment:\012"
- init/main.c:1429 [main]run_init_process =p " %s\012"
+ init/main.c:1424 [main]run_init_process =p " with arguments:\n"
+ init/main.c:1426 [main]run_init_process =p " %s\n"
+ init/main.c:1427 [main]run_init_process =p " with environment:\n"
+ init/main.c:1429 [main]run_init_process =p " %s\n"
Error messages go to console/syslog::
--
2.44.0
^ permalink raw reply [flat|nested] 24+ messages in thread* [PATCH v8 03/35] test-dyndbg: fixup CLASSMAP usage error
2024-04-29 19:31 [PATCH v8 00/35] fix CONFIG_DRM_USE_DYNAMIC_DEBUG=y regression Jim Cromie
2024-04-29 19:31 ` [PATCH v8 01/35] dyndbg: fix old BUG_ON in >control parser Jim Cromie
2024-04-29 19:31 ` [PATCH v8 02/35] docs/dyndbg: update examples \012 to \n Jim Cromie
@ 2024-04-29 19:31 ` Jim Cromie
2024-04-29 19:31 ` [PATCH v8 04/35] dyndbg: reword "class unknown," to "class:_UNKNOWN_" Jim Cromie
` (19 subsequent siblings)
22 siblings, 0 replies; 24+ messages in thread
From: Jim Cromie @ 2024-04-29 19:31 UTC (permalink / raw)
To: jbaron, gregkh, linux-kernel, dri-devel, amd-gfx, intel-gvt-dev,
intel-gfx
Cc: ukaszb, linux-doc, daniel.vetter, tvrtko.ursulin, jani.nikula,
ville.syrjala, seanpaul, robdclark, groeck, yanivt, bleung,
Jim Cromie
A more careful reading of logging output from test_dynamic_debug.ko
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
107 says: HI is unknown, 105,106 have LOW/MID and MID/HI skew.
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. I coded in an error, intending to verify
err detection, then forgot, and missed that it was there.
So this patch fixes a bad usage of DECLARE_DYNDBG_CLASSMAP(), showing
that it is too error-prone. As noted in test-mod 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 !!
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.44.0
^ permalink raw reply [flat|nested] 24+ messages in thread* [PATCH v8 04/35] dyndbg: reword "class unknown," to "class:_UNKNOWN_"
2024-04-29 19:31 [PATCH v8 00/35] fix CONFIG_DRM_USE_DYNAMIC_DEBUG=y regression Jim Cromie
` (2 preceding siblings ...)
2024-04-29 19:31 ` [PATCH v8 03/35] test-dyndbg: fixup CLASSMAP usage error Jim Cromie
@ 2024-04-29 19:31 ` Jim Cromie
2024-04-29 19:31 ` [PATCH v8 05/35] dyndbg: make ddebug_class_param union members same size Jim Cromie
` (18 subsequent siblings)
22 siblings, 0 replies; 24+ messages in thread
From: Jim Cromie @ 2024-04-29 19:31 UTC (permalink / raw)
To: jbaron, gregkh, linux-kernel, dri-devel, amd-gfx, intel-gvt-dev,
intel-gfx
Cc: ukaszb, linux-doc, daniel.vetter, tvrtko.ursulin, jani.nikula,
ville.syrjala, seanpaul, robdclark, groeck, yanivt, bleung,
Jim Cromie
When a dyndbg classname is unknown to a kernel module (as before
previous patch), the callsite is un-addressable via >control queries.
The control-file displays this condition as "class unknown,"
currently. That spelling is sub-optimal, so change it to
"class:_UNKNOWN_" to loudly announce the erroneous situation, and to
make it exceedingly greppable.
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 f2c5e7910bb1..73ccf947d4aa 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -1154,7 +1154,7 @@ static int ddebug_proc_show(struct seq_file *m, void *p)
if (class)
seq_printf(m, " class:%s", class);
else
- seq_printf(m, " class unknown, _id:%d", dp->class_id);
+ seq_printf(m, " class:_UNKNOWN_ _id:%d", dp->class_id);
}
seq_puts(m, "\n");
--
2.44.0
^ permalink raw reply [flat|nested] 24+ messages in thread* [PATCH v8 05/35] dyndbg: make ddebug_class_param union members same size
2024-04-29 19:31 [PATCH v8 00/35] fix CONFIG_DRM_USE_DYNAMIC_DEBUG=y regression Jim Cromie
` (3 preceding siblings ...)
2024-04-29 19:31 ` [PATCH v8 04/35] dyndbg: reword "class unknown," to "class:_UNKNOWN_" Jim Cromie
@ 2024-04-29 19:31 ` Jim Cromie
2024-04-29 19:31 ` [PATCH v8 06/35] dyndbg: replace classmap list with a vector Jim Cromie
` (17 subsequent siblings)
22 siblings, 0 replies; 24+ messages in thread
From: Jim Cromie @ 2024-04-29 19:31 UTC (permalink / raw)
To: jbaron, gregkh, linux-kernel, dri-devel, amd-gfx, intel-gvt-dev,
intel-gfx
Cc: ukaszb, linux-doc, daniel.vetter, tvrtko.ursulin, jani.nikula,
ville.syrjala, seanpaul, robdclark, groeck, yanivt, bleung,
Jim Cromie
struct ddebug_class_param keeps a ref to the state-storage of the
param; make both class-types use the same unsigned long storage type.
ISTM this is simpler and safer.
Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
include/linux/dynamic_debug.h | 2 +-
lib/dynamic_debug.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index 4fcbf4d4fd0a..5231aaf361c4 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -124,7 +124,7 @@ struct _ddebug_info {
struct ddebug_class_param {
union {
unsigned long *bits;
- unsigned int *lvl;
+ unsigned long *lvl;
};
char flags[8];
const struct ddebug_class_map *map;
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 73ccf947d4aa..152b04c05981 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -799,7 +799,7 @@ int param_get_dyndbg_classes(char *buffer, const struct kernel_param *kp)
case DD_CLASS_TYPE_LEVEL_NAMES:
case DD_CLASS_TYPE_LEVEL_NUM:
- return scnprintf(buffer, PAGE_SIZE, "%d\n", *dcp->lvl);
+ return scnprintf(buffer, PAGE_SIZE, "%ld\n", *dcp->lvl);
default:
return -1;
}
--
2.44.0
^ permalink raw reply [flat|nested] 24+ messages in thread* [PATCH v8 06/35] dyndbg: replace classmap list with a vector
2024-04-29 19:31 [PATCH v8 00/35] fix CONFIG_DRM_USE_DYNAMIC_DEBUG=y regression Jim Cromie
` (4 preceding siblings ...)
2024-04-29 19:31 ` [PATCH v8 05/35] dyndbg: make ddebug_class_param union members same size Jim Cromie
@ 2024-04-29 19:31 ` Jim Cromie
2024-04-29 19:31 ` [PATCH v8 07/35] dyndbg: ddebug_apply_class_bitmap - add module arg, select on it Jim Cromie
` (16 subsequent siblings)
22 siblings, 0 replies; 24+ messages in thread
From: Jim Cromie @ 2024-04-29 19:31 UTC (permalink / raw)
To: jbaron, gregkh, linux-kernel, dri-devel, amd-gfx, intel-gvt-dev,
intel-gfx
Cc: ukaszb, linux-doc, daniel.vetter, tvrtko.ursulin, jani.nikula,
ville.syrjala, seanpaul, robdclark, groeck, yanivt, bleung,
Jim Cromie
Classmaps are stored in an elf section/array, but are individually
list-linked onto dyndbg's per-module ddebug_table for operation.
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.
IOW, this treats classmaps similarly to _ddebugs, which are already
kept as vector-refs (address+len).
So this changes:
struct ddebug_class_map drops list-head link.
struct ddebug_table drops the list-head maps, and gets: classes &
num_classes for the start-addy and num_classes, placed to improve
struct packing.
The loading: in ddebug_attach_module_classes(), replace the
for-the-modname list-add loop, with a forloop that finds the module's
subrange (start,length) of matching classmaps within the possibly
builtin classmaps vector, and saves those to the ddebug_table.
The reading/using: change list-foreach loops in ddebug_class_name() &
ddebug_find_valid_class() to walk the array from start to length.
Also:
Move #define __outvar up, above an added use in a fn-prototype.
Simplify ddebug_attach_module_classes args, ref has both addy,len.
no functional changes
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 5231aaf361c4..b53217e4b711 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -83,7 +83,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 152b04c05981..46e4cdd8e6be 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 {
@@ -147,13 +148,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;
@@ -164,7 +167,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
@@ -1114,9 +1116,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];
@@ -1200,30 +1203,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);
+ }
}
/*
@@ -1256,10 +1260,9 @@ static int ddebug_add_module(struct _ddebug_info *di, const char *modname)
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);
@@ -1372,8 +1375,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.44.0
^ permalink raw reply [flat|nested] 24+ messages in thread* [PATCH v8 07/35] dyndbg: ddebug_apply_class_bitmap - add module arg, select on it
2024-04-29 19:31 [PATCH v8 00/35] fix CONFIG_DRM_USE_DYNAMIC_DEBUG=y regression Jim Cromie
` (5 preceding siblings ...)
2024-04-29 19:31 ` [PATCH v8 06/35] dyndbg: replace classmap list with a vector Jim Cromie
@ 2024-04-29 19:31 ` Jim Cromie
2024-04-29 19:31 ` [PATCH v8 08/35] dyndbg: split param_set_dyndbg_classes to _module & wrapper fns Jim Cromie
` (15 subsequent siblings)
22 siblings, 0 replies; 24+ messages in thread
From: Jim Cromie @ 2024-04-29 19:31 UTC (permalink / raw)
To: jbaron, gregkh, linux-kernel, dri-devel, amd-gfx, intel-gvt-dev,
intel-gfx
Cc: ukaszb, linux-doc, daniel.vetter, tvrtko.ursulin, jani.nikula,
ville.syrjala, seanpaul, robdclark, groeck, yanivt, bleung,
Jim Cromie
Add param: query_module to ddebug_apply_class_bitmap(), and pass it
thru to _ddebug_queries(), replacing NULL with query_module. This
allows its caller to update just one module, or all (as currently).
We'll use this later to propagate drm.debug to each USEr as they're
modprobed.
No functional change.
Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
after `modprobe i915`, heres the module dependencies,
though not all on drm.debug.
bash-5.2# lsmod
Module Size Used by
i915 3133440 0
drm_buddy 20480 1 i915
ttm 90112 1 i915
i2c_algo_bit 16384 1 i915
video 61440 1 i915
wmi 32768 1 video
drm_display_helper 200704 1 i915
drm_kms_helper 208896 2 drm_display_helper,i915
drm 606208 5 drm_kms_helper,drm_display_helper,drm_buddy,i915,ttm
cec 57344 2 drm_display_helper,i915
---
lib/dynamic_debug.c | 19 ++++++++++++-------
1 file changed, 12 insertions(+), 7 deletions(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 46e4cdd8e6be..a1fd2e9dbafb 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -605,7 +605,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];
@@ -613,7 +614,8 @@ static int ddebug_apply_class_bitmap(const struct ddebug_class_param *dcp,
int matches = 0;
int bi, ct;
- v2pr_info("apply: 0x%lx to: 0x%lx\n", *new_bits, *old_bits);
+ v2pr_info("apply bitmap: 0x%lx to: 0x%lx for %s\n", *new_bits, *old_bits,
+ query_modname ?: "");
for (bi = 0; bi < map->length; bi++) {
if (test_bit(bi, new_bits) == test_bit(bi, old_bits))
@@ -622,12 +624,15 @@ static int ddebug_apply_class_bitmap(const struct ddebug_class_param *dcp,
snprintf(query, QUERY_SIZE, "class %s %c%s", map->class_names[bi],
test_bit(bi, new_bits) ? '+' : '-', dcp->flags);
- ct = ddebug_exec_queries(query, NULL);
+ ct = ddebug_exec_queries(query, query_modname);
matches += ct;
v2pr_info("bit_%d: %d matches on class: %s -> 0x%lx\n", bi,
ct, map->class_names[bi], *new_bits);
}
+ v2pr_info("applied bitmap: 0x%lx to: 0x%lx for %s\n", *new_bits, *old_bits,
+ query_modname ?: "");
+
return matches;
}
@@ -682,7 +687,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]);
@@ -692,7 +697,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);
@@ -755,7 +760,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:
@@ -768,7 +773,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.44.0
^ permalink raw reply [flat|nested] 24+ messages in thread* [PATCH v8 08/35] dyndbg: split param_set_dyndbg_classes to _module & wrapper fns
2024-04-29 19:31 [PATCH v8 00/35] fix CONFIG_DRM_USE_DYNAMIC_DEBUG=y regression Jim Cromie
` (6 preceding siblings ...)
2024-04-29 19:31 ` [PATCH v8 07/35] dyndbg: ddebug_apply_class_bitmap - add module arg, select on it Jim Cromie
@ 2024-04-29 19:31 ` Jim Cromie
2024-04-29 19:31 ` [PATCH v8 09/35] dyndbg: drop NUM_TYPE_ARRAY Jim Cromie
` (14 subsequent siblings)
22 siblings, 0 replies; 24+ messages in thread
From: Jim Cromie @ 2024-04-29 19:31 UTC (permalink / raw)
To: jbaron, gregkh, linux-kernel, dri-devel, amd-gfx, intel-gvt-dev,
intel-gfx
Cc: ukaszb, linux-doc, daniel.vetter, tvrtko.ursulin, jani.nikula,
ville.syrjala, seanpaul, robdclark, groeck, yanivt, bleung,
Jim Cromie
Split api-fn: param_set_dyndbg_classes(), adding modname param and
passing NULL in from api-fn.
The new arg allows caller to specify that only one module is affected
by a prdbgs update. This selectivity will be used later to narrow the
scope of changes made.
no functional change.
Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
lib/dynamic_debug.c | 37 ++++++++++++++++++++++---------------
1 file changed, 22 insertions(+), 15 deletions(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index a1fd2e9dbafb..4a48f830507f 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -711,18 +711,9 @@ static int param_set_dyndbg_classnames(const char *instr, const struct kernel_pa
return 0;
}
-/**
- * param_set_dyndbg_classes - class FOO >control
- * @instr: string echo>d to sysfs, input depends on map_type
- * @kp: kp->arg has state: bits/lvl, map, map_type
- *
- * Enable/disable prdbgs by their class, as given in the arguments to
- * DECLARE_DYNDBG_CLASSMAP. For LEVEL map-types, enforce relative
- * levels by bitpos.
- *
- * Returns: 0 or <0 if error.
- */
-int param_set_dyndbg_classes(const char *instr, const struct kernel_param *kp)
+static int param_set_dyndbg_module_classes(const char *instr,
+ const struct kernel_param *kp,
+ const char *modnm)
{
const struct ddebug_class_param *dcp = kp->arg;
const struct ddebug_class_map *map = dcp->map;
@@ -759,8 +750,8 @@ int param_set_dyndbg_classes(const char *instr, const struct kernel_param *kp)
KP_NAME(kp), inrep, CLASSMAP_BITMASK(map->length));
inrep &= CLASSMAP_BITMASK(map->length);
}
- v2pr_info("bits:%lx > %s\n", inrep, KP_NAME(kp));
- totct += ddebug_apply_class_bitmap(dcp, &inrep, dcp->bits, NULL);
+ v2pr_info("bits:0x%lx > %s.%s\n", inrep, modnm ?: "*", KP_NAME(kp));
+ totct += ddebug_apply_class_bitmap(dcp, &inrep, dcp->bits, modnm);
*dcp->bits = inrep;
break;
case DD_CLASS_TYPE_LEVEL_NUM:
@@ -773,7 +764,7 @@ int param_set_dyndbg_classes(const char *instr, const struct kernel_param *kp)
old_bits = CLASSMAP_BITMASK(*dcp->lvl);
new_bits = CLASSMAP_BITMASK(inrep);
v2pr_info("lvl:%ld bits:0x%lx > %s\n", inrep, new_bits, KP_NAME(kp));
- totct += ddebug_apply_class_bitmap(dcp, &new_bits, &old_bits, NULL);
+ totct += ddebug_apply_class_bitmap(dcp, &new_bits, &old_bits, modnm);
*dcp->lvl = inrep;
break;
default:
@@ -782,6 +773,22 @@ int param_set_dyndbg_classes(const char *instr, const struct kernel_param *kp)
vpr_info("%s: total matches: %d\n", KP_NAME(kp), totct);
return 0;
}
+
+/**
+ * param_set_dyndbg_classes - class FOO >control
+ * @instr: string echo>d to sysfs, input depends on map_type
+ * @kp: kp->arg has state: bits/lvl, map, map_type
+ *
+ * Enable/disable prdbgs by their class, as given in the arguments to
+ * DECLARE_DYNDBG_CLASSMAP. For LEVEL map-types, enforce relative
+ * levels by bitpos.
+ *
+ * Returns: 0 or <0 if error.
+ */
+int param_set_dyndbg_classes(const char *instr, const struct kernel_param *kp)
+{
+ return param_set_dyndbg_module_classes(instr, kp, NULL);
+}
EXPORT_SYMBOL(param_set_dyndbg_classes);
/**
--
2.44.0
^ permalink raw reply [flat|nested] 24+ messages in thread* [PATCH v8 09/35] dyndbg: drop NUM_TYPE_ARRAY
2024-04-29 19:31 [PATCH v8 00/35] fix CONFIG_DRM_USE_DYNAMIC_DEBUG=y regression Jim Cromie
` (7 preceding siblings ...)
2024-04-29 19:31 ` [PATCH v8 08/35] dyndbg: split param_set_dyndbg_classes to _module & wrapper fns Jim Cromie
@ 2024-04-29 19:31 ` Jim Cromie
2024-04-29 19:31 ` [PATCH v8 10/35] dyndbg: reduce verbose/debug clutter Jim Cromie
` (13 subsequent siblings)
22 siblings, 0 replies; 24+ messages in thread
From: Jim Cromie @ 2024-04-29 19:31 UTC (permalink / raw)
To: jbaron, gregkh, linux-kernel, dri-devel, amd-gfx, intel-gvt-dev,
intel-gfx
Cc: ukaszb, linux-doc, daniel.vetter, tvrtko.ursulin, jani.nikula,
ville.syrjala, seanpaul, robdclark, groeck, yanivt, bleung,
Jim Cromie
ARRAY_SIZE works here, since array decl is complete.
no functional change
Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
include/linux/dynamic_debug.h | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index b53217e4b711..8116d0a0d33a 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -106,11 +106,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.44.0
^ permalink raw reply [flat|nested] 24+ messages in thread* [PATCH v8 10/35] dyndbg: reduce verbose/debug clutter
2024-04-29 19:31 [PATCH v8 00/35] fix CONFIG_DRM_USE_DYNAMIC_DEBUG=y regression Jim Cromie
` (8 preceding siblings ...)
2024-04-29 19:31 ` [PATCH v8 09/35] dyndbg: drop NUM_TYPE_ARRAY Jim Cromie
@ 2024-04-29 19:31 ` Jim Cromie
2024-04-29 19:31 ` [PATCH v8 11/35] dyndbg: silence debugs with no-change updates Jim Cromie
` (12 subsequent siblings)
22 siblings, 0 replies; 24+ messages in thread
From: Jim Cromie @ 2024-04-29 19:31 UTC (permalink / raw)
To: jbaron, gregkh, linux-kernel, dri-devel, amd-gfx, intel-gvt-dev,
intel-gfx
Cc: ukaszb, linux-doc, daniel.vetter, tvrtko.ursulin, jani.nikula,
ville.syrjala, seanpaul, robdclark, groeck, yanivt, bleung,
Jim Cromie
currently, for verbose=3, these are logged (blank lines for clarity):
dyndbg: query 0: "class DRM_UT_CORE +p" mod:*
dyndbg: split into words: "class" "DRM_UT_CORE" "+p"
dyndbg: op='+'
dyndbg: flags=0x1
dyndbg: *flagsp=0x1 *maskp=0xffffffff
dyndbg: parsed: func="" file="" module="" format="" lineno=0-0 class=...
dyndbg: no matches for query
dyndbg: no-match: func="" file="" module="" format="" lineno=0-0 class=...
dyndbg: processed 1 queries, with 0 matches, 0 errs
That is excessive, so this patch:
- shrinks 3 lines of 2nd stanza to single line
- drops 1st 2 lines of 3rd stanza
3rd line is like 1st, with result, not procedure.
2nd line is just status, retold in 4th, with more info.
New output:
dyndbg: query 0: "class DRM_UT_CORE +p" mod:*
dyndbg: split into words: "class" "DRM_UT_CORE" "+p"
dyndbg: op='+' flags=0x1 *flagsp=0x1 *maskp=0xffffffff
dyndbg: no-match: func="" file="" module="" format="" lineno=0-0 class=...
dyndbg: processed 1 queries, with 0 matches, 0 errs
no functional change.
Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
lib/dynamic_debug.c | 14 +++-----------
1 file changed, 3 insertions(+), 11 deletions(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 4a48f830507f..368381dbd266 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -266,9 +266,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;
}
@@ -501,7 +498,6 @@ static int ddebug_parse_flags(const char *str, struct flag_settings *modifiers)
pr_err("bad flag-op %c, at start of %s\n", *str, str);
return -EINVAL;
}
- v3pr_info("op='%c'\n", op);
for (; *str ; ++str) {
for (i = ARRAY_SIZE(opt_array) - 1; i >= 0; i--) {
@@ -515,7 +511,6 @@ static int ddebug_parse_flags(const char *str, struct flag_settings *modifiers)
return -EINVAL;
}
}
- v3pr_info("flags=0x%x\n", modifiers->flags);
/* calculate final flags, mask based upon op */
switch (op) {
@@ -531,7 +526,7 @@ static int ddebug_parse_flags(const char *str, struct flag_settings *modifiers)
modifiers->flags = 0;
break;
}
- v3pr_info("*flagsp=0x%x *maskp=0x%x\n", modifiers->flags, modifiers->mask);
+ v3pr_info("op='%c' flags=0x%x maskp=0x%x\n", op, modifiers->flags, modifiers->mask);
return 0;
}
@@ -541,7 +536,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);
@@ -559,10 +554,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.44.0
^ permalink raw reply [flat|nested] 24+ messages in thread* [PATCH v8 11/35] dyndbg: silence debugs with no-change updates
2024-04-29 19:31 [PATCH v8 00/35] fix CONFIG_DRM_USE_DYNAMIC_DEBUG=y regression Jim Cromie
` (9 preceding siblings ...)
2024-04-29 19:31 ` [PATCH v8 10/35] dyndbg: reduce verbose/debug clutter Jim Cromie
@ 2024-04-29 19:31 ` Jim Cromie
2024-04-29 19:31 ` [PATCH v8 12/35] dyndbg: tighten ddebug_class_name() 1st arg type Jim Cromie
` (11 subsequent siblings)
22 siblings, 0 replies; 24+ messages in thread
From: Jim Cromie @ 2024-04-29 19:31 UTC (permalink / raw)
To: jbaron, gregkh, linux-kernel, dri-devel, amd-gfx, intel-gvt-dev,
intel-gfx
Cc: ukaszb, linux-doc, daniel.vetter, tvrtko.ursulin, jani.nikula,
ville.syrjala, seanpaul, robdclark, groeck, yanivt, bleung,
Jim Cromie
In ddebug_apply_class_bitmap(), check for actual changes to the bits
before announcing them, to declutter logs.
no functional change.
Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
lib/dynamic_debug.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 368381dbd266..8320cadeb251 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -595,7 +595,7 @@ static int ddebug_exec_queries(char *query, const char *modname)
return nfound;
}
-/* apply a new bitmap to the sys-knob's current bit-state */
+/* apply a new class-param setting */
static int ddebug_apply_class_bitmap(const struct ddebug_class_param *dcp,
unsigned long *new_bits, unsigned long *old_bits,
const char *query_modname)
@@ -606,8 +606,9 @@ static int ddebug_apply_class_bitmap(const struct ddebug_class_param *dcp,
int matches = 0;
int bi, ct;
- v2pr_info("apply bitmap: 0x%lx to: 0x%lx for %s\n", *new_bits, *old_bits,
- query_modname ?: "");
+ if (*new_bits != *old_bits)
+ v2pr_info("apply bitmap: 0x%lx to: 0x%lx for %s\n", *new_bits,
+ *old_bits, query_modname ?: "'*'");
for (bi = 0; bi < map->length; bi++) {
if (test_bit(bi, new_bits) == test_bit(bi, old_bits))
@@ -622,8 +623,9 @@ static int ddebug_apply_class_bitmap(const struct ddebug_class_param *dcp,
v2pr_info("bit_%d: %d matches on class: %s -> 0x%lx\n", bi,
ct, map->class_names[bi], *new_bits);
}
- v2pr_info("applied bitmap: 0x%lx to: 0x%lx for %s\n", *new_bits, *old_bits,
- query_modname ?: "");
+ if (*new_bits != *old_bits)
+ v2pr_info("applied bitmap: 0x%lx to: 0x%lx for %s\n", *new_bits,
+ *old_bits, query_modname ?: "'*'");
return matches;
}
--
2.44.0
^ permalink raw reply [flat|nested] 24+ messages in thread* [PATCH v8 12/35] dyndbg: tighten ddebug_class_name() 1st arg type
2024-04-29 19:31 [PATCH v8 00/35] fix CONFIG_DRM_USE_DYNAMIC_DEBUG=y regression Jim Cromie
` (10 preceding siblings ...)
2024-04-29 19:31 ` [PATCH v8 11/35] dyndbg: silence debugs with no-change updates Jim Cromie
@ 2024-04-29 19:31 ` Jim Cromie
2024-04-29 19:31 ` [PATCH v8 13/35] dyndbg: tighten fn-sig of ddebug_apply_class_bitmap Jim Cromie
` (10 subsequent siblings)
22 siblings, 0 replies; 24+ messages in thread
From: Jim Cromie @ 2024-04-29 19:31 UTC (permalink / raw)
To: jbaron, gregkh, linux-kernel, dri-devel, amd-gfx, intel-gvt-dev,
intel-gfx
Cc: ukaszb, linux-doc, daniel.vetter, tvrtko.ursulin, jani.nikula,
ville.syrjala, seanpaul, robdclark, groeck, yanivt, bleung,
Jim Cromie
Change function's 1st arg-type, and deref in the caller.
The fn doesn't need any other fields in the struct.
no functional change.
Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
lib/dynamic_debug.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 8320cadeb251..882354e1e78f 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -1120,12 +1120,12 @@ static void *ddebug_proc_next(struct seq_file *m, void *p, loff_t *pos)
#define class_in_range(class_id, map) \
(class_id >= map->base && class_id < map->base + map->length)
-static const char *ddebug_class_name(struct ddebug_iter *iter, struct _ddebug *dp)
+static const char *ddebug_class_name(struct ddebug_table *dt, struct _ddebug *dp)
{
- struct ddebug_class_map *map = iter->table->classes;
- int i, nc = iter->table->num_classes;
+ struct ddebug_class_map *map = dt->classes;
+ int i;
- for (i = 0; i < nc; i++, map++)
+ for (i = 0; i < dt->num_classes; i++, map++)
if (class_in_range(dp->class_id, map))
return map->class_names[dp->class_id - map->base];
@@ -1159,7 +1159,7 @@ static int ddebug_proc_show(struct seq_file *m, void *p)
seq_puts(m, "\"");
if (dp->class_id != _DPRINTK_CLASS_DFLT) {
- class = ddebug_class_name(iter, dp);
+ class = ddebug_class_name(iter->table, dp);
if (class)
seq_printf(m, " class:%s", class);
else
--
2.44.0
^ permalink raw reply [flat|nested] 24+ messages in thread* [PATCH v8 13/35] dyndbg: tighten fn-sig of ddebug_apply_class_bitmap
2024-04-29 19:31 [PATCH v8 00/35] fix CONFIG_DRM_USE_DYNAMIC_DEBUG=y regression Jim Cromie
` (11 preceding siblings ...)
2024-04-29 19:31 ` [PATCH v8 12/35] dyndbg: tighten ddebug_class_name() 1st arg type Jim Cromie
@ 2024-04-29 19:31 ` Jim Cromie
2024-04-29 19:31 ` [PATCH v8 14/35] dyndbg: reduce verbose=3 messages in ddebug_add_module Jim Cromie
` (9 subsequent siblings)
22 siblings, 0 replies; 24+ messages in thread
From: Jim Cromie @ 2024-04-29 19:31 UTC (permalink / raw)
To: jbaron, gregkh, linux-kernel, dri-devel, amd-gfx, intel-gvt-dev,
intel-gfx
Cc: ukaszb, linux-doc, daniel.vetter, tvrtko.ursulin, jani.nikula,
ville.syrjala, seanpaul, robdclark, groeck, yanivt, bleung,
Jim Cromie
old_bits arg is currently a pointer to the input bits, but this could
allow inadvertent changes to the input by the fn. Disallow this.
And constify new_bits while here.
Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
lib/dynamic_debug.c | 21 +++++++++++----------
1 file changed, 11 insertions(+), 10 deletions(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 882354e1e78f..d4a0ae31d059 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -597,7 +597,8 @@ static int ddebug_exec_queries(char *query, const char *modname)
/* apply a new class-param setting */
static int ddebug_apply_class_bitmap(const struct ddebug_class_param *dcp,
- unsigned long *new_bits, unsigned long *old_bits,
+ const unsigned long *new_bits,
+ const unsigned long old_bits,
const char *query_modname)
{
#define QUERY_SIZE 128
@@ -606,12 +607,12 @@ static int ddebug_apply_class_bitmap(const struct ddebug_class_param *dcp,
int matches = 0;
int bi, ct;
- if (*new_bits != *old_bits)
+ if (*new_bits != old_bits)
v2pr_info("apply bitmap: 0x%lx to: 0x%lx for %s\n", *new_bits,
- *old_bits, query_modname ?: "'*'");
+ old_bits, query_modname ?: "'*'");
for (bi = 0; bi < map->length; bi++) {
- if (test_bit(bi, new_bits) == test_bit(bi, old_bits))
+ if (test_bit(bi, new_bits) == test_bit(bi, &old_bits))
continue;
snprintf(query, QUERY_SIZE, "class %s %c%s", map->class_names[bi],
@@ -623,9 +624,9 @@ static int ddebug_apply_class_bitmap(const struct ddebug_class_param *dcp,
v2pr_info("bit_%d: %d matches on class: %s -> 0x%lx\n", bi,
ct, map->class_names[bi], *new_bits);
}
- if (*new_bits != *old_bits)
+ if (*new_bits != old_bits)
v2pr_info("applied bitmap: 0x%lx to: 0x%lx for %s\n", *new_bits,
- *old_bits, query_modname ?: "'*'");
+ old_bits, query_modname ?: "'*'");
return matches;
}
@@ -681,7 +682,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, NULL);
+ 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]);
@@ -691,7 +692,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, NULL);
+ 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);
@@ -745,7 +746,7 @@ static int param_set_dyndbg_module_classes(const char *instr,
inrep &= CLASSMAP_BITMASK(map->length);
}
v2pr_info("bits:0x%lx > %s.%s\n", inrep, modnm ?: "*", KP_NAME(kp));
- totct += ddebug_apply_class_bitmap(dcp, &inrep, dcp->bits, modnm);
+ totct += ddebug_apply_class_bitmap(dcp, &inrep, *dcp->bits, modnm);
*dcp->bits = inrep;
break;
case DD_CLASS_TYPE_LEVEL_NUM:
@@ -758,7 +759,7 @@ static int param_set_dyndbg_module_classes(const char *instr,
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, modnm);
+ totct += ddebug_apply_class_bitmap(dcp, &new_bits, old_bits, modnm);
*dcp->lvl = inrep;
break;
default:
--
2.44.0
^ permalink raw reply [flat|nested] 24+ messages in thread* [PATCH v8 14/35] dyndbg: reduce verbose=3 messages in ddebug_add_module
2024-04-29 19:31 [PATCH v8 00/35] fix CONFIG_DRM_USE_DYNAMIC_DEBUG=y regression Jim Cromie
` (12 preceding siblings ...)
2024-04-29 19:31 ` [PATCH v8 13/35] dyndbg: tighten fn-sig of ddebug_apply_class_bitmap Jim Cromie
@ 2024-04-29 19:31 ` Jim Cromie
2024-04-29 19:31 ` [PATCH v8 15/35] dyndbg-API: remove DD_CLASS_TYPE_(DISJOINT|LEVEL)_NAMES and code Jim Cromie
` (8 subsequent siblings)
22 siblings, 0 replies; 24+ messages in thread
From: Jim Cromie @ 2024-04-29 19:31 UTC (permalink / raw)
To: jbaron, gregkh, linux-kernel, dri-devel, amd-gfx, intel-gvt-dev,
intel-gfx
Cc: ukaszb, linux-doc, daniel.vetter, tvrtko.ursulin, jani.nikula,
ville.syrjala, seanpaul, robdclark, groeck, yanivt, bleung,
Jim Cromie
When modprobing a module, dyndbg currently logs/says "add-module", and
then "skipping" if the module has no prdbgs. Instead just check 1st
and return quietly.
no functional change
Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
lib/dynamic_debug.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index d4a0ae31d059..43a8e04b8599 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -1245,11 +1245,10 @@ static int ddebug_add_module(struct _ddebug_info *di, const char *modname)
{
struct ddebug_table *dt;
- v3pr_info("add-module: %s.%d sites\n", modname, di->num_descs);
- if (!di->num_descs) {
- v3pr_info(" skip %s\n", modname);
+ if (!di->num_descs)
return 0;
- }
+
+ v3pr_info("add-module: %s %d sites\n", modname, di->num_descs);
dt = kzalloc(sizeof(*dt), GFP_KERNEL);
if (dt == NULL) {
--
2.44.0
^ permalink raw reply [flat|nested] 24+ messages in thread* [PATCH v8 15/35] dyndbg-API: remove DD_CLASS_TYPE_(DISJOINT|LEVEL)_NAMES and code
2024-04-29 19:31 [PATCH v8 00/35] fix CONFIG_DRM_USE_DYNAMIC_DEBUG=y regression Jim Cromie
` (13 preceding siblings ...)
2024-04-29 19:31 ` [PATCH v8 14/35] dyndbg: reduce verbose=3 messages in ddebug_add_module Jim Cromie
@ 2024-04-29 19:31 ` Jim Cromie
2024-04-29 19:31 ` [PATCH v8 16/35] dyndbg-API: fix DECLARE_DYNDBG_CLASSMAP Jim Cromie
` (7 subsequent siblings)
22 siblings, 0 replies; 24+ messages in thread
From: Jim Cromie @ 2024-04-29 19:31 UTC (permalink / raw)
To: jbaron, gregkh, linux-kernel, dri-devel, amd-gfx, intel-gvt-dev,
intel-gfx
Cc: ukaszb, linux-doc, daniel.vetter, tvrtko.ursulin, jani.nikula,
ville.syrjala, seanpaul, robdclark, groeck, yanivt, bleung,
Jim Cromie
Remove the NAMED class types; these 2 classmap types accept class
names at the PARAM interface, for example:
echo +DRM_UT_CORE,-DRM_UT_KMS > /sys/module/drm/parameters/debug_names
The code works, but its only used by test-dynamic-debug, and wasn't
asked for by anyone else, so reduce test-surface, simplify things.
also rename enum class_map_type to enum ddebug_class_map_type.
Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
include/linux/dynamic_debug.h | 23 ++------
lib/dynamic_debug.c | 102 +++-------------------------------
lib/test_dynamic_debug.c | 26 ---------
3 files changed, 14 insertions(+), 137 deletions(-)
diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index 8116d0a0d33a..dd304e231f08 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -58,27 +58,16 @@ 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.
- * expecting hex input. Built for drm.debug, basis for other types.
+ * DD_CLASS_TYPE_DISJOINT_BITS: classes are independent, mapped to bits[0..N].
+ * Expects hex input. Built for drm.debug, basis for other types.
*/
DD_CLASS_TYPE_LEVEL_NUM,
/**
- * DD_CLASS_TYPE_LEVEL_NUM: input is numeric level, 0-N.
- * N turns on just bits N-1 .. 0, so N=0 turns all bits off.
- */
- DD_CLASS_TYPE_DISJOINT_NAMES,
- /**
- * DD_CLASS_TYPE_DISJOINT_NAMES: input is a CSV of [+-]CLASS_NAMES,
- * classes are independent, like _DISJOINT_BITS.
- */
- DD_CLASS_TYPE_LEVEL_NAMES,
- /**
- * DD_CLASS_TYPE_LEVEL_NAMES: input is a CSV of [+-]CLASS_NAMES,
- * intended for names like: INFO,DEBUG,TRACE, with a module prefix
- * avoid EMERG,ALERT,CRIT,ERR,WARNING: they're not debug
+ * DD_CLASS_TYPE_LEVEL_NUM: input is numeric level, 0..N.
+ * Input N turns on bits 0..N-1
*/
};
@@ -88,7 +77,7 @@ 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;
};
/**
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 43a8e04b8599..d5701207febc 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -636,76 +636,6 @@ static int ddebug_apply_class_bitmap(const struct ddebug_class_param *dcp,
#define CLASSMAP_BITMASK(width) ((1UL << (width)) - 1)
-/* accept comma-separated-list of [+-] classnames */
-static int param_set_dyndbg_classnames(const char *instr, const struct kernel_param *kp)
-{
- const struct ddebug_class_param *dcp = kp->arg;
- const struct ddebug_class_map *map = dcp->map;
- unsigned long curr_bits, old_bits;
- char *cl_str, *p, *tmp;
- int cls_id, totct = 0;
- bool wanted;
-
- cl_str = tmp = kstrdup_and_replace(instr, '\n', '\0', GFP_KERNEL);
- if (!tmp)
- return -ENOMEM;
-
- /* start with previously set state-bits, then modify */
- curr_bits = old_bits = *dcp->bits;
- vpr_info("\"%s\" > %s:0x%lx\n", cl_str, KP_NAME(kp), curr_bits);
-
- for (; cl_str; cl_str = p) {
- p = strchr(cl_str, ',');
- if (p)
- *p++ = '\0';
-
- if (*cl_str == '-') {
- wanted = false;
- cl_str++;
- } else {
- wanted = true;
- if (*cl_str == '+')
- cl_str++;
- }
- cls_id = match_string(map->class_names, map->length, cl_str);
- if (cls_id < 0) {
- pr_err("%s unknown to %s\n", cl_str, KP_NAME(kp));
- continue;
- }
-
- /* have one or more valid class_ids of one *_NAMES type */
- switch (map->map_type) {
- case DD_CLASS_TYPE_DISJOINT_NAMES:
- /* the +/- pertains to a single bit */
- if (test_bit(cls_id, &curr_bits) == wanted) {
- v3pr_info("no change on %s\n", cl_str);
- continue;
- }
- curr_bits ^= BIT(cls_id);
- 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]);
- break;
- case DD_CLASS_TYPE_LEVEL_NAMES:
- /* cls_id = N in 0..max. wanted +/- determines N or N-1 */
- 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, 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);
- break;
- default:
- pr_err("illegal map-type value %d\n", map->map_type);
- }
- }
- kfree(tmp);
- vpr_info("total matches: %d\n", totct);
- return 0;
-}
-
static int param_set_dyndbg_module_classes(const char *instr,
const struct kernel_param *kp,
const char *modnm)
@@ -714,29 +644,17 @@ static int param_set_dyndbg_module_classes(const char *instr,
const struct ddebug_class_map *map = dcp->map;
unsigned long inrep, new_bits, old_bits;
int rc, totct = 0;
-
- switch (map->map_type) {
-
- case DD_CLASS_TYPE_DISJOINT_NAMES:
- case DD_CLASS_TYPE_LEVEL_NAMES:
- /* handle [+-]classnames list separately, we are done here */
- return param_set_dyndbg_classnames(instr, kp);
-
- case DD_CLASS_TYPE_DISJOINT_BITS:
- case DD_CLASS_TYPE_LEVEL_NUM:
- /* numeric input, accept and fall-thru */
- rc = kstrtoul(instr, 0, &inrep);
- if (rc) {
- pr_err("expecting numeric input: %s > %s\n", instr, KP_NAME(kp));
- return -EINVAL;
- }
- break;
- default:
- pr_err("%s: bad map type: %d\n", KP_NAME(kp), map->map_type);
+ char *nl;
+
+ rc = kstrtoul(instr, 0, &inrep);
+ if (rc) {
+ nl = strchr(instr, '\n');
+ if (nl)
+ *nl = '\0';
+ pr_err("expecting numeric input, not: %s > %s\n", instr, KP_NAME(kp));
return -EINVAL;
}
- /* only _BITS,_NUM (numeric) map-types get here */
switch (map->map_type) {
case DD_CLASS_TYPE_DISJOINT_BITS:
/* expect bits. mask and warn if too many */
@@ -801,12 +719,8 @@ int param_get_dyndbg_classes(char *buffer, const struct kernel_param *kp)
const struct ddebug_class_map *map = dcp->map;
switch (map->map_type) {
-
- case DD_CLASS_TYPE_DISJOINT_NAMES:
case DD_CLASS_TYPE_DISJOINT_BITS:
return scnprintf(buffer, PAGE_SIZE, "0x%lx\n", *dcp->bits);
-
- case DD_CLASS_TYPE_LEVEL_NAMES:
case DD_CLASS_TYPE_LEVEL_NUM:
return scnprintf(buffer, PAGE_SIZE, "%ld\n", *dcp->lvl);
default:
diff --git a/lib/test_dynamic_debug.c b/lib/test_dynamic_debug.c
index a01f0193a419..9e950a911b6c 100644
--- a/lib/test_dynamic_debug.c
+++ b/lib/test_dynamic_debug.c
@@ -74,13 +74,6 @@ DECLARE_DYNDBG_CLASSMAP(map_disjoint_bits, DD_CLASS_TYPE_DISJOINT_BITS, 0,
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");
-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,
@@ -88,13 +81,6 @@ DECLARE_DYNDBG_CLASSMAP(map_level_num, DD_CLASS_TYPE_LEVEL_NUM, 14,
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");
-DD_SYS_WRAP(level_names, p);
-DD_SYS_WRAP(level_names, T);
-
/* stand-in for all pr_debug etc */
#define prdbg(SYM) __pr_debug_cls(SYM, #SYM " msg\n")
@@ -102,10 +88,6 @@ static void do_cats(void)
{
pr_debug("doing categories\n");
- prdbg(LOW);
- prdbg(MID);
- prdbg(HI);
-
prdbg(D2_CORE);
prdbg(D2_DRIVER);
prdbg(D2_KMS);
@@ -129,14 +111,6 @@ static void do_levels(void)
prdbg(V5);
prdbg(V6);
prdbg(V7);
-
- prdbg(L1);
- prdbg(L2);
- prdbg(L3);
- prdbg(L4);
- prdbg(L5);
- prdbg(L6);
- prdbg(L7);
}
static void do_prints(void)
--
2.44.0
^ permalink raw reply [flat|nested] 24+ messages in thread* [PATCH v8 16/35] dyndbg-API: fix DECLARE_DYNDBG_CLASSMAP
2024-04-29 19:31 [PATCH v8 00/35] fix CONFIG_DRM_USE_DYNAMIC_DEBUG=y regression Jim Cromie
` (14 preceding siblings ...)
2024-04-29 19:31 ` [PATCH v8 15/35] dyndbg-API: remove DD_CLASS_TYPE_(DISJOINT|LEVEL)_NAMES and code Jim Cromie
@ 2024-04-29 19:31 ` Jim Cromie
2024-04-29 19:31 ` [PATCH v8 17/35] selftests-dyndbg: add tools/testing/selftests/dynamic_debug/* Jim Cromie
` (6 subsequent siblings)
22 siblings, 0 replies; 24+ messages in thread
From: Jim Cromie @ 2024-04-29 19:31 UTC (permalink / raw)
To: jbaron, gregkh, linux-kernel, dri-devel, amd-gfx, intel-gvt-dev,
intel-gfx
Cc: ukaszb, linux-doc, daniel.vetter, tvrtko.ursulin, jani.nikula,
ville.syrjala, seanpaul, robdclark, groeck, yanivt, bleung,
Jim Cromie
DECLARE_DYNDBG_CLASSMAP() has a design error; its usage fails a basic
K&R rule: "define once, refer many times".
It is used across DRM core & drivers, each use re-defines the classmap
understood by that module; and all must match for the modules to
respond together when DRM.debug categories are enabled. This is
brittle, rubbish.
Worse, it causes the CONFIG_DRM_USE_DYNAMIC_DEBUG=Y regression; 1st
drm.ko loads, and dyndbg initializes its DRM.debug callsites, then a
drm-driver loads, but too late - it missed the DRM.debug enablement.
So replace it with 2 macros:
DYNDBG_CLASSMAP_DEFINE - invoked once from core - drm.ko
DYNDBG_CLASSMAP_USE - from all drm drivers and helpers.
DYNDBG_CLASSMAP_DEFINE: it reuses a renamed DECLARE_DYNDBG_CLASSMAP to
construct the struct classmap variable, but it drops the static
qualifier, and exports it instead.
DYNDBG_CLASSMAP_USE: then refers to the exported var by name:
* used from drivers, helper-mods
* lets us drop the repetitive "classname" args
* fixes 2nd-defn problem
* creates a ddebug_class_user record in new __dyndbg_class_users section
this allows ddebug_add_module(etal) to handle them per-module.
DECLARE_DYNDBG_CLASSMAP is preserved temporarily, to decouple DRM
adaptation work and avoid compile-errs before its done. IOW, DRM
gets fixed when they commit the adopt-new-api patches.
The DEFINE,USE distinction, and the separate usage record, allows
dyndbg to initialize the drivers & helpers DRM.debug callsites
separately after each is modprobed.
Basically, the classmap init-scan repeated for classmap-users.
To review, dyndbg's existing __dyndbg_classes[] section does:
. catalogs the classmaps defined by a module (or builtin modules)
. authorizes dyndbg to >control those class'd prdbgs for the module.
. DYNDBG_CLASSMAP_DEFINE creates classmaps in this section.
This patch adds __dyndbg_class_users[] section:
. catalogs uses/references to the classmap definitions.
. authorizes dyndbg to >control those class'd prdbgs in ref'g module.
. DYNDBG_CLASSMAP_USE() creates classmap-user records in this new section.
Now ddebug_add_module(etal) can handle classmap-uses similar to (and
after) classmaps; when a dependent module is loaded, its parent's
kernel params are scanned to find if a param is wired to dyndbg's
param-ops, whose classmap ref is the one being looked for.
To support this, theres a few data/header changes:
new struct ddebug_class_user
contains: user-module-name, &classmap-defn
it records drm-driver's use of a classmap in the section, allowing lookup
struct ddebug_info gets 2 more fields to keep the new section with the others:
class_users, num_class_users.
set by dynamic_debug_init() for builtins.
or by kernel/module/main:load_info() for loadable modules.
vmlinux.lds.h: new BOUNDED_SECTION for __dyndbg_class_users
dynamic_debug.c has 2 changes in ddebug_add_module(), ddebug_change():
ddebug_add_module() already calls ddebug_attach_module_classes()
to handle classmaps DEFINEd by a module, now it also calls
ddebug_attach_user_module_classes() to handle USEd classmaps. To
avoid this work when possible, 1st scan the module's descriptors and
count the number of class'd pr_debugs.
ddebug_attach_user_module_classes() scans the module's class_users
section, follows the refs to the parent's classmap, and calls
ddebug_apply_params() on each. It also avoids work by checking the
module's class-ct.
ddebug_apply_params(new fn):
It scans module's/builtin kernel-params, calls ddebug_match_apply_kparam
for each to find the params/sysfs-nodes which may be wired to a classmap.
ddebug_match_apply_kparam(new fn):
1st, it tests the kernel-param.ops is dyndbg's; this guarantees that
the attached arg is a struct ddebug_class_param, which has a ref to
the param's state, and to the classmap defining the param's handling.
2nd, it requires that the classmap ref'd by the kparam is the one
we're called for; modules can use many separate classmaps (as
test_dynamic_debug does).
Then apply the "parent" kparam's setting to the dependent module,
using ddebug_apply_class_bitmap().
ddebug_change(and callees) also gets adjustments:
ddebug_find_valid_class(): This does a search over the module's
classmaps, looking for the class FOO echo'd to >control. So now it
searches over __dyndbg_class_users[] after __dyndbg_classes[].
ddebug_class_name(): return class-names for defined AND used classes.
test_dynamic_debug.c, test_dynamic_debug_submod.c:
This demonstrates the 2 types of classmaps & sysfs-params, following
the 4-part recipe:
1. define an enum for the classmap: DRM.debug has DRM_UT_{CORE,KMS,...}
multiple classes must share 0-62 classid space.
2. DYNDBG_CLASSMAP_DEFINE(.. DRM_UT_{CORE,KMS,...})
3. DYNDBG_CLASSMAP_PARAM* (classmap)
4. DYNDBG_CLASSMAP_USE()
by _submod only, skipping 2,3
Move all the enum declarations together, to better explain how they
share the 0..62 class-id space available to a module (non-overlapping
subranges).
reorg macros 2,3 by name. This gives a tabular format, making it easy
to see the pattern of repetition, and the points of change.
And extend the test to replicate the 2-module (parent & dependent)
scenario which caused the CONFIG_DRM_USE_DYNAMIC_DEBUG=y regression
seen in drm & drivers.
The _submod.c is a 2-line file: #define _SUBMOD, #include parent.
This gives identical complements of prdbgs in parent & _submod, and
thus identical print behavior when all of: >control, >params, and
parent->_submod propagation are working correctly.
It also puts all the parent/_submod declarations together in the same
source, with the new ifdef _SUBMOD block invoking DYNDBG_CLASSMAP_USE
for the 2 test-interfaces. I think this is clearer.
These 2 modules are both tristate, allowing 3 super/sub combos: Y/Y,
Y/M, M/M (not N/N). Y/Y testing exposed a missing __align(8) in the
_METADATA macro, which M/M didn't see because the module-loader memory
placement constrains it instead.
DEBUG details:
``#define DEBUG`` in src enables all pr_debugs after it, including any
class'd pr_debugs; its not necessarily all-or-nothing, unless you do
the define in a header. Also, the only way to disable them is with
the classmap-kparam or using "class foo -p" >control queries.
Fixes: aad0214f3026 ("dyndbg: add DECLARE_DYNDBG_CLASSMAP macro")
Ref: bb2ff6c27bc9 ("drm: Disable dynamic debug as broken")
Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
v8 - split drm parts to separate commits.
preserve DECLARE_DYNDBG_CLASSMAP to decouple DRM, no flag day.
v7 - previous submission-blocking bug:
missing __align(8) in DYNAMIC_DEBUG_DECLARE_METADATA on
ddebug_class_user caused corrupt records, but only for builtin
modules; module loader code probably pinned allocations to the right
alignment naturally, hiding the bug for typical builds.
v6- get rid of WARN_ON_ONCE
v?- fix _var expanded 2x in macro
---
MAINTAINERS | 2 +-
include/asm-generic/vmlinux.lds.h | 1 +
include/linux/dynamic_debug.h | 61 ++++++++--
kernel/module/main.c | 3 +
lib/Kconfig.debug | 24 +++-
lib/Makefile | 3 +
lib/dynamic_debug.c | 183 ++++++++++++++++++++++++++----
lib/test_dynamic_debug.c | 111 ++++++++++++------
lib/test_dynamic_debug_submod.c | 10 ++
9 files changed, 330 insertions(+), 68 deletions(-)
create mode 100644 lib/test_dynamic_debug_submod.c
diff --git a/MAINTAINERS b/MAINTAINERS
index ebf03f5f0619..cf9fccbc6bde 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7525,7 +7525,7 @@ M: Jim Cromie <jim.cromie@gmail.com>
S: Maintained
F: include/linux/dynamic_debug.h
F: lib/dynamic_debug.c
-F: lib/test_dynamic_debug.c
+F: lib/test_dynamic_debug*.c
DYNAMIC INTERRUPT MODERATION
M: Tal Gilboa <talgi@nvidia.com>
diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index f7749d0f2562..f1d8e64b244c 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -365,6 +365,7 @@
/* implement dynamic printk debug */ \
. = ALIGN(8); \
BOUNDED_SECTION_BY(__dyndbg_classes, ___dyndbg_classes) \
+ BOUNDED_SECTION_BY(__dyndbg_class_users, ___dyndbg_class_users) \
BOUNDED_SECTION_BY(__dyndbg, ___dyndbg) \
LIKELY_PROFILE() \
BRANCH_PROFILE() \
diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index dd304e231f08..d1fc3035e19c 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -72,8 +72,8 @@ enum ddebug_class_map_type {
};
struct ddebug_class_map {
- struct module *mod;
- const char *mod_name; /* needed for builtins */
+ const struct module *mod; /* NULL for builtins */
+ const char *mod_name;
const char **class_names;
const int length;
const int base; /* index of 1st .class_id, allows split/shared space */
@@ -81,11 +81,33 @@ struct ddebug_class_map {
};
/**
- * DECLARE_DYNDBG_CLASSMAP - declare classnames known by a module
- * @_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
+ * DYNDBG_CLASSMAP_DEFINE - define a set of debug-classes used by a module.
+ * @_var: name of the classmap, exported for other modules coordinated use.
+ * @_type: enum ddebug_class_map_type, chooses bits/verbose, numeric/names.
+ * @_base: offset of 1st class-name, used to share 0..62 classid space
+ * @classes: vals are stringified enum-vals, like DRM_UT_*
+ *
+ * Defines and exports a struct ddebug_class_map whose @classes are
+ * used to validate a "class FOO .." >control command on the module
+ */
+#define __DYNDBG_CLASSMAP_DEFINE(_var, _maptype, _base, ...) \
+ const char *_var##_classnames[] = { __VA_ARGS__ }; \
+ struct ddebug_class_map __aligned(8) __used \
+ __section("__dyndbg_classes") _var = { \
+ .mod = THIS_MODULE, \
+ .mod_name = KBUILD_MODNAME, \
+ .base = _base, \
+ .map_type = _maptype, \
+ .length = ARRAY_SIZE(_var##_classnames), \
+ .class_names = _var##_classnames, \
+ }
+#define DYNDBG_CLASSMAP_DEFINE(_var, ...) \
+ __DYNDBG_CLASSMAP_DEFINE(_var, __VA_ARGS__); \
+ EXPORT_SYMBOL(_var)
+
+/*
+ XXX: keep this until DRM applies adaptations, it differs from
+ __DYNDBG_CLASSMAP_DEFINE only in the static on the struct decl.
*/
#define DECLARE_DYNDBG_CLASSMAP(_var, _maptype, _base, ...) \
static const char *_var##_classnames[] = { __VA_ARGS__ }; \
@@ -99,12 +121,37 @@ struct ddebug_class_map {
.class_names = _var##_classnames, \
}
+struct ddebug_class_user {
+ char *user_mod_name;
+ struct ddebug_class_map *map;
+};
+
+/**
+ * DYNDBG_CLASSMAP_USE - refer to a classmap, DEFINEd elsewhere.
+ * @_var: name of the exported classmap var
+ *
+ * This registers a module's use of another module's classmap defn, so
+ * dyndbg can authorize "class DRM_CORE ..." >control commands upon
+ * this module.
+ */
+#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 __aligned(8) __used \
+ __section("__dyndbg_class_users") _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_users;
unsigned int num_descs;
unsigned int num_classes;
+ unsigned int num_class_users;
};
struct ddebug_class_param {
diff --git a/kernel/module/main.c b/kernel/module/main.c
index e1e8a7a9d6c1..9f7ce0f0e6e5 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -2217,6 +2217,9 @@ static int find_module_sections(struct module *mod, struct load_info *info)
mod->dyndbg_info.classes = section_objs(info, "__dyndbg_classes",
sizeof(*mod->dyndbg_info.classes),
&mod->dyndbg_info.num_classes);
+ mod->dyndbg_info.class_users = section_objs(info, "__dyndbg_class_users",
+ sizeof(*mod->dyndbg_info.class_users),
+ &mod->dyndbg_info.num_class_users);
#endif
return 0;
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index c63a5fbf1f1c..bcc2549c01e9 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -2796,12 +2796,26 @@ config TEST_STATIC_KEYS
If unsure, say N.
config TEST_DYNAMIC_DEBUG
- tristate "Test DYNAMIC_DEBUG"
- depends on DYNAMIC_DEBUG
+ tristate "Build test-dynamic-debug module"
+ depends on DYNAMIC_DEBUG || DYNAMIC_DEBUG_CORE
help
- This module registers a tracer callback to count enabled
- pr_debugs in a 'do_debugging' function, then alters their
- enablements, calls the function, and compares counts.
+ This module exersizes/demonstrates dyndbg's classmap API, by
+ creating 2 classes: a DISJOINT classmap (supporting DRM.debug)
+ and a LEVELS/VERBOSE classmap (like verbose2 > verbose1).
+
+ If unsure, say N.
+
+config TEST_DYNAMIC_DEBUG_SUBMOD
+ tristate "Build test-dynamic-debug submodule"
+ default m
+ depends on DYNAMIC_DEBUG || DYNAMIC_DEBUG_CORE
+ depends on TEST_DYNAMIC_DEBUG
+ help
+ This sub-module uses a classmap defined and exported by the
+ parent module, recapitulating drm & driver's shared use of
+ drm.debug to control enabled debug-categories.
+ It is tristate, independent of parent, to allow testing all
+ proper combinations of parent=y/m submod=y/m.
If unsure, say N.
diff --git a/lib/Makefile b/lib/Makefile
index ffc6b2341b45..d3e6a48f202b 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -82,6 +82,7 @@ obj-$(CONFIG_TEST_USER_COPY) += test_user_copy.o
obj-$(CONFIG_TEST_STATIC_KEYS) += test_static_keys.o
obj-$(CONFIG_TEST_STATIC_KEYS) += test_static_key_base.o
obj-$(CONFIG_TEST_DYNAMIC_DEBUG) += test_dynamic_debug.o
+obj-$(CONFIG_TEST_DYNAMIC_DEBUG_SUBMOD) += test_dynamic_debug_submod.o
obj-$(CONFIG_TEST_PRINTF) += test_printf.o
obj-$(CONFIG_TEST_SCANF) += test_scanf.o
@@ -240,6 +241,8 @@ obj-$(CONFIG_HAVE_ARCH_TRACEHOOK) += syscall.o
obj-$(CONFIG_DYNAMIC_DEBUG_CORE) += dynamic_debug.o
#ensure exported functions have prototypes
CFLAGS_dynamic_debug.o := -DDYNAMIC_DEBUG_MODULE
+CFLAGS_test_dynamic_debug.o := -DDYNAMIC_DEBUG_MODULE
+CFLAGS_test_dynamic_debug_submod.o := -DDYNAMIC_DEBUG_MODULE
obj-$(CONFIG_SYMBOLIC_ERRNAME) += errname.o
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index d5701207febc..f0a274a3cc9e 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_users[];
+extern struct ddebug_class_user __stop___dyndbg_class_users[];
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_users;
+ unsigned int num_ddebugs, num_classes, num_class_users;
};
struct ddebug_query {
@@ -148,21 +151,39 @@ static void vpr_info_dq(const struct ddebug_query *query, const char *msg)
query->first_lineno, query->last_lineno, query->class_string);
}
+#define vpr_dt_info(dt_p, msg_p, ...) ({ \
+ struct ddebug_table const *_dt = dt_p; \
+ v2pr_info(msg_p " module:%s nd:%d nc:%d nu:%d\n", ##__VA_ARGS__, \
+ _dt->mod_name, _dt->num_ddebugs, _dt->num_classes, \
+ _dt->num_class_users); \
+ })
+
#define __outvar /* filled by callee */
static struct ddebug_class_map *ddebug_find_valid_class(struct ddebug_table const *dt,
const char *class_string,
__outvar int *class_id)
{
struct ddebug_class_map *map;
+ struct ddebug_class_user *cli;
int i, idx;
- for (map = dt->classes, i = 0; i < dt->num_classes; i++, map++) {
+ for (i = 0, map = dt->classes; i < dt->num_classes; i++, map++) {
idx = match_string(map->class_names, map->length, class_string);
if (idx >= 0) {
*class_id = idx + map->base;
+ vpr_dt_info(dt, "good-class: %s.%s ", map->mod_name, class_string);
return map;
}
}
+ for (i = 0, cli = dt->class_users; i < dt->num_class_users; i++, cli++) {
+ idx = match_string(cli->map->class_names, cli->map->length, class_string);
+ if (idx >= 0) {
+ *class_id = idx + cli->map->base;
+ vpr_dt_info(dt, "class-ref: %s.%s ",
+ cli->user_mod_name, class_string);
+ return cli->map;
+ }
+ }
*class_id = -ENOENT;
return NULL;
}
@@ -559,7 +580,7 @@ static int ddebug_exec_query(char *query_string, const char *modname)
/* handle multiple queries in query string, continue on error, return
last error or number of matching callsites. Module name is either
- in param (for boot arg) or perhaps in query string.
+ in the modname arg (for boot args) or perhaps in query string.
*/
static int ddebug_exec_queries(char *query, const char *modname)
{
@@ -688,12 +709,12 @@ static int param_set_dyndbg_module_classes(const char *instr,
}
/**
- * param_set_dyndbg_classes - class FOO >control
+ * param_set_dyndbg_classes - set all classes in a classmap
* @instr: string echo>d to sysfs, input depends on map_type
- * @kp: kp->arg has state: bits/lvl, map, map_type
+ * @kp: kp->arg has state: bits/lvl, classmap, map_type
*
- * Enable/disable prdbgs by their class, as given in the arguments to
- * DECLARE_DYNDBG_CLASSMAP. For LEVEL map-types, enforce relative
+ * For all classes in the classmap, enable/disable them per the input
+ * (depending on map_type). For LEVEL map-types, enforce relative
* levels by bitpos.
*
* Returns: 0 or <0 if error.
@@ -1038,12 +1059,17 @@ static void *ddebug_proc_next(struct seq_file *m, void *p, loff_t *pos)
static const char *ddebug_class_name(struct ddebug_table *dt, struct _ddebug *dp)
{
struct ddebug_class_map *map = dt->classes;
+ struct ddebug_class_user *cli = dt->class_users;
int i;
for (i = 0; i < dt->num_classes; i++, map++)
if (class_in_range(dp->class_id, map))
return map->class_names[dp->class_id - map->base];
+ for (i = 0; i < dt->num_class_users; i++, cli++)
+ if (class_in_range(dp->class_id, cli->map))
+ return cli->map->class_names[dp->class_id - cli->map->base];
+
return NULL;
}
@@ -1124,31 +1150,133 @@ static const struct proc_ops proc_fops = {
.proc_write = ddebug_proc_write
};
+static const char * const ddebug_classmap_typenames[] = {
+ "DISJOINT_BITS", "LEVEL_NUM"
+};
+
+#define vpr_cm_info(cm_p, msg_p, ...) ({ \
+ struct ddebug_class_map const *_cm = cm_p; \
+ v2pr_info(msg_p " module:%s base:%d len:%d type:%s\n", ##__VA_ARGS__, \
+ _cm->mod_name, _cm->base, _cm->length, \
+ ddebug_classmap_typenames[_cm->map_type]); \
+ })
+
+static void ddebug_sync_classbits(const struct ddebug_class_param *dcp, const char *modname)
+{
+ /* clamp initial bitvec, mask off hi-bits */
+ if (*dcp->bits & ~CLASSMAP_BITMASK(dcp->map->length)) {
+ *dcp->bits &= CLASSMAP_BITMASK(dcp->map->length);
+ v2pr_info("preset classbits: %lx\n", *dcp->bits);
+ }
+ /* force class'd prdbgs (in USEr module) to match (DEFINEr module) class-param */
+ ddebug_apply_class_bitmap(dcp, dcp->bits, ~0, modname);
+ ddebug_apply_class_bitmap(dcp, dcp->bits, 0, modname);
+}
+
+static void ddebug_match_apply_kparam(const struct kernel_param *kp,
+ const struct ddebug_class_map *map,
+ const char *modnm)
+{
+ struct ddebug_class_param *dcp;
+
+ if (kp->ops != ¶m_ops_dyndbg_classes)
+ return;
+
+ dcp = (struct ddebug_class_param *)kp->arg;
+
+ if (map == dcp->map) {
+ v2pr_info("found kp:%s =0x%lx", kp->name, *dcp->bits);
+ vpr_cm_info(map, "mapped to:");
+ ddebug_sync_classbits(dcp, modnm);
+ }
+}
+
+static void ddebug_apply_params(const struct ddebug_class_map *cm, const char *modnm)
+{
+ const struct kernel_param *kp;
+#if IS_ENABLED(CONFIG_MODULES)
+ int i;
+
+ if (cm->mod) {
+ vpr_cm_info(cm, "loaded class:");
+ for (i = 0, kp = cm->mod->kp; i < cm->mod->num_kp; i++, kp++)
+ ddebug_match_apply_kparam(kp, cm, modnm);
+ }
+#endif
+ if (!cm->mod) {
+ vpr_cm_info(cm, "builtin class:");
+ for (kp = __start___param; kp < __stop___param; kp++)
+ ddebug_match_apply_kparam(kp, cm, modnm);
+ }
+}
+
+/*
+ * Find this module's classmaps in a sub/whole-range of the builtin/
+ * modular classmap vector/section. Save the start and length of the
+ * subrange at its edges.
+ */
static void ddebug_attach_module_classes(struct ddebug_table *dt, struct _ddebug_info *di)
{
struct ddebug_class_map *cm;
int i, nc = 0;
- /*
- * Find this module's classmaps in a subrange/wholerange of
- * the builtin/modular classmap vector/section. Save the start
- * and length of the subrange at its edges.
- */
- for (cm = di->classes, i = 0; i < di->num_classes; i++, cm++) {
+ for (i = 0, cm = di->classes; i < di->num_classes; i++, cm++) {
if (!strcmp(cm->mod_name, dt->mod_name)) {
- if (!nc) {
- v2pr_info("start subrange, class[%d]: module:%s base:%d len:%d ty:%d\n",
- i, cm->mod_name, cm->base, cm->length, cm->map_type);
+ vpr_cm_info(cm, "classes[%d]:", i);
+ if (!nc++)
dt->classes = cm;
- }
- nc++;
}
}
- if (nc) {
- dt->num_classes = nc;
- vpr_info("module:%s attached %d classes\n", dt->mod_name, nc);
+ if (!nc)
+ return;
+
+ vpr_info("module:%s attached %d classes\n", dt->mod_name, nc);
+ dt->num_classes = nc;
+
+ for (i = 0, cm = dt->classes; i < dt->num_classes; i++, cm++)
+ ddebug_apply_params(cm, cm->mod_name);
+}
+
+/*
+ * propagates class-params thru their classmaps to class-users. this
+ * means a query against the dt/module, which means it must be on the
+ * list to be seen by ddebug_change.
+ */
+static void ddebug_attach_user_module_classes(struct ddebug_table *dt,
+ const struct _ddebug_info *di)
+{
+ struct ddebug_class_user *cli;
+ int i, nc = 0;
+
+ /*
+ * For builtins: scan the array, find start/length of this
+ * module's refs, save to dt. For loadables, this is the
+ * whole array.
+ */
+ for (i = 0, cli = di->class_users; i < di->num_class_users; i++, cli++) {
+
+ if (WARN_ON_ONCE(!cli || !cli->map || !cli->user_mod_name))
+ continue;
+
+ if (!strcmp(cli->user_mod_name, dt->mod_name)) {
+
+ vpr_cm_info(cli->map, "class_ref[%d] %s -> %s", i,
+ cli->user_mod_name, cli->map->mod_name);
+ if (!nc++)
+ dt->class_users = cli;
+ }
}
+ if (!nc)
+ return;
+
+ dt->num_class_users = nc;
+
+ /* now iterate dt */
+ for (i = 0, cli = dt->class_users; i < dt->num_class_users; i++, cli++)
+ ddebug_apply_params(cli->map, cli->user_mod_name);
+
+ vpr_dt_info(dt, "attach-client-module: ");
}
/*
@@ -1158,6 +1286,8 @@ static void ddebug_attach_module_classes(struct ddebug_table *dt, struct _ddebug
static int ddebug_add_module(struct _ddebug_info *di, const char *modname)
{
struct ddebug_table *dt;
+ struct _ddebug *iter;
+ int i, class_ct = 0;
if (!di->num_descs)
return 0;
@@ -1181,13 +1311,20 @@ static int ddebug_add_module(struct _ddebug_info *di, const char *modname)
INIT_LIST_HEAD(&dt->link);
- if (di->classes && di->num_classes)
+ for (i = 0, iter = di->descs; i < di->num_descs; i++, iter++)
+ if (iter->class_id != _DPRINTK_CLASS_DFLT)
+ class_ct++;
+
+ if (class_ct && 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 (class_ct && di->num_class_users)
+ ddebug_attach_user_module_classes(dt, di);
+
vpr_info("%3u debug prints in module %s\n", di->num_descs, modname);
return 0;
}
@@ -1337,8 +1474,10 @@ static int __init dynamic_debug_init(void)
struct _ddebug_info di = {
.descs = __start___dyndbg,
.classes = __start___dyndbg_classes,
+ .class_users = __start___dyndbg_class_users,
.num_descs = __stop___dyndbg - __start___dyndbg,
.num_classes = __stop___dyndbg_classes - __start___dyndbg_classes,
+ .num_class_users = __stop___dyndbg_class_users - __start___dyndbg_class_users,
};
#ifdef CONFIG_MODULES
diff --git a/lib/test_dynamic_debug.c b/lib/test_dynamic_debug.c
index 9e950a911b6c..ff9b879286d5 100644
--- a/lib/test_dynamic_debug.c
+++ b/lib/test_dynamic_debug.c
@@ -6,11 +6,15 @@
* Jim Cromie <jim.cromie@gmail.com>
*/
-#define pr_fmt(fmt) "test_dd: " fmt
+#if defined(TEST_DYNAMIC_DEBUG_SUBMOD)
+ #define pr_fmt(fmt) "test_dd_submod: " fmt
+#else
+ #define pr_fmt(fmt) "test_dd: " fmt
+#endif
#include <linux/module.h>
-/* run tests by reading or writing sysfs node: do_prints */
+/* re-gen output by reading or writing sysfs node: do_prints */
static void do_prints(void); /* device under test */
static int param_set_do_prints(const char *instr, const struct kernel_param *kp)
@@ -29,24 +33,39 @@ static const struct kernel_param_ops param_ops_do_prints = {
};
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)
- * Additionally, here:
- * - tie together sysname, mapname, bitsname, flagsname
- */
-#define DD_SYS_WRAP(_model, _flags) \
- static unsigned long bits_##_model; \
- static struct ddebug_class_param _flags##_model = { \
+#define CLASSMAP_BITMASK(width, base) (((1UL << (width)) - 1) << base)
+
+/* sysfs param wrapper, proto-API */
+#define DYNDBG_CLASSMAP_PARAM_(_model, _flags, _init) \
+ static unsigned long bits_##_model = _init; \
+ 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)
+ module_param_cb(_flags##_##_model, ¶m_ops_dyndbg_classes, \
+ &_flags##_##_model, 0600)
+#ifdef DEBUG
+#define DYNDBG_CLASSMAP_PARAM(_model, _flags) DYNDBG_CLASSMAP_PARAM_(_model, _flags, ~0)
+#else
+#define DYNDBG_CLASSMAP_PARAM(_model, _flags) DYNDBG_CLASSMAP_PARAM_(_model, _flags, 0)
+#endif
+
+/*
+ * Demonstrate/test all 4 class-typed classmaps with a sys-param.
+ *
+ * Each is 3 part: client-enum decl, _DEFINE, _PARAM.
+ * Declare them in blocks to show patterns of use (repetitions and
+ * changes) within each.
+ *
+ * 1st, dyndbg expects a client-provided enum-type as source of
+ * category/classid truth. DRM has DRM_UT_<CORE,DRIVER,KMS,etc>.
+ *
+ * Modules with multiple CLASSMAPS must have enums with distinct
+ * value-ranges, arranged below with explicit enum_sym = X inits.
+ *
+ * Declare all 4 enums now, for different types
+ */
/* numeric input, independent bits */
enum cat_disjoint_bits {
@@ -60,26 +79,51 @@ 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");
-DD_SYS_WRAP(disjoint_bits, p);
-DD_SYS_WRAP(disjoint_bits, 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,
- "V0", "V1", "V2", "V3", "V4", "V5", "V6", "V7");
-DD_SYS_WRAP(level_num, p);
-DD_SYS_WRAP(level_num, T);
+
+/* recapitulate DRM's parent(drm.ko) <-- _submod(drivers,helpers) */
+#if !defined(TEST_DYNAMIC_DEBUG_SUBMOD)
+/*
+ * In single user, or parent / coordinator (drm.ko) modules, define
+ * classmaps on the client enums above, and then declares the PARAMS
+ * ref'g the classmaps. Each is exported.
+ */
+DYNDBG_CLASSMAP_DEFINE(map_disjoint_bits, DD_CLASS_TYPE_DISJOINT_BITS,
+ D2_CORE,
+ "D2_CORE",
+ "D2_DRIVER",
+ "D2_KMS",
+ "D2_PRIME",
+ "D2_ATOMIC",
+ "D2_VBL",
+ "D2_STATE",
+ "D2_LEASE",
+ "D2_DP",
+ "D2_DRMRES");
+
+DYNDBG_CLASSMAP_DEFINE(map_level_num, DD_CLASS_TYPE_LEVEL_NUM,
+ V0, "V0", "V1", "V2", "V3", "V4", "V5", "V6", "V7");
+
+/*
+ * now add the sysfs-params
+ */
+
+DYNDBG_CLASSMAP_PARAM(disjoint_bits, p);
+DYNDBG_CLASSMAP_PARAM(level_num, p);
+
+#else /* TEST_DYNAMIC_DEBUG_SUBMOD */
+
+/*
+ * in submod/drm-drivers, use the classmaps defined in top/parent
+ * module above.
+ */
+
+DYNDBG_CLASSMAP_USE(map_disjoint_bits);
+DYNDBG_CLASSMAP_USE(map_level_num);
+
+#endif
/* stand-in for all pr_debug etc */
#define prdbg(SYM) __pr_debug_cls(SYM, #SYM " msg\n")
@@ -115,6 +159,7 @@ static void do_levels(void)
static void do_prints(void)
{
+ pr_debug("do_prints:\n");
do_cats();
do_levels();
}
diff --git a/lib/test_dynamic_debug_submod.c b/lib/test_dynamic_debug_submod.c
new file mode 100644
index 000000000000..9a893402ce1a
--- /dev/null
+++ b/lib/test_dynamic_debug_submod.c
@@ -0,0 +1,10 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Kernel module for testing dynamic_debug
+ *
+ * Authors:
+ * Jim Cromie <jim.cromie@gmail.com>
+ */
+
+#define TEST_DYNAMIC_DEBUG_SUBMOD
+#include "test_dynamic_debug.c"
--
2.44.0
^ permalink raw reply [flat|nested] 24+ messages in thread* [PATCH v8 17/35] selftests-dyndbg: add tools/testing/selftests/dynamic_debug/*
2024-04-29 19:31 [PATCH v8 00/35] fix CONFIG_DRM_USE_DYNAMIC_DEBUG=y regression Jim Cromie
` (15 preceding siblings ...)
2024-04-29 19:31 ` [PATCH v8 16/35] dyndbg-API: fix DECLARE_DYNDBG_CLASSMAP Jim Cromie
@ 2024-04-29 19:31 ` Jim Cromie
2024-04-29 19:31 ` [PATCH v8 18/35] selftests-dyndbg: exit 127 if no facility Jim Cromie
` (5 subsequent siblings)
22 siblings, 0 replies; 24+ messages in thread
From: Jim Cromie @ 2024-04-29 19:31 UTC (permalink / raw)
To: jbaron, gregkh, linux-kernel, dri-devel, amd-gfx, intel-gvt-dev,
intel-gfx
Cc: ukaszb, linux-doc, daniel.vetter, tvrtko.ursulin, jani.nikula,
ville.syrjala, seanpaul, robdclark, groeck, yanivt, bleung,
Jim Cromie
Add a selftest script for dynamic-debug. The config requires
CONFIG_TEST_DYNAMIC_DEBUG=m (and CONFIG_TEST_DYNAMIC_DEBUG_SUBMOD=m),
which tacitly requires either CONFIG_DYNAMIC_DEBUG=y or
CONFIG_DYNAMIC_DEBUG_CORE=y
ATM this has just basic_tests(), it modifies pr_debug flags in a few
builtins (init/main, params), counts the callsite flags changed, and
verifies against expected values.
This is backported from another feature branch; the support-fns (thx
Lukas) have unused features at the moment, they'll get used shortly.
The script enables simple virtme-ng testing:
$> vng --verbose --name v6.8-32-g30d431000676 --user root \
--cwd ../.. -a dynamic_debug.verbose=2 -p 4 \
./tools/testing/selftests/dynamic_debug/dyndbg_selftest.sh
virtme: waiting for virtiofsd to start
virtme: use 'microvm' QEMU architecture
...
[ 4.136168] virtme-init: Setting hostname to v6.8-32-g30d431000676...
[ 4.240874] virtme-init: starting script
test_dynamic_debug_submod not there
test_dynamic_debug not there
...
[ 4.474435] virtme-init: script returned {0}
Powering off.
[ 4.529318] ACPI: PM: Preparing to enter system sleep state S5
[ 4.529991] kvm: exiting hardware virtualization
[ 4.530428] reboot: Power down
And add dynamic_debug to TARGETS, so `make run_tests` sees it properly
for the impatient, set TARGETS explicitly:
bash-5.2# make TARGETS=dynamic_debug run_tests
make[1]: ...
TAP version 13
1..1
[ 35.552922] dyndbg: read 3 bytes from userspace
[ 35.553099] dyndbg: query 0: "=_" mod:*
[ 35.553544] dyndbg: processed 1 queries, with 1778 matches, 0 errs
...
TLDR:
This selftest is slightly naive wrt the init state of call-site flags.
In particular, it fails if class'd pr_debugs have been set
$ cat /etc/modprobe.d/drm-test.conf
options drm dyndbg=class,DRM_UT_CORE,+mfslt%class,DRM_UT_KMS,+mf
By Contract, class'd pr_debugs are protected from alteration by
default (only by direct "class FOO" queries), so the "=_" logged above
(TAP version 13) cannot affect the DRM_UT_CORE,KMS pr_debugs.
These class'd flag-settings, added by modprobe, alter the counts of
flag-matching patterns, breaking the tests' expectations.
Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
Co-developed-by: Łukasz Bartosik <ukaszb@chromium.org>
Signed-off-by: Łukasz Bartosik <ukaszb@chromium.org>
---
MAINTAINERS | 1 +
tools/testing/selftests/Makefile | 1 +
.../testing/selftests/dynamic_debug/Makefile | 9 +
tools/testing/selftests/dynamic_debug/config | 2 +
.../dynamic_debug/dyndbg_selftest.sh | 231 ++++++++++++++++++
5 files changed, 244 insertions(+)
create mode 100644 tools/testing/selftests/dynamic_debug/Makefile
create mode 100644 tools/testing/selftests/dynamic_debug/config
create mode 100755 tools/testing/selftests/dynamic_debug/dyndbg_selftest.sh
diff --git a/MAINTAINERS b/MAINTAINERS
index cf9fccbc6bde..bdffd192d3df 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7526,6 +7526,7 @@ S: Maintained
F: include/linux/dynamic_debug.h
F: lib/dynamic_debug.c
F: lib/test_dynamic_debug*.c
+F: tools/testing/selftest/dynamic_debug/*
DYNAMIC INTERRUPT MODERATION
M: Tal Gilboa <talgi@nvidia.com>
diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
index e1504833654d..84edf0bd8e80 100644
--- a/tools/testing/selftests/Makefile
+++ b/tools/testing/selftests/Makefile
@@ -20,6 +20,7 @@ TARGETS += drivers/s390x/uvdevice
TARGETS += drivers/net/bonding
TARGETS += drivers/net/team
TARGETS += dt
+TARGETS += dynamic_debug
TARGETS += efivarfs
TARGETS += exec
TARGETS += fchmodat2
diff --git a/tools/testing/selftests/dynamic_debug/Makefile b/tools/testing/selftests/dynamic_debug/Makefile
new file mode 100644
index 000000000000..6d06fa7f1040
--- /dev/null
+++ b/tools/testing/selftests/dynamic_debug/Makefile
@@ -0,0 +1,9 @@
+# SPDX-License-Identifier: GPL-2.0-only
+# borrowed from Makefile for user memory selftests
+
+# No binaries, but make sure arg-less "make" doesn't trigger "run_tests"
+all:
+
+TEST_PROGS := dyndbg_selftest.sh
+
+include ../lib.mk
diff --git a/tools/testing/selftests/dynamic_debug/config b/tools/testing/selftests/dynamic_debug/config
new file mode 100644
index 000000000000..d080da571ac0
--- /dev/null
+++ b/tools/testing/selftests/dynamic_debug/config
@@ -0,0 +1,2 @@
+CONFIG_TEST_DYNAMIC_DEBUG=m
+CONFIG_TEST_DYNAMIC_DEBUG_SUBMOD=m
diff --git a/tools/testing/selftests/dynamic_debug/dyndbg_selftest.sh b/tools/testing/selftests/dynamic_debug/dyndbg_selftest.sh
new file mode 100755
index 000000000000..1be70af26a38
--- /dev/null
+++ b/tools/testing/selftests/dynamic_debug/dyndbg_selftest.sh
@@ -0,0 +1,231 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0-only
+
+V=${V:=0} # invoke as V=1 $0 for global verbose
+RED="\033[0;31m"
+GREEN="\033[0;32m"
+YELLOW="\033[0;33m"
+BLUE="\033[0;34m"
+MAGENTA="\033[0;35m"
+CYAN="\033[0;36m"
+NC="\033[0;0m"
+error_msg=""
+
+function vx () {
+ echo $1 > /sys/module/dynamic_debug/parameters/verbose
+}
+
+function ddgrep () {
+ grep $1 /proc/dynamic_debug/control
+}
+
+function doprints () {
+ cat /sys/module/test_dynamic_debug/parameters/do_prints
+}
+
+function ddcmd () {
+ exp_exit_code=0
+ num_args=$#
+ if [ "${@:$#}" = "pass" ]; then
+ num_args=$#-1
+ elif [ "${@:$#}" = "fail" ]; then
+ num_args=$#-1
+ exp_exit_code=1
+ fi
+ args=${@:1:$num_args}
+ output=$((echo "$args" > /proc/dynamic_debug/control) 2>&1)
+ exit_code=$?
+ error_msg=$(echo $output | cut -d ":" -f 5 | sed -e 's/^[[:space:]]*//')
+ handle_exit_code $BASH_LINENO $FUNCNAME $exit_code $exp_exit_code
+}
+
+function handle_exit_code() {
+ local exp_exit_code=0
+ [ $# == 4 ] && exp_exit_code=$4
+ if [ $3 -ne $exp_exit_code ]; then
+ echo -e "${RED}: $BASH_SOURCE:$1 $2() expected to exit with code $exp_exit_code"
+ [ $3 == 1 ] && echo "Error: '$error_msg'"
+ exit
+ fi
+}
+
+# $1 - pattern to match, pattern in $1 is enclosed by spaces for a match ""\s$1\s"
+# $2 - number of times the pattern passed in $1 is expected to match
+# $3 - optional can be set either to "-r" or "-v"
+# "-r" means relaxed matching in this case pattern provided in $1 is passed
+# as is without enclosing it with spaces
+# "-v" prints matching lines
+# $4 - optional when $3 is set to "-r" then $4 can be used to pass "-v"
+function check_match_ct {
+ pattern="\s$1\s"
+ exp_cnt=0
+
+ [ "$3" == "-r" ] && pattern="$1"
+ let cnt=$(ddgrep "$pattern" | wc -l)
+ if [ $V -eq 1 ] || [ "$3" == "-v" ] || [ "$4" == "-v" ]; then
+ echo -ne "${BLUE}" && ddgrep $pattern && echo -ne "${NC}"
+ fi
+ [ $# -gt 1 ] && exp_cnt=$2
+ if [ $cnt -ne $exp_cnt ]; then
+ echo -e "${RED}: $BASH_SOURCE:$BASH_LINENO check failed expected $exp_cnt on $1, got $cnt"
+ exit
+ else
+ echo ": $cnt matches on $1"
+ fi
+}
+
+# $1 - trace instance name
+# #2 - if > 0 then directory is expected to exist, if <= 0 then otherwise
+# $3 - "-v" for verbose
+function check_trace_instance_dir {
+ if [ -e /sys/kernel/tracing/instances/$1 ]; then
+ if [ "$3" == "-v" ] ; then
+ echo "ls -l /sys/kernel/tracing/instances/$1: "
+ ls -l /sys/kernel/tracing/instances/$1
+ fi
+ if [ $2 -le 0 ]; then
+ echo -e "${RED}: $BASH_SOURCE:$BASH_LINENO error trace instance \
+ '/sys/kernel/tracing/instances/$1' does exist"
+ exit
+ fi
+ else
+ if [ $2 -gt 0 ]; then
+ echo -e "${RED}: $BASH_SOURCE:$BASH_LINENO error trace instance \
+ '/sys/kernel/tracing/instances/$1' does not exist"
+ exit
+ fi
+ fi
+}
+
+function tmark {
+ echo $* > /sys/kernel/tracing/trace_marker
+}
+
+# $1 - trace instance name
+# $2 - line number
+# $3 - if > 0 then the instance is expected to be opened, otherwise
+# the instance is expected to be closed
+function check_trace_instance {
+ output=$(tail -n9 /proc/dynamic_debug/control | grep ": Opened trace instances" \
+ | xargs -n1 | grep $1)
+ if [ "$output" != $1 ] && [ $3 -gt 0 ]; then
+ echo -e "${RED}: $BASH_SOURCE:$2 trace instance $1 is not opened"
+ exit
+ fi
+ if [ "$output" == $1 ] && [ $3 -le 0 ]; then
+ echo -e "${RED}: $BASH_SOURCE:$2 trace instance $1 is not closed"
+ exit
+ fi
+}
+
+function is_trace_instance_opened {
+ check_trace_instance $1 $BASH_LINENO 1
+}
+
+function is_trace_instance_closed {
+ check_trace_instance $1 $BASH_LINENO 0
+}
+
+# $1 - trace instance directory to delete
+# $2 - if > 0 then directory is expected to be deleted successfully, if <= 0 then otherwise
+function del_trace_instance_dir() {
+ exp_exit_code=1
+ [ $2 -gt 0 ] && exp_exit_code=0
+ output=$((rmdir /sys/kernel/debug/tracing/instances/$1) 2>&1)
+ exit_code=$?
+ error_msg=$(echo $output | cut -d ":" -f 3 | sed -e 's/^[[:space:]]*//')
+ handle_exit_code $BASH_LINENO $FUNCNAME $exit_code $exp_exit_code
+}
+
+function error_log_ref {
+ # to show what I got
+ : echo "# error-log-ref: $1"
+ : echo cat \$2
+}
+
+function ifrmmod {
+ lsmod | grep $1 2>&1>/dev/null || echo $1 not there
+ lsmod | grep $1 2>&1>/dev/null && rmmod $1
+}
+
+# $1 - text to search for
+function search_trace() {
+ search_trace_name 0 1 $1
+}
+
+# $1 - trace instance name, 0 for global event trace
+# $2 - line number counting from the bottom
+# $3 - text to search for
+function search_trace_name() {
+ if [ "$1" = "0" ]; then
+ buf=$(cat /sys/kernel/debug/tracing/trace)
+ line=$(tail -$2 /sys/kernel/debug/tracing/trace | head -1 | sed -e 's/^[[:space:]]*//')
+ else
+ buf=$(cat /sys/kernel/debug/tracing/instances/$1/trace)
+ line=$(tail -$2 /sys/kernel/debug/tracing/instances/$1/trace | head -1 | \
+ sed -e 's/^[[:space:]]*//')
+ fi
+ if [ $2 = 0 ]; then
+ # whole-buf check
+ output=$(echo $buf | grep "$3")
+ else
+ output=$(echo $line | grep "$3")
+ fi
+ if [ "$output" = "" ]; then
+ echo -e "${RED}: $BASH_SOURCE:$BASH_LINENO search for '$3' failed \
+ in line '$line' or '$buf'"
+ exit
+ fi
+ if [ $V = 1 ]; then
+ echo -e "${MAGENTA}: search_trace_name in $1 found: \n$output \nin:${BLUE} $buf ${NC}"
+ fi
+}
+
+# $1 - error message to check
+function check_err_msg() {
+ if [ "$error_msg" != "$1" ]; then
+ echo -e "${RED}: $BASH_SOURCE:$BASH_LINENO error message '$error_msg' \
+ does not match with '$1'"
+ exit
+ fi
+}
+
+function basic_tests {
+ echo -e "${GREEN}# BASIC_TESTS ${NC}"
+ ddcmd =_ # zero everything (except class'd sites)
+ check_match_ct =p 0
+ # there are several main's :-/
+ ddcmd module main file */module/main.c +p
+ check_match_ct =p 14
+ ddcmd =_
+ check_match_ct =p 0
+ # multi-cmd input, newline separated, with embedded comments
+ cat <<"EOF" > /proc/dynamic_debug/control
+ module main +mf # multi-query
+ module main file init/main.c +ml # newline separated
+EOF
+ # the intersection of all those main's is hard to track/count
+ # esp when mixed with overlapping greps
+ check_match_ct =mf 21
+ check_match_ct =ml 0
+ check_match_ct =mfl 6
+ ddcmd =_
+}
+
+
+tests_list=(
+ basic_tests
+)
+
+# Run tests
+
+ifrmmod test_dynamic_debug_submod
+ifrmmod test_dynamic_debug
+
+for test in "${tests_list[@]}"
+do
+ $test
+ echo ""
+done
+echo -en "${GREEN}# Done on: "
+date
--
2.44.0
^ permalink raw reply [flat|nested] 24+ messages in thread* [PATCH v8 18/35] selftests-dyndbg: exit 127 if no facility
2024-04-29 19:31 [PATCH v8 00/35] fix CONFIG_DRM_USE_DYNAMIC_DEBUG=y regression Jim Cromie
` (16 preceding siblings ...)
2024-04-29 19:31 ` [PATCH v8 17/35] selftests-dyndbg: add tools/testing/selftests/dynamic_debug/* Jim Cromie
@ 2024-04-29 19:31 ` Jim Cromie
2024-04-29 19:31 ` [PATCH v8 19/35] dyndbg-API: promote DYNDBG_CLASSMAP_PARAM to API Jim Cromie
` (4 subsequent siblings)
22 siblings, 0 replies; 24+ messages in thread
From: Jim Cromie @ 2024-04-29 19:31 UTC (permalink / raw)
To: jbaron, gregkh, linux-kernel, dri-devel, amd-gfx, intel-gvt-dev,
intel-gfx
Cc: ukaszb, linux-doc, daniel.vetter, tvrtko.ursulin, jani.nikula,
ville.syrjala, seanpaul, robdclark, groeck, yanivt, bleung,
Jim Cromie
Test if /proc/dynamic_debug/control exists, exit 127 otherwise.
This distinguishes an untestable config from both pass & fail.
The 127 choice is pretty arbitrary, but imitating bisect.
That control file's presense guarantees that dynamic-debugging is
configured (unless /proc is off, unusually), without dealing with the
<debugfs> mount.
Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
tools/testing/selftests/dynamic_debug/dyndbg_selftest.sh | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/tools/testing/selftests/dynamic_debug/dyndbg_selftest.sh b/tools/testing/selftests/dynamic_debug/dyndbg_selftest.sh
index 1be70af26a38..cb77ae142520 100755
--- a/tools/testing/selftests/dynamic_debug/dyndbg_selftest.sh
+++ b/tools/testing/selftests/dynamic_debug/dyndbg_selftest.sh
@@ -11,6 +11,11 @@ CYAN="\033[0;36m"
NC="\033[0;0m"
error_msg=""
+[ -e /proc/dynamic_debug/control ] || {
+ echo -e "${RED}: kernel not configured for this test ${NC}"
+ exit 127
+}
+
function vx () {
echo $1 > /sys/module/dynamic_debug/parameters/verbose
}
--
2.44.0
^ permalink raw reply [flat|nested] 24+ messages in thread* [PATCH v8 19/35] dyndbg-API: promote DYNDBG_CLASSMAP_PARAM to API
2024-04-29 19:31 [PATCH v8 00/35] fix CONFIG_DRM_USE_DYNAMIC_DEBUG=y regression Jim Cromie
` (17 preceding siblings ...)
2024-04-29 19:31 ` [PATCH v8 18/35] selftests-dyndbg: exit 127 if no facility Jim Cromie
@ 2024-04-29 19:31 ` Jim Cromie
2024-04-29 19:31 ` [PATCH v8 20/35] dyndbg-doc: add classmap info to howto Jim Cromie
` (3 subsequent siblings)
22 siblings, 0 replies; 24+ messages in thread
From: Jim Cromie @ 2024-04-29 19:31 UTC (permalink / raw)
To: jbaron, gregkh, linux-kernel, dri-devel, amd-gfx, intel-gvt-dev,
intel-gfx
Cc: ukaszb, linux-doc, daniel.vetter, tvrtko.ursulin, jani.nikula,
ville.syrjala, seanpaul, robdclark, groeck, yanivt, bleung,
Jim Cromie
move the DYNDBG_CLASSMAP_PARAM macro from test-dynamic-debug.c into
the header, and refine it, by distinguishing the 2 use cases:
1.DYNDBG_CLASSMAP_PARAM_REF
for DRM, to pass in extern __drm_debug by name.
dyndbg keeps bits in it, so drm can still use it as before
2.DYNDBG_CLASSMAP_PARAM
new user (test_dynamic_debug) doesn't need to share state,
decls a static long unsigned int to store the bitvec.
__DYNDBG_CLASSMAP_PARAM
bottom layer - allocate,init a ddebug-class-param, module-param-cb.
Also clean up and improve comments in test-code, and add
MODULE_DESCRIPTIONs.
Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
fixup drm-print.h add PARAM_REF forwarding macros
with DYNDBG_CLASSMAP_PARAM_REF in the API, add DRM_ variant
---
include/linux/dynamic_debug.h | 37 ++++++++++++++++-
lib/dynamic_debug.c | 70 ++++++++++++++++++++++-----------
lib/test_dynamic_debug.c | 50 +++++++++--------------
lib/test_dynamic_debug_submod.c | 9 ++++-
4 files changed, 110 insertions(+), 56 deletions(-)
diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index d1fc3035e19c..090fe9554db7 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -91,7 +91,7 @@ struct ddebug_class_map {
* used to validate a "class FOO .." >control command on the module
*/
#define __DYNDBG_CLASSMAP_DEFINE(_var, _maptype, _base, ...) \
- const char *_var##_classnames[] = { __VA_ARGS__ }; \
+ static const char *_var##_classnames[] = { __VA_ARGS__ }; \
struct ddebug_class_map __aligned(8) __used \
__section("__dyndbg_classes") _var = { \
.mod = THIS_MODULE, \
@@ -163,6 +163,41 @@ struct ddebug_class_param {
const struct ddebug_class_map *map;
};
+/**
+ * DYNDBG_CLASSMAP_PARAM - wrap a dyndbg-classmap with a controlling sys-param
+ * @_name sysfs node name
+ * @_var name of the struct classmap var defining the controlled classes
+ * @_flags flags to be toggled, typically just 'p'
+ *
+ * Creates a sysfs-param to control the classes defined by the
+ * classmap. Keeps bits in a private/static
+ */
+#define DYNDBG_CLASSMAP_PARAM(_name, _var, _flags) \
+ static unsigned long _name##_bvec; \
+ __DYNDBG_CLASSMAP_PARAM(_name, _name##_bvec, _var, _flags)
+
+/**
+ * DYNDBG_CLASSMAP_PARAM_REF - wrap a dyndbg-classmap with a controlling sys-param
+ * @_name sysfs node name
+ * @_bits name of the module's unsigned long bit-vector, ex: __drm_debug
+ * @_var name of the struct classmap var defining the controlled classes
+ * @_flags flags to be toggled, typically just 'p'
+ *
+ * Creates a sysfs-param to control the classmap, keeping bitvec in user @_bits.
+ * This lets drm use __drm_debug elsewhere too.
+ */
+#define DYNDBG_CLASSMAP_PARAM_REF(_name, _bits, _var, _flags) \
+ __DYNDBG_CLASSMAP_PARAM(_name, _bits, _var, _flags)
+
+#define __DYNDBG_CLASSMAP_PARAM(_name, _bits, _var, _flags) \
+ static struct ddebug_class_param _name##_##_flags = { \
+ .bits = &(_bits), \
+ .flags = #_flags, \
+ .map = &(_var), \
+ }; \
+ module_param_cb(_name, ¶m_ops_dyndbg_classes, \
+ &_name##_##_flags, 0600)
+
/*
* pr_debug() and friends are globally enabled or modules have selectively
* enabled them.
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index f0a274a3cc9e..31fd67597928 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -657,6 +657,30 @@ static int ddebug_apply_class_bitmap(const struct ddebug_class_param *dcp,
#define CLASSMAP_BITMASK(width) ((1UL << (width)) - 1)
+static void ddebug_class_param_clamp_input(unsigned long *inrep, const struct kernel_param *kp)
+{
+ const struct ddebug_class_param *dcp = kp->arg;
+ const struct ddebug_class_map *map = dcp->map;
+
+ switch (map->map_type) {
+ case DD_CLASS_TYPE_DISJOINT_BITS:
+ /* expect bits. mask and warn if too many */
+ if (*inrep & ~CLASSMAP_BITMASK(map->length)) {
+ pr_warn("%s: input: 0x%lx exceeds mask: 0x%lx, masking\n",
+ KP_NAME(kp), *inrep, CLASSMAP_BITMASK(map->length));
+ *inrep &= CLASSMAP_BITMASK(map->length);
+ }
+ break;
+ case DD_CLASS_TYPE_LEVEL_NUM:
+ /* input is bitpos, of highest verbosity to be enabled */
+ if (*inrep > map->length) {
+ pr_warn("%s: level:%ld exceeds max:%d, clamping\n",
+ KP_NAME(kp), *inrep, map->length);
+ *inrep = map->length;
+ }
+ break;
+ }
+}
static int param_set_dyndbg_module_classes(const char *instr,
const struct kernel_param *kp,
const char *modnm)
@@ -675,26 +699,15 @@ static int param_set_dyndbg_module_classes(const char *instr,
pr_err("expecting numeric input, not: %s > %s\n", instr, KP_NAME(kp));
return -EINVAL;
}
+ ddebug_class_param_clamp_input(&inrep, kp);
switch (map->map_type) {
case DD_CLASS_TYPE_DISJOINT_BITS:
- /* expect bits. mask and warn if too many */
- if (inrep & ~CLASSMAP_BITMASK(map->length)) {
- pr_warn("%s: input: 0x%lx exceeds mask: 0x%lx, masking\n",
- KP_NAME(kp), inrep, CLASSMAP_BITMASK(map->length));
- inrep &= CLASSMAP_BITMASK(map->length);
- }
v2pr_info("bits:0x%lx > %s.%s\n", inrep, modnm ?: "*", KP_NAME(kp));
totct += ddebug_apply_class_bitmap(dcp, &inrep, *dcp->bits, modnm);
*dcp->bits = inrep;
break;
case DD_CLASS_TYPE_LEVEL_NUM:
- /* input is bitpos, of highest verbosity to be enabled */
- if (inrep > map->length) {
- pr_warn("%s: level:%ld exceeds max:%d, clamping\n",
- KP_NAME(kp), inrep, map->length);
- inrep = map->length;
- }
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));
@@ -1161,16 +1174,27 @@ static const char * const ddebug_classmap_typenames[] = {
ddebug_classmap_typenames[_cm->map_type]); \
})
-static void ddebug_sync_classbits(const struct ddebug_class_param *dcp, const char *modname)
+static void ddebug_sync_classbits(const struct kernel_param *kp, const char *modname)
{
- /* clamp initial bitvec, mask off hi-bits */
- if (*dcp->bits & ~CLASSMAP_BITMASK(dcp->map->length)) {
- *dcp->bits &= CLASSMAP_BITMASK(dcp->map->length);
- v2pr_info("preset classbits: %lx\n", *dcp->bits);
+ struct ddebug_class_param *dcp = kp->arg;
+ unsigned long new_bits;
+
+ ddebug_class_param_clamp_input(dcp->bits, kp);
+
+ switch (dcp->map->map_type) {
+ case DD_CLASS_TYPE_DISJOINT_BITS:
+ v2pr_info(" %s: classbits: 0x%lx\n", KP_NAME(kp), *dcp->bits);
+ ddebug_apply_class_bitmap(dcp, dcp->bits, 0UL, modname);
+ break;
+ case DD_CLASS_TYPE_LEVEL_NUM:
+ new_bits = CLASSMAP_BITMASK(*dcp->lvl);
+ v2pr_info(" %s: lvl:%ld bits:0x%lx\n", KP_NAME(kp), *dcp->lvl, new_bits);
+ ddebug_apply_class_bitmap(dcp, &new_bits, 0UL, modname);
+ break;
+ default:
+ pr_err("bad map type %d\n", dcp->map->map_type);
+ return;
}
- /* force class'd prdbgs (in USEr module) to match (DEFINEr module) class-param */
- ddebug_apply_class_bitmap(dcp, dcp->bits, ~0, modname);
- ddebug_apply_class_bitmap(dcp, dcp->bits, 0, modname);
}
static void ddebug_match_apply_kparam(const struct kernel_param *kp,
@@ -1185,9 +1209,9 @@ static void ddebug_match_apply_kparam(const struct kernel_param *kp,
dcp = (struct ddebug_class_param *)kp->arg;
if (map == dcp->map) {
- v2pr_info("found kp:%s =0x%lx", kp->name, *dcp->bits);
- vpr_cm_info(map, "mapped to:");
- ddebug_sync_classbits(dcp, modnm);
+ v2pr_info(" found kp:%s =0x%lx", kp->name, *dcp->bits);
+ vpr_cm_info(map, " mapped to:");
+ ddebug_sync_classbits(kp, modnm);
}
}
diff --git a/lib/test_dynamic_debug.c b/lib/test_dynamic_debug.c
index ff9b879286d5..b3c6f90b9fe8 100644
--- a/lib/test_dynamic_debug.c
+++ b/lib/test_dynamic_debug.c
@@ -1,6 +1,7 @@
// SPDX-License-Identifier: GPL-2.0-only
/*
- * Kernel module for testing dynamic_debug
+ * Kernel module to test/demonstrate dynamic_debug features,
+ * particularly classmaps and their support for subsystems like DRM.
*
* Authors:
* Jim Cromie <jim.cromie@gmail.com>
@@ -35,24 +36,8 @@ module_param_cb(do_prints, ¶m_ops_do_prints, NULL, 0600);
#define CLASSMAP_BITMASK(width, base) (((1UL << (width)) - 1) << base)
-/* sysfs param wrapper, proto-API */
-#define DYNDBG_CLASSMAP_PARAM_(_model, _flags, _init) \
- static unsigned long bits_##_model = _init; \
- 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)
-#ifdef DEBUG
-#define DYNDBG_CLASSMAP_PARAM(_model, _flags) DYNDBG_CLASSMAP_PARAM_(_model, _flags, ~0)
-#else
-#define DYNDBG_CLASSMAP_PARAM(_model, _flags) DYNDBG_CLASSMAP_PARAM_(_model, _flags, 0)
-#endif
-
/*
- * Demonstrate/test all 4 class-typed classmaps with a sys-param.
+ * Demonstrate/test both types of classmaps, each with a sys-param.
*
* Each is 3 part: client-enum decl, _DEFINE, _PARAM.
* Declare them in blocks to show patterns of use (repetitions and
@@ -64,7 +49,7 @@ module_param_cb(do_prints, ¶m_ops_do_prints, NULL, 0600);
* Modules with multiple CLASSMAPS must have enums with distinct
* value-ranges, arranged below with explicit enum_sym = X inits.
*
- * Declare all 4 enums now, for different types
+ * Declare all enums now, for different types
*/
/* numeric input, independent bits */
@@ -83,12 +68,15 @@ enum cat_disjoint_bits {
/* numeric verbosity, V2 > V1 related */
enum cat_level_num { V0 = 14, V1, V2, V3, V4, V5, V6, V7 };
-/* recapitulate DRM's parent(drm.ko) <-- _submod(drivers,helpers) */
+/*
+ * use/demonstrate multi-module-group classmaps, as for DRM
+ */
#if !defined(TEST_DYNAMIC_DEBUG_SUBMOD)
/*
- * In single user, or parent / coordinator (drm.ko) modules, define
- * classmaps on the client enums above, and then declares the PARAMS
- * ref'g the classmaps. Each is exported.
+ * For module-groups of 1+, define classmaps with names (stringified
+ * enum-symbols) copied from above. 1-to-1 mapping is recommended.
+ * The classmap is exported, so that other modules in the group can
+ * link to it and control their prdbgs.
*/
DYNDBG_CLASSMAP_DEFINE(map_disjoint_bits, DD_CLASS_TYPE_DISJOINT_BITS,
D2_CORE,
@@ -107,19 +95,18 @@ DYNDBG_CLASSMAP_DEFINE(map_level_num, DD_CLASS_TYPE_LEVEL_NUM,
V0, "V0", "V1", "V2", "V3", "V4", "V5", "V6", "V7");
/*
- * now add the sysfs-params
+ * for use-cases that want it, provide a sysfs-param to set the
+ * classes in the classmap. It is at this interface where the
+ * "v3>v2" property is applied to DD_CLASS_TYPE_LEVEL_NUM inputs.
*/
-
-DYNDBG_CLASSMAP_PARAM(disjoint_bits, p);
-DYNDBG_CLASSMAP_PARAM(level_num, p);
+DYNDBG_CLASSMAP_PARAM(p_disjoint_bits, map_disjoint_bits, p);
+DYNDBG_CLASSMAP_PARAM(p_level_num, map_level_num, p);
#else /* TEST_DYNAMIC_DEBUG_SUBMOD */
-
/*
- * in submod/drm-drivers, use the classmaps defined in top/parent
- * module above.
+ * the +1 members of a multi-module group refer to the classmap
+ * DEFINEd (and exported) above.
*/
-
DYNDBG_CLASSMAP_USE(map_disjoint_bits);
DYNDBG_CLASSMAP_USE(map_level_num);
@@ -180,5 +167,6 @@ static void __exit test_dynamic_debug_exit(void)
module_init(test_dynamic_debug_init);
module_exit(test_dynamic_debug_exit);
+MODULE_DESCRIPTION("test/demonstrate dynamic-debug features");
MODULE_AUTHOR("Jim Cromie <jim.cromie@gmail.com>");
MODULE_LICENSE("GPL");
diff --git a/lib/test_dynamic_debug_submod.c b/lib/test_dynamic_debug_submod.c
index 9a893402ce1a..0d15f3ffe466 100644
--- a/lib/test_dynamic_debug_submod.c
+++ b/lib/test_dynamic_debug_submod.c
@@ -1,6 +1,9 @@
// SPDX-License-Identifier: GPL-2.0
/*
- * Kernel module for testing dynamic_debug
+ * Kernel module to test/demonstrate dynamic_debug features,
+ * particularly classmaps and their support for subsystems, like DRM,
+ * which defines its drm_debug classmap in drm module, and uses it in
+ * helpers & drivers.
*
* Authors:
* Jim Cromie <jim.cromie@gmail.com>
@@ -8,3 +11,7 @@
#define TEST_DYNAMIC_DEBUG_SUBMOD
#include "test_dynamic_debug.c"
+
+MODULE_DESCRIPTION("test/demonstrate dynamic-debug subsystem support");
+MODULE_AUTHOR("Jim Cromie <jim.cromie@gmail.com>");
+MODULE_LICENSE("GPL");
--
2.44.0
^ permalink raw reply [flat|nested] 24+ messages in thread* [PATCH v8 20/35] dyndbg-doc: add classmap info to howto
2024-04-29 19:31 [PATCH v8 00/35] fix CONFIG_DRM_USE_DYNAMIC_DEBUG=y regression Jim Cromie
` (18 preceding siblings ...)
2024-04-29 19:31 ` [PATCH v8 19/35] dyndbg-API: promote DYNDBG_CLASSMAP_PARAM to API Jim Cromie
@ 2024-04-29 19:31 ` Jim Cromie
2024-04-29 19:31 ` [PATCH v8 21/35] dyndbg: treat comma as a token separator Jim Cromie
` (2 subsequent siblings)
22 siblings, 0 replies; 24+ messages in thread
From: Jim Cromie @ 2024-04-29 19:31 UTC (permalink / raw)
To: jbaron, gregkh, linux-kernel, dri-devel, amd-gfx, intel-gvt-dev,
intel-gfx
Cc: ukaszb, linux-doc, daniel.vetter, tvrtko.ursulin, jani.nikula,
ville.syrjala, seanpaul, robdclark, groeck, yanivt, bleung,
Jim Cromie
Describe the 3 API macros providing dynamic_debug's classmaps
DYNDBG_CLASSMAP_DEFINE - create, exports a module's classmap
DYNDBG_CLASSMAP_USE - refer to exported map
DYNDBG_CLASSMAP_PARAM - bind control param to the classmap
DYNDBG_CLASSMAP_PARAM_REF + use module's storage - __drm_debug
cc: linux-doc@vger.kernel.org
Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
v5 adjustments per Randy Dunlap
v7 checkpatch fixes
v8 more
---
.../admin-guide/dynamic-debug-howto.rst | 63 ++++++++++++++++++-
1 file changed, 62 insertions(+), 1 deletion(-)
diff --git a/Documentation/admin-guide/dynamic-debug-howto.rst b/Documentation/admin-guide/dynamic-debug-howto.rst
index 6a8ce5a34382..742eb4230c6e 100644
--- a/Documentation/admin-guide/dynamic-debug-howto.rst
+++ b/Documentation/admin-guide/dynamic-debug-howto.rst
@@ -225,7 +225,6 @@ the ``p`` flag has meaning, other flags are ignored.
Note the regexp ``^[-+=][fslmpt_]+$`` matches a flags specification.
To clear all flags at once, use ``=_`` or ``-fslmpt``.
-
Debug messages during Boot Process
==================================
@@ -375,3 +374,65 @@ just a shortcut for ``print_hex_dump(KERN_DEBUG)``.
For ``print_hex_dump_debug()``/``print_hex_dump_bytes()``, format string is
its ``prefix_str`` argument, if it is constant string; or ``hexdump``
in case ``prefix_str`` is built dynamically.
+
+Dynamic Debug classmaps
+=======================
+
+Dyndbg allows selection/grouping of *prdbg* callsites using structural
+info: module, file, function, line. Classmaps allow authors to add
+their own domain-oriented groupings using class-names. Classmaps are
+exported, so they referencable from other modules.
+
+ # enable classes individually
+ :#> ddcmd class DRM_UT_CORE +p
+ :#> ddcmd class DRM_UT_KMS +p
+ # or more selectively
+ :#> ddcmd class DRM_UT_CORE module drm +p
+
+The "class FOO" syntax protects class'd prdbgs from generic overwrite::
+
+ # IOW this doesn't wipe any DRM.debug settings
+ :#> ddcmd -p
+
+To support the DRM.debug parameter, DYNDBG_CLASSMAP_PARAM* updates all
+classes in a classmap, mapping param-bits 0..N onto the classes:
+DRM_UT_<*> for the DRM use-case.
+
+Dynamic Debug Classmap API
+==========================
+
+DYNDBG_CLASSMAP_DEFINE - modules use this to create classmaps, naming
+each of the classes (stringified enum-symbols: "DRM_UT_<*>"), and
+type, and mapping the class-names to consecutive _class_ids.
+
+By doing so, modules tell dyndbg that they have prdbgs with those
+class_ids, and they authorize dyndbg to accept "class FOO" for the
+module defining the classmap, and its contained classnames.
+
+DYNDBG_CLASSMAP_USE - drm drivers invoke this to ref the CLASSMAP that
+drm DEFINEs. This shares the classmap definition, and authorizes
+dyndbg to apply changes to the user module's class'd pr_debugs. It
+also tells dyndbg how to initialize the user's prdbgs at modprobe,
+based upon the current setting of the parent's controlling param.
+
+There are 2 types of classmaps:
+
+ DD_CLASS_TYPE_DISJOINT_BITS: classes are independent, like DRM.debug
+ DD_CLASS_TYPE_LEVEL_NUM: classes are relative, ordered (V3 > V2)
+
+DYNDBG_CLASSMAP_PARAM - modelled after module_param_cb, it refers to a
+DEFINEd classmap, and associates it to the param's data-store. This
+state is then applied to DEFINEr and USEr modules when they're modprobed.
+
+This interface also enforces the DD_CLASS_TYPE_LEVEL_NUM relation
+amongst the contained classnames; all classes are independent in the
+control parser itself.
+
+Modules or module-groups (drm & drivers) can define multiple
+classmaps, as long as they share the limited 0..62 per-module-group
+_class_id range, without overlap.
+
+``#define DEBUG`` will enable all pr_debugs in scope, including any
+class'd ones. This won't be reflected in the PARAM readback value,
+but the class'd pr_debug callsites can be forced off by toggling the
+classmap-kparam all-on then all-off.
--
2.44.0
^ permalink raw reply [flat|nested] 24+ messages in thread* [PATCH v8 21/35] dyndbg: treat comma as a token separator
2024-04-29 19:31 [PATCH v8 00/35] fix CONFIG_DRM_USE_DYNAMIC_DEBUG=y regression Jim Cromie
` (19 preceding siblings ...)
2024-04-29 19:31 ` [PATCH v8 20/35] dyndbg-doc: add classmap info to howto Jim Cromie
@ 2024-04-29 19:31 ` Jim Cromie
2024-04-29 19:31 ` [PATCH v8 22/35] selftests-dyndbg: add comma_terminator_tests Jim Cromie
2024-05-08 19:32 ` [PATCH v8 00/35] fix CONFIG_DRM_USE_DYNAMIC_DEBUG=y regression jim.cromie
22 siblings, 0 replies; 24+ messages in thread
From: Jim Cromie @ 2024-04-29 19:31 UTC (permalink / raw)
To: jbaron, gregkh, linux-kernel, dri-devel, amd-gfx, intel-gvt-dev,
intel-gfx
Cc: ukaszb, linux-doc, daniel.vetter, tvrtko.ursulin, jani.nikula,
ville.syrjala, seanpaul, robdclark, groeck, yanivt, bleung,
Jim Cromie
Treat comma as a token terminator, just like a space. This allows a
user to avoid quoting hassles when spaces are otherwise needed:
:#> modprobe drm dyndbg=class,DRM_UT_CORE,+p\;class,DRM_UT_KMS,+p
or as a boot arg:
drm.dyndbg=class,DRM_UT_CORE,+p # todo: support multi-query here
Given the many ways a boot-line +args can be assembled and then passed
in/down/around shell based tools, this may allow side-stepping all
sorts of quoting hassles thru those layers.
existing query format:
modprobe test_dynamic_debug dyndbg="class D2_CORE +p"
new format:
modprobe test_dynamic_debug dyndbg=class,D2_CORE,+p
Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
Co-developed-by: Łukasz Bartosik <ukaszb@chromium.org>
Signed-off-by: Łukasz Bartosik <ukaszb@chromium.org>
---
lib/dynamic_debug.c | 17 +++++++++++++----
1 file changed, 13 insertions(+), 4 deletions(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 31fd67597928..c1bc728cb050 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -290,6 +290,14 @@ static int ddebug_change(const struct ddebug_query *query,
return nfound;
}
+static char *skip_spaces_and_commas(const char *str)
+{
+ str = skip_spaces(str);
+ while (*str == ',')
+ str = skip_spaces(++str);
+ return (char *)str;
+}
+
/*
* Split the buffer `buf' into space-separated words.
* Handles simple " and ' quoting, i.e. without nested,
@@ -303,8 +311,8 @@ static int ddebug_tokenize(char *buf, char *words[], int maxwords)
while (*buf) {
char *end;
- /* Skip leading whitespace */
- buf = skip_spaces(buf);
+ /* Skip leading whitespace and comma */
+ buf = skip_spaces_and_commas(buf);
if (!*buf)
break; /* oh, it was trailing whitespace */
if (*buf == '#')
@@ -320,7 +328,7 @@ static int ddebug_tokenize(char *buf, char *words[], int maxwords)
return -EINVAL; /* unclosed quote */
}
} else {
- for (end = buf; *end && !isspace(*end); end++)
+ for (end = buf; *end && !isspace(*end) && *end != ','; end++)
;
if (end == buf) {
pr_err("parse err after word:%d=%s\n", nwords,
@@ -592,7 +600,8 @@ static int ddebug_exec_queries(char *query, const char *modname)
if (split)
*split++ = '\0';
- query = skip_spaces(query);
+ query = skip_spaces_and_commas(query);
+
if (!query || !*query || *query == '#')
continue;
--
2.44.0
^ permalink raw reply [flat|nested] 24+ messages in thread* [PATCH v8 22/35] selftests-dyndbg: add comma_terminator_tests
2024-04-29 19:31 [PATCH v8 00/35] fix CONFIG_DRM_USE_DYNAMIC_DEBUG=y regression Jim Cromie
` (20 preceding siblings ...)
2024-04-29 19:31 ` [PATCH v8 21/35] dyndbg: treat comma as a token separator Jim Cromie
@ 2024-04-29 19:31 ` Jim Cromie
2024-05-08 19:32 ` [PATCH v8 00/35] fix CONFIG_DRM_USE_DYNAMIC_DEBUG=y regression jim.cromie
22 siblings, 0 replies; 24+ messages in thread
From: Jim Cromie @ 2024-04-29 19:31 UTC (permalink / raw)
To: jbaron, gregkh, linux-kernel, dri-devel, amd-gfx, intel-gvt-dev,
intel-gfx
Cc: ukaszb, linux-doc, daniel.vetter, tvrtko.ursulin, jani.nikula,
ville.syrjala, seanpaul, robdclark, groeck, yanivt, bleung,
Jim Cromie
New fn validates parsing and effect of queries using combinations of
commas and spaces to delimit the tokens.
It manipulates pr-debugs in builtin module/params, so might have deps
I havent foreseen on odd configurations.
Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
.../selftests/dynamic_debug/dyndbg_selftest.sh | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/tools/testing/selftests/dynamic_debug/dyndbg_selftest.sh b/tools/testing/selftests/dynamic_debug/dyndbg_selftest.sh
index cb77ae142520..7a7d437e948b 100755
--- a/tools/testing/selftests/dynamic_debug/dyndbg_selftest.sh
+++ b/tools/testing/selftests/dynamic_debug/dyndbg_selftest.sh
@@ -217,9 +217,23 @@ EOF
ddcmd =_
}
+function comma_terminator_tests {
+ echo -e "${GREEN}# COMMA_TERMINATOR_TESTS ${NC}"
+ # try combos of spaces & commas
+ check_match_ct '\[params\]' 4 -r
+ ddcmd module,params,=_ # commas as spaces
+ ddcmd module,params,+mpf # turn on module's pr-debugs
+ check_match_ct =pmf 4
+ ddcmd ,module ,, , params, -p
+ check_match_ct =mf 4
+ ddcmd " , module ,,, , params, -m" #
+ check_match_ct =f 4
+ ddcmd =_
+}
tests_list=(
basic_tests
+ comma_terminator_tests
)
# Run tests
--
2.44.0
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH v8 00/35] fix CONFIG_DRM_USE_DYNAMIC_DEBUG=y regression
2024-04-29 19:31 [PATCH v8 00/35] fix CONFIG_DRM_USE_DYNAMIC_DEBUG=y regression Jim Cromie
` (21 preceding siblings ...)
2024-04-29 19:31 ` [PATCH v8 22/35] selftests-dyndbg: add comma_terminator_tests Jim Cromie
@ 2024-05-08 19:32 ` jim.cromie
22 siblings, 0 replies; 24+ messages in thread
From: jim.cromie @ 2024-05-08 19:32 UTC (permalink / raw)
To: jbaron, gregkh, linux-kernel, dri-devel, amd-gfx, intel-gvt-dev,
intel-gfx
Cc: ukaszb, linux-doc, daniel.vetter, tvrtko.ursulin, jani.nikula,
ville.syrjala, seanpaul, robdclark, groeck, yanivt, bleung
On Mon, Apr 29, 2024 at 1:32 PM Jim Cromie <jim.cromie@gmail.com> wrote:
>
> hi Greg, Jason, DRM-folk,
>
> This patchset fixes the CONFIG_DRM_USE_DYNAMIC_DEBUG=y regression,
> Fixes: bb2ff6c27bc9 ("drm: Disable dynamic debug as broken")
>
> this is v8.
> Its also here:
> https://github.com/jimc/linux/tree/dd-classmap-fix-8a
>
This patchset didnt get picked up by drm patchwork
maybe its tired my stochastic renaming, sorry about that
https://patchwork.freedesktop.org/project/intel-gfx/series/?ordering=-last_updated
125063fix DRM_USE_DYNAMIC_DEBUG=y
regressionwarningNew23jim.cromie@gmail.comNone2023-11-01
123572fix DRM_USE_DYNAMIC_DEBUG
regressionfailureNew22jim.cromie@gmail.comNone2023-09-11
113363fix DRM_USE_DYNAMIC_DEBUG regressionwarningIn progress
22jim.cromie@gmail.comNone2023-08-01
111651DRM_USE_DYNAMIC_DEBUG
regressionfailureNew20jim.cromie@gmail.comNone2023-01-13
111631DRM - avoid regression in -rc, fix comment
is there something missing for a DRM patchwork run ?
does it kick out because theres non-drm subsystem stuff in there ?
thanks
> v7 had at least 2 problems:
>
> https://lore.kernel.org/lkml/20231101002609.3533731-1-jim.cromie@gmail.com/
> https://patchwork.freedesktop.org/series/125066/
>
> 1. missing __align(8) in METATDATA macro, giving too much placement
> freedom to linker, caused weird segvs following non-ptr vals, but for
> builtin modules only. found by lkp-test.
>
> 2. the main patch changed both the dyndbg API, and the drm/drivers.
> This was a flag-day annoyance, and not practical. Fix by preserving
> old API macro until "later", and splitting the patch and set into 2
> sequential subsets. removal can wait.
>
> What was broken ?
>
> Booting a modular kernel with drm.debug=0x1ff, this enabled pr_debugs
> only in drm itself, not the yet-to-be loaded driver + helpers. I had
> tested with scripts doing lots of modprobes with dyndbg=<> options
> permuting. I didn't notice that I didn't really test without them.
>
> The deeper cause was my design error, a violation of the K&R rule:
> "define once, refer many times".
>
> DECLARE_DYNDBG_CLASSMAP defined the classmap, and was used everywhere,
> re-declaring the same static classmap repeatedly. Jani Nikula actually
> picked up on this at the time, but didn't scream loudly enough for
> anyone to notice, I know I didn't get it then. One patchset across 2
> trees didn't help either.
>
> The revised classmap API "splits" it to def & ref.
>
> DYNDBG_CLASSMAP_DEFINE fixes & updates the busted macro, EXPORTing the
> classmap instead. It gets invoked once per subsystem, by the
> parent/builtin, drm.ko for DRM.
>
> DYNDBG_CLASSMAP_USE in drivers and helpers refer to the classmap by
> name, which links the 2 modules (like __drm_debug already does).
>
> These 2 tell dyndbg to map "class FOO" to the defined FOO_ID, which
> allows it to make those changes via >control.
>
> DYNDBG_CLASSMAP_PARAM*, defines the controlling kparam, and binds it
> to both the _var, and the _DEFINEd classmap. So drm uses this to bind
> the classmap to __drm_debug.
>
> It provides the common control-point for the sub-system; it is applied
> to the classmaps during modprobe of both _DEFINEr and USErs. It also
> enforces the relative nature of LEVEL classmaps, ie V3>V2.
>
> DECLARE_DYNDBG_CLASSMAP is preserved to decouple the DRM patches.
>
> A new struct and elf section contain the _USEs; on modprobe, these are
> scanned similarly to the _DEFINEs, but follow the references to their
> defining modules, find the kparam wired to the classmap, and apply its
> classmap settings to the USEr. This action is what V1 missed, which
> is why drivers failed to enable debug during modprobe.
>
> In order to recapitulate the regression scenario without involving
> DRM, the patchset (v6 I think) adds test_dynamic_debug_submod, which
> is a duplicate of its parent; _submod.c #defines _SUBMOD, and then
> includes parent.
>
> This puts _DEFINE and _USE close together in the same file, for
> obviousness. It also guarantees that the submod always has the same
> complement of debug()s, giving consistent output from both when
> classmaps are working properly, as tested when changing callsites via
> both param and >control.
>
> To provide a turn-key selftest, the patchset also adds
> ./tools/testing/selftests/dynamic_debug/dyndbg_selftest.sh, pilfered
> from a debug-to-trace patchset I and Lukasz Bartozik have been working
> out. It starts with basic_tests, then to test 2 new parsing
> delimiters, which simplify testing of the classmap functionality.
>
> It works nicely from virtme-ng:
>
> [jimc@frodo vx]$ vrun_ -- ./tools/testing/selftests/dynamic_debug/dyndbg_selftest.sh
> doing: vng --verbose --name v6.9-rc5-34-g2f1ace6e1c68 \
> --user root --cwd ../.. \
> -a dynamic_debug.verbose=2 -p 4 \
> -- ./tools/testing/selftests/dynamic_debug/dyndbg_selftest.sh
> virtme: waiting for virtiofsd to start
> ...
> [ 3.546739] ip (260) used greatest stack depth: 12368 bytes left
> [ 3.609288] virtme-init: starting script
> test_dynamic_debug_submod not there
> test_dynamic_debug not there
> # BASIC_TESTS
> ...
> # Done on: Fri Apr 26 20:45:08 MDT 2024
> [ 4.765751] virtme-init: script returned {0}
> Powering off.
> [ 4.805790] ACPI: PM: Preparing to enter system sleep state S5
> [ 4.806223] kvm: exiting hardware virtualization
> [ 4.806564] reboot: Power down
> [jimc@frodo vx]$
>
>
> I've been running the kernel on my x86 desktop & laptop, booting with
> drm.debug=0x1f, then turning it all-off after sleep 15.
>
> a few highlights from a bare-metal boot:
>
> here modprobe amdgpu; dyndbg applies last bit/class/category, and finishes init,
> then drm and amdgpu start logging as they execute
> ...
> [ 9.019696] gandalf kernel: dyndbg: query 0: "class DRM_UT_ATOMIC +p" mod:amdgpu
> [ 9.019704] gandalf kernel: dyndbg: class-ref: amdgpu.DRM_UT_ATOMIC module:amdgpu nd:4754 nc:0 nu:1
> [ 9.020012] gandalf kernel: dyndbg: processed 1 queries, with 21 matches, 0 errs
> [ 9.020017] gandalf kernel: dyndbg: bit_4: 21 matches on class: DRM_UT_ATOMIC -> 0x1f
> [ 9.020021] gandalf kernel: dyndbg: applied bitmap: 0x1f to: 0x0 for amdgpu
> [ 9.020026] gandalf kernel: dyndbg: attach-client-module: module:amdgpu nd:4754 nc:0 nu:1
> [ 9.020031] gandalf kernel: dyndbg: 4754 debug prints in module amdgpu
> [ 9.055065] gandalf kernel: [drm] amdgpu kernel modesetting enabled.
> [ 9.055138] gandalf kernel: [drm:amdgpu_acpi_detect [amdgpu]] No matching acpi device found for AMD3000
> [ 9.055564] gandalf kernel: amdgpu: Virtual CRAT table created for CPU
> [ 9.055585] gandalf kernel: amdgpu: Topology: Add CPU node
> [ 9.055752] gandalf kernel: amdgpu 0000:0c:00.0: enabling device (0006 -> 0007)
> [ 9.055821] gandalf kernel: [drm] initializing kernel modesetting (NAVI10 0x1002:0x731F 0x148C:0x2398 0xC1).
> [ 9.055835] gandalf kernel: [drm] register mmio base: 0xFCB00000
> [ 9.055839] gandalf kernel: [drm] register mmio size: 524288
> [ 9.059148] gandalf kernel: [drm:amdgpu_discovery_set_ip_blocks [amdgpu]] number of dies: 1
> [ 9.059387] gandalf kernel: [drm:amdgpu_discovery_set_ip_blocks [amdgpu]] number of hardware IPs on die0: 39
> [ 9.059623] gandalf kernel: [drm:amdgpu_discovery_set_ip_blocks [amdgpu]] ATHUB(35) #0 v2.0.0:
> [ 9.059856] gandalf kernel: [drm:amdgpu_discovery_set_ip_blocks [amdgpu]] 0x00000c00
> [ 9.060096] gandalf kernel: [drm:amdgpu_discovery_set_ip_blocks [amdgpu]] 0x02408c00
> [ 9.060328] gandalf kernel: [drm:amdgpu_discovery_set_ip_blocks [amdgpu]] set register base offset for ATHUB
>
> a startup script, after sleep 15, turns off the logging:
>
> echo 0 > /sys/module/drm/parameters/debug
>
> heres 1st 2 bits/classes/categories being turned off:
>
> [ 29.105991] gandalf kernel: [drm:amdgpu_ih_process [amdgpu]] amdgpu_ih_process: rptr 90752, wptr 90784
> [ 29.118086] gandalf kernel: dyndbg: bits:0x0 > *.debug
> [ 29.118096] gandalf kernel: dyndbg: apply bitmap: 0x0 to: 0x1f for '*'
> [ 29.118102] gandalf kernel: dyndbg: query 0: "class DRM_UT_CORE -p" mod:*
> [ 29.118122] gandalf kernel: dyndbg: good-class: drm.DRM_UT_CORE module:drm nd:338 nc:1 nu:0
> [ 29.119548] gandalf kernel: dyndbg: class-ref: drm_kms_helper.DRM_UT_CORE module:drm_kms_helper nd:93
> nc:0 nu:1
> [ 29.119552] gandalf kernel: dyndbg: class-ref: drm_display_helper.DRM_UT_CORE module:drm_display_helper nd:151 nc:0 nu:1
> [ 29.119737] gandalf kernel: dyndbg: class-ref: amdgpu.DRM_UT_CORE module:amdgpu nd:4754 nc:0 nu:1
> [ 29.122181] gandalf kernel: [drm:amdgpu_ih_process [amdgpu]] amdgpu_ih_process: rptr 90784, wptr 90816
> [ 29.127687] gandalf kernel: dyndbg: processed 1 queries, with 466 matches, 0 errs
> [ 29.127690] gandalf kernel: dyndbg: bit_0: 466 matches on class: DRM_UT_CORE -> 0x0
> [ 29.127692] gandalf kernel: dyndbg: query 0: "class DRM_UT_DRIVER -p" mod:*
> [ 29.127696] gandalf kernel: dyndbg: good-class: drm.DRM_UT_DRIVER module:drm nd:338 nc:1 nu:0
> [ 29.127699] gandalf kernel: dyndbg: class-ref: drm_kms_helper.DRM_UT_DRIVER module:drm_kms_helper nd:93 nc:0 nu:1
> [ 29.127701] gandalf kernel: dyndbg: class-ref: drm_display_helper.DRM_UT_DRIVER module:drm_display_helper nd:151 nc:0 nu:1
> [ 29.127885] gandalf kernel: dyndbg: class-ref: amdgpu.DRM_UT_DRIVER module:amdgpu nd:4754 nc:0 nu:1
> [ 29.152925] gandalf kernel: dyndbg: processed 1 queries, with 1384 matches, 0 errs
>
>
> The resulting journal is ~14.6k lines, written in the 1st 15 (29)
> seconds of startup. I'm unsure what the 15/29 discrepancy might
> indicate/betray, besides a lot of logging work. sleep 15 is not the
> best stopwatch.
>
> recent spins thru lkp-test have also been SUCCESS-ful.
>
> Jim Cromie (35):
>
> old-bugs & cleanups:
> dyndbg: fix old BUG_ON in >control parser
> docs/dyndbg: update examples \012 to \n
> test-dyndbg: fixup CLASSMAP usage error
> dyndbg: reword "class unknown," to "class:_UNKNOWN_"
> dyndbg: make ddebug_class_param union members same size
> dyndbg: replace classmap list with a vector
> dyndbg: ddebug_apply_class_bitmap - add module arg, select on it
> dyndbg: split param_set_dyndbg_classes to _module & wrapper fns
> dyndbg: drop NUM_TYPE_ARRAY
> dyndbg: reduce verbose/debug clutter
> dyndbg: silence debugs with no-change updates
> dyndbg: tighten ddebug_class_name() 1st arg type
> dyndbg: tighten fn-sig of ddebug_apply_class_bitmap
> dyndbg: reduce verbose=3 messages in ddebug_add_module
> dyndbg-API: remove DD_CLASS_TYPE_(DISJOINT|LEVEL)_NAMES and code
>
> core fix & selftests:
> dyndbg-API: fix DECLARE_DYNDBG_CLASSMAP
> selftests-dyndbg: add tools/testing/selftests/dynamic_debug/*
> selftests-dyndbg: exit 127 if no facility
> dyndbg-API: promote DYNDBG_CLASSMAP_PARAM to API
> dyndbg-doc: add classmap info to howto
> dyndbg: treat comma as a token separator
> selftests-dyndbg: add comma_terminator_tests
> dyndbg: split multi-query strings with %
> selftests-dyndbg: test_percent_splitting multi-cmds on module classes
> docs/dyndbg: explain new delimiters: comma, percent
> selftests-dyndbg: add test_mod_submod
> selftests-dyndbg: test dyndbg-to-tracefs
> dyndbg-doc: explain flags parse 1st
> dyndbg: add __counted_by annotations
>
> DRM parts
> drm+drivers: adapt to use DYNDBG_CLASSMAP_{DEFINE,USE}
> drm-dyndbg: adapt to use DYNDBG_CLASSMAP_PARAM
> drm: use correct ccflags-y spelling
> drm-drivers: DRM_CLASSMAP_USE in 2nd batch of drivers, helpers
> drm: restore CONFIG_DRM_USE_DYNAMIC_DEBUG un-BROKEN
> drm-print: workaround compiler meh
>
> .../admin-guide/dynamic-debug-howto.rst | 99 ++-
> MAINTAINERS | 3 +-
> drivers/gpu/drm/Kconfig | 3 +-
> drivers/gpu/drm/Makefile | 3 +-
> 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/drm_gem_shmem_helper.c | 2 +
> drivers/gpu/drm/drm_print.c | 38 +-
> drivers/gpu/drm/gud/gud_drv.c | 2 +
> drivers/gpu/drm/i915/i915_params.c | 12 +-
> drivers/gpu/drm/mgag200/mgag200_drv.c | 2 +
> drivers/gpu/drm/nouveau/nouveau_drm.c | 12 +-
> drivers/gpu/drm/qxl/qxl_drv.c | 2 +
> drivers/gpu/drm/radeon/radeon_drv.c | 2 +
> drivers/gpu/drm/udl/udl_main.c | 2 +
> drivers/gpu/drm/vkms/vkms_drv.c | 2 +
> drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 2 +
> include/asm-generic/vmlinux.lds.h | 1 +
> include/drm/drm_print.h | 10 +
> include/linux/dynamic_debug.h | 130 ++-
> kernel/module/main.c | 3 +
> lib/Kconfig.debug | 24 +-
> lib/Makefile | 3 +
> lib/dynamic_debug.c | 443 ++++++----
> lib/test_dynamic_debug.c | 131 +--
> lib/test_dynamic_debug_submod.c | 17 +
> tools/testing/selftests/Makefile | 1 +
> .../testing/selftests/dynamic_debug/Makefile | 9 +
> tools/testing/selftests/dynamic_debug/config | 2 +
> .../dynamic_debug/dyndbg_selftest.sh | 765 ++++++++++++++++++
> 31 files changed, 1398 insertions(+), 363 deletions(-)
> create mode 100644 lib/test_dynamic_debug_submod.c
> create mode 100644 tools/testing/selftests/dynamic_debug/Makefile
> create mode 100644 tools/testing/selftests/dynamic_debug/config
> create mode 100755 tools/testing/selftests/dynamic_debug/dyndbg_selftest.sh
>
> --
> 2.44.0
>
^ permalink raw reply [flat|nested] 24+ messages in thread