Messages in this thread |  | | From | jim.cromie@gmail ... | Date | Wed, 17 Jun 2020 07:23:34 -0600 | Subject | Re: [PATCH v2 20/24] dyndbg: WIP towards debug-print-class based callsite controls |
| |
On Wed, Jun 17, 2020 at 3:52 AM Petr Mladek <pmladek@suse.com> wrote: > > On Wed 2020-06-17 10:31:54, Daniel Thompson wrote: > > On Tue, Jun 16, 2020 at 02:05:27PM -0700, Joe Perches wrote: > > > On Tue, 2020-06-16 at 15:45 +0200, Petr Mladek wrote: > > > > On Sat 2020-06-13 09:57:34, Jim Cromie wrote: > > > > > 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. > > > > > > > > I agree that it might make sense to provide some common mechanism. > > > [] > > > > My problem with this approach is that it is too generic. Each class > > > > would have different meaning in each subsystem. > > > > > > > > It might help to replace any existing variants. But it would be hard > > > > for developers debugging the code. They would need to study/remember > > > > the meaning of these groups for particular subsystems. They would > > > > need to set different values for different messages. > > > > > > > > Could you please provide more details about the potential users? > > > > Would be possible to find some common patterns and common > > > > meaning of the groups? > > > > > > I doubt the utility of common patterns. > > > Verbosity is common but groupings are not. > > > > > > Look at the DRM patterns vs other groups. > > > > I've seen drm.debug mentioned a couple of times but the comments about > > it seem to only learn part of what is shows us. > > > > drm.debug is a form of common grouping but it acts at a sub-system level > > rather then whole system (and gives a whole sub-system enable/disable). > > This is where grouping makes most sense. > > > > The result is that drm.debug is easy to document, both in official > > kernel docs and in other resources (like the arch distro documentation). > > Having controls that are easy to document makes them easy to find and > > thus sub-system grouping leads directly to higher quality bug reports. > > Thanks a lot for explanation. > > Now, could anyone please tell me how this new dynamic debug feature > would allow to replace drm.debug option? > > I mean what steps/commands will be needed instead of, for example > drm.debug=0x3 command line option? > > Best Regards, > Petr
[jimc@frodo linux.git]$ git log -1 commit 12a67ffb3e63c40027e251b44b2abc77463dc2da (HEAD -> dd-v3) Author: Jim Cromie <jim.cromie@gmail.com> Date: Tue Jun 16 15:36:37 2020 -0600
dyndbg: export ddebug_exec_queries
Exporting ddebug_exec_queries will allow module authors using dynamic debug to actually control/enable/disable their own pr-debug callsites dynamically.
With it, module authors can tie any update of their internal debug variables to a corresponding ddebug_exec_queries invocation, which will modify all their selected callsites accordingly.
Generally, authors would exec +p or -p on some subsets of their set of callsites, and leave fmlt flags for user to choose the appropriate amount of structural context desired in the logs.
Depending upon the user needs, the module might be known, and therefore a waste of screen width, function would be valuable, file would be long and familiar to the author, etc.. That said, author could harness the message-prefix facility if they saw fit to do so. Any author preferences can be overridden with echo >control
Is it safe ?
ddebug_exec_queries() is currently 'exposed' to user space in several limited ways;
1 it is called from module-load callback, where it implements the $modname.dyndbg=+p "fake" parameter provided to all modules.
2 it handles query input from >control directly
IOW, it is "fully" exposed to local root user; exposing the same functionality to other kernel modules is no additional risk.
The other big issue to check is locking:
dyndbg has a single mutex, taken by ddebug_change to handle >control, and by ddebug_proc_(start|stop) to span `cat control`. ISTM this proposed export presents no locking problems.
drm use case:
drm.debug=0x03 appears to be a kernel boot-arg example, setting 2 internal debug flags. Each bit is probably controlling a separate subset of all debug-prints, they may be overlapping subsets.
Those subsets are *definitely* expressible as a few dyndbg queries each. Any arbitrary subset is.
drm.dyndbg='file drm_gem_* +p' # gem debug drm.dyndbg='file *_gem_* +p' # *gem debug
With this proposed export, drm authors could exec these examples, most likely in the callback that handles updates to the drm.debug variable. (END)
I should note that none of this needs the WIP patch
|  |