Re: [RFC][PATCH 3/4] tracing/kprobes: Fail to unregister if probeevent files are open

[Masami Hiramatsu]

(2013/07/06 2:26), Oleg Nesterov wrote:
> On 07/05, Masami Hiramatsu wrote:
>>
>> (2013/07/05 3:48), Oleg Nesterov wrote:
>>> On 07/04, Masami Hiramatsu wrote:
>>>>
>>>> Actually disable_kprobe() doesn't ensure to finish the current running
>>>> kprobe handlers.
>>>
>>> Yes. in fact disable_trace_probe(file != NULL) does, but perf doesn't.
>>
>> Ah, right. we did that.
> 
> And thus we only need to synchronize kprobe_dispatcher()->kprobe_perf_func()
> path. And afaics kprobe_perf_func() doesn't use anything which can be freed
> by trace_remove_event_call?
> 
>>>> OTOH, unregister_kprobe() waits for that.
>>>
>>> Yes.
>>>
>>> So I think we only need to move kfree(tp->call.print_fmt).
> 
> OOPS. I was wrong. It seems that ->print_fmt is only for event/format ?
> Then it is fine to kfree it right after trace_remove_event_call().
> 
>>> So the sequence should be:
>>>
>>> 	if (trace_remove_event_call(...))
>>> 		return;
>>>
>>> 	/* does synchronize_sched */
>>> 	unregister_kprobe();
>>>
>>> 	kfree(everything);
>>>
>>> Agreed?
>>
>> If we can free everything after all, I'd like to do so.
>> Hmm, but AFAICS, trace_remove_event_call() supposes that
>> all event is disabled completely.
> 
> Yes, but kprobe_trace_func() is really disabled?

No, currently, doesn't. We need to fix that.
(Perhaps, uprobes too.)

>> A safe way is to wait rcu always right after disable_*probe
>> in disable_trace_probe. If we have an unused link, we can
>> free it after that.
> 
> Aaaah... I am starting to understand... Even if kprobe_perf_func()
> is fine, synchronize_sched() is calles _before_ disable_kprobe()
> and thus it can't synchronize with the handlers which hit this probe
> after we start synchronize_sched().
> 
> Did you mean this or I misssed something else?

Right, thus perf path also need to be synchronized.

Thank you,


-- 
Masami HIRAMATSU
IT Management Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@xxxxxxxxxxx


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/