Message-ID: <CAJfuBxzE=A0vzsjNai_jU_16R_P0haYA-FHnjZcaHOR_3fy__A@mail.gmail.com>
Date:   Thu, 11 Jun 2020 15:19:21 -0600
From:   jim.cromie@...il.com
To:     Daniel Thompson <daniel.thompson@...aro.org>
Cc:     Stanimir Varbanov <stanimir.varbanov@...aro.org>,
        Joe Perches <joe@...ches.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Linux Documentation List <linux-doc@...r.kernel.org>,
        LKML <linux-kernel@...r.kernel.org>, linux-media@...r.kernel.org,
        linux-arm-msm@...r.kernel.org, linux-btrfs@...r.kernel.org,
        linux-acpi@...r.kernel.org, netdev@...r.kernel.org,
        Jason Baron <jbaron@...mai.com>
Subject: Re: [PATCH v3 6/7] venus: Make debug infrastructure more flexible

trimmed..

> > > Currently I think there not enough "levels" to map something like
> > > drm.debug to the new dyn dbg feature. I don't think it is intrinsic
> > > but I couldn't find the bit of the code where the 5-bit level in struct
> > > _ddebug is converted from a mask to a bit number and vice-versa.
> >
> > Here [1] is Joe's initial suggestion. But I decided that bitmask is a
> > good start for the discussion.
> >
> > I guess we can add new member uint "level" in struct _ddebug so that we
> > can cover more "levels" (types, groups).
>
> I don't think it is allocating only 5 bits that is the problem!
>
> The problem is that those 5 bits need not be encoded as a bitmask by
> dyndbg, that can simply be the category code for the message. They only
> need be converted into a mask when we compare them to the mask provided
> by the user.
>
>
> Daniel.


heres what I have in mind.  whats described here is working.
I'll send it out soon

commit 20298ec88cc2ed64269c8be7b287a24e60a5347e
Author: Jim Cromie <jim.cromie@...il.com>
Date:   Wed Jun 10 12:55:08 2020 -0600

    dyndbg: WIP towards module->debugflags based callsite controls

    There are *lots* of ad-hoc debug printing solutions in kernel,
    this is a 1st attempt at providing a common mechanism for many of them.

    Basically, there are 2 styles of debug printing:
    - levels, with increasing verbosity, 1-10 forex
    - bits/flags, independently controlling separate groups of dprints

    This patch does bits/flags (with no distinction made yet between 2)

    API:

    - change pr_debug(...)  -->  pr_debug_typed(type_id=0, ...)
    - all existing uses have type_id=0
    - developer creates exclusive types of log messages with type_id>0
      1, 2, 3 are disjoint groups, for example: hi, mid, low

    - !!type_id is just an additional callsite selection criterion

      Qfoo() { echo module foo $* >/proc/dynamic_debug/control }
      Qfoo +p               # all groups, including default 0
      Qfoo mflags 1 +p      # only group 1
      Qfoo mflags 12 +p     # TBD[1]: groups 1 or 2
      Qfoo mflags 0 +p      # ignored atm TBD[2]
      Qfoo mflags af +p     # TBD[3]: groups a or f (10 or 15)

    so patch does:

    - add u32 debugflags to struct module. Each bit is a separate print-class.

    - add unsigned int mflags into struct ddebug_query
      mflags matched in ddebug_change

    - add unsigned int type_id:5 to struct _ddebug
      picks a single debugflag bit.  No subclass or multitype nonsense.
      nice and dense, packs with other members.
      we will have a lot of struct _ddebugs.

    - in ddebug_change()
      filter on !! module->debugflags,
      IFF query->module is given, and matches dt->mod_name
      and query->mflags is given, and bitmatches module->debugflags

    - in parse_query()
      accept new query term: mflags $arg
      populate query->mflags
      arg-type needs some attention, but basic plumbing is there

    WIP: not included:

    - pr_debug_typed( bitpos=0, ....)
      aka: pr_debug_class() or pr_debug_id()
      the bitpos is 1<<shift, allowing a single type. no ISA relations.
      this covers OP's high,mid,low case, many others

    - no way to exersize new code in ddebug_change
      need pr_debug_typed() to make a (not-null) typed callsite.
      also no way to set module->debugflags

    Im relying on:
    cdf6d00696 dynamic_debug: don't duplicate modname in ddebug_add_module

    which copies the ptr-val from module->name to dt->mod_name, which
    allowed == tests instead of strcmp.

    That equivalence and a (void*) cast in use of container_of() seem to
    do the trick to get the module, then module->debugflags.

    Notes:

    1- A query ANDs all its query terms together, so Qfoo() above
    requires both "module foo" AND all additional query terms given in $*

    But since callsite type_id creates disjoint groups, "mflags 12" is
    nonsense if it means groups 1 AND 2.  Here, 1 OR 2 is meaningful, if
    its not judged to be too confusing.

    2- im not sure what this does atm, or should do
       Qfoo mflags 0 +p      # select only untyped ? or no flags check at all ?

    3- since modflags has 32 bits, [1-9a-v] could select any single type_id
       it is succinct, but arcane.
       its a crude alphanumeric map for developers to make flags mnemonic
       maybe query keyword should be mbits

Powered by blists - more mailing lists