----- Original Message ----- > From: "Frank Ch. Eigler" <f...@redhat.com> > To: "Steven Rostedt" <rost...@goodmis.org> > Cc: "Mathieu Desnoyers" <mathieu.desnoy...@efficios.com>, > linux-kernel@vger.kernel.org, "Ingo Molnar" > <mi...@kernel.org>, "Frederic Weisbecker" <fweis...@gmail.com>, "Andrew > Morton" <a...@linux-foundation.org>, > "Johannes Berg" <johannes.b...@intel.com> > Sent: Tuesday, March 11, 2014 10:26:50 AM > Subject: Re: [for-next][PATCH 08/20] tracing: Warn if a tracepoint is not set > via debugfs > > Hi, Steven - > > > > So it is a deferred-activation kind of call, with no way of knowing > > > when or if the tracepoints will start coming in. Why is that > > > supported at all, considering that clients could monitor modules > > > coming & going via the module_notifier chain, and update registration > > > at that time? > > > > That's my argument. > > Was there an answer?
Let's step back and look at the overall picture here, along with the possible solutions that are available so far. My intent is to let end users specify "I want to trace this specific tracepoint" from the tracer interface as long as the tracepoint probe provider module is loaded (the file with CREATE_TRACE_POINTS defined). For instance, the tracepoint call site can be within a driver loaded by the USB stack when a USB device is plugged in. While tracing is enabled, the user may want to plug in the said USB device to investigate what is the culprit of an issue he would be facing when the the device is plugged in. There seems to be 2 elegant ways to achieve this while giving feedback to tracers about whether there are active tracepoint callsites or not: 1) We add a trace_has_callsites_enabled() (or any better name) function to tracepoint.h, which allows tracers to query whether a given tracepoint name has any callsites enabled, 2) We change tracepoint_probe_register() so it unregisters the tracepoint and return -ENODEV if there are no callsites enabled, and deal with the use-case I explain above with module coming and going within the tracer. This duplicates part of the tracepoint infrastructure into the tracer though, which is why I was not so keen on going for this solution. The other solution proposed by Steven (returning -ENODEV without unregistering the tracepoint) does not appear to be an elegant solution, as we discussed earlier in this thread. It kind of weird to have a negative value treated as an OK special case. The other solution proposed by Steven in an earlier thread was to tie tracepoints very deeply with module loading infrastructure and add module parameters specifically to specify if tracepoint callsites need to be enabled on module load. This approach unfortunately expect that everyone interacts with module loading, as root, in a system-wide (no multi-session) fashion and is not suitable for the user-base we are targeting. I seems to be a user experience disaster IMHO. I'm OK as long as we have an elegant way forward. Ideally I would have prefered (1) to eliminate code duplication between tracers and tracepoint infrastructure (we have to reimplement a hash table similar to tracepoints within the tracer with solution (2)), but (2) technically works too. Thanks, Mathieu > > > > > >> + entry = get_tracepoint(name); > > > >> + /* Make sure the entry was enabled */ > > > >> + if (!entry || !entry->enabled) > > > >> + ret = -ENODEV; > > > > > > For what it's worth, I agree with Mathieu that this sort of successful > > > failure result code is a problem for tracking what needs cleanup and > > > what doesn't. (In systemtap's case, if this change gets merged, we'll > > > have to treat -ENODEV as if it were 0.) > > > > Does systemtap enable tracepoints before they are created? That is, do > > you allow enabling of a tracepoint in a module that is not loaded yet? > > We have no formal opinion on whether or not this makes sense. If the > kernel permits it, fine. > > > If not, than you want this as an error. > > But it's not exactly an error! It's a success of sorts, and means > that later on we have to unregister the callback, just as if it were > successful. > > > - FChE > -- Mathieu Desnoyers EfficiOS Inc. http://www.efficios.com -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
