Messages in this thread | | | From | jim.cromie@gmail ... | Date | Tue, 14 Nov 2023 08:40:32 -0700 | Subject | Re: [PATCH v1 11/12] dyndbg: write debug logs to trace instance |
| |
On Tue, Nov 14, 2023 at 12:45 AM Łukasz Bartosik <lb@semihalf.com> wrote: > > wt., 14 lis 2023 o 02:08 <jim.cromie@gmail.com> napisał(a): > >
> > > > > 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. >
Ok, I got your point now.
how about we call it "0" ? it should be an obvious "magical" value, cuz it doesnt need to be open'd, and cant be close'd
then we can revert to global tracebuf by its "name" echo module foo +T:0 > /proc/dynamic_debug/control
we probably should also limit the trace-instance-names to ^\w+
> Or did I miss answer to that in our long discussion :> ?
nope :-)
thanks, Jim
> > 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. > > > > > > > > > > > > > > > > > > > :-)
| |