Re: [PATCH v3 20/21] dyndbg: add user-flag, negating-flags, and filtering on flags

[jim . cromie]

On Thu, Jun 18, 2020 at 12:17 PM Jason Baron <jbaron@xxxxxxxxxx> wrote:
>
>
>
> On 6/18/20 1:40 PM, Petr Mladek wrote:
> > On Thu 2020-06-18 18:19:12, Petr Mladek wrote:
> >> On Wed 2020-06-17 10:25:35, Jim Cromie wrote:
> >>> 1. Add a user-flag [u] which works like the [pfmlt] flags, but has no
> >>> effect on callsite behavior; it allows incremental marking of
> >>> arbitrary sets of callsites.
> >>>
> >>> 2. Add [PFMLTU] flags, which negate their counterparts; P===!p etc.
> >>> And in ddebug_read_flags():
> >>>    current code does:       [pfmltu_] -> flags
> >>>    copy it to:              [PFMLTU_] -> mask
> >>>
> >>> also disallow both of a pair: ie no 'pP', no true & false.
> >>>
> >>> 3. Add filtering ops into ddebug_change(), right after all the
> >>> callsite-property selections are complete.  These filter on the
> >>> callsite's current flagstate before applying modflags.
> >>>
> >>> Why ?
> >>>
> >>> The u-flag & filter flags
> >>>
> >>> The 'u' flag lets the user assemble an arbitary set of callsites.
> >>> Then using filter flags, user can activate the 'u' callsite set.
> >>>
> >>>   #> echo 'file foo.c +u; file bar.c +u' > control   # and repeat
> >>>   #> echo 'u+p' > control
> >>>
> >>> Of course, you can continue to just activate your set without ever
> >>> marking it 1st, but you could trivially add the markup as you go, then
> >>> be able to use it as a constraint later, to undo or modify your set.
> >>>
> >>>   #> echo 'file foo.c +up' >control
> >>>   .. monitor, debug, finish ..
> >>>   #> echo 'u-p' >control
> >>>
> >>>   # then later resume
> >>>   #> echo 'u+p' >control
> >>>
> >>>   # disable some cluttering messages, and remove from u-set
> >>>   #> echo 'file noisy.c function:jabber_* u-pu' >control
> >>>
> >>>   # for doc, recollection
> >>>   grep =pu control > my-favorite-callsites
> >>>
> >>> Note:
> >>>
> >>> Your flagstate after boot is generally not all =_. -DDEBUG will arm
> >>> compiled callsites by default, $builtinmod.dyndbg=+p bootargs can
> >>> enable them early, and $module.dyndbg=+p bootargs will arm them when
> >>> the module is loaded.  But you could manage them with u-flags:
> >>>
> >>>   #> echo '-t' >control             # clear t-flag to use it as 2ndary markup
> >>>   #> echo 'p+ut' >control   # mark the boot-enabled set of callsites
> >>>   #> echo '-p' >control             # clean your dmesg -w stream
> >>>
> >>>   ... monitor, debug ..
> >>>   #> echo 'module of_interest $qterms +pu' >control # build your set of useful debugs
> >>>   #> echo 'module of_interest $qterms UT+pu' >control       # same, but dont alter ut marked set
> >>
> >> Does anyone requested this feature, please?
> >>
> >> For me, it is really hard to imagine people using these complex and hacky
> >> steps.
> >
> > I think that all this is motivated by adding support for module
> > specific groups.
> >
> > What about storing the group as yet another information for each
> > message? I mean the same way as we store module name, file, line,
> > function name.
> >
> > Then we could add API to define group for a given message:
> >
> >    pr_debug_group(group_id, fmt, ...);
> >
> > the interface for the control file might be via new keyword "group".
> > You could then do something like:
> >
> >    echo module=drm group=0x3 +p >control
> >
> > But more importantly you should add functions that might be called
> > when the drm.debug parameter is changes. I have already mentioned
> > it is another reply:
> >
> >     dd_enable_module_group(module_name, group_id);
> >     dd_disable_module_group(module_name, group_id);
> >
> >
> > It will _not_ need any new flag or flag filtering.
> >
> > Best Regards,
> > Petr
> >
>
> Yes, I'm wondering as well if people are really going to use the
> new flags and filter flags - I mentioned that here:
> https://lkml.org/lkml/2020/6/12/732
>

yes, I saw, and replied there.
but since that was v1, and we're on v3, we should refresh.

the central use-case is above, 1-liner version summarized here:

1- enable sites as you chase a problem with +up
2- examine them with grep =pu
3- change the set to suit, either by adding or subtracting callsites.
4- continue debugging, and changing callsites to suit
5- grep =pu control > ~/debugging-session-task1-callsites
6- echo up-p >control   # disable for now, leave u-set for later
7- do other stuff
8 echo uP+p >control # reactivate useful debug-state and resume


> The grouping stuff is already being used by lots of modules so
> that seems useful.

I now dont see the need.

given N debug callsites, any group can be defined by <N queries,
probably a lot less
if module authors can use ddebug_exec_queries(), cuz its exported, (15/21)
then they can act (+p or -p) on those sets defined by <N queries.

and now any callsite can be in any number of groups, not just one.
It would be prudent to evaluate such groupings case by case,
because the intersecting callsites are subject to "last manipulator wins"
but its unnecessary to insist that all sets are disjoint.
Unlike pr_debug_n, however its spelled.

>
> Thanks,
>
> -Jason