[lkml]   [2009]   [Aug]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
    SubjectRe: [PATCH] tracing/profile: Fix profile_disable vs module_unload
    * Steven Rostedt ( wrote:
    > On Thu, 27 Aug 2009, Mathieu Desnoyers wrote:
    > > * Steven Rostedt ( 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.


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