On Thu, Jun 18, 2020 at 1:40 PM Jason Baron <jbaron@xxxxxxxxxx> wrote: > > > > On 6/18/20 3:11 PM, jim.cromie@xxxxxxxxx wrote: > > On Thu, Jun 18, 2020 at 12:17 PM Jason Baron <jbaron@xxxxxxxxxx> 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 <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. > > > > 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