Message-ID: <CAJfuBxxFWD0rEjm-va+Bjmf-m2nfOD_+ZEqKy22WX6QdugQCUw@mail.gmail.com>
Date: Thu, 11 Dec 2025 10:12:03 +1300
From: jim.cromie@...il.com
To: Jason Baron <jbaron@...mai.com>
Cc: linux-kernel@...r.kernel.org, dri-devel@...ts.freedesktop.org, 
	gregkh@...uxfoundation.org, ukaszb@...omium.org, louis.chauvet@...tlin.com
Subject: Re: [PATCH v6 00/31] drm/dyndbg: Fix dynamic debug classmap regression

On Thu, Dec 11, 2025 at 8:09 AM Jason Baron <jbaron@...mai.com> wrote:
>
>
>
> 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?
>

Yes. that is correct. class CORE is global.
If 2 different DEFINE()s give that classname,
the defining modules will both respond to `class CORE +p > control`
but they will get independent int values (which could be the same, but
dont have to be)

DRM is our case in point.
I reused DRM_UT_CORE...
because I didnt have a good reason to change it
that said, Daniel Vetter noted that the _UT_ part doesnt have a real reason.
So theres some space for a discussion, when I resend that patchset.

`module drm class DRM_UT_CORE +p > control`
will narrow the query and avoid all the drivers/helpers,
which could be what someone wants.
class DRM_UT_CORE would select drivers and helpers too,
so the DRM_UT_  disambiguation is appropriate.

I'll reread the docs to see if theres a bit more I can add to further
explain this.
Do you have any suggestions for wording to achieve this ?

thx,
JIm



> Thanks,
>
> -Jason

Powered by blists - more mailing lists