(2014/07/04 1:22), Oleg Nesterov wrote: >> One possible scenario is here; someone disables an event and tries to remove >> it (both will be done by different syscalls). If we don't synchronize >> the first disabling, the event flag set disabled, but the event itself >> is not disabled. Thus event handler is still possible to be running >> somewhere when it is removed. > > See above. "remove" can't succed until all ftrace_event_file's are inactive. > And probe_event_disable() does uprobe_unregister() in this case which (again) > serializes with the callbacks itself.
Ah, I see. kprobes uses disable_kprobe() instead of unregister, and that is not serialized. >> The other path of __trace_remove_event_call() is trace_module_remove_events() >> which is not related to kprobes/uprobes (Even so, there is no obvious check >> of >> that.) > > Yes, uprobe can ignore modules ;) > >>> 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() ? >> >> I think it doesn't need to call synchronize_sched() because >> event_trace_del_tracer() ensures that all events are disabled >> (handlers are not running anymore) > > Not really, afaics... Well yes, it calls __ftrace_set_clr_event_nolock(), > but this can race with the callbacks; this doesn't necessary call > uprobe_unregister() because there can be other active ftrace_event_file's. > So we need to synchronize before we start to destroy the data. > > So do you agree that we can remove that synchronize_sched() in trace_uprobe.c > and replace it with call_rcu? Yes for uprobes, kprobes still need it. :) Thank you, -- Masami HIRAMATSU Software Platform Research Dept. Linux Technology Research Center Hitachi, Ltd., Yokohama Research Laboratory E-mail: masami.hiramatsu...@hitachi.com -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/