Re: [PATCH v6 00/31] drm/dyndbg: Fix dynamic debug classmap regression

[Jason Baron <jbaron@akamai.com>]

On 11/18/25 3:18 PM, Jim Cromie wrote:
> !-------------------------------------------------------------------|
>    This Message Is From an External Sender
>    This message came from outside your organization.
> |-------------------------------------------------------------------!
> 
> hello all,
> 
> commit aad0214f3026 ("dyndbg: add DECLARE_DYNDBG_CLASSMAP macro")
> 
> added dyndbg's "classmaps" feature, which brought dyndbg's 0-off-cost
> debug to DRM.  Dyndbg wired to /sys/module/drm/parameters/debug,
> mapped its bits to classes named "DRM_UT_*", and effected the callsite
> enablements only on updates to the sys-node (and underlying >control).
> 
> Sadly, it hit a CI failure, resulting in:
> commit bb2ff6c27bc9 ("drm: Disable dynamic debug as broken")
> 
> The regression was that drivers, when modprobed, did not get the
> drm.debug=0xff turn-on action, because that had already been done for
> drm.ko itself.
> 
> The core design bug is in the DECLARE_DYNDBG_CLASSMAP macro.  Its use
> in both drm.ko (ie core) and all drivers.ko meant that they couldn't
> fundamentally distinguish their respective roles.  They each
> "re-defined" the classmap separately, breaking K&R-101.
> 
> My ad-hoc test scripting helped to hide the error from me, by 1st
> testing various combos of boot-time module.dyndbg=... and
> drm.debug=... configurations, and then inadvertently relying upon
> those initializations.
> 
> This series addresses both failings:
> 
> It replaces DECLARE_DYNDBG_CLASSMAP with
> 
> - `DYNAMIC_DEBUG_CLASSMAP_DEFINE`: Used by core modules (e.g.,
>    `drm.ko`) to define their classmaps.  Based upon DECLARE, it exports
>    the classmap so USE can use it.
> 
> - `DYNAMIC_DEBUG_CLASSMAP_USE`: this lets other "subsystem" users
>    create a linkage to the classmap defined elsewhere (ie drm.ko).
>    These users can then find their "parent" and apply its settings.
> 
> It adds a selftest script, and a 2nd "sub-module" to recapitulate
> DRM's multi-module "subsystem" use-case, including the specific
> failure scenario.
> 
> It also adds minor parsing enhancements, allowing easier construction
> of multi-part debug configurations.  These enhancements are used to
> test classmaps in particular, but are not otherwize required.
> 
> Thank you for your review.
> 
> P.S. Id also like to "tease" some other work:
> 
> 1. patchset to send pr_debugs to tracefs on +T flag
> 
>     allows 63 "private" tracebufs, 1 "common" one (at 0)
>     "drm.debug_2trace=0x1ff" is possible
>     from Lukas Bartoski
> 
> 2. patchset to save 40% of DATA_DATA footprint
> 
>     move (modname,filename,function) to struct _ddebug_site
>     save their descriptor intervals to 3 maple-trees
>     3 accessors fetch on descriptor, from trees
>     move __dyndbg_sites __section to INIT_DATA
> 
> 3. patchset to cache dynamic-prefixes
>     should hide 2.s cost increase.
> 
> 

Hi Jim,

I just wanted to confirm my understanding that the class names here are 
'global'. That is if say two different modules both used say the name 
"core" in their DYNAMIC_DEBUG_CLASSMAP_DEFINE() name array, then if the 
user did: echo "class core +p > control", then that would enable all the 
sites that had the class name "core" in both modules. One could add the 
"module" modifier to the request if needed.

One could prepend the module name to the class names to make them unique 
but it's not much different from adding a separate 'module blah' in the 
request. So probably fine as is, but maybe worth calling out in the docs 
a bit?

Thanks,

-Jason