Messages in this thread | | | From | Łukasz Bartosik <> | Date | Tue, 14 Nov 2023 23:09:41 +0100 | Subject | Re: [PATCH v1 11/12] dyndbg: write debug logs to trace instance |
| |
wt., 14 lis 2023 o 16:41 <jim.cromie@gmail.com> napisał(a): > > 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 >
I like it. It resembles surprise emoji :0
> we probably should also limit the trace-instance-names to ^\w+ >
Ack
I think this closes the last open topic to discuss for now. I will get back with next patchset version to you soon.
> > 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. > > > > > > > > > > > > > > > > > > > > > > :-)
| |