Re: [PATCH v3 6/7] venus: Make debug infrastructure more flexible

[jim . cromie]

calling out some thinkos

On Thu, Jun 11, 2020 at 3:19 PM <jim.cromie@xxxxxxxxx> wrote:
>
> heres what I have in mind.  whats described here is working.
> I'll send it out soon
>
> commit 20298ec88cc2ed64269c8be7b287a24e60a5347e
> Author: Jim Cromie <jim.cromie@xxxxxxxxx>
> 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, ...)

pr_debug, pr_debug_n now in printk.h

_?_?dynamic_.+_cl  adaptations in dynamic_debug.h

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

this is feeling wrong now.
setting these bits would have to trigger an update via ddebug_exec_query
kinda like setting a bit would trigger
       echo module $foo mflags $bitpos +p > control

its possible, but not 1st, or 2nd perhaps.
In general Im quite leery of rigging up some callback to do it.

its prudent to effect all debug changes via >control

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

wrong, ddebug_change cannot respond to changes of debugflags,
most it could do is consult it on queries


>     - 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, ....)'

now done, as pr_debug_n, pr_debug in printk.h

Ive adapted the macros with a "_cl(cl, " insertion,

also added trailing prcls to control output

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

close enough to see the thinkos