Messages in this thread | | | From | jim.cromie@gmail ... | Date | Mon, 13 Nov 2023 18:08:18 -0700 | Subject | Re: [PATCH v1 11/12] dyndbg: write debug logs to trace instance |
| |
On Mon, Nov 13, 2023 at 4:44 PM Łukasz Bartosik <lb@semihalf.com> wrote: > > pon., 13 lis 2023 o 19:59 <jim.cromie@gmail.com> napisał(a): > > > > On Sun, Nov 12, 2023 at 9:32 AM Łukasz Bartosik <lb@semihalf.com> wrote: > > > > > > pt., 10 lis 2023 o 21:03 <jim.cromie@gmail.com> napisał(a): > > > > > > > > On Fri, Nov 10, 2023 at 7:53 AM Łukasz Bartosik <lb@semihalf.com> wrote: > > > > > > > > > > sob., 4 lis 2023 o 22:49 <jim.cromie@gmail.com> napisał(a): > > > > > > > > > > > > On Fri, Nov 3, 2023 at 7:10 AM Łukasz Bartosik <lb@semihalf.com> wrote: > > > > > > > > > > > > > > When trace is enabled (T flag is set) and trace_dst field is set > > > > > > > to value greater than 0 (0 is reserved for trace events) then > > > > > > > debug logs will be written to trace instance pointed by trace_dst > > > > > > > value, for example when trace_dst value is 2 then debug logs will > > > > > > > be written to <debugfs>/tracing/instances/dyndbg_inst_2 instance. > > > > > > > Given trace instance will not be initialized until debug logs are > > > > > > > requested to be written to it and afer init will persist until > > > > > > > reboot. > > > > > > > > > > > > > > > > > > > restating 00 comments - > > > > > > > > > > > > you can get rid of integer destination ids by adding a new command: open/close > > > > > > > > > > > > $> echo \ > > > > > > open kms-instance \;\ > > > > > > class DRM_UT_KMS -T:kms-instance # preset-dests-disable-sites \;\ > > > > > > > /proc/dynamic_debug/control > > > > > > > > > > > > > > > > Instead of using above command to preset destination we could preset > > > > > destination with open command. I mean last successful > > > > > open would preset destination ? What do you think ? > > > > > > > > > > > > > I dont think it works - if open maps to a dest-number, (or implicit as > > > > TOP-of-stack) > > > > then you just have +T<dest-number> (or +T <implicit tos>) > > > > rather than +T:dest-name > > > > and you still have to keep track of what dest-numbers were already used. > > > > (or every new dest needs an explicit OPEN before it) > > > > > > > > and how do you then get back to default instance ? > > > > open 0 ? > > > > close <previous-handle> ? > > > > > > > > > > > > by using names, all opens can be at the top, > > > > (and thus document in 1 block all the named-instances) > > > > and any named dest that hasnt been opened is an error > > > > (not just reusing previous OPEN) > > > > > > > > > > Sorry, I should have been more specific with my proposal. Let me use > > > an example to clarify it: > > > open usb # -> create trace instance "usb" and make it default > > > echo module usbcore +T > /proc/dynamic_debug/control ## --> write usbcore > > > ## debug logs to trace instance named usb > > > open tbt --> create trace instance "tbt" and make it default > > > echo module aaa +T:usb > /proc/dynamic_debug/control --> write aaa > > > debug logs to trace instance named usb, instance usb has to be used > > > explicitly > > > > > > because now tbt is default trace instance > > > > that feels too magical/ action at a distance. > > > > ISTM it also muddles what the "default" is: > > > > my-default: > > what each callsite's current/preset dest is/was > > the only way to set it is with explicit [-+]T:outstream > > > > I see your point, I will follow your suggestion. > > > your-default: > > whatever was last opened. whether it was 2 or 50 lines above, > > or set weeks ago, the last time somebody opened an instance. > > > > and as more instances are created > > (potentially by different users? > > after all there are separate instances, > > and presumably separate interests), > > the default gets less predictable. > > > > > > > echo module bbb +T > /proc/dynamic_debug/control --> write bbb debug > > > logs to trace instance named tbt > > > close tbt --> close tbt trace instance, I omit this step but in order > > > for an instance to be successful closed it must not be used by any > > > callsite, after > > > closing tbt instance the usb becomes default instance > > > > so after 'close tbt', the previous 'open usb' is now top-of-stack ? > > > > how does that affect all existing callsite-users of tbt ? > > do they continue to use the trace-instance theyve been writing to ? > > If not, then reverting to the global instance seems much more predictable. > > > > Or are you proposing that the close fails because the instance is still in use ? > > this seems least surprising, > > and more robust in the face of the next 'open foo' > > which could otherwize reuse the dst_id mapped to tbt > > > > > > > > > > I agree that your method of setting default trace instance is more flexible: > > > class DRM_UT_KMS -T:kms-instance # preset-dests-disable-sites > > > > > > Maybe we can combine both to set default trace destination ? > > > > > > Also I think we need a reserved keyword for writing debug logs to > > > trace events - maybe "event" keyword ? > > > > do you mean "event" as a selector, like module, function, class, etc ? > > if so, what are the values ? > > any event under /sys/kernel/debug/tracing/events/ ? > > > > how does this get used ? > > > > I meant that we need to reserve name/keyword to enable writing debug > logs to trace events (prdbg and devdbg), for example > echo module usbcore +T:event > /proc/dynamic_debug/control > > Or do you anticipate other way to do it ?
way back, when I had even fewer clues, I sent patches to call trace-printk when +T was set. Steve didnt like it, I think cuz it could flood the tracebuf.
Thats why I added the prdbg and devdbg event-types, so that they could be disabled easily using /sys/kernel/debug/tracing/ putting them squarely under trace-control.
Note that this puts 2 off-switches in series, both tracefs and >control can disable all the pr_debug traffic, tracefs by event-type, and >control at individual callsite level.
echo 1 > /sys/kernel/debug/tracing/dyndbg/enable echo 1 > /sys/kernel/debug/tracing/dyndbg/prdbg/enable echo 1 > /sys/kernel/debug/tracing/dyndbg/devdbg/enable
I briefly thought about linking the 2 off-switches, but punted cuz I thought it complicated things, (how exactly would they get coupled?) and I didnt want to distract from larger goals
Does that address your question ?
On a related point, I also added drm_dbg and drm_devdbg. Those are issued from __drm_dbg & __drm_dev_dbg respectively when CONFIG_DRM_USE_DYNAMIC_DEBUG=n.
Im not sure theyre more useful than confusing yet.
> > > > > > > > > > > > > > > > > > > > > > > and +T w/o dest means use existing setting, not just 0 (unless thats > > > > > > the existing setting) > > > > > > > > > > > > > > > > Sounds good. > > > > > > > > > > > > > :-)
| |