lkml.org 
[lkml]   [2011]   [Dec]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCHv2 03/10] ftrace: Add enable/disable ftrace_ops control interface
On Tue, Dec 20, 2011 at 10:25:53AM -0500, Steven Rostedt wrote:
> On Tue, 2011-12-20 at 15:57 +0100, Jiri Olsa wrote:
>
> > >
> > > If the above is called with preemption enabled, it will not do what is
> > > expected. We could disable function tracing on one CPU and then
> > > re-enable it for another CPU even though it is already enabled.
> >
> > It is only called inside perf reg callback within the
> > schedule function where the preemption is disabled.
> >
> > The ftrace_function_enable is called when task is scheduled in
> > on respective cpu. Likewise the ftrace_function_disable is called
> > when task is scheduled out on respective cpu.
>
> Yes I know how you use it, but this is an open API. It may be currently
> only used by perf today, but that doesn't mean that it wont be used by
> others. There's no documentation on how to use it. I don't look at this
> and say, "oh this is used by perf, we only need to worry about how perf
> uses it". That doesn't scale. It needs to be documented on how to use
> it, and if it requires preemption disabled when calling it, it should
> definitely be stated that fact and it may need a
> WARN_ON(preempt_count()) or something.

I've already added

if (WARN_ON_ONCE(!(ops->flags & FTRACE_OPS_FL_CONTROL)))
return;

will add similar for preemption and make comments for both functions

thanks,
jirka

>
> -- Steve
>
>


\
 
 \ /
  Last update: 2011-12-20 16:37    [W:0.138 / U:0.560 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site