lkml.org 
[lkml]   [2009]   [Aug]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] tracing/profile: Fix profile_disable vs module_unload
* Steven Rostedt (rostedt@goodmis.org) wrote:
>
> On Thu, 27 Aug 2009, Mathieu Desnoyers wrote:
>
> > * Steven Rostedt (rostedt@goodmis.org) wrote:
> > >
> > > On Thu, 27 Aug 2009, Mathieu Desnoyers wrote:
> > >
> > > > Looks good. Just don't forget to eventually add the "synchronize" calls
> > > > between tracepoint unregistration and the removal of their module. There
> > > > is a race condition in the way you do it currently.
> > >
> > > I'm trying to figure out the race here. What will disappear in the
> > > tracepoint? Could you give a brief example of the issue.
> >
> > Sure,
> >
> > Let's say we have a tracepoint in module instrumented.c, and a probe in
> > module probe.c. The probe is registered by module probe.c init through
> > the tracepoint infrastructure to connect to the tracepoint in
> > instrumented.c. Unregistration is done in probe.c module exit.
> >
> > As the instrumented code get executed (let's say periodically), it calls
> > the connected probe. Preemption is disabled around the call.
> >
> > If you unload the probe.c module, the module exit will unregister the
> > probe, but the probe code can still be in use by another CPU. You have
> > to wait for quiescent state with the tracepoint synchronize (which is
> > just a wrapper over synchronize_sched() before you are allowed to
> > complete module unload. Otherwise, you will end up reclaiming module
> > memory that is still used by probe execution.
> >
> > A test-case for this would be to create a probe with a delay in it, and
> > an instrumented module calling the instrumentation in a loop. On a SMP
> > system, running the instrumented code and probe load/unload in loops
> > should trigger this race.
>
> Thanks for the explanation. So let me see if I get this correct.
>
> For this race to occur, the probe (code that hooks to the tracepoint) must
> be in module that does not contain the tracepoint. We don't even need more
> than one module, this could occur even with a core tracepoint. If a module
> registers it, if it unregisters before unloading, the tracepoint may be
> hit before the unregister and executing while the module is unloading.
>
> I don't think we need to worry about this with the case of TRACE_EVENT and
> ftrace.h. The reason is that the trace point and probes are always in the
> same location. The MACROS set up the probe code with the modules. Thus, to
> remove the module, you must also remove the tracepoint itself along with
> the probe. If you can be executing in the probe, then you must have hit
> the trace point. If you hit the trace point, then you are executing code
> inside the module you are removing, which is a bug in the module code
> itself.
>
> Using the ftrace.h MACROS limits the use of tracepoints and this race
> does not exist. I feel we are safe not needing to have the
> tracepoint_synchronize_unregister within the ftrace.h code.
>

Looks right. If you can guarantee that the probe is only called from
tracepoints located within the same module as the probe, you should be
safe without tracepoint_synchronize_unregister. It's worth adding a
comment in ftrace.h explaining that though.

Mathieu

> -- Steve
>

--
Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68


\
 
 \ /
  Last update: 2009-08-27 18:07    [from the cache]
©2003-2011 Jasper Spaans. Advertise on this site