----- Original Message ----- > From: "Steven Rostedt" <rost...@goodmis.org> > To: "Mathieu Desnoyers" <mathieu.desnoy...@efficios.com> > Cc: "Frank Ch. Eigler" <f...@redhat.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>, "Linus Torvalds" <torva...@linux-foundation.org>, > "Peter Zijlstra" > <pet...@infradead.org>, "Thomas Gleixner" <t...@linutronix.de>, "Greg > Kroah-Hartman" <gre...@linuxfoundation.org>, > "lttng-dev" <lttng-...@lists.lttng.org>, "Rusty Russell" > <ru...@rustcorp.com.au> > Sent: Wednesday, March 12, 2014 3:30:06 PM > Subject: Re: [for-next][PATCH 08/20] tracing: Warn if a tracepoint is not set > via debugfs > > On Wed, 12 Mar 2014 14:58:02 -0400 > Steven Rostedt <rost...@goodmis.org> wrote: > > > > > Two modules should not have the same name. Is there any duplicate > > tracepoints you are aware of. Namespace collisions in tracepoints > > should be avoided, as that would cause people to trace things they did > > not intend on tracing. > > > > That should be a new patch as well. Enforce unique tracepoint names. > > This may be why you are not understanding what I want. It's the way > things are implemented today, which I believe are wrong. I see what you > did. You have probes that are registered, and tracepoints that are > where the code lies. You just add and remove probes from a hash list, > and then you loop through all the tracepoints seeing if the iter->name > matches a probe->name. > > I'm fine with keeping the probe separate, but there really should be > no more than just a one to one mapping between probes and tracepoints. > Have the probe point to the matching tracepoint. The probe is > registered, it enables the tracepoint static key, when it's ref count > goes to zero, it disables the tracepoint static key. We can get rid of > that loop then, as well as the duplicate names between probes and > tracepoints.
Right there, this is not possible for a few reasons, namely: - loop unrolling performed by the compiler can duplicate a tracepoint, even if it is only there once in the source code, - inlining performed by the compiler may do the same, - LTO, whenever it will start being used for the kernel, may do the same, and also spread call sites across modules. There can be no 1 to 1 mapping between a probe function and a callsite due to those compilers optimisations, even if we enforce the strictest coding style rules possible on their use. Thanks, Mathieu > > Here's the steps we should take: > > 1) Prevent duplicate tracepoints. They are just namespace collisions > that we already try to avoid. How to do this? We may need to add a > hlist_node to the tracepoint structure, and keep them in a hash by name. > Check for collisions when the name is added to the hash. > > 2) Change the way tracepoints are enabled. Do not do a loop of all > tracepoints, but instead have the first probe of a tracepoint enable > it, and the last one to disable it. This would require a pointer from > the probe to the tracepoint it represents. Again, it should not > represent more than one. > > 3) On module unload, it would be the responsibility of the user to > unload all the tracepoints that may have been enabled for a module. We > can add a mod pointer in the probe to make this easier, as well as to > the tp_module structure. > > The way tracepoints are today are to handle two completely different > tracepoints with the same name. That should be avoided, and will make > things much less complex. > > Then you can easily handle the accounting of modules loading and > unloading in your module, and the tracepoint code will match what the > rest of the kernel does for resource management. > > -- Steve > -- 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/
