Re: [for-next][PATCH 08/20] tracing: Warn if a tracepoint is not set via debugfs

[Steven Rostedt]

On Wed, 12 Mar 2014 14:58:02 -0400
Steven Rostedt <rostedt@xxxxxxxxxxx> 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.

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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/