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    [W:0.041 / U:0.100 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site