Message-ID: <ea069c8f-b724-409c-9dc6-345b3f9d6d85@bootlin.com>
Date: Mon, 8 Sep 2025 16:38:09 +0200
From: Louis Chauvet <louis.chauvet@...tlin.com>
To: Jim Cromie <jim.cromie@...il.com>, linux-kernel@...r.kernel.org,
 jbaron@...mai.com, gregkh@...uxfoundation.org, ukaszb@...omium.org
Cc: dri-devel@...ts.freedesktop.org, amd-gfx@...ts.freedesktop.org,
 intel-gvt-dev@...ts.freedesktop.org, intel-gfx@...ts.freedesktop.org,
 daniel.vetter@...ll.ch, tvrtko.ursulin@...ux.intel.com,
 jani.nikula@...el.com, ville.syrjala@...ux.intel.com, seanpaul@...omium.org,
 robdclark@...il.com, groeck@...gle.com, yanivt@...gle.com,
 bleung@...gle.com, quic_saipraka@...cinc.com, will@...nel.org,
 catalin.marinas@....com, quic_psodagud@...cinc.com, maz@...nel.org,
 arnd@...db.de, linux-arm-kernel@...ts.infradead.org,
 linux-arm-msm@...r.kernel.org, mingo@...hat.com, linux-doc@...r.kernel.org
Subject: Re: [PATCH v4 29/58] docs/dyndbg: add classmap info to howto



Le 03/08/2025 à 05:57, Jim Cromie a écrit :
> Describe the 3 API macros providing dynamic_debug's classmaps
> 
> DYNDBG_CLASSMAP_DEFINE - create & export a 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
> 
> TBD: some of this might be over-specification, or just over-talked.
> 
> NB: The _DEFINE & _USE model makes the user dependent on the definer,
> just like EXPORT_SYMBOL(__drm_debug) already does.
> 
> cc: linux-doc@...r.kernel.org
> Signed-off-by: Jim Cromie <jim.cromie@...il.com>

Reviewed-by: Louis Chauvet <louis.chauvet@...tlin.com>

