| Message ID | 20250125064619.8305-1-jim.cromie@gmail.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Fix CONFIG_DRM_USE_DYNAMIC_DEBUG=y | expand |
Despite a few latent flaws (1-3 below), classmaps-v1 went in with:
ee7d633f2dfb drm-print.h: include dyndbg header
84ec67288c10 drm_print: wrap drm_*_dbg in dyndbg descriptor factory macro
e820f52577b1 drm_print: interpose drm_*dbg with forwarding macros
f158936b60a7 drm: POC drm on dyndbg - use in core, 2 helpers, 3 drivers.
0406faf25fb1 drm_print: condense enum drm_debug_category
and
6ea3bf466ac6 dyndbg: test DECLARE_DYNDBG_CLASSMAP, sysfs nodes
b9400852c080 dyndbg: add drm.debug style (drm/parameters/debug) bitmap support
ace7c4bbb240 doc-dyndbg: edit dynamic-debug-howto for brevity, audience
753914ed85ac doc-dyndbg: describe "class CLASS_NAME" query support
a4a2a427413e dyndbg: validate class FOO by checking with module
c45f67ace832 dyndbg: add ddebug_attach_module_classes
66f4006b6ace kernel/module: add __dyndbg_classes section
aad0214f3026 dyndbg: add DECLARE_DYNDBG_CLASSMAP macro
3fc95d80a536 dyndbg: add __pr_debug_cls for testing
ca90fca7f7b5 dyndbg: add class_id to pr_debug callsites
b7b4eebdba7b dyndbg: gather __dyndbg[] state into struct _ddebug_info
Those flaws:
1. Regression during initialization. classmaps-v1 inits drm-dbg
callsites' static-keys when drm is "modprobed/initd" with
drm.debug=<initval>, but not during driver & helper modprobes.
Previously __drm_debug vs DRM_UT_* was checked at runtime, which
made this a non-issue. My test scripts passed dyndbg=<options>,
which obscured this lack of "propagation" to drivers, and I didn't
pick up on it.
2. DECLARE_DYNDBG_CLASSMAP violated a K&R rule "define once, refer
after", and is the root cause under 1.
3. classmaps-v1 was something of a flag-day patchset; dyndbg & DRM
parts were overly coupled, and test-dyndbg didn't validate multi-
module classes.
Consequently we got:
commit bb2ff6c27bc9 ("drm: Disable dynamic debug as broken")
Classmaps-v2 fixes these as follows:
2. replace DECLARE_DYNDBG_CLASSMAP with DYNDBG_CLASSMAP_DEFINE
(invoked once in drm-core) and DYNDBG_CLASSMAP_USE (repeatedly, in
drivers & helpers).
_DEFINE now exports the classmap, which _USE can repeatedly refer to.
_USE adds a usage/linkage record referring to the _DEFINEd (&
exported) classmap, in a new __dyndbg_class_users section.
1. Now at modprobe, dyndbg scans the new section, follows the linkage
to the _DEFINEr module, finds the (optional) kernel-param controlling
the classmap, examines its drm.debug=<initval>, and applies it to the
module being initialized.
3. test/validate dyndbg for multi-module classes wo DRM involvement.
A. add test_dynamic_debug_submod.ko, a clone of parent.
This allows recapitulating various multi-module scenarios.
B. add tools/testing/selftests/dynamic_debug/*
This verifies spec'd behavior, including the multi-module scenarios
made available in 3a.
C. to decouple dyndbg from DRM, DECLARE_DYNDBG_CLASSMAP is preserved
until after DRM adapts to the api change.
Since the last rev sent here/LKML:
https://lore.kernel.org/lkml/20240716185806.1572048-1-jim.cromie@gmail.com/
Ive rebased onto recent drm-tip, and tested with drm-trybot:
https://patchwork.freedesktop.org/series/139147/
and made the following additions:
1- drop class "protection" special case, per JBaron's preference.
current use is marked BROKEN so nobody to affect.
now framed as policy-choice:
#define ddebug_client_module_protects_classes false
2- compile-time arg-tests in DYNDBG_CLASSMAP_DEFINE
implement several required constraints, fail obviously.
3- modprobe time check of conflicting class-id reservations
only affects 2+classmaps users.
compile-time solution not apparent.
4- dyndbg can now cause modprobe to fail.
needed toq catch 3.
maybe some loose ends here on failure.
5- refactor & rename ddebug_attach_*module_classes
reduce repetetive boilerplate on 2 types: maps, users.
rework mostly brought forward in patchset to reduce churn
TBD: maybe squash more.
The recent trybot submissions (against drm-tip) pass CI.BAT, and fail
one or few CI.IGT tests randomly; re-tests do not repeat the failures.
its also at github.com:jimc/linux.git
commit c0f15eda9c3676149dedbc5a5fc0faee9550a2f7 (HEAD -> dd-fix-9s-ontip, ghlinux/dd-fix-9s-ontip)
Im running it on my laptop & desktop w/o issues.
The drivers/gpu/drm patches are RFC, I think there might be a place to
put a single DRM_CLASSMAP_USE(drm_ddbug_classes) to replace the
sprinkling of USEs in drivers and helpers. IIRC, I tried adding a
_DEFINE into drm_drv.c, that didn't do it.
I think the dyndbg core additions are ready for review and merging
into a (next-next) test/integration tree, I'd like to find out.
Jim Cromie (63):
pre-cleanup:
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
checkpatch: add an exception to the do-while wrapper advice
core change:
dyndbg-API: replace DECLARE_DYNDBG_CLASSMAP
dyndbg: check DYNDBG_CLASSMAP_DEFINE args at compile-time
dyndbg: add/use for_subvec() to reduce boilerplate
dyndbg: make proper substructs in _ddebug_info
dyndbg: drop premature optimization in ddebug_add_module
dyndbg: allow ddebug_add_module to fail
dyndbg: rework ddebug_attach_*module_classes()
dyndbg: fail modprobe on ddebug_class_range_overlap()
dyndbg: hoist the range-overlap checks
ddebug: cleanup-range-overlap fails
dyndbg-test: change do_prints testpoint to accept a loopct
selftests-dyndbg: add tools/testing/selftests/dynamic_debug/*
dyndbg-API: promote DYNDBG_CLASSMAP_PARAM to API
dyndbg-doc: add classmap info to howto
new features:
dyndbg: treat comma as a token separator
selftests-dyndbg: add comma_terminator_tests
dyndbg: split multi-query strings with %
selftests-dyndbg: test_percent_splitting
docs/dyndbg: explain new delimiters: comma, percent
finishing:
selftests-dyndbg: add test_mod_submod
docs/dyndbg: explain flags parse 1st
dyndbg: change __dynamic_func_call_cls* macros into expressions
dyndbg: drop "protection" of class'd pr_debugs from legacy queries
DRM: (phase 2)
drm: use correct ccflags-y spelling
checkpatch: dont warn about unused macro arg on empty body
drm-dyndbg: adapt drm core to use dyndbg classmaps-v2
drm-dyndbg: adapt DRM to invoke DYNDBG_CLASSMAP_PARAM
drm-print: fix config-dependent unused variable
drm-dyndbg: DRM_CLASSMAP_USE in amdgpu driver
drm-dyndbg: DRM_CLASSMAP_USE in i915 driver
drm-dyndbg: DRM_CLASSMAP_USE in drm_crtc_helper
drm-dyndbg: DRM_CLASSMAP_USE in drm_dp_helper
drm-dyndbg: DRM_CLASSMAP_USE in nouveau
drm-dyndbg: add DRM_CLASSMAP_USE to Xe driver
drm-dyndbg: add DRM_CLASSMAP_USE to virtio_gpu
drm-dyndbg: add DRM_CLASSMAP_USE to simpledrm
drm-dyndbg: add DRM_CLASSMAP_USE to bochs
drm-dyndbg: add DRM_CLASSMAP_USE to etnaviv
drm-dyndbg: add DRM_CLASSMAP_USE to gma500 driver
drm-dyndbg: add DRM_CLASSMAP_USE to radeon
drm-dyndbg: add DRM_CLASSMAP_USE to vmwgfx driver
drm-dyndbg: add DRM_CLASSMAP_USE to vkms driver
drm-dyndbg: add DRM_CLASSMAP_USE to udl driver
drm-dyndbg: add DRM_CLASSMAP_USE to mgag200 driver
drm-dyndbg: add DRM_CLASSMAP_USE to the gud driver
drm-dyndbg: add DRM_CLASSMAP_USE to the qxl driver
drm-dyndbg: add DRM_CLASSMAP_USE to the drm_gem_shmem_helper driver
drm: restore CONFIG_DRM_USE_DYNAMIC_DEBUG un-BROKEN
.../admin-guide/dynamic-debug-howto.rst | 116 +++-
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/etnaviv/etnaviv_drv.c | 2 +
drivers/gpu/drm/gma500/psb_drv.c | 2 +
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/tiny/bochs.c | 2 +
drivers/gpu/drm/tiny/simpledrm.c | 2 +
drivers/gpu/drm/udl/udl_main.c | 2 +
drivers/gpu/drm/virtio/virtgpu_drv.c | 2 +
drivers/gpu/drm/vkms/vkms_drv.c | 2 +
drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 2 +
drivers/gpu/drm/xe/xe_drm_client.c | 2 +
include/asm-generic/vmlinux.lds.h | 1 +
include/drm/drm_print.h | 12 +
include/linux/dynamic_debug.h | 193 ++++--
kernel/module/main.c | 15 +-
lib/Kconfig.debug | 24 +-
lib/Makefile | 3 +
lib/dynamic_debug.c | 555 +++++++++++-------
lib/test_dynamic_debug.c | 181 +++---
lib/test_dynamic_debug_submod.c | 17 +
scripts/checkpatch.pl | 3 +-
tools/testing/selftests/Makefile | 1 +
.../testing/selftests/dynamic_debug/Makefile | 9 +
tools/testing/selftests/dynamic_debug/config | 2 +
.../dynamic_debug/dyndbg_selftest.sh | 364 ++++++++++++
38 files changed, 1206 insertions(+), 425 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.47.0
On Fri, Jan 24, 2025 at 11:45:14PM -0700, Jim Cromie wrote: > This series fixes dynamic-debug's support for DRM debug-categories. > Classmaps-v1 evaded full review, and got committed in 2 chunks: > > b7b4eebdba7b..6ea3bf466ac6 # core dyndbg changes > 0406faf25fb1..ee7d633f2dfb # drm adoption > > DRM-CI found a regression during init with drm.debug=<initval>; the > static-keys under the drm-dbgs in drm.ko got enabled, but those in > drivers & helpers did not. > > Root Problem: > > DECLARE_DYNDBG_CLASSMAP violated a K&R rule "define once, refer > afterwards". Replace it with DYNDBG_CLASSMAP_DEFINE (invoked once in > drm-core) and DYNDBG_CLASSMAP_USE (invoked repeatedly, in drivers & > helpers). > > _DEFINE exports the classmap it creates (in drm.ko), other modules > _USE the classmap. The _USE adds a record ref'g the _DEFINEd (& > exported) classmap, in a 2nd __dyndbg_class_users section. > > So now at modprobe, dyndbg scans the new section after the 1st > __dyndbg_class_maps section, follows the linkage to the _DEFINEr > module, finds the (optional) kernel-param controlling the classmap, > examines its drm.debug=<initval>, and applies it to the module being > initialized. > > To recapitulate the multi-module problem wo DRM involvement, Add: > > A. tools/testing/selftests/dynamic_debug/* > > This alters pr_debugs in the test-modules, counts the results and > checks them against expectations. It uses this formula to test most > of the control grammar, including the new class keyword. > > B. test_dynamic_debug_submod.ko > > This alters the test-module to build both parent & _submod ko's, with > _DEFINE and _USE inside #if/#else blocks. This recap's DRM's 2 module > failure scenario, allowing A to exersize several cases. > > The #if/#else puts the 2 macro uses together for clarity, and gives > the 2 modules identical sets of debugs. > > Recent DRM-CI tests are here: > https://patchwork.freedesktop.org/series/139147/ > > Previous rev: > https://lore.kernel.org/lkml/20240716185806.1572048-1-jim.cromie@gmail.com/ > > Noteworthy Additions: > > 1- drop class "protection" special case, per JBaron's preference. > only current use is marked BROKEN so nobody to affect. > now framed as policy-choice: > #define ddebug_client_module_protects_classes() false > subsystems wanting protection can change this. > > 2- compile-time arg-tests in DYNDBG_CLASSMAP_DEFINE > implement several required constraints, and fail obviously. > > 3- modprobe time check of conflicting class-id reservations > only affects 2+classmaps users. > compile-time solution not apparent. > > 4- dyndbg can now cause modprobe to fail. > needed to catch 3. > maybe some loose ends here on failure. > > 5- refactor & rename ddebug_attach_*module_classes > reduce repetetive boilerplate on 2 types: maps, users. > rework mostly brought forward in patchset to reduce churn > TBD: maybe squash more. > > Several recent trybot submissions (against drm-tip) have been passing > CI.BAT, and failing one or few CI.IGT tests randomly; re-tests do not > reliably repeat the failures. > > its also at github.com:jimc/linux.git > dd-fix-9[st]-ontip & dd-fix-9-13 > > Ive been running it on my desktop w/o issues. > > The drivers/gpu/drm patches are RFC, I think there might be a single > place to call DRM_CLASSMAP_USE(drm_dedbug_classes) to replace the > sprinkling of _USEs in drivers and helpers. IIRC, I tried adding a > _DEFINE into drm_drv.c, that didn't do it, so I punted for now. > > I think the dyndbg core additions are ready for review and merging > into a (next-next) test/integration tree. So whose tree should this go through? And I think the last patch in this series isn't correct, it looks like a 000 email somehow. thanks, greg k-h
On Thu, Feb 20, 2025 at 09:31:41AM +0100, Greg KH wrote: > On Fri, Jan 24, 2025 at 11:45:14PM -0700, Jim Cromie wrote: > > This series fixes dynamic-debug's support for DRM debug-categories. > > Classmaps-v1 evaded full review, and got committed in 2 chunks: > > > > b7b4eebdba7b..6ea3bf466ac6 # core dyndbg changes > > 0406faf25fb1..ee7d633f2dfb # drm adoption > > > > DRM-CI found a regression during init with drm.debug=<initval>; the > > static-keys under the drm-dbgs in drm.ko got enabled, but those in > > drivers & helpers did not. > > > > Root Problem: > > > > DECLARE_DYNDBG_CLASSMAP violated a K&R rule "define once, refer > > afterwards". Replace it with DYNDBG_CLASSMAP_DEFINE (invoked once in > > drm-core) and DYNDBG_CLASSMAP_USE (invoked repeatedly, in drivers & > > helpers). > > > > _DEFINE exports the classmap it creates (in drm.ko), other modules > > _USE the classmap. The _USE adds a record ref'g the _DEFINEd (& > > exported) classmap, in a 2nd __dyndbg_class_users section. > > > > So now at modprobe, dyndbg scans the new section after the 1st > > __dyndbg_class_maps section, follows the linkage to the _DEFINEr > > module, finds the (optional) kernel-param controlling the classmap, > > examines its drm.debug=<initval>, and applies it to the module being > > initialized. > > > > To recapitulate the multi-module problem wo DRM involvement, Add: > > > > A. tools/testing/selftests/dynamic_debug/* > > > > This alters pr_debugs in the test-modules, counts the results and > > checks them against expectations. It uses this formula to test most > > of the control grammar, including the new class keyword. > > > > B. test_dynamic_debug_submod.ko > > > > This alters the test-module to build both parent & _submod ko's, with > > _DEFINE and _USE inside #if/#else blocks. This recap's DRM's 2 module > > failure scenario, allowing A to exersize several cases. > > > > The #if/#else puts the 2 macro uses together for clarity, and gives > > the 2 modules identical sets of debugs. > > > > Recent DRM-CI tests are here: > > https://patchwork.freedesktop.org/series/139147/ > > > > Previous rev: > > https://lore.kernel.org/lkml/20240716185806.1572048-1-jim.cromie@gmail.com/ > > > > Noteworthy Additions: > > > > 1- drop class "protection" special case, per JBaron's preference. > > only current use is marked BROKEN so nobody to affect. > > now framed as policy-choice: > > #define ddebug_client_module_protects_classes() false > > subsystems wanting protection can change this. > > > > 2- compile-time arg-tests in DYNDBG_CLASSMAP_DEFINE > > implement several required constraints, and fail obviously. > > > > 3- modprobe time check of conflicting class-id reservations > > only affects 2+classmaps users. > > compile-time solution not apparent. > > > > 4- dyndbg can now cause modprobe to fail. > > needed to catch 3. > > maybe some loose ends here on failure. > > > > 5- refactor & rename ddebug_attach_*module_classes > > reduce repetetive boilerplate on 2 types: maps, users. > > rework mostly brought forward in patchset to reduce churn > > TBD: maybe squash more. > > > > Several recent trybot submissions (against drm-tip) have been passing > > CI.BAT, and failing one or few CI.IGT tests randomly; re-tests do not > > reliably repeat the failures. > > > > its also at github.com:jimc/linux.git > > dd-fix-9[st]-ontip & dd-fix-9-13 > > > > Ive been running it on my desktop w/o issues. > > > > The drivers/gpu/drm patches are RFC, I think there might be a single > > place to call DRM_CLASSMAP_USE(drm_dedbug_classes) to replace the > > sprinkling of _USEs in drivers and helpers. IIRC, I tried adding a > > _DEFINE into drm_drv.c, that didn't do it, so I punted for now. > > > > I think the dyndbg core additions are ready for review and merging > > into a (next-next) test/integration tree. > > So whose tree should this go through? I'm trying to get some drm folks to review/test this, but thus far not much success :-/ I think it's good stuff, but I'm somewhat hesitant if no one else agrees that it's useful for CI or in-field crash-recording or whatever ... I guess worst case we can land it and hope it attracts more folks? Wrt tree I don't care, but I guess we should then also land the drm side too. -Sima > And I think the last patch in this series isn't correct, it looks like a > 000 email somehow. > > thanks, > > greg k-h
Le 20/02/2025 à 10:45, Simona Vetter a écrit : > On Thu, Feb 20, 2025 at 09:31:41AM +0100, Greg KH wrote: >> On Fri, Jan 24, 2025 at 11:45:14PM -0700, Jim Cromie wrote: >>> This series fixes dynamic-debug's support for DRM debug-categories. >>> Classmaps-v1 evaded full review, and got committed in 2 chunks: >>> >>> b7b4eebdba7b..6ea3bf466ac6 # core dyndbg changes >>> 0406faf25fb1..ee7d633f2dfb # drm adoption >>> >>> DRM-CI found a regression during init with drm.debug=<initval>; the >>> static-keys under the drm-dbgs in drm.ko got enabled, but those in >>> drivers & helpers did not. >>> >>> Root Problem: >>> >>> DECLARE_DYNDBG_CLASSMAP violated a K&R rule "define once, refer >>> afterwards". Replace it with DYNDBG_CLASSMAP_DEFINE (invoked once in >>> drm-core) and DYNDBG_CLASSMAP_USE (invoked repeatedly, in drivers & >>> helpers). >>> >>> _DEFINE exports the classmap it creates (in drm.ko), other modules >>> _USE the classmap. The _USE adds a record ref'g the _DEFINEd (& >>> exported) classmap, in a 2nd __dyndbg_class_users section. >>> >>> So now at modprobe, dyndbg scans the new section after the 1st >>> __dyndbg_class_maps section, follows the linkage to the _DEFINEr >>> module, finds the (optional) kernel-param controlling the classmap, >>> examines its drm.debug=<initval>, and applies it to the module being >>> initialized. >>> >>> To recapitulate the multi-module problem wo DRM involvement, Add: >>> >>> A. tools/testing/selftests/dynamic_debug/* >>> >>> This alters pr_debugs in the test-modules, counts the results and >>> checks them against expectations. It uses this formula to test most >>> of the control grammar, including the new class keyword. >>> >>> B. test_dynamic_debug_submod.ko >>> >>> This alters the test-module to build both parent & _submod ko's, with >>> _DEFINE and _USE inside #if/#else blocks. This recap's DRM's 2 module >>> failure scenario, allowing A to exersize several cases. >>> >>> The #if/#else puts the 2 macro uses together for clarity, and gives >>> the 2 modules identical sets of debugs. >>> >>> Recent DRM-CI tests are here: >>> https://patchwork.freedesktop.org/series/139147/ >>> >>> Previous rev: >>> https://lore.kernel.org/lkml/20240716185806.1572048-1-jim.cromie@gmail.com/ >>> >>> Noteworthy Additions: >>> >>> 1- drop class "protection" special case, per JBaron's preference. >>> only current use is marked BROKEN so nobody to affect. >>> now framed as policy-choice: >>> #define ddebug_client_module_protects_classes() false >>> subsystems wanting protection can change this. >>> >>> 2- compile-time arg-tests in DYNDBG_CLASSMAP_DEFINE >>> implement several required constraints, and fail obviously. >>> >>> 3- modprobe time check of conflicting class-id reservations >>> only affects 2+classmaps users. >>> compile-time solution not apparent. >>> >>> 4- dyndbg can now cause modprobe to fail. >>> needed to catch 3. >>> maybe some loose ends here on failure. >>> >>> 5- refactor & rename ddebug_attach_*module_classes >>> reduce repetetive boilerplate on 2 types: maps, users. >>> rework mostly brought forward in patchset to reduce churn >>> TBD: maybe squash more. >>> >>> Several recent trybot submissions (against drm-tip) have been passing >>> CI.BAT, and failing one or few CI.IGT tests randomly; re-tests do not >>> reliably repeat the failures. >>> >>> its also at github.com:jimc/linux.git >>> dd-fix-9[st]-ontip & dd-fix-9-13 >>> >>> Ive been running it on my desktop w/o issues. >>> >>> The drivers/gpu/drm patches are RFC, I think there might be a single >>> place to call DRM_CLASSMAP_USE(drm_dedbug_classes) to replace the >>> sprinkling of _USEs in drivers and helpers. IIRC, I tried adding a >>> _DEFINE into drm_drv.c, that didn't do it, so I punted for now. >>> >>> I think the dyndbg core additions are ready for review and merging >>> into a (next-next) test/integration tree. >> >> So whose tree should this go through? > > I'm trying to get some drm folks to review/test this, but thus far not > much success :-/ I think it's good stuff, but I'm somewhat hesitant if no I tested the VKMS driver with this, and it works! Tested-by: Louis Chauvet <louis.chauvet@bootlin.com> > one else agrees that it's useful for CI or in-field crash-recording or > whatever ... > > I guess worst case we can land it and hope it attracts more folks? > > Wrt tree I don't care, but I guess we should then also land the drm side > too. > -Sima > >> And I think the last patch in this series isn't correct, it looks like a >> 000 email somehow. >> >> thanks, >> >> greg k-h >
hello everyone, sorry for the late reply. I have a cleaner version cooking now. less inter-commit churn, by bringing more cleanups forward. I'll send a -v2 soon. (lets forget all the meandering crap versions I sent) Louis, thanks for testing !!!!! I wrote the test script and submod.ko so the lib/* parts would stand by themselves. And this time, I left the old DECLARE_ macro, so DRM doesnt get a flag-day breakage :-) But for ease of testing, I'll keep the DRM parts in the series. Taking 1st N commits is normal workflow ? On Fri, Feb 28, 2025 at 9:24 AM Louis Chauvet <louis.chauvet@bootlin.com> wrote: > > > > Le 20/02/2025 à 10:45, Simona Vetter a écrit : > > On Thu, Feb 20, 2025 at 09:31:41AM +0100, Greg KH wrote: > >> On Fri, Jan 24, 2025 at 11:45:14PM -0700, Jim Cromie wrote: > >>> This series fixes dynamic-debug's support for DRM debug-categories. > >>> Classmaps-v1 evaded full review, and got committed in 2 chunks: > >>> > >>> b7b4eebdba7b..6ea3bf466ac6 # core dyndbg changes > >>> 0406faf25fb1..ee7d633f2dfb # drm adoption > >>> > >>> DRM-CI found a regression during init with drm.debug=<initval>; the > >>> static-keys under the drm-dbgs in drm.ko got enabled, but those in > >>> drivers & helpers did not. > >>> > >>> Root Problem: > >>> > >>> DECLARE_DYNDBG_CLASSMAP violated a K&R rule "define once, refer > >>> afterwards". Replace it with DYNDBG_CLASSMAP_DEFINE (invoked once in > >>> drm-core) and DYNDBG_CLASSMAP_USE (invoked repeatedly, in drivers & > >>> helpers). > >>> > >>> _DEFINE exports the classmap it creates (in drm.ko), other modules > >>> _USE the classmap. The _USE adds a record ref'g the _DEFINEd (& > >>> exported) classmap, in a 2nd __dyndbg_class_users section. > >>> > >>> So now at modprobe, dyndbg scans the new section after the 1st > >>> __dyndbg_class_maps section, follows the linkage to the _DEFINEr > >>> module, finds the (optional) kernel-param controlling the classmap, > >>> examines its drm.debug=<initval>, and applies it to the module being > >>> initialized. > >>> > >>> To recapitulate the multi-module problem wo DRM involvement, Add: > >>> > >>> A. tools/testing/selftests/dynamic_debug/* > >>> > >>> This alters pr_debugs in the test-modules, counts the results and > >>> checks them against expectations. It uses this formula to test most > >>> of the control grammar, including the new class keyword. > >>> > >>> B. test_dynamic_debug_submod.ko > >>> > >>> This alters the test-module to build both parent & _submod ko's, with > >>> _DEFINE and _USE inside #if/#else blocks. This recap's DRM's 2 module > >>> failure scenario, allowing A to exersize several cases. > >>> > >>> The #if/#else puts the 2 macro uses together for clarity, and gives > >>> the 2 modules identical sets of debugs. > >>> > >>> Recent DRM-CI tests are here: > >>> https://patchwork.freedesktop.org/series/139147/ > >>> > >>> Previous rev: > >>> https://lore.kernel.org/lkml/20240716185806.1572048-1-jim.cromie@gmail.com/ > >>> > >>> Noteworthy Additions: > >>> > >>> 1- drop class "protection" special case, per JBaron's preference. > >>> only current use is marked BROKEN so nobody to affect. > >>> now framed as policy-choice: > >>> #define ddebug_client_module_protects_classes() false > >>> subsystems wanting protection can change this. > >>> > >>> 2- compile-time arg-tests in DYNDBG_CLASSMAP_DEFINE > >>> implement several required constraints, and fail obviously. > >>> > >>> 3- modprobe time check of conflicting class-id reservations > >>> only affects 2+classmaps users. > >>> compile-time solution not apparent. > >>> > >>> 4- dyndbg can now cause modprobe to fail. > >>> needed to catch 3. > >>> maybe some loose ends here on failure. > >>> > >>> 5- refactor & rename ddebug_attach_*module_classes > >>> reduce repetetive boilerplate on 2 types: maps, users. > >>> rework mostly brought forward in patchset to reduce churn > >>> TBD: maybe squash more. > >>> > >>> Several recent trybot submissions (against drm-tip) have been passing > >>> CI.BAT, and failing one or few CI.IGT tests randomly; re-tests do not > >>> reliably repeat the failures. > >>> > >>> its also at github.com:jimc/linux.git > >>> dd-fix-9[st]-ontip & dd-fix-9-13 > >>> > >>> Ive been running it on my desktop w/o issues. > >>> > >>> The drivers/gpu/drm patches are RFC, I think there might be a single > >>> place to call DRM_CLASSMAP_USE(drm_dedbug_classes) to replace the > >>> sprinkling of _USEs in drivers and helpers. IIRC, I tried adding a > >>> _DEFINE into drm_drv.c, that didn't do it, so I punted for now. > >>> > >>> I think the dyndbg core additions are ready for review and merging > >>> into a (next-next) test/integration tree. > >> > >> So whose tree should this go through? > > > > I'm trying to get some drm folks to review/test this, but thus far not > > much success :-/ I think it's good stuff, but I'm somewhat hesitant if no > > I tested the VKMS driver with this, and it works! > > Tested-by: Louis Chauvet <louis.chauvet@bootlin.com> > > > one else agrees that it's useful for CI or in-field crash-recording or > > whatever ... > > > > I guess worst case we can land it and hope it attracts more folks? > > > > Wrt tree I don't care, but I guess we should then also land the drm side > > too. > > -Sima > > > >> And I think the last patch in this series isn't correct, it looks like a > >> 000 email somehow. > >> > >> thanks, > >> > >> greg k-h > > > > -- > Louis Chauvet, Bootlin > Embedded Linux and Kernel engineering > https://bootlin.com >