Re: [RFC PATCH 03/10] ftrace: Drop the ftrace_profile_enabled checks in tracing hot path

[Frederic Weisbecker <fweisbec@gmail.com>]

On Thu, Jan 21, 2010 at 09:05:17PM -0500, Steven Rostedt wrote:
> On Fri, 2010-01-22 at 02:16 +0100, Frederic Weisbecker wrote:
> > Every time we enter the function profiler tracing callbacks, we first
> > check if the function profiling is enabled.
> > 
> > This check is useless because we register the function graph
> > callbacks only after the hashlist has been initialized.
> 
> Unfortunately, since the previous patch is incorrect, it makes this one
> buggy too.
> 
> If you remove the check to ftrace_profile_enabled, the call to the
> profiled code could have been preempted and pending to be called.
> 
> Stop machine may remove all calls to the tracing, but it only affects
> new hits. Pending hits may still exist.
> 
> If you remove this check, and the user re-enables the profiling, then
> all PER_CPU hashs will be reset. If in the process of this happening,
> the task with the pending trace wakes up, it may access the PER_CPU list
> and corrupt it.


Indeed.



> Now for the reason I Cc'd Paul and Mathieu...
> 
> If we had a synchronize_sched() like function that would wait and return
> when all preempted tasks have been scheduled again and went to either
> userspace or called schedule directly, then we could actually do this.
> 
> After unregistering the function graph trace, you call this
> "synchronize_tasks()" and it will guarantee that all currently preempted
> tasks have either went to userspace or have called schedule() directly.
> Then it would be safe to remove this check.



Good point!

I fear that would require heavy hooks in the scheduler though...