lkml.org 
[lkml]   [2011]   [Dec]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCHv2 03/10] ftrace: Add enable/disable ftrace_ops control interface
From
Date
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.

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.

-- Steve




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