On 6/11/20 5:19 PM, jim.cromie@gmail.com wrote:

trimmed..

quoted

quoted

quoted

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

Cool. thanks for working on this!

commit 20298ec88cc2ed64269c8be7b287a24e60a5347e
Author: Jim Cromie [off-list ref]
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)

I think it might be nice to have this proposal to integrate level too
so we see how it fits in. Maybe level is just about we enable things?
So you still mark callsites with pr_debug_typed(), but we can eanble
them via something like, 'echo module foo level N +p  >
/proc/dynamic_debug/control'. And that and 'p' to all callsites <= N.
So anybody using pr_debug_typed() can use either style to enable/disable.

    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

So I thought this meant select group twelve. Aren't there 32 groups?

      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.

what is this for again?

    - 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 ?

seems like it should group 0 which you're calling untyped. So you can write group
0 call sites as pr_debug(); or pr_debug_typed(0, ...); I'm not sure why we should
treat it differently other than it can be written without the pr_debug_typed().

    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

Also wondering if the control file should display the type (if non-zero). I think
that would be nice.

Thanks,

-Jason