2024-04-29 19:39:42

by Jim Cromie

[permalink] [raw]
Subject: [PATCH v8 20/35] dyndbg-doc: add classmap info to howto

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: [email protected]
Signed-off-by: Jim Cromie <[email protected]>
---
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



2024-04-29 19:39:49

by Jim Cromie

[permalink] [raw]
Subject: [PATCH v8 21/35] dyndbg: treat comma as a token separator

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 <[email protected]>
Co-developed-by: Łukasz Bartosik <[email protected]>
Signed-off-by: Łukasz Bartosik <[email protected]>
---
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


2024-04-29 19:40:20

by Jim Cromie

[permalink] [raw]
Subject: [PATCH v8 24/35] selftests-dyndbg: test_percent_splitting multi-cmds on module classes

New fn tests multi-queries composed with % splitters. It uses both
test_dynamic_debug and test_dynamic_debug_submod, and manipulates
several classes at once. So despite the syntactic-oriented name, it
also tests classmaps.

Signed-off-by: Jim Cromie <[email protected]>
---
.../dynamic_debug/dyndbg_selftest.sh | 20 +++++++++++++++++++
1 file changed, 20 insertions(+)

diff --git a/tools/testing/selftests/dynamic_debug/dyndbg_selftest.sh b/tools/testing/selftests/dynamic_debug/dyndbg_selftest.sh
index 7a7d437e948b..ddb04c0a7fd2 100755
--- a/tools/testing/selftests/dynamic_debug/dyndbg_selftest.sh
+++ b/tools/testing/selftests/dynamic_debug/dyndbg_selftest.sh
@@ -231,9 +231,29 @@ function comma_terminator_tests {
ddcmd =_
}

+function test_percent_splitting {
+ echo -e "${GREEN}# TEST_PERCENT_SPLITTING - multi-command splitting on % ${NC}"
+ ifrmmod test_dynamic_debug_submod
+ ifrmmod test_dynamic_debug
+ ddcmd =_
+ modprobe test_dynamic_debug dyndbg=class,D2_CORE,+pf%class,D2_KMS,+pt%class,D2_ATOMIC,+pm
+ check_match_ct =pf 1
+ check_match_ct =pt 1
+ check_match_ct =pm 1
+ check_match_ct test_dynamic_debug 23 -r
+ # add flags to those callsites
+ ddcmd class,D2_CORE,+mf%class,D2_KMS,+lt%class,D2_ATOMIC,+ml
+ check_match_ct =pmf 1
+ check_match_ct =plt 1
+ check_match_ct =pml 1
+ check_match_ct test_dynamic_debug 23 -r
+ ifrmmod test_dynamic_debug
+}
+
tests_list=(
basic_tests
comma_terminator_tests
+ test_percent_splitting
)

# Run tests
--
2.44.0


2024-04-29 19:41:04

by Jim Cromie

[permalink] [raw]
Subject: [PATCH v8 28/35] dyndbg-doc: explain flags parse 1st

When writing queries to >control, flags are parsed 1st, since they are
the only required field. So if the flags draw an error, then keyword
errors aren't reported. This can be mildly confusing/annoying, so
explain it instead.

This note could be moved up to just after the grammar id's the flags,
and before the match-spec is detailed. Opinions ?

Signed-off-by: Jim Cromie <[email protected]>
---
Documentation/admin-guide/dynamic-debug-howto.rst | 10 ++++++++++
1 file changed, 10 insertions(+)

diff --git a/Documentation/admin-guide/dynamic-debug-howto.rst b/Documentation/admin-guide/dynamic-debug-howto.rst
index 7b570f29ae98..ccf3704f2143 100644
--- a/Documentation/admin-guide/dynamic-debug-howto.rst
+++ b/Documentation/admin-guide/dynamic-debug-howto.rst
@@ -106,6 +106,16 @@ The match-spec's select *prdbgs* from the catalog, upon which to apply
the flags-spec, all constraints are ANDed together. An absent keyword
is the same as keyword "*".

+Note: because the match-spec can be empty, the flags are checked 1st,
+then the pairs of keyword values. Flag errs will hide keyword errs:
+
+ bash-5.2# ddcmd mod bar +foo
+ dyndbg: read 13 bytes from userspace
+ dyndbg: query 0: "mod bar +foo" mod:*
+ dyndbg: unknown flag 'o'
+ dyndbg: flags parse failed
+ dyndbg: processed 1 queries, with 0 matches, 1 errs
+
A match specification is a keyword, which selects the attribute of
the callsite to be compared, and a value to compare against. Possible
keywords are:::
--
2.44.0


2024-04-29 19:41:08

by Jim Cromie

[permalink] [raw]
Subject: [PATCH v8 27/35] selftests-dyndbg: test dyndbg-to-tracefs

Add a series of trace-tests: test_actual_trace() etc, to validate that
the dyndbg-to-tracefs feature (using +T flag) works as intended. The
1st test uses the global tracebuf, the rest use/excercise private
tracebufs.

These tests are currently optional, via "TRACE" arg1, because the
feature code is in-the-lab. But its an objective test, and pretty
user-interface oriented.

IOW this passes:
:#> ./tools/testing/selftests/dynamic_debug/dyndbg_selftest.sh
but this fails:
:#> ./tools/testing/selftests/dynamic_debug/dyndbg_selftest.sh TRACE

So its won't break selftests success.

This allows the patch to be committed now w/o inducing selftest
failures, and the tests enabled later, with the promised code.

Signed-off-by: Jim Cromie <[email protected]>
Co-developed-by: Łukasz Bartosik <[email protected]>
Signed-off-by: Łukasz Bartosik <[email protected]>
---
.../dynamic_debug/dyndbg_selftest.sh | 435 ++++++++++++++++++
1 file changed, 435 insertions(+)

diff --git a/tools/testing/selftests/dynamic_debug/dyndbg_selftest.sh b/tools/testing/selftests/dynamic_debug/dyndbg_selftest.sh
index 54acee58cb4e..65f31418870f 100755
--- a/tools/testing/selftests/dynamic_debug/dyndbg_selftest.sh
+++ b/tools/testing/selftests/dynamic_debug/dyndbg_selftest.sh
@@ -308,6 +308,405 @@ function test_mod_submod {
check_match_ct =p 14 -v
}

