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

    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.

    -- Steve



    \
     
     \ /
      Last update: 2009-08-27 17:53    [W:0.039 / U:93.924 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site