Re: [PATCH v1] dynamic_debug: add support for logs destination

[jim . cromie]

adding in Jason, not sure how he got missed.

On Mon, Oct 16, 2023 at 9:13 AM Łukasz Bartosik <lb@xxxxxxxxxxxx> wrote:
>
> czw., 12 paź 2023 o 20:48 <jim.cromie@xxxxxxxxx> napisał(a):
> >
> > > If you want the kernel to keep separate flight recorders I guess we could
> > > add that, but I don't think it currently exists for the dyndbg stuff at
> > > least. Maybe a flight recorder v2 feature, once the basics are in.
> > >
> >
> > dyndbg has   +p    writes to syslog
> > +T  would separately independently write the same to global trace
> >
> > This would allow  graceful switchover to tracefs,
> > without removing logging from dmesg, where most folks
> > (and any monitor tools) would expect it.
> >
> > Lukas (iiuc) wants to steer each site to just 1 destination.
> > Or maybe (in addition to +p > syslog) one trace destination,
> > either global via events, or a separate tracebuf
> >
> > Im ambivalent, but thinking the smooth rollover from syslog to trace
> > might be worth having to ease migration / weaning off syslog.
> >
> > And we have a 4 byte hole in struct _ddebug we could just use.
>
> I'm glad you brought that up. This means we can leave class_id field
> untouched, have separate +T in flags (for consistency)
> and dst_id can be easily 8 bits wide.
>
> Also can you point me to the latest version of writing debug logs to
> trace events (+T option).
> I would like to base trace instances work on that because both are
> closely related.
>

Ive got 3 branches, all stacked up on rc6
(last I checked, they were clean on drm-tip)

https://github.com/jimc/linux/tree/dd-fix-7c
the regression fix, reposting this version next.
it also grabs another flag bit: _DPRINTK_FLAGS_INCL_LOOKUP


https://github.com/jimc/linux/tree/dd-shrink-3b
split modname,filename,function to new __dyndbg_sites section
loads the 3 column values and their intervals into 3 maple trees
and implements 3 accessor functions to retrieve the vals
from the descriptor addresses.
it "works" but doesnt erase intervals properly on rmmod
it also claims another flag - _DPRINTK_FLAGS_PREFIX_CACHED
and uses it to mark cached prefix fragment that get created for enabled calls.

https://github.com/jimc/linux/tree/dd-kitchen-sink
this adds the +T flag stuff.
its still a little fuzzy, esp around the descriptor arg -
using it to render the prefix str at buffer-render time
got UNSAFE warnings - likely due to loadable module
descriptors which could or did go away between
capture and render (after rmmod)




> > Unless the align 8 is optional on 32-bits,
>
> I verified with "gcc -g -m32 ..." that the align(8) is honored on 32 bits.
>

this is sorta the opposite of what I was probing, but I think result
is the same.
istm  align(8) is only for JUMP_LABEL, nothing else in the struct
appears to need it
so moving it to the top trades the hole for padding.



> > I think we're never gonna close the hole anywhere.
> >
>
> :)
>
> > is align 8 a generic expression of an architectural simplifying constraint ?
> > or a need for 1-7 ptr offsets ?
> >
> >
> >
> >
> > > > That's my idea of it. It is interesting to see how far the requirements
> > > > can be reasonably realised.
> > >
> > > I think aside from the "make it available directly to unpriviledged
> > > userspace" everything sounds reasonable and doable.
> > >
> > > More on the process side of things, I think Jim is very much looking for
> > > acks and tested-by by people who are interested in better drm logging
> > > infra. That should help that things are moving in a direction that's
> > > actually useful, even when it's not yet entirely complete.
> > >
> >
> > yes, please.  Now posted at
> >
> > https://lore.kernel.org/lkml/20231012172137.3286566-1-jim.cromie@xxxxxxxxx/T/#t
> >
> > Lukas, I managed to miss your email in the send phase.
> > please consider yourself a direct recipient :-)
> >
> > thanks everyone
> >
> > > Cheers, Sima
> > > --
> > > Daniel Vetter
> > > Software Engineer, Intel Corporation
> > > http://blog.ffwll.ch