Messages in this thread | | | Date | Thu, 27 Aug 2009 11:51:29 -0400 (EDT) | From | Steven Rostedt <> | Subject | Re: [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
| |