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

[Masami Hiramatsu <mhiramat@redhat.com>]

Mathieu Desnoyers wrote:
> * Steven Rostedt (rostedt@goodmis.org) wrote:
>> On Thu, 2010-01-21 at 23:09 -0500, Mathieu Desnoyers wrote:
>>> * Steven Rostedt (rostedt@goodmis.org) wrote:
>>
>>>> Hmm, interesting. Maybe something like that might work. But what if
>>>> CONFIG_PREEMPT is enabled but CONFIG_FREEZER is not?
>>>
>>> Then you may want to make the function tracer depend on CONFIG_FREEZER,
>>> but maybe Masami has other ideas ?
>>
>> egad no! This is just to help add guarantees to those that use the
>> function tracer that when the tracing is disabled, it is guaranteed that
>> no more tracing will be called by the function tracer. Currently,
>> nothing relies on this. But we may add cases that might need this.
> 
> Yep, identifying tracer quiescent state can become handy.
> 
>>
>> In fact, only those that need this requirement would need to do this
>> trick. Anyway, we could make those depend on CONFIG_FREEZER, but that
>> just seems to be a strange dependency.
> 
> This makes me wonder (question for Masami)...
> 
> static int __kprobes check_safety(void)
> {
>         int ret = 0;
> #if defined(CONFIG_PREEMPT) && defined(CONFIG_FREEZER)
>         ret = freeze_processes();
>         if (ret == 0) {
>                 struct task_struct *p, *q;
>                 do_each_thread(p, q) {
>                         if (p != current && p->state == TASK_RUNNING &&
>                             p->pid != 0) {
>                                 printk("Check failed: %s is running\n",p->comm);
>                                 ret = -1;
>                                 goto loop_end;
>                         }
>                 } while_each_thread(p, q);
>         }
> loop_end:
>         thaw_processes();
> #else
>         synchronize_sched();
> #endif
>         return ret;
> }
> 
> How does that deal with CONFIG_PREEMPT && !CONFIG_FREEZER exactly ?

In that case, it just disables kprobe booster, and never use this
safety check.

Actually, this safety check is currently only for boosted probes,
which is transparently enabled on all kprobes. This means that we
can fall back to normal kprobes if we can't use the booster.
(the functionality of probing still exists, but not boosted,
 becomes slow.)

I'm not so sure how much cost can be reduced by dropping
ftrace_profile_enabled(). I agree that using freeze_processes() will
help you, but if there is no alternative(e.g. use ftrace_profile_enable()
only if CONFIG_PREEMPT&&!CONFIG_FREEZER), I don't recommend to use it.

If we can make synchronize_tasks(), I'd like to use it in above safety
check too:-)

Thank you,

-- 
Masami Hiramatsu

Software Engineer
Hitachi Computer Products (America), Inc.
Software Solutions Division

e-mail: mhiramat@redhat.com