Messages in this thread | | | Date | Tue, 20 Dec 2011 16:35:06 +0100 | From | Jiri Olsa <> | Subject | Re: [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 > >
| |