[Intel-gfx] [PATCH v4 12/41] dyndbg: add DECLARE_DYNDBG_CLASSMAP

[jim.cromie at gmail.com]

On Fri, Jul 22, 2022 at 2:23 PM Jason Baron <jbaron at akamai.com> wrote:
>
>
>
> On 7/20/22 11:32, Jim Cromie wrote:
> > DECLARE_DYNDBG_CLASSMAP lets modules declare a set of classnames, this
> > opt-in authorizes dyndbg to allow enabling of prdbgs by their class:
> >
> >    :#> echo class DRM_UT_KMS +p > /proc/dynamic_debug/control
> >
> > This is just the setup; following commits deliver.
> >
> > The macro declares and initializes a static struct ddebug_class_map:
> >
> >  - maps approved class-names to class_ids used in module,
> >    by array order. forex: DRM_UT_*
> >  - class-name vals allow validation of "class FOO" queries
> >    using macro is opt-in
> >  - enum class_map_type - determines interface, behavior
> >
> > Each module has its own .class_id space, and only known class-names
> > will be authorized for a manipulation.  Only DRM stuff should know this:
> >
> >   :#> echo class DRM_UT_CORE +p > control     # across all modules
> >
> > pr_debugs (with default class_id) are still controllable as before.
> >
> > DECLARE_DYNDBG_CLASSMAP(_var, _maptype, _base, classes...) is::
> >
> >  _var: name of the static struct var. user passes to module_param_cb()
> >        if they want a sysfs node. (ive only done it this way).
> >
> >  _maptype: this is hard-coded to DD_CLASS_TYPE_DISJOINT for now.
> >
> >  _base: usually 0, it allows splitting 31 classes into subranges, so
> >       that multiple classes / sysfs-nodes can share the module's
> >       class-id space.
> >
> >  classes: list of class_name strings, these are mapped to class-ids
> >         starting at _base.  This class-names list must have a
> >         corresponding ENUM, with SYMBOLS that match the literals,
> >         and 1st enum val = _base.
> >
> > enum class_map_type has 4 values, on 2 factors::
> >
> >  - classes are disjoint/independent vs relative/x<y/verbosity.
> >    disjoint is basis, verbosity by overlay.
> >
> >  - input NUMBERS vs [+-]CLASS_NAMES
> >    uints, ideally hex.  vs  +DRM_UT_CORE,-DRM_UT_KMS
> >
>
> Could the naming here directly reflect the 2 factors? At least for me
> I think it's more readable. So something like:
>

yeah, those were 1st-pass names.
your names are more regular

> DD_CLASS_TYPE_DISJOINT_NUM
> DD_CLASS_TYPE_DISJOINT_NAME
> DD_CLASS_TYPE_LEVEL_NUM
> DD_CLASS_TYPE_LEVEL_NAME

s/_NAME/_NAMES/  - the plural suggests list,comma-separated

s/(DISJOINT)_NUM/$1_BITS/  - strongly signals expected input form

LEVEL - while VERBOSE spoke to me, LEVEL has legacy: include/linux/kern_levels.h
LEVELS - pluralize ?
NUM - Im not crazy about, but it stands well in opposition to _BITS