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@xxxxxxxxxxx> 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@xxxxxxxxx/ > >>> > >>> 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@xxxxxxxxxxx> > > > 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 >
