[lkml]   [2014]   [Jul]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
Subjectprobe_event_disable()->synchronize_sched() (Was: tracing/uprobes: Revert "Support mix of ftrace and perf")
Namhyung, Masami,

Please look at the question below. Perhaps we discussed this before,
but I can recall nothing.

On 06/30, Oleg Nesterov wrote:
> Actually, I'll probably try to make the patch tomorrow. It looks simple
> enough, the main complication is CONFIG_PERF. And, to keep this patch
> simple, I won't try to optimize (say) the TP_FLAG_TRACE-comes-first
> case which could avoid uprobe_apply().

I regret very much I said this ;) OK, I'll probably try anyway, but...

> Yes, I still think it would be better to change the register/unregister
> API first, but I do not know when I do this ;)

OK, we can do this later.

But it turns out that trace_uprobe.c needs other cleanups, and I simply
can't uglify this code more without these cleanups... Starting from
set/clear TP_FLAG_*. I'll recheck, but it seems that there is no reason
to carefully set (say) TP_FLAG_TRACE before uprobe_register() and then
clear it if _register() fails. And uprobe_dispatcher() is very ugly if
is_ret_probe(). And more. So it needs a series.

And. I am puzzled by probe_event_disable()->synchronize_sched(). Why
do we need it? I mean, why we can't use call_rcu() ? The comment says
"synchronize with u{,ret}probe_trace_func" but it doesn't explain _why_
do we need to sync.

I thought that perhaps the caller needs to synch with the callbacks.
Say, __trace_remove_event_call() can destroy the data which can be used
by the callbacks. But no, this is only possible if we are going to call
uprobe_unregister(), and this adds the necessary serialization.

So why? Looks like, the only reason is instance_rmdir() which can do
TRACE_REG_UNREGISTER and after that destroy this ftrace_event_file?
But event_trace_del_tracer() also has synchronize_sched() right after
__ftrace_set_clr_event_nolock() ?

So please tell me why do we need this synchronize_sched ;) And imo
this should be documented.


 \ /
  Last update: 2014-07-01 22:21    [W:0.105 / U:32.532 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site