Messages in this thread | | | From | Łukasz Bartosik <> | Date | Tue, 14 Nov 2023 08:44:49 +0100 | Subject | Re: [PATCH v1 11/12] dyndbg: write debug logs to trace instance |
| |
wt., 14 lis 2023 o 02:08 <jim.cromie@gmail.com> napisał(a): > > 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 ? >
Jim,
Thanks but it doesn't answer my question.
How do you plan to enable output to tracefs event at a callsite level ?
In my original proposal it was enabled by setting trace destination to 0. Since we are moving to names instead of numbers now I guess we need to reserve a name for it not to clash with trace instance names provided by users. That's why I proposed to reserve name "event" for that purpose and be used as +T:event.
Or did I miss answer to that in our long discussion :> ?
Thanks, Lukasz
> 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. > > > > > > > > > > > > > > > > :-)
| |