Messages in this thread | | | Date | Thu, 27 Aug 2009 12:18:29 -0400 (EDT) | From | Steven Rostedt <> | Subject | Re: [PATCH] tracing/profile: Fix profile_disable vs module_unload |
| |
On Thu, 27 Aug 2009, Peter Zijlstra wrote:
> On Thu, 2009-08-27 at 11:57 -0400, Steven Rostedt wrote: > > On Thu, 27 Aug 2009, Peter Zijlstra wrote: > > > > > On Wed, 2009-08-26 at 15:48 -0400, Steven Rostedt wrote: > > > > \ > > > > @@ -399,16 +413,20 @@ static int ftrace_profile_enable_##call(struct ftrace_event_call *event_call) \ > > > > { \ > > > > int ret = 0; \ > > > > \ > > > > - if (!atomic_inc_return(&event_call->profile_count)) \ > > > > + if (!atomic_inc_return(&event_call->profile_count)) { \ > > > > + event_trace_up_ref(); \ > > > > ret = register_trace_##call(ftrace_profile_##call); \ > > > > + } \ > > > > \ > > > > return ret; \ > > > > } \ > > > > \ > > > > static void ftrace_profile_disable_##call(struct ftrace_event_call *event_call)\ > > > > { \ > > > > - if (atomic_add_negative(-1, &event_call->profile_count)) \ > > > > + if (atomic_add_negative(-1, &event_call->profile_count)) { \ > > > > unregister_trace_##call(ftrace_profile_##call); \ > > > > + event_trace_down_ref(); \ > > > > + } \ > > > > } > > > > > > Ha, almost, make that {un,}register_ftrace_##call() and stick the module > > > gunk in there, and I'm ok. > > > > Peter, that {un,}register_tace_##call is the tracepoint code. The > > DEFINE_TRACE creates those functions. This is the lowest level we can go > > in the ftrace.h file. > > Did you miss the extra f?
I thought that was a typo since there is no register_ftrace_##call.
> > You can wrap the regular {un,}register_trace_##call() things in > {un,}register_/F/trace_##call() functions which also take care of the > module stuff.
Oh, you mean you want me to create a wrapper for it? Note, this does not affect the normal trace "enable" code, because there's a module call back in trace_events.c that disables all the trace points in a module before unloading. And this in turn will unregister all events.
> Your patch is virtually identical to the one that started this thread, > everybody using the TRACE_EVENT() things will still need to worry about > modules.
That's not true. You are not using the TRACE_EVENT, you are modifying the TRACE_EVENT. That is different. I don't see how a user could hit this issue. You hit it because you modified what was in ftrace.h, and that does not count as a user. That's a developer ;-)
-- Steve
| |