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

[Mathieu Desnoyers]

----- Original Message -----
> From: "Steven Rostedt" <rostedt@xxxxxxxxxxx>
> To: "Mathieu Desnoyers" <mathieu.desnoyers@xxxxxxxxxxxx>
> Cc: "Frank Ch. Eigler" <fche@xxxxxxxxxx>, linux-kernel@xxxxxxxxxxxxxxx, "Ingo Molnar" <mingo@xxxxxxxxxx>, "Frederic
> Weisbecker" <fweisbec@xxxxxxxxx>, "Andrew Morton" <akpm@xxxxxxxxxxxxxxxxxxxx>, "Johannes Berg"
> <johannes.berg@xxxxxxxxx>, "Linus Torvalds" <torvalds@xxxxxxxxxxxxxxxxxxxx>, "Peter Zijlstra"
> <peterz@xxxxxxxxxxxxx>, "Thomas Gleixner" <tglx@xxxxxxxxxxxxx>, "Greg Kroah-Hartman" <gregkh@xxxxxxxxxxxxxxxxxxx>,
> "lttng-dev" <lttng-dev@xxxxxxxxxxxxxxx>, "Rusty Russell" <rusty@xxxxxxxxxxxxxxx>
> Sent: Wednesday, March 12, 2014 12:18:13 PM
> Subject: Re: [for-next][PATCH 08/20] tracing: Warn if a tracepoint is not set via debugfs
> 
> On Wed, 12 Mar 2014 16:05:36 +0000 (UTC)
> Mathieu Desnoyers <mathieu.desnoyers@xxxxxxxxxxxx> wrote:
> 
> > ----- Original Message -----
> > > From: "Steven Rostedt" <rostedt@xxxxxxxxxxx>
> > > To: "Mathieu Desnoyers" <mathieu.desnoyers@xxxxxxxxxxxx>
> > > Cc: "Frank Ch. Eigler" <fche@xxxxxxxxxx>, linux-kernel@xxxxxxxxxxxxxxx,
> > > "Ingo Molnar" <mingo@xxxxxxxxxx>, "Frederic
> > > Weisbecker" <fweisbec@xxxxxxxxx>, "Andrew Morton"
> > > <akpm@xxxxxxxxxxxxxxxxxxxx>, "Johannes Berg"
> > > <johannes.berg@xxxxxxxxx>, "Linus Torvalds"
> > > <torvalds@xxxxxxxxxxxxxxxxxxxx>, "Peter Zijlstra"
> > > <peterz@xxxxxxxxxxxxx>, "Thomas Gleixner" <tglx@xxxxxxxxxxxxx>, "Greg
> > > Kroah-Hartman" <gregkh@xxxxxxxxxxxxxxxxxxx>,
> > > "lttng-dev" <lttng-dev@xxxxxxxxxxxxxxx>, "Rusty Russell"
> > > <rusty@xxxxxxxxxxxxxxx>
> > > Sent: Wednesday, March 12, 2014 11:46:11 AM
> > > Subject: Re: [for-next][PATCH 08/20] tracing: Warn if a tracepoint is not
> > > set via debugfs
> > > 
> > > 
> > > To sum up this thread, and get the signal vs noise ratio up.
> > > 
> > > On Wed, 12 Mar 2014 11:11:00 -0400
> > > Steven Rostedt <rostedt@xxxxxxxxxxx> wrote:
> > > 
> > > 
> > > > The solution I like the most that I believe will work for both of us,
> > > > is to to move this magic "enable tracepoint in the future" to your
> > > > LTTng module. Have your module register a module load and unload
> > > > handler
> > > > to be able to see the tracepoints that exist in the module, and you can
> > > > enable them then. When a module is unloaded, your module can do the
> > > > accounting to record that, and the state of its tracepoints.
> > > 
> > > This is my final proposal.
> > > 
> > > I'll add the patch that removes the tracepoint on failure along with
> > > returning -ENODEV. That way, there will be no registered tracepoints
> > > that do not exist.
> > > 
> > > I'll also make sure that on module unload, the tracepoints are disabled
> > > for the module as well.
> > 
> > Do you mean that the tracepoint probe will be unregistered from within
> > tracepoint.c whenever all modules containing tracepoint call sites are
> > unloaded ? If so, how do you plan to handle ownership of the "name",
> > "probe" and "data" pointer ? They belong to the tracer. Would they simply
> > leak ?
> 
> Are you telling me it's not possible to delete the entire probe?

The ownership flow is the following:

1) Tracer creates name, probe, data objects. The probe can be typically
   code within a probe provider module, which needs to have a reference
   count incremented. The name and data objects can be dynamically allocated,
   or in some special cases part of a probe provider module (again with
   refcount incremented).

2) The tracer registers the tracepoint probe. If registration returns 0,
   the tracer should not free those elements until it calls tracepoint
   probe unregister for that (name, probe, data) tuple.

3) Tracer calls tracepoint probe unregister for the (name, probe, data) tuple.

4) Tracer calls tracepoint_synchronize_unregister() to ensure quiescence
   of tracepoint call sites wrt the probe that has just been unregistered.

5) Tracer can free/unref the probe provider module.

> 
> What I'm proposing is to do what the trace events do. Delete everything
> associated to the tracepoints associated to the module.

Is your intent to have a module "going" notifier in tracepoint.c managing
ownership of objects it does not own ? If not, I guess I'm not understanding
your proposal fully.

Thanks,

Mathieu

-- 
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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/