Re: [PATCH v1 11/12] dyndbg: write debug logs to trace instance

[jim . cromie]

On Sun, Nov 12, 2023 at 9:32 AM Łukasz Bartosik <lb@xxxxxxxxxxxx> wrote:
>
> pt., 10 lis 2023 o 21:03 <jim.cromie@xxxxxxxxx> napisał(a):
> >
> > On Fri, Nov 10, 2023 at 7:53 AM Łukasz Bartosik <lb@xxxxxxxxxxxx> wrote:
> > >
> > > sob., 4 lis 2023 o 22:49 <jim.cromie@xxxxxxxxx> napisał(a):
> > > >
> > > > On Fri, Nov 3, 2023 at 7:10 AM Łukasz Bartosik <lb@xxxxxxxxxxxx> 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

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 ?

>
>
> >
> > > >
> > > > and +T  w/o dest means use existing setting, not just 0 (unless thats
> > > > the existing setting)
> > > >
> > >
> > > Sounds good.
> > >
> >
> > :-)