Re: [PATCH v3 20/21] dyndbg: add user-flag, negating-flags, and filtering on flags
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. Not to say that using t-flag as a markup looks like a real hack. People either always need the line number in the kernel log or they do not need it at all. Let me repeat. Please, stop this non-sense. Best Regards, Petr
Re: [PATCH v3 20/21] dyndbg: add user-flag, negating-flags, and filtering on flags
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
Re: [PATCH v3 20/21] dyndbg: add user-flag, negating-flags, and filtering on flags
On Thu, Jun 18, 2020 at 10:19 AM Petr Mladek wrote: > > On Wed 2020-06-17 10:25:35, Jim Cromie wrote: OK. Please tell me how this chunk of prose fails to explain a use case for the u-flag we can differ on how useful it looks. if u-flag is useful, then filtering on flags is also needed, to use the flag to enable/disable an arbitrary set of callsites all the other "flag abuse" you disliked in last patch is avoidable, unless 2 people are chasing 2 separate problems, and need to keep their sets distinct > > 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. > > Not to say that using t-flag as a markup looks like a real hack. > People either always need the line number in the kernel log or > they do not need it at all. > > Let me repeat. Please, stop this non-sense. > > Best Regards, > Petr
Re: [PATCH v3 20/21] dyndbg: add user-flag, negating-flags, and filtering on flags
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 The grouping stuff is already being used by lots of modules so that seems useful. Thanks, -Jason
Re: [PATCH v3 20/21] dyndbg: add user-flag, negating-flags, and filtering on flags
On Thu, Jun 18, 2020 at 12:17 PM Jason Baron 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 > Thanks, > > -Jason
Re: [PATCH v3 20/21] dyndbg: add user-flag, negating-flags, and filtering on flags
On 6/18/20 3:11 PM, jim.cro...@gmail.com wrote: > On Thu, Jun 18, 2020 at 12:17 PM Jason Baron 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://urldefense.proofpoint.com/v2/url?u=https-3A__lkml.org_lkml_2020_6_12_732&d=DwIBaQ&c=96ZbZZcaMF4w0F4jpN6LZg&r=1fLh1mlLqbfetnnGsbwXfpwmGlG4m83mXgtV4vZ1B1A&m=vltk6sSzPDQIqO4gGkJeDY6jcEarG4xTztab2EHtPFY&s=6x1EHNoRxebA99Tu-C2i0s5dmdzyEF9bXVcv_cYoM_I&e= >> >> > > 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 probably a lot less > if module authors can use ddebug_exec_queries(), cuz its e
Re: [PATCH v3 20/21] dyndbg: add user-flag, negating-flags, and filtering on flags
On Thu, Jun 18, 2020 at 1:40 PM Jason Baron wrote:
>
>
>
> On 6/18/20 3:11 PM, jim.cro...@gmail.com wrote:
> > On Thu, Jun 18, 2020 at 12:17 PM Jason Baron wrote:
> >>
> >
> >> 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 > 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 >
> > 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.
> >
>
> hmm - so I think you are saying there is then no need to change the
> calling functions themselves - its still 'pr_debug()'. You could even
> use the 'format' qualifier for example to implement your groups that
> way.
>
> For example:
>
> pr_debug("failure type1: blah");
> pr_debug("failure type2: blah blah");
>
> and then do: ddebug_exec_queries("format type1 +p", module);
Exactly
and using format, which always have user relevant info,
and often some severity indication (forex warn info err)
are a workable classification scheme already in use at least informally
So Id expect that this classification can often be done in 1 query.
define the set of callsites in 1 query-string, add +p or -p to it, and
manipulate away.
Amplifying,
this is the only user interface of consequence in dyndbg.
/sys/.../verbose doesnt count
Letting module authors use it is the full-featured way,
everything else is crap (movie reference)
and would require far more maintenance
>
> I would be curious to see what Stanimir thinks of this proposal
> and whether it would work for his venus driver, which is what
> prompted this module group discussion.
>
Indeed.
Id also like to hear from drm folks
./drm/amd/display/include/logger_types.h:#define DC_LOG_SURFACE(...)
pr_debug("[SURFACE]:"__VA_ARGS__)
./drm/amd/display/include/logger_types.h:#define
DC_LOG_HW_LINK_TRAINING(...)
pr_debug("[HW_LINK_TRAINING]:"__VA_ARGS__)
./drm/amd/display/include/logger_types.h:#define DC_LOG_HW_AUDIO(...)
pr_debug("[HW_AUDIO]:"__VA_ARGS__)
./drm/amd/display/include/logger_types.h:#define DC_LOG_SCALER(...)
pr_debug("[SCALER]:"__VA_ARGS__)
./drm/amd/display/include/logger_types.h:#define DC_LOG_BIOS(...)
pr_debug("[BIOS]:"__VA_ARGS__)
./drm/amd/display/include/logger_types.h:#define
DC_LOG_BANDWIDTH_CALCS(...) pr_debug("[BANDWIDTH_CALCS]:"__VA_ARGS__)
./drm/amd/display/include/logger_types.h:#define DC_LOG_DML(...)
pr_debug("[DML]:"__VA_ARGS__)
./drm/amd/display/include/logger_types.h:#define DC_LOG_IF_TRACE(...)
pr_debug("[IF_TRACE]:"__VA_ARGS__)
./drm/amd/display/include/logger_types.h:#define DC_LOG_GAMMA(...)
pr_debug("[GAMMA]:"__VA_ARGS__)
./drm/amd/display/include/logger_types.h:#define DC_LOG_ALL_GAMMA(...)
pr_debug("[GAMMA]:"__VA_ARGS__)
./drm/amd/display/include/logger_types.h:#define
DC_LOG_ALL_TF_CHANNELS(...) pr_debug("[GAMMA]:"__VA_ARGS__)
those defines suggest that they are already doing this with existing formats
with the export,
they can implement this group control using dyndbg with little effort.
including the tie-in to the __debug var if thats useful
and of course, user can add or subtract from that set ad-hoc.
> Thanks,
>
> -Jason
thanks
jimc
Re: [PATCH v3 20/21] dyndbg: add user-flag, negating-flags, and filtering on flags
Hi Jason, Jim, On 6/18/20 10:40 PM, Jason Baron wrote: > > > On 6/18/20 3:11 PM, jim.cro...@gmail.com wrote: >> On Thu, Jun 18, 2020 at 12:17 PM Jason Baron 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://urldefense.proofpoint.com/v2/url?u=https-3A__lkml.org_lkml_2020_6_12_732&d=DwIBaQ&c=96ZbZZcaMF4w0F4jpN6LZg&r=1fLh1mlLqbfetnnGsbwXfpwmGlG4m83mXgtV4vZ1B1A&m=vltk6sSzPDQIqO4gGkJeDY6jcEarG4xTztab2EHtPFY&s=6x1EHNoRxebA99Tu-C2i0s5dmdzyEF9bXVcv_cYoM_I&e= >>> >>> >> >> 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 se
Re: [PATCH v3 20/21] dyndbg: add user-flag, negating-flags, and filtering on flags
On Thu, Jun 18, 2020 at 4:34 PM Stanimir Varbanov
wrote:
>
> Hi Jason, Jim,
>
> > I would be curious to see what Stanimir thinks of this proposal
> > and whether it would work for his venus driver, which is what
> > prompted this module group discussion.
>
> Hmm, we spin in a circle :)
>
> Infact this was my first way of implementing the groups in Venus driver,
> you can see it at [1].
>
> +#define VDBGL(fmt, args...) pr_debug("VENUSL: " fmt, ##args)
> +#define VDBGM(fmt, args...) pr_debug("VENUSM: " fmt, ##args)
> +#define VDBGH(fmt, args...) pr_debug("VENUSH: " fmt, ##args)
> +#define VDBGFW(fmt, args...) pr_debug("VENUSFW: " fmt, ##args)
>
I recall :-)
I think Greg K-Hs distaste for those defines was for using them,
as it tosses the utility of grep pr_debug
pr_debug("VENUSM:"
is barely longer than
VDBGM
with ddebug_exec_queries, you can leverage the existing format.
>
> [1] https://lkml.org/lkml/2020/5/21/668
>
> --
> regards,
> Stan
thanks
Jim
Re: [PATCH v3 20/21] dyndbg: add user-flag, negating-flags, and filtering on flags
On Thu 2020-06-18 13:11:05, jim.cro...@gmail.com wrote: > On Thu, Jun 18, 2020 at 12:17 PM Jason Baron wrote: > > 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. No, the repply only explains how the interface might be used. There is no prove that people would actually use it. > 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 In short, this feature allows repeatedly enable/disable some slowly growing maze of debug messages. Who need this, please? !!! If I am debugging then I add/remove debug messages. But I never enable/disable all of them repeatedly. Also this is far from the original problem. It was about debugging a single driver (venus, drm). In this case, people need something easy to use. The following is the easy way: drm.debug = area_of_interest venus.debug = level_of_interest echo module=drm group=area_of_interest +p >control [*] echo module=venus group=level_of_interes +p >control Anyway, why filtering and 'u' flag would be necessary to debug these drivers? Is anyone going to use it? I would really like to hear the motivation for these features. Has anyone asked for them? Or are these just some "interesting" ideas from some brainstorming? [*] Well, I wonder if the dyndbg interface would even be useful for drm because it it is actually split into many modules. So it might require creating/maintaining several filters. Best Regards, Petr PS: This is probably my last mail in this thread. It goes in cycle. You repeatedly explain how many possibilities the new features allow. I repeatedly doubt that they are worth it. I also proposed another solution for the original problem (venus) but it has never been commented. I just hope that these features will not get merged without a clear interest in them.
Re: [PATCH v3 20/21] dyndbg: add user-flag, negating-flags, and filtering on flags
On Fri 2020-06-19 09:45:55, Petr Mladek wrote: > On Thu 2020-06-18 13:11:05, jim.cro...@gmail.com wrote: > > On Thu, Jun 18, 2020 at 12:17 PM Jason Baron wrote: > > > 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. > > No, the repply only explains how the interface might be used. There is > no prove that people would actually use it. > > > 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 > > In short, this feature allows repeatedly enable/disable some > slowly growing maze of debug messages. Who need this, please? !!! > > If I am debugging then I add/remove debug messages. But I never > enable/disable all of them repeatedly. Not to say that I usually need to reboot when I reproduce the problem and before I could try it again. So all dyndbg flags gets lost between two tests anyway. Best Regards, Petr
Re: [PATCH v3 20/21] dyndbg: add user-flag, negating-flags, and filtering on flags
On Fri, Jun 19, 2020 at 10:10:24AM +0200, Petr Mladek wrote: > On Fri 2020-06-19 09:45:55, Petr Mladek wrote: > > On Thu 2020-06-18 13:11:05, jim.cro...@gmail.com wrote: > > > On Thu, Jun 18, 2020 at 12:17 PM Jason Baron wrote: > > > > 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. > > > > No, the repply only explains how the interface might be used. There is > > no prove that people would actually use it. > > > > > 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 > > > > In short, this feature allows repeatedly enable/disable some > > slowly growing maze of debug messages. Who need this, please? !!! > > > > If I am debugging then I add/remove debug messages. But I never > > enable/disable all of them repeatedly. > > Not to say that I usually need to reboot when I reproduce the problem > and before I could try it again. So all dyndbg flags gets lost > between two tests anyway. I agree, this feels way too complex for no good reason. Users only need a specific set of "run this command to enable messages and send us the logs" instructions. Nothing complex like this at all. thanks, greg k-h
Re: [PATCH v3 20/21] dyndbg: add user-flag, negating-flags, and filtering on flags
On 6/18/20 6:48 PM, jim.cro...@gmail.com wrote:
> On Thu, Jun 18, 2020 at 4:34 PM Stanimir Varbanov
> wrote:
>>
>> Hi Jason, Jim,
>>
>
>
>
>>> I would be curious to see what Stanimir thinks of this proposal
>>> and whether it would work for his venus driver, which is what
>>> prompted this module group discussion.
>>
>> Hmm, we spin in a circle :)
>>
>> Infact this was my first way of implementing the groups in Venus driver,
>> you can see it at [1].
>>
>> +#define VDBGL(fmt, args...) pr_debug("VENUSL: " fmt, ##args)
>> +#define VDBGM(fmt, args...) pr_debug("VENUSM: " fmt, ##args)
>> +#define VDBGH(fmt, args...) pr_debug("VENUSH: " fmt, ##args)
>> +#define VDBGFW(fmt, args...) pr_debug("VENUSFW: " fmt, ##args)
>>
>
> I recall :-)
>
> I think Greg K-Hs distaste for those defines was for using them,
> as it tosses the utility of grep pr_debug
>
> pr_debug("VENUSM:"
> is barely longer than
> VDBGM
>
> with ddebug_exec_queries, you can leverage the existing format.
>
>>
Ok, yes, I like this approach because its simple (just exports
ddebug_exec_queries()), and it seems to be quite flexible. Module
authors can 'tag' their queries any way they want.
We could provide some structure, if desired, something like:
#define DYNAMIC_DEBUG_LOW "-V "
#define DYNAMIC_DEBUG_MED "-VV "
#define DYNAMIC_DEBUG_HIGH "-VVV "
#define DYNAMIC_DEBUG_REALLY_HIGH "- "
And then these could be added to the pr_debug() so:
#define VDBGL(fmt, args...) pr_debug("VENUSL: " DYNAMIC_DEBUG_LOW fmt,
##args)
or
#define VDBGL(fmt, args...) pr_debug(DYNAMIC_DEBUG_LOW fmt, ##args)
or just:
pr_debug(DYNAMIC_DEBUG_LOW "ERROR HERE: %d", err)
Thanks,
-Jason
>> [1]
>> https://urldefense.proofpoint.com/v2/url?u=https-3A__lkml.org_lkml_2020_5_21_668&d=DwIBaQ&c=96ZbZZcaMF4w0F4jpN6LZg&r=1fLh1mlLqbfetnnGsbwXfpwmGlG4m83mXgtV4vZ1B1A&m=29UzIGELhVL1znJsgyjGDKGIEdSkSlCsmAh0jpbWHVQ&s=_szr6DQOsbdQ-oYCR9-fs4b-XG_fotTiObUfG3z6UtY&e=
>>
>>
>> --
>> regards,
>> Stan
>
> thanks
> Jim
>
Re: [PATCH v3 20/21] dyndbg: add user-flag, negating-flags, and filtering on flags
On Wed, 2020-06-17 at 10:25 -0600, 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. Does this set selection also allow for selection by increasing decimal level? Can sites be enabled for a value less than x?
Re: [PATCH v3 20/21] dyndbg: add user-flag, negating-flags, and filtering on flags
On Wed, Jun 17, 2020 at 4:13 PM Joe Perches wrote: > > On Wed, 2020-06-17 at 10:25 -0600, 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. > > Does this set selection also allow for selection by > increasing decimal level? > > Can sites be enabled for a value less than x? > > no, theres no levels added here. that would be a variation on the WIP pr-class patch in v2, dropped from v3 legacy dyndbg - select on RO callsite info only - file, module, func, line. flag state is write only. filterflags - additionally select on callsite's flagstate, ie reading the current flagstate please look at the export patch 15/21 for how I now think levels, and drm.debug=0x03 can be done.