> ---
> v3- rework protection around PARAM
> 
> v0.5 adjustments per Randy Dunlap
> v0.7 checkpatch fixes
> v0.8 more
> v0.9 rewords
> 
> fixup-howto
> ---
>   .../admin-guide/dynamic-debug-howto.rst       | 137 ++++++++++++++++--
>   1 file changed, 126 insertions(+), 11 deletions(-)
> 
> diff --git a/Documentation/admin-guide/dynamic-debug-howto.rst b/Documentation/admin-guide/dynamic-debug-howto.rst
> index 1ceadf4f28f9f..556e00299ed35 100644
> --- a/Documentation/admin-guide/dynamic-debug-howto.rst
> +++ b/Documentation/admin-guide/dynamic-debug-howto.rst
> @@ -146,7 +146,9 @@ keywords are:::
>     "1-30" is valid range but "1 - 30" is not.
>   
>   
> -The meanings of each keyword are:
> +Keywords:::
> +
> +The meanings of each keyword are::
>   
>   func
>       The given string is compared against the function name
> @@ -194,16 +196,6 @@ format
>   	format "nfsd: SETATTR"  // a neater way to match a format with whitespace
>   	format 'nfsd: SETATTR'  // yet another way to match a format with whitespace
>   
> -class
> -    The given class_name is validated against each module, which may
> -    have declared a list of known class_names.  If the class_name is
> -    found for a module, callsite & class matching and adjustment
> -    proceeds.  Examples::
> -
> -	class DRM_UT_KMS	# a DRM.debug category
> -	class JUNK		# silent non-match
> -	// class TLD_*		# NOTICE: no wildcard in class names
> -
>   line
>       The given line number or range of line numbers is compared
>       against the line number of each ``pr_debug()`` callsite.  A single
> @@ -218,6 +210,24 @@ line
>   	line -1605          // the 1605 lines from line 1 to line 1605
>   	line 1600-          // all lines from line 1600 to the end of the file
>   
> +class
> +
> +    The given class_name is validated against each module, which may
> +    have declared a list of class_names it accepts.  If the class_name
> +    accepted by a module, callsite & class matching and adjustment
> +    proceeds.  Examples::
> +
> +	class DRM_UT_KMS	# a DRM.debug category
> +	class JUNK		# silent non-match
> +	// class TLD_*		# NOTICE: no wildcard in class names
> +
> +.. note ::
> +
> +    Unlike other keywords, classes are "name-to-change", not
> +    "omitting-constraint-allows-change".  See Dynamic Debug Classmaps
> +
> +Flags:::
> +
>   The flags specification comprises a change operation followed
>   by one or more flag characters.  The change operation is one
>   of the characters::
> @@ -394,3 +404,108 @@ 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
> +=======================
> +
> +The "class" keyword selects prdbgs based on author supplied,
> +domain-oriented names.  This complements the nested-scope keywords:
> +module, file, function, line.
> +
> +The main difference from the others: classes must be named to be
> +changed.  This protects them from generic overwrite:
> +
> +  # IOW this cannot undo any DRM.debug settings
> +  :#> ddcmd -p
> +
> +This protection is needed; /sys/module/drm/parameters/debug is ABI.
> +DRM.debug is authoritative when dyndbg is not used, dyndbg's PARAM
> +cannot undermine that guarantee just because its optional for DRM to
> +use it.
> +
> +  :#> echo 0x1ff > /sys/module/drm/parameters/debug
> +
> +So each class must be enabled individually (no wildcards):
> +
> +  :#> ddcmd class DRM_UT_CORE +p
> +  :#> ddcmd class DRM_UT_KMS +p
> +  # or more selectively
> +  :#> ddcmd class DRM_UT_CORE module drm +p
> +
> +That makes direct >control wordy and annoying, but it is a secondary
> +interface; it is not intended to replace the ABI, just slide in
> +underneath and reimplement it.
> +
> +However, since the sysfs/kparam is the ABI, if a classmap DEFINEr
> +doesn't also add a _CLASSMAP_PARAM, there is no ABI, and no protection
> +is needed.  In that case, class'd prdbgs would be enabled/disabled by
> +legacy (class-less) queries, as a convenience, and because there's no
> +need to enforce irrelevant rules.
> +
> +
> +Dynamic Debug Classmap API
> +==========================
> +
> +DRM.debug is built upon:
> +
> +- enum drm_debug_category: DRM_UT_<*> - <T> for short
> +- 23 categorized api macros: drm_dbg_<T>(), DRM_DEBUG_<T>()
> +- 5000 calls to them
> +- all calling to __pr_debug_cls(<T>, ...)
> +
> +Those compile-time const short ints are good for optimizing compilers;
> +a primary classmaps design goal was to keep that property.
> +So basically .class_id === category.
> +
> +Then we use the drm_categories DRM_UT_* enum for both the classnames
> +(stringified enum symbols) and their numeric values.
> +
> +Its expected that future users will also use categorized macros and an
> +enum-defined categorization scheme like DRM's, with dyndbg inserted in
> +similarly.
> +
> +DYNAMIC_DEBUG_CLASSMAP_DEFINE(var,type,_base,classnames) - this maps
> +classnames (a list of strings) onto class-ids consecutively, starting
> +at _base, it also maps the names onto CLASSMAP_PARAM bits 0..N.
> +
> +DYNAMIC_DEBUG_CLASSMAP_USE(var) - modules call this to refer to the
> +var _DEFINEd elsewhere (and exported).
> +
> +Classmaps are opt-in: modules invoke _DEFINE or _USE to authorize
> +dyndbg to update those classes.  "class FOO" queries are validated
> +against the classes, this finds the classid to alter; classes are not
> +directly selectable by their classid.
> +
> +NB: It is an inherent API limitation that the following are possible:
> +
> +  // these would be caught in review
> +  __pr_debug_cls(0, "fake DRM_UT_CORE msg");  // this works
> +  __pr_debug_cls(62, "un-known classid msg"); // this compiles, does nothing
> +
> +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)
> +
> +DYNAMIC_DEBUG_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.
> +
> +The PARAM interface also enforces the DD_CLASS_TYPE_LEVEL_NUM relation
> +amongst the contained classnames; all classes are independent in the
> +control parser itself; there is no implied meaning in names like "V4".
> +
> +Modules or module-groups (drm & drivers) can define multiple
> +classmaps, as long as they (all the classmaps) share the limited 0..62
> +per-module-group _class_id range, without overlap.
> +
> +If a module encounters a conflict between 2 classmaps its USEing, we
> +can extend the _USE macro with an offset to allow de-conflicting the
> +respective ranges.  Or they use the DEFINErs macro-api, but with new
> +enum symbols.
> +
> +``#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.

-- 
Louis Chauvet, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Powered by blists - more mailing lists