Messages in this thread | | | Date | Tue, 20 Dec 2011 15:57:27 +0100 | From | Jiri Olsa <> | Subject | Re: [PATCHv2 03/10] ftrace: Add enable/disable ftrace_ops control interface |
| |
On Mon, Dec 19, 2011 at 02:35:35PM -0500, Steven Rostedt wrote: > On Mon, 2011-12-05 at 18:22 +0100, Jiri Olsa wrote: > > > > +static inline void enable_ftrace_function(struct ftrace_ops *ops) > > +{ > > + atomic_t *disabled = this_cpu_ptr(ops->disabled); > > + atomic_dec(disabled); > > +} > > + > > +static inline void disable_ftrace_function(struct ftrace_ops *ops) > > +{ > > + atomic_t *disabled = this_cpu_ptr(ops->disabled); > > + atomic_inc(disabled); > > +} > > + > > The above should be renamed to ftrace_function_enable/disable(), and > they should pass in the cpu. There may be a case we want to disable > ftrace functions on another CPU. > > Not to mention, this is the perfect example of "this_cpu_ptr" being used > incorrectly. It's not made for this purpose, and again the naming of > "this_cpu" totally confuses other kernel developers. We need to change > this name as it was agreed to at Kernel Summit.
ok, will make the changes
> > 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.
thanks, jirka
| |