+# tests below here are all actually using dyndbg->trace,
+# and verifying the writes
+
+function test_actual_trace {
+ echo -e "${GREEN}# TEST_ACTUAL_TRACE ${NC}"
+ ddcmd =_
+ echo > /sys/kernel/tracing/trace
+ echo 1 >/sys/kernel/tracing/tracing_on
+ echo 1 >/sys/kernel/tracing/events/dyndbg/enable
+ modprobe test_dynamic_debug dyndbg=class,D2_CORE,+T:0
+ search_trace "D2_CORE msg"
+ search_trace_name 0 1 "D2_CORE msg"
+ check_match_ct =T 1
+ tmark "trace-mark"
+ search_trace "trace-mark"
+ doprints
+ search_trace "D2_CORE msg"
+ ifrmmod test_dynamic_debug
+}
+
+function self_start {
+ echo \# open, modprobe +T:selftest
+ ddcmd open selftest
+ check_trace_instance_dir selftest 1
+ is_trace_instance_opened selftest
+ modprobe test_dynamic_debug dyndbg=+T:selftest.mf
+ check_match_ct =T:selftest.mf 5
+}
+
+function self_end_normal {
+ echo \# disable -T:selftest, rmmod, close
+ ddcmd module test_dynamic_debug -T:selftest # leave mf
+ check_match_ct =:selftest.mf 5 -v
+ ddcmd module test_dynamic_debug +:0
+ ddcmd close selftest
+ is_trace_instance_closed selftest
+ ifrmmod test_dynamic_debug
+}
+
+function self_end_disable_anon {
+ echo \# disable, close, rmmod
+ ddcmd module test_dynamic_debug -T
+ check_match_ct =:selftest.mf 5
+ ddcmd module test_dynamic_debug +:0
+ ddcmd close selftest
+ is_trace_instance_closed selftest
+ ifrmmod test_dynamic_debug
+}
+
+function self_end_disable_anon_mf {
+ echo \# disable, close, rmmod
+ ddcmd module test_dynamic_debug -Tf
+ check_match_ct =:selftest.m 5
+ ddcmd module test_dynamic_debug +:0
+ ddcmd close selftest
+ is_trace_instance_closed selftest
+ ifrmmod test_dynamic_debug
+}
+
+function self_end_nodisable {
+ echo \# SKIPPING: ddcmd module test_dynamic_debug -T:selftest
+ ddcmd close selftest fail # close fails because selftest is still being used
+ check_err_msg "Device or resource busy"
+ check_match_ct =T:selftest.mf 5
+ rmmod test_dynamic_debug
+ ddcmd close selftest # now selftest can be closed because rmmod removed
+ # all callsites which were using it
+ is_trace_instance_closed selftest
+}
+
+function self_end_delete_directory {
+ del_trace_instance_dir selftest 0
+ check_err_msg "Device or resource busy"
+ ddcmd module test_dynamic_debug -mT:selftest
+ check_match_ct =:selftest.f 5
+ del_trace_instance_dir selftest 0
+ check_err_msg "Device or resource busy"
+ ddcmd module test_dynamic_debug +:0
+ ddcmd close selftest
+ check_trace_instance_dir selftest 1
+ is_trace_instance_closed selftest
+ del_trace_instance_dir selftest 1
+ check_trace_instance_dir selftest 0
+}
+
+function test_early_close () {
+ ddcmd open kparm_stream
+ ddcmd module usbcore +T:kparm_stream.mf
+ check_match_ct =T:usb_stream.mf 161
+ echo ":not-running # ddcmd module usbcore -T:kparm_stream.mf"
+ ddcmd close kparm_stream
+}
+
+function self_test_ {
+ echo "# SELFTEST $1"
+ self_start
+ self_end_$1
+}
+
+function cycle_tests_normal {
+ echo -e "${GREEN}# CYCLE_TESTS_NORMAL ${NC}"
+ self_test_ normal # ok
+ self_test_ disable_anon # ok
+ self_test_ normal # ok
+ self_test_ disable_anon_mf # ok
+}
+
+function cycle_not_best_practices {
+ echo -e "${GREEN}# CYCLE_TESTS_PROBLEMS ${NC}"
+ self_test_ nodisable
+ self_test_ normal
+ self_test_ delete_directory
+}
+
+# proper life cycle - open, enable:named, disable:named, close
+function test_private_trace_simple_proper {
+ echo -e "${GREEN}# TEST_PRIVATE_TRACE_1 ${NC}"
+ # ddcmd close kparm_stream
+ ddcmd open kparm_stream
+ ddcmd module params +T:kparm_stream.mf
+ check_match_ct =T:kparm_stream.mf 4
+ ddcmd module params -T:kparm_stream.mf
+ check_match_ct =T:kparm_stream.mf 0
+ is_trace_instance_opened kparm_stream
+ ddcmd module params +:0
+ ddcmd close kparm_stream
+ is_trace_instance_closed kparm_stream
+ ddcmd =_
+ check_trace_instance_dir kparm_stream 1
+ is_trace_instance_closed kparm_stream
+ del_trace_instance_dir kparm_stream 1
+ check_trace_instance_dir kparm_stream 0
+}
+
+function test_private_trace_2 {
+ echo -e "${GREEN}# TEST_PRIVATE_TRACE_2 ${NC}"
+ ddcmd =_
+ echo > /sys/kernel/tracing/trace
+ echo 1 >/sys/kernel/tracing/tracing_on
+ echo 1 >/sys/kernel/tracing/events/dyndbg/enable
+ ddcmd open foo
+ is_trace_instance_opened foo
+ check_trace_instance_dir foo 1
+
+ modprobe test_dynamic_debug
+ ddcmd class,D2_CORE,+T:foo.l,%class,D2_KMS,+fT:foo.ml
+ check_match_ct =T:foo.l 1
+ check_match_ct =T:foo.mfl 1
+
+ # purpose ?
+ echo 1 >/sys/kernel/tracing/tracing_on
+ echo 1 >/sys/kernel/tracing/events/dyndbg/enable
+
+ tmark test_private_trace about to do_prints
+ search_trace "test_private_trace about to do_prints"
+ search_trace_name "0" 1 "test_private_trace about to do_prints"
+
+ doprints
+ ddcmd class,D2_CORE,-T:foo
+ ddcmd close foo fail
+ check_err_msg "Device or resource busy"
+ ddcmd class,D2_KMS,-T:foo
+ ddcmd close foo
+ check_trace_instance_dir foo 1
+ is_trace_instance_closed foo
+ ddcmd close bar fail
+ check_err_msg "No such file or directory"
+ ifrmmod test_dynamic_debug
+ search_trace_name foo 2 "D2_CORE msg"
+ search_trace_name foo 1 "D2_KMS msg"
+ del_trace_instance_dir foo 1
+ check_trace_instance_dir foo 0
+}
+
+function test_private_trace_3 {
+ echo -e "${GREEN}# TEST_PRIVATE_TRACE_3 ${NC}"
+ ddcmd =_
+ echo > /sys/kernel/tracing/trace
+ echo 1 >/sys/kernel/tracing/tracing_on
+ echo 1 >/sys/kernel/tracing/events/dyndbg/enable
+ ddcmd open foo \; open bar
+ is_trace_instance_opened foo
+ is_trace_instance_opened bar
+ modprobe test_dynamic_debug
+ ddcmd class,D2_CORE,+T:foo
+ ddcmd class,D2_KMS,+T:foo
+ ddcmd class D2_CORE +T:foo \; class D2_KMS +T:foo
+ ddcmd "class,D2_CORE,+T:foo;,class,D2_KMS,+T:foo"
+ ddcmd class,D2_CORE,+T:foo\;class,D2_KMS,+T:foo
+ check_match_ct =T 2 -v
+ check_match_ct =Tl 0
+ check_match_ct =Tmf 0
+ echo 1 >/sys/kernel/tracing/tracing_on
+ echo 1 >/sys/kernel/tracing/events/dyndbg/enable
+ tmark test_private_trace_2 about to do_prints
+ doprints
+ rmmod test_dynamic_debug
+ ddcmd "close bar;close foo"
+ is_trace_instance_closed bar
+ is_trace_instance_closed foo
+ search_trace_name foo 2 "D2_CORE msg"
+ search_trace_name foo 1 "D2_KMS msg"
+ del_trace_instance_dir foo 1
+ check_trace_instance_dir foo 0
+ search_trace "test_private_trace_2 about to do_prints"
+ del_trace_instance_dir bar 1
+ check_trace_instance_dir bar 0
+}
+
+function test_private_trace_4 {
+ echo -e "${GREEN}# TEST_PRIVATE_TRACE_4 ${NC}"
+ ddcmd =_
+ echo > /sys/kernel/tracing/trace
+ echo 1 >/sys/kernel/tracing/tracing_on
+ echo 1 >/sys/kernel/tracing/events/dyndbg/enable
+
+ ddcmd open foo
+ modprobe test_dynamic_debug dyndbg=class,D2_CORE,+T:foo%class,D2_KMS,+T:foo
+ check_match_ct =Tl 0
+ check_match_ct =Tmf 0
+ check_match_ct =T 2
+
+ # are these 2 doing anything ?
+ echo 1 >/sys/kernel/tracing/tracing_on
+ echo 1 >/sys/kernel/tracing/events/dyndbg/enable
+
+ tmark should be ready
+ search_trace_name "0" 0 "should be ready" # in global trace
+
+ doprints
+ search_trace_name foo 2 "D2_CORE msg" # in private buf
+ search_trace_name foo 1 "D2_KMS msg"
+
+ # premature delete
+ del_trace_instance_dir foo 0
+ check_trace_instance_dir foo 1 # doesn't delete
+ ifrmmod test_dynamic_debug
+
+ ddcmd "close foo"
+ is_trace_instance_closed foo
+ del_trace_instance_dir foo 1 # delete works now
+
+ check_trace_instance_dir foo 0
+ search_trace "should be ready"
+}
+
+# $1 - trace-buf-name (or "0")
+# $2 - reference-buffer
+function search_in_trace_for {
+ bufname=$1; shift;
+ ref=$2;
+ ref2=$(echo $ref | cut -c20-)
+ echo $ref2
+}
+
+function test_private_trace_mixed_class {
+ echo -e "${GREEN}# TEST_PRIVATE_TRACE_5 ${NC}"
+ ddcmd =_
+ ddcmd module,params,+T:unopened fail
+ check_err_msg "Invalid argument"
+ is_trace_instance_closed unopened
+ check_trace_instance_dir unopened 0
+
+ ddcmd open bupkus
+ is_trace_instance_opened bupkus
+ check_trace_instance_dir bupkus 1
+ modprobe test_dynamic_debug \
+ dyndbg=class,D2_CORE,+T:bupkus.mf%class,D2_KMS,+T:bupkus.mf%class,V3,+T:bupkus.mf
+
+ # test various name misses
+ ddcmd "module params =T:bupkus1" fail # miss on name
+ check_err_msg "Invalid argument"
+ ddcmd "module params =T:unopened" fail # unopened
+ check_err_msg "Invalid argument"
+
+ ddcmd "module params =mlfT:bupkus." # we allow dot at the end of flags
+ ddcmd "module params =T:bupkus."
+ ddcmd "module params =:bupkus."
+ ddcmd "module params =:0."
+
+ check_match_ct =T:bupkus.mf 3 # the 3 classes enabled above
+ # enable the 5 non-class'd pr_debug()s
+ ddcmd "module test_dynamic_debug =T:bupkus"
+ check_match_ct =T:bupkus 8 -r # 8=5+3
+
+ doprints
+ ddcmd close,bupkus fail
+ check_err_msg "Device or resource busy"
+ ddcmd "module * -T:0" # misses class'd ones
+ ddcmd close,bupkus fail
+
+ ddcmd class,D2_CORE,-T:0%class,D2_KMS,-T:0%class,V3,-T:0 # turn off class'd and set dest to 0
+ ddcmd close,bupkus
+ is_trace_instance_closed bupkus
+
+ # check results
+ eyeball_ref=<<EOD
+ modprobe-1173 [001] ..... 7.781008: 0: test_dynamic_debug:do_cats: test_dd: D2_CORE msg
+ modprobe-1173 [001] ..... 7.781010: 0: test_dynamic_debug:do_cats: test_dd: D2_KMS msg
+ modprobe-1173 [001] ..... 7.781010: 0: test_dynamic_debug:do_levels: test_dd: V3 msg
+ cat-1214 [001] ..... 7.905494: 0: test_dd: doing categories
+ cat-1214 [001] ..... 7.905495: 0: test_dynamic_debug:do_cats: test_dd: D2_CORE msg
+ cat-1214 [001] ..... 7.905496: 0: test_dynamic_debug:do_cats: test_dd: D2_KMS msg
+ cat-1214 [001] ..... 7.905497: 0: test_dd: doing levels
+ cat-1214 [001] ..... 7.905498: 0: test_dynamic_debug:do_levels: test_dd: V3 msg
+EOD
+
+ # validate the 3 enabled class'd sites, with mf prefix
+ search_trace_name bupkus 0 "test_dynamic_debug:do_cats: test_dd: D2_CORE msg"
+ search_trace_name bupkus 0 "test_dynamic_debug:do_cats: test_dd: D2_KMS msg"
+ search_trace_name bupkus 0 "test_dynamic_debug:do_levels: test_dd: V3 msg"
+
+ search_trace_name bupkus 0 "test_dd: doing levels"
+ search_trace_name bupkus 0 "test_dd: doing categories"
+
+ # reopen wo error
+ ddcmd open bupkus
+ is_trace_instance_opened bupkus
+ check_trace_instance_dir bupkus 1
+
+ ddcmd "module test_dynamic_debug =T:bupkus" # rearm the 5 plain-old prdbgs
+ check_match_ct =T:bupkus 5
+
+ doprints # 2nd time
+ search_trace_name bupkus 0 "test_dd: doing categories"
+ search_trace_name bupkus 0 "test_dd: doing levels""
+ search_trace_name bupkus 2 "test_dd: doing categories"
+ search_trace_name bupkus 1 "test_dd: doing levels""
+
+ ddcmd close,bupkus fail
+ check_err_msg "Device or resource busy"
+ del_trace_instance_dir bupkus 0
+ check_err_msg "Device or resource busy"
+ check_trace_instance_dir bupkus 1
+ is_trace_instance_opened bupkus
+ check_trace_instance_dir bupkus 1
+
+ # drop last users, now delete works
+ ddcmd "module * -T:0"
+ ddcmd close,bupkus
+ is_trace_instance_closed bupkus
+ del_trace_instance_dir bupkus 1
+ check_trace_instance_dir bupkus 0
+ ifrmmod test_dynamic_debug
+}
+
+function test_private_trace_overlong_name {
+ echo -e "${GREEN}# TEST_PRIVATE_TRACE_overlong_name ${NC}"
+ ddcmd =_
+ name="A_bit_lengthy_trace_instance_name"
+ ddcmd open $name
+ is_trace_instance_opened $name
+ check_trace_instance_dir $name 1
+
+ ddcmd "file kernel/module/main.c +T:$name.mf "
+ check_match_ct =T:A_bit_lengthy_trace_....mf 14
+
+ ddcmd "module * -T"
+ check_match_ct =:A_bit_lengthy_trace_....mf 14
+ check_match_ct kernel/module/main.c 14 -r # to be certain
+
+ ddcmd "module * -T:0"
+ ddcmd close,$name
+ is_trace_instance_closed $name
+ del_trace_instance_dir $name 1
+ check_trace_instance_dir $name 0
+}
+
+function test_private_trace_fill_trace_index {
+ echo -e "${GREEN}# TEST_PRIVATE_TRACE_fill_trace_index ${NC}"
+ ddcmd =_
+ name="trace_instance_"
+ for i in {1..63}
+ do
+ ddcmd open $name$i
+ is_trace_instance_opened $name$i
+ check_trace_instance_dir $name$i 1
+ done
+ # catch the 1-too-many err
+ ddcmd "open too_many" fail
+ check_err_msg "No space left on device"
+
+ ddcmd 'file kernel/module/main.c =T:trace_instance_63.m'
+ check_match_ct =T:trace_instance_63.m 14
+
+ for i in {1..62}
+ do
+ ddcmd close $name$i
+ is_trace_instance_closed $name$i
+ del_trace_instance_dir $name$i 1
+ check_trace_instance_dir $name$i 0
+ done
+ ddcmd "module * -T:0"
+ ddcmd close,trace_instance_63
+ is_trace_instance_closed trace_instance_63
+ del_trace_instance_dir trace_instance_63 1
+ check_trace_instance_dir trace_instance_63 0
+}
+
tests_list=(
basic_tests
# these require test_dynamic_debug*.ko
@@ -315,9 +714,28 @@ tests_list=(
test_percent_splitting
test_mod_submod
)
+trace_tests=(
+ # these tests write to tracebuf, and check its contents
+ test_actual_trace
+ cycle_tests_normal
+ cycle_not_best_practices
+ test_private_trace_1
+ test_private_trace_simple_proper
+ test_private_trace_2
+ test_private_trace_3
+ test_private_trace_4
+ test_private_trace_mixed_class
+ test_private_trace_mixed_class # again, to test pre,post conditions
+
+ test_private_trace_overlong_name
+
+ # works, takes 30 sec
+ test_private_trace_fill_trace_index
+)

# Run tests

+# rmmod modules to clear their possibly modified state
ifrmmod test_dynamic_debug_submod
ifrmmod test_dynamic_debug

@@ -326,5 +744,22 @@ do
$test
echo ""
done
+
+if [[ "$1" = "TRACE" ]] ; then
+
+ # rmmod modules to clear their possibly modified state
+ ifrmmod test_dynamic_debug_submod
+ ifrmmod test_dynamic_debug
+
+ for test in "${trace_tests[@]}"
+ do
+ $test
+ echo ""
+ done
+fi
+
+# leave modules in place at end
+# so that evidence of mishap is present
+
echo -en "${GREEN}# Done on: "
date
--
2.44.0


2024-04-29 19:41:25

by Jim Cromie

[permalink] [raw]
Subject: [PATCH v8 22/35] selftests-dyndbg: add comma_terminator_tests

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 <[email protected]>
---
.../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


2024-04-29 19:41:29

by Jim Cromie

[permalink] [raw]
Subject: [PATCH v8 23/35] dyndbg: split multi-query strings with %

Multi-query strings have long allowed:

modprobe drm dyndbg="class DRM_UT_CORE +p; class DRM_UT_KMS +p"
modprobe drm dyndbg=<<EOX
class DRM_UT_CORE +p
class DRM_UT_KMS +p
EOX

More recently, the need for quoting was avoided by treating a comma
like a space/token-terminator:

modprobe drm dyndbg=class,DRM_UT_CORE,+p\;class,DRM_UT_KMS,+p

That worked, but it left unfinished business; the semicolon in the
example above is a shell special-char (one of the bash control
operators), so it is brittle when passed in/down/around scripts. In
particular, it fails when passed to vng (virtme-ng).

So this patch adds '%' to the existing ';' and '\n' multi-cmd
separators, which is more shell-friendly, and also avoids quoting and
escaping hassles.

NOTE: it does break format matching on '%' patterns:

bash-5.2# ddcmd 'format "find-me: %foo" +p'
[ 203.900581] dyndbg: read 26 bytes from userspace
[ 203.900883] dyndbg: query 0: "format "find-me: " mod:*
[ 203.901118] dyndbg: unclosed quote: find-me:
[ 203.901355] dyndbg: tokenize failed
[ 203.901529] dyndbg: query 1: "foo" +p" mod:*
[ 203.901957] dyndbg: split into words: "foo"" "+p"
[ 203.902243] dyndbg: op='+' flags=0x1 maskp=0xffffffff
[ 203.902458] dyndbg: expecting pairs of match-spec <value>
[ 203.902703] dyndbg: query parse failed
[ 203.902871] dyndbg: processed 2 queries, with 0 matches, 2 errs
bash: echo: write error: Invalid argument

The '%' splits the input into 2 queries, and both fail. Given the
limited utility of matching against the working parts of a format
string "foo: %d bar %s", nothing is actually lost here.

Signed-off-by: Jim Cromie <[email protected]>
---
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 c1bc728cb050..625838bd74aa 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -596,7 +596,7 @@ static int ddebug_exec_queries(char *query, const char *modname)
int i, errs = 0, exitcode = 0, rc, nfound = 0;

for (i = 0; query; query = split) {
- split = strpbrk(query, ";\n");
+ split = strpbrk(query, "%;\n");
if (split)
*split++ = '\0';

--
2.44.0


2024-04-29 19:41:58

by Jim Cromie

[permalink] [raw]
Subject: [PATCH v8 25/35] docs/dyndbg: explain new delimiters: comma, percent

Add mention of comma and percent delimiters into the respective
paragraphs describing their equivalents: space and newline.

Signed-off-by: Jim Cromie <[email protected]>
---
.../admin-guide/dynamic-debug-howto.rst | 18 ++++++++++--------
1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/Documentation/admin-guide/dynamic-debug-howto.rst b/Documentation/admin-guide/dynamic-debug-howto.rst
index 742eb4230c6e..7b570f29ae98 100644
--- a/Documentation/admin-guide/dynamic-debug-howto.rst
+++ b/Documentation/admin-guide/dynamic-debug-howto.rst
@@ -73,16 +73,18 @@ Command Language Reference
==========================

At the basic lexical level, a command is a sequence of words separated
-by spaces or tabs. So these are all equivalent::
+by spaces, tabs, or commas. So these are all equivalent::

:#> ddcmd file svcsock.c line 1603 +p
:#> ddcmd "file svcsock.c line 1603 +p"
:#> ddcmd ' file svcsock.c line 1603 +p '
+ :#> ddcmd file,svcsock.c,line,1603,+p

-Command submissions are bounded by a write() system call.
-Multiple commands can be written together, separated by ``;`` or ``\n``::
+Command submissions are bounded by a write() system call. Multiple
+commands can be written together, separated by ``%``, ``;`` or ``\n``::

- :#> ddcmd "func pnpacpi_get_resources +p; func pnp_assign_mem +p"
+ :#> ddcmd func foo +p % func bar +p
+ :#> ddcmd func foo +p \; func bar +p
:#> ddcmd <<"EOC"
func pnpacpi_get_resources +p
func pnp_assign_mem +p
@@ -104,7 +106,6 @@ The match-spec's select *prdbgs* from the catalog, upon which to apply
the flags-spec, all constraints are ANDed together. An absent keyword
is the same as keyword "*".

-
A match specification is a keyword, which selects the attribute of
the callsite to be compared, and a value to compare against. Possible
keywords are:::
@@ -128,7 +129,6 @@ keywords are:::
``line-range`` cannot contain space, e.g.
"1-30" is valid range but "1 - 30" is not.

-
The meanings of each keyword are:

func
@@ -153,9 +153,11 @@ module
The given string is compared against the module name
of each callsite. The module name is the string as
seen in ``lsmod``, i.e. without the directory or the ``.ko``
- suffix and with ``-`` changed to ``_``. Examples::
+ suffix and with ``-`` changed to ``_``.
+
+ Examples::

- module sunrpc
+ module,sunrpc # with ',' as token separator
module nfsd
module drm* # both drm, drm_kms_helper

--
2.44.0


2024-04-29 19:42:13

by Jim Cromie

[permalink] [raw]
Subject: [PATCH v8 26/35] selftests-dyndbg: add test_mod_submod

This new test-fn runs 3 module/submodule modprobe scenarios, variously
using both the generic dyndbg=<queries> modprobe arg, and the
test-module's classmap-params to manipulate the test-mod*'s pr_debugs.
In all cases, the current flag-settings are counted and tested vs
expectations.

The 3rd scenario recapitulates the DRM_USE_DYNAMIC_DEBUG=y failure.

1. 2 modprobes (super then sub), with separate dyndbg=class-settings
check module specific flag settings

2. modprobe submod, supermod is auto-loaded
set supermod class-params
check expected enablements in super & submod

3. modprobe super, with param=setting (like drm.debug=0x1ef)
modprobe submod
validate submod's class'd pr_debugs get properly enabled

The test uses multi-queries, with both commas and percents (to avoid
spaces and quoting). This is the main reason the test wasn't earlier
in the patchset, closer to the classmap patches its validating.

With some tedium, the tests could be refactored to split out early
tests which avoid multi-cmds, and test only the class-params.

Signed-off-by: Jim Cromie <[email protected]>
---
.../dynamic_debug/dyndbg_selftest.sh | 60 +++++++++++++++++++
1 file changed, 60 insertions(+)

diff --git a/tools/testing/selftests/dynamic_debug/dyndbg_selftest.sh b/tools/testing/selftests/dynamic_debug/dyndbg_selftest.sh
index ddb04c0a7fd2..54acee58cb4e 100755
--- a/tools/testing/selftests/dynamic_debug/dyndbg_selftest.sh
+++ b/tools/testing/selftests/dynamic_debug/dyndbg_selftest.sh
@@ -250,10 +250,70 @@ function test_percent_splitting {
ifrmmod test_dynamic_debug
}

+function test_mod_submod {
+ echo -e "${GREEN}# TEST_MOD_SUBMOD ${NC}"
+ ifrmmod test_dynamic_debug_submod
+ ifrmmod test_dynamic_debug
+ ddcmd =_
+
+ # modprobe with class enablements
+ modprobe test_dynamic_debug dyndbg=class,D2_CORE,+pf%class,D2_KMS,+pt%class,D2_ATOMIC,+pm
+ check_match_ct '\[test_dynamic_debug\]' 23 -r
+ check_match_ct =pf 1
+ check_match_ct =pt 1
+ check_match_ct =pm 1
+
+ modprobe test_dynamic_debug_submod
+ check_match_ct test_dynamic_debug_submod 23 -r
+ check_match_ct '\[test_dynamic_debug\]' 23 -r
+ check_match_ct test_dynamic_debug 46 -r
+
+ # change classes again, this time submod too
+ ddcmd class,D2_CORE,+mf%class,D2_KMS,+lt%class,D2_ATOMIC,+ml "# add some prefixes"
+ check_match_ct =pmf 1 -v
+ check_match_ct =plt 1 -v
+ check_match_ct =pml 1 -v
+ # submod changed too
+ check_match_ct =mf 1 -v
+ check_match_ct =lt 1 -v
+ check_match_ct =ml 1 -v
+
+ # now work the classmap-params
+ # fresh start, to clear all above flags (test-fn limits)
+ ifrmmod test_dynamic_debug_submod
+ ifrmmod test_dynamic_debug
+ modprobe test_dynamic_debug_submod # get supermod too
+
+ echo 1 > /sys/module/test_dynamic_debug/parameters/p_disjoint_bits
+ echo 4 > /sys/module/test_dynamic_debug/parameters/p_level_num
+ # 2 mods * ( V1-3 + D2_CORE )
+ check_match_ct =p 8 -v
+ echo 3 > /sys/module/test_dynamic_debug/parameters/p_disjoint_bits
+ echo 0 > /sys/module/test_dynamic_debug/parameters/p_level_num
+ # 2 mods * ( D2_CORE, D2_DRIVER )
+ check_match_ct =p 4 -v
+ echo 0x16 > /sys/module/test_dynamic_debug/parameters/p_disjoint_bits
+ echo 0 > /sys/module/test_dynamic_debug/parameters/p_level_num
+ # 2 mods * ( D2_DRIVER, D2_KMS, D2_ATOMIC )
+ check_match_ct =p 6 -v
+
+ # recap DRM_USE_DYNAMIC_DEBUG regression
+ ifrmmod test_dynamic_debug_submod
+ ifrmmod test_dynamic_debug
+ # set super-mod params
+ modprobe test_dynamic_debug p_disjoint_bits=0x16 p_level_num=5
+ check_match_ct =p 7 -v
+ modprobe test_dynamic_debug_submod
+ # see them picked up by submod
+ check_match_ct =p 14 -v
+}
+
tests_list=(
basic_tests
+ # these require test_dynamic_debug*.ko
comma_terminator_tests
test_percent_splitting
+ test_mod_submod
)

# Run tests
--
2.44.0


2024-04-29 19:42:15

by Jim Cromie

[permalink] [raw]
Subject: [PATCH v8 33/35] drm-drivers: DRM_CLASSMAP_USE in 2nd batch of drivers, helpers

Add a DRM_CLASSMAP_USE declaration to 2nd batch of helpers and *_drv.c
files. For drivers, add the decl just above the module's PARAMs,
since it identifies the "inherited" drm.debug param.

Note: with CONFIG_DRM_USE_DYNAMIC_DEBUG=y, a module not also declaring
DRM_CLASSMAP_USE will have its class'd prdbgs stuck in the initial
(disabled, but for DEBUG) state.

The stuck sites are evident in /proc/dynamic_debug/control as:

class:_UNKNOWN_ _id:N # control's last column

rather than a proper "enumeration":

class:DRM_UT_CORE

TLDR: This set of updates was found by choosing M for all DRM-config
items I found (not allmodconfig), building & modprobing them, and
grepping "class unknown," control. There may yet be others.

Signed-off-by: Jim Cromie <[email protected]>
---
drivers/gpu/drm/drm_gem_shmem_helper.c | 2 ++
drivers/gpu/drm/gud/gud_drv.c | 2 ++
drivers/gpu/drm/mgag200/mgag200_drv.c | 2 ++
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 ++
8 files changed, 16 insertions(+)

diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c
index e435f986cd13..066d906e3199 100644
--- a/drivers/gpu/drm/drm_gem_shmem_helper.c
+++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
@@ -23,6 +23,8 @@
#include <drm/drm_prime.h>
#include <drm/drm_print.h>

+DRM_CLASSMAP_USE(drm_debug_classes);
+
MODULE_IMPORT_NS(DMA_BUF);

/**
diff --git a/drivers/gpu/drm/gud/gud_drv.c b/drivers/gpu/drm/gud/gud_drv.c
index 9d7bf8ee45f1..5b555045fce4 100644
--- a/drivers/gpu/drm/gud/gud_drv.c
+++ b/drivers/gpu/drm/gud/gud_drv.c
@@ -31,6 +31,8 @@

#include "gud_internal.h"

+DRM_CLASSMAP_USE(drm_debug_classes);
+
/* Only used internally */
static const struct drm_format_info gud_drm_format_r1 = {
.format = GUD_DRM_FORMAT_R1,
diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.c b/drivers/gpu/drm/mgag200/mgag200_drv.c
index 573dbe256aa8..88c5e24cc894 100644
--- a/drivers/gpu/drm/mgag200/mgag200_drv.c
+++ b/drivers/gpu/drm/mgag200/mgag200_drv.c
@@ -25,6 +25,8 @@ static int mgag200_modeset = -1;
MODULE_PARM_DESC(modeset, "Disable/Enable modesetting");
module_param_named(modeset, mgag200_modeset, int, 0400);

+DRM_CLASSMAP_USE(drm_debug_classes);
+
int mgag200_init_pci_options(struct pci_dev *pdev, u32 option, u32 option2)
{
struct device *dev = &pdev->dev;
diff --git a/drivers/gpu/drm/qxl/qxl_drv.c b/drivers/gpu/drm/qxl/qxl_drv.c
index beee5563031a..1971bfa8a8a6 100644
--- a/drivers/gpu/drm/qxl/qxl_drv.c
+++ b/drivers/gpu/drm/qxl/qxl_drv.c
@@ -65,6 +65,8 @@ module_param_named(modeset, qxl_modeset, int, 0400);
MODULE_PARM_DESC(num_heads, "Number of virtual crtcs to expose (default 4)");
module_param_named(num_heads, qxl_num_crtc, int, 0400);

+DRM_CLASSMAP_USE(drm_debug_classes);
+
static struct drm_driver qxl_driver;
static struct pci_driver qxl_pci_driver;

diff --git a/drivers/gpu/drm/radeon/radeon_drv.c b/drivers/gpu/drm/radeon/radeon_drv.c
index 7bf08164140e..d22308328c76 100644
--- a/drivers/gpu/drm/radeon/radeon_drv.c
+++ b/drivers/gpu/drm/radeon/radeon_drv.c
@@ -247,6 +247,8 @@ int radeon_cik_support = 1;
MODULE_PARM_DESC(cik_support, "CIK support (1 = enabled (default), 0 = disabled)");
module_param_named(cik_support, radeon_cik_support, int, 0444);

+DRM_CLASSMAP_USE(drm_debug_classes);
+
static struct pci_device_id pciidlist[] = {
radeon_PCI_IDS
};
diff --git a/drivers/gpu/drm/udl/udl_main.c b/drivers/gpu/drm/udl/udl_main.c
index 3ebe2ce55dfd..ba57c14454e5 100644
--- a/drivers/gpu/drm/udl/udl_main.c
+++ b/drivers/gpu/drm/udl/udl_main.c
@@ -19,6 +19,8 @@

#define NR_USB_REQUEST_CHANNEL 0x12

+DRM_CLASSMAP_USE(drm_debug_classes);
+
#define MAX_TRANSFER (PAGE_SIZE*16 - BULK_SIZE)
#define WRITES_IN_FLIGHT (20)
#define MAX_VENDOR_DESCRIPTOR_SIZE 256
diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c
index dd0af086e7fa..086797c4b82b 100644
--- a/drivers/gpu/drm/vkms/vkms_drv.c
+++ b/drivers/gpu/drm/vkms/vkms_drv.c
@@ -39,6 +39,8 @@

static struct vkms_config *default_config;

+DRM_CLASSMAP_USE(drm_debug_classes);
+
static bool enable_cursor = true;
module_param_named(enable_cursor, enable_cursor, bool, 0444);
MODULE_PARM_DESC(enable_cursor, "Enable/Disable cursor support");
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
index 58fb40c93100..c159f4d186a3 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
@@ -275,6 +275,8 @@ static int vmw_probe(struct pci_dev *, const struct pci_device_id *);
static int vmwgfx_pm_notifier(struct notifier_block *nb, unsigned long val,
void *ptr);

+DRM_CLASSMAP_USE(drm_debug_classes);
+
MODULE_PARM_DESC(restrict_iommu, "Try to limit IOMMU usage for TTM pages");
module_param_named(restrict_iommu, vmw_restrict_iommu, int, 0600);
MODULE_PARM_DESC(force_coherent, "Force coherent TTM pages");
--
2.44.0


2024-04-29 19:42:23

by Jim Cromie

[permalink] [raw]
Subject: [PATCH v8 34/35] drm: restore CONFIG_DRM_USE_DYNAMIC_DEBUG un-BROKEN

Time for some quality CI

Signed-off-by: Jim Cromie <[email protected]>
---
drivers/gpu/drm/Kconfig | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index 5a0c476361c3..b2ea73ae48f0 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -54,8 +54,7 @@ config DRM_DEBUG_MM

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


2024-04-29 19:42:42

by Jim Cromie

[permalink] [raw]
Subject: [PATCH v8 29/35] dyndbg: add __counted_by annotations

Tell the compiler about our vectors (array,length), in 2 places:

h: struct _ddebug_info, which keeps refs to the __dyndbg_* ELF/DATA
sections, these are all vectors with a length.

c: struct ddebug_table, which has sub-refs into _ddebug_info.*

Signed-off-by: Jim Cromie <[email protected]>
---
include/linux/dynamic_debug.h | 6 +++---
lib/dynamic_debug.c | 6 +++---
2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index 090fe9554db7..c54d2a4e1d48 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -146,9 +146,9 @@ struct ddebug_class_user {

/* 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;
+ struct _ddebug *descs __counted_by(num_descs);
+ struct ddebug_class_map *classes __counted_by(num_classes);
+ struct ddebug_class_user *class_users __counted_by(num_class_users);
unsigned int num_descs;
unsigned int num_classes;
unsigned int num_class_users;
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 625838bd74aa..390a35508fb6 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -49,9 +49,9 @@ 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;
- struct ddebug_class_user *class_users;
+ struct _ddebug *ddebugs __counted_by(num_ddebugs);
+ struct ddebug_class_map *classes __counted_by(num_classes);
+ struct ddebug_class_user *class_users __counted_by(num_class_users);
unsigned int num_ddebugs, num_classes, num_class_users;
};

--
2.44.0


2024-04-29 19:43:00

by Jim Cromie

[permalink] [raw]
Subject: [PATCH v8 30/35] drm+drivers: adapt to use DYNDBG_CLASSMAP_{DEFINE,USE}

Follow dynamic_debug API change from DECLARE_DYNDBG_CLASSMAP to
DYNDBG_CLASSMAP_{DEFINE,USE}.

Prior to this, we used DECLARE_DYNDBG_CLASSMAP, which was preserved to
decouple DRM conversion. I'm unsure of the full functionality
in-between, a round of lkp-testing will help.

Fixes: f158936b60a7 ("drm: POC drm on dyndbg - use in core, 2 helpers, 3 drivers.")

Signed-off-by: Jim Cromie <[email protected]>
---
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_print.c | 25 +++++++++++++------------
drivers/gpu/drm/i915/i915_params.c | 12 +-----------
drivers/gpu/drm/nouveau/nouveau_drm.c | 12 +-----------
include/drm/drm_print.h | 8 ++++++++
7 files changed, 26 insertions(+), 67 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index e4277298cf1a..b287f0cfd8fa 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -217,17 +217,7 @@ int amdgpu_damage_clips = -1; /* auto */

static void amdgpu_drv_delayed_reset_work_handler(struct work_struct *work);

-DECLARE_DYNDBG_CLASSMAP(drm_debug_classes, DD_CLASS_TYPE_DISJOINT_BITS, 0,
- "DRM_UT_CORE",
- "DRM_UT_DRIVER",
- "DRM_UT_KMS",
- "DRM_UT_PRIME",
- "DRM_UT_ATOMIC",
- "DRM_UT_VBL",
- "DRM_UT_STATE",
- "DRM_UT_LEASE",
- "DRM_UT_DP",
- "DRM_UT_DRMRES");
+DRM_CLASSMAP_USE(drm_debug_classes);

struct amdgpu_mgpu_info mgpu_info = {
.mutex = __MUTEX_INITIALIZER(mgpu_info.mutex),
diff --git a/drivers/gpu/drm/display/drm_dp_helper.c b/drivers/gpu/drm/display/drm_dp_helper.c
index f5d4be897866..d3a7df09846f 100644
--- a/drivers/gpu/drm/display/drm_dp_helper.c
+++ b/drivers/gpu/drm/display/drm_dp_helper.c
@@ -41,17 +41,7 @@

#include "drm_dp_helper_internal.h"

-DECLARE_DYNDBG_CLASSMAP(drm_debug_classes, DD_CLASS_TYPE_DISJOINT_BITS, 0,
- "DRM_UT_CORE",
- "DRM_UT_DRIVER",
- "DRM_UT_KMS",
- "DRM_UT_PRIME",
- "DRM_UT_ATOMIC",
- "DRM_UT_VBL",
- "DRM_UT_STATE",
- "DRM_UT_LEASE",
- "DRM_UT_DP",
- "DRM_UT_DRMRES");
+DRM_CLASSMAP_USE(drm_debug_classes);

struct dp_aux_backlight {
struct backlight_device *base;
diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c
index 2dafc39a27cb..e9d229a393f4 100644
--- a/drivers/gpu/drm/drm_crtc_helper.c
+++ b/drivers/gpu/drm/drm_crtc_helper.c
@@ -50,17 +50,7 @@

#include "drm_crtc_helper_internal.h"

-DECLARE_DYNDBG_CLASSMAP(drm_debug_classes, DD_CLASS_TYPE_DISJOINT_BITS, 0,
- "DRM_UT_CORE",
- "DRM_UT_DRIVER",
- "DRM_UT_KMS",
- "DRM_UT_PRIME",
- "DRM_UT_ATOMIC",
- "DRM_UT_VBL",
- "DRM_UT_STATE",
- "DRM_UT_LEASE",
- "DRM_UT_DP",
- "DRM_UT_DRMRES");
+DRM_CLASSMAP_USE(drm_debug_classes);

/**
* DOC: overview
diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c
index 699b7dbffd7b..4a5f2317229b 100644
--- a/drivers/gpu/drm/drm_print.c
+++ b/drivers/gpu/drm/drm_print.c
@@ -55,18 +55,19 @@ MODULE_PARM_DESC(debug, "Enable debug output, where each bit enables a debug cat
#if !defined(CONFIG_DRM_USE_DYNAMIC_DEBUG)
module_param_named(debug, __drm_debug, ulong, 0600);
#else
-/* classnames must match vals of enum drm_debug_category */
-DECLARE_DYNDBG_CLASSMAP(drm_debug_classes, DD_CLASS_TYPE_DISJOINT_BITS, 0,
- "DRM_UT_CORE",
- "DRM_UT_DRIVER",
- "DRM_UT_KMS",
- "DRM_UT_PRIME",
- "DRM_UT_ATOMIC",
- "DRM_UT_VBL",
- "DRM_UT_STATE",
- "DRM_UT_LEASE",
- "DRM_UT_DP",
- "DRM_UT_DRMRES");
+/* classnames must match value-symbols of enum drm_debug_category */
+DRM_CLASSMAP_DEFINE(drm_debug_classes, DD_CLASS_TYPE_DISJOINT_BITS,
+ DRM_UT_CORE,
+ "DRM_UT_CORE",
+ "DRM_UT_DRIVER",
+ "DRM_UT_KMS",
+ "DRM_UT_PRIME",
+ "DRM_UT_ATOMIC",
+ "DRM_UT_VBL",
+ "DRM_UT_STATE",
+ "DRM_UT_LEASE",
+ "DRM_UT_DP",
+ "DRM_UT_DRMRES");

static struct ddebug_class_param drm_debug_bitmap = {
.bits = &__drm_debug,
diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
index de43048543e8..dccf12d05105 100644
--- a/drivers/gpu/drm/i915/i915_params.c
+++ b/drivers/gpu/drm/i915/i915_params.c
@@ -29,17 +29,7 @@
#include "i915_params.h"
#include "i915_drv.h"

-DECLARE_DYNDBG_CLASSMAP(drm_debug_classes, DD_CLASS_TYPE_DISJOINT_BITS, 0,
- "DRM_UT_CORE",
- "DRM_UT_DRIVER",
- "DRM_UT_KMS",
- "DRM_UT_PRIME",
- "DRM_UT_ATOMIC",
- "DRM_UT_VBL",
- "DRM_UT_STATE",
- "DRM_UT_LEASE",
- "DRM_UT_DP",
- "DRM_UT_DRMRES");
+DRM_CLASSMAP_USE(drm_debug_classes);

#define i915_param_named(name, T, perm, desc) \
module_param_named(name, i915_modparams.name, T, perm); \
diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c b/drivers/gpu/drm/nouveau/nouveau_drm.c
index a947e1d5f309..27995c0c9b31 100644
--- a/drivers/gpu/drm/nouveau/nouveau_drm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
@@ -72,17 +72,7 @@
#include "nouveau_uvmm.h"
#include "nouveau_sched.h"

-DECLARE_DYNDBG_CLASSMAP(drm_debug_classes, DD_CLASS_TYPE_DISJOINT_BITS, 0,
- "DRM_UT_CORE",
- "DRM_UT_DRIVER",
- "DRM_UT_KMS",
- "DRM_UT_PRIME",
- "DRM_UT_ATOMIC",
- "DRM_UT_VBL",
- "DRM_UT_STATE",
- "DRM_UT_LEASE",
- "DRM_UT_DP",
- "DRM_UT_DRMRES");
+DRM_CLASSMAP_USE(drm_debug_classes);

MODULE_PARM_DESC(config, "option string to pass to driver core");
static char *nouveau_config;
diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
index 9cc473e5d353..905fc25bf65a 100644
--- a/include/drm/drm_print.h
+++ b/include/drm/drm_print.h
@@ -140,6 +140,14 @@ enum drm_debug_category {
DRM_UT_DRMRES
};

+#ifdef CONFIG_DRM_USE_DYNAMIC_DEBUG
+#define DRM_CLASSMAP_DEFINE(...) DYNDBG_CLASSMAP_DEFINE(__VA_ARGS__)
+#define DRM_CLASSMAP_USE(name) DYNDBG_CLASSMAP_USE(name)
+#else
+#define DRM_CLASSMAP_DEFINE(...)
+#define DRM_CLASSMAP_USE(name)
+#endif
+
static inline bool drm_debug_enabled_raw(enum drm_debug_category category)
{
return unlikely(__drm_debug & BIT(category));
--
2.44.0


2024-04-29 19:43:13

by Jim Cromie

[permalink] [raw]
Subject: [PATCH v8 31/35] drm-dyndbg: adapt to use DYNDBG_CLASSMAP_PARAM

use new export
---
drivers/gpu/drm/drm_print.c | 8 ++------
include/drm/drm_print.h | 6 ++++--
2 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c
index 4a5f2317229b..efdf82f8cbbb 100644
--- a/drivers/gpu/drm/drm_print.c
+++ b/drivers/gpu/drm/drm_print.c
@@ -69,12 +69,8 @@ DRM_CLASSMAP_DEFINE(drm_debug_classes, DD_CLASS_TYPE_DISJOINT_BITS,
"DRM_UT_DP",
"DRM_UT_DRMRES");

-static struct ddebug_class_param drm_debug_bitmap = {
- .bits = &__drm_debug,
- .flags = "p",
- .map = &drm_debug_classes,
-};
-module_param_cb(debug, &param_ops_dyndbg_classes, &drm_debug_bitmap, 0600);
+DRM_CLASSMAP_PARAM_REF(debug, __drm_debug, drm_debug_classes, p);
+
#endif

void __drm_puts_coredump(struct drm_printer *p, const char *str)
diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
index 905fc25bf65a..95c667934bbb 100644
--- a/include/drm/drm_print.h
+++ b/include/drm/drm_print.h
@@ -141,11 +141,13 @@ enum drm_debug_category {
};

#ifdef CONFIG_DRM_USE_DYNAMIC_DEBUG
-#define DRM_CLASSMAP_DEFINE(...) DYNDBG_CLASSMAP_DEFINE(__VA_ARGS__)
-#define DRM_CLASSMAP_USE(name) DYNDBG_CLASSMAP_USE(name)
+#define DRM_CLASSMAP_DEFINE(...) DYNDBG_CLASSMAP_DEFINE(__VA_ARGS__)
+#define DRM_CLASSMAP_USE(name) DYNDBG_CLASSMAP_USE(name)
+#define DRM_CLASSMAP_PARAM_REF(...) DYNDBG_CLASSMAP_PARAM_REF(__VA_ARGS__)
#else
#define DRM_CLASSMAP_DEFINE(...)
#define DRM_CLASSMAP_USE(name)
+#define DRM_CLASSMAP_PARAM_REF(...)
#endif

static inline bool drm_debug_enabled_raw(enum drm_debug_category category)
--
2.44.0


2024-04-29 19:43:25

by Jim Cromie

[permalink] [raw]
Subject: [PATCH v8 32/35] drm: use correct ccflags-y spelling

Incorrectly spelled CFLAGS- failed to add -DDYNAMIC_DEBUG_MODULE,
which broke builds with:

CONFIG_DRM_USE_DYNAMIC_DEBUG=y
CONFIG_DYNAMIC_DEBUG_CORE=y
CONFIG_DYNAMIC_DEBUG=n

Also add subdir-ccflags so that all drivers pick up the addition.

Fixes: 84ec67288c10 ("drm_print: wrap drm_*_dbg in dyndbg descriptor factory macro")
Signed-off-by: Jim Cromie <[email protected]>
---
drivers/gpu/drm/Makefile | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 104b42df2e95..313516fc2ad5 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -3,7 +3,8 @@
# Makefile for the drm device driver. This driver provides support for the
# Direct Rendering Infrastructure (DRI) in XFree86 4.1.0 and higher.

-CFLAGS-$(CONFIG_DRM_USE_DYNAMIC_DEBUG) += -DDYNAMIC_DEBUG_MODULE
+ccflags-$(CONFIG_DRM_USE_DYNAMIC_DEBUG) += -DDYNAMIC_DEBUG_MODULE
+subdir-ccflags-$(CONFIG_DRM_USE_DYNAMIC_DEBUG) += -DDYNAMIC_DEBUG_MODULE

drm-y := \
drm_aperture.o \
--
2.44.0


2024-04-29 19:44:01

by Jim Cromie

[permalink] [raw]
Subject: [PATCH v8 35/35] drm-print: workaround compiler meh

For some reason I cannot grok, I get an unused variable 'category'
warning/error, though the usage follows immediately. This drops the
local var and directly derefs in the macro-call, which somehow avoids
the warning.

commit 9fd6f61a297e ("drm/print: add drm_dbg_printer() for drm device specific printer")
CC: Jani Nikula <[email protected]>
Signed-off-by: Jim Cromie <[email protected]>
---
drivers/gpu/drm/drm_print.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c
index efdf82f8cbbb..c400441cd77e 100644
--- a/drivers/gpu/drm/drm_print.c
+++ b/drivers/gpu/drm/drm_print.c
@@ -183,11 +183,10 @@ void __drm_printfn_dbg(struct drm_printer *p, struct va_format *vaf)
{
const struct drm_device *drm = p->arg;
const struct device *dev = drm ? drm->dev : NULL;
- enum drm_debug_category category = p->category;
const char *prefix = p->prefix ?: "";
const char *prefix_pad = p->prefix ? " " : "";

- if (!__drm_debug_enabled(category))
+ if (!__drm_debug_enabled(p->category))
return;

/* Note: __builtin_return_address(0) is useless here. */
--
2.44.0


2024-04-30 18:50:26

by Jim Cromie

[permalink] [raw]
Subject: Re: [PATCH v8 29/35] dyndbg: add __counted_by annotations

On Mon, Apr 29, 2024 at 1:39 PM Jim Cromie <[email protected]> wrote:
>
> Tell the compiler about our vectors (array,length), in 2 places:
>

these are not flex-arrays, using counted-by is wrong here.

Ive dropped this commit, series rebases clean wo it.


> h: struct _ddebug_info, which keeps refs to the __dyndbg_* ELF/DATA
> sections, these are all vectors with a length.
>