Messages in this thread | | | Date | Thu, 1 Aug 2013 16:33:07 +0200 | From | Oleg Nesterov <> | Subject | Re: [RFC][PATCH 3/4] tracing/kprobes: Fail to unregister if probe event files are open |
| |
On 08/01, Steven Rostedt wrote: > > On Thu, 2013-08-01 at 15:34 +0200, Oleg Nesterov wrote: > > > > > > __unregister_trace_probe(tp); > > > list_del(&tp->list) > > > unregister_probe_event(tp) <-- fails! > > > free_trace_probe(tp) > > > > Yes. But again, this doesn't explain why unregister_probe_event()-> > > __trace_remove_event_call() can't simply proceed and > > do ftrace_event_enable_disable() + remove_event_from_tracers(). > > The problem is with the soft disable.
Exactly! This is another (also unlikely) race we need to prevent.
> so the > i_private wont work.
Yes, and this is another reason why trace_remove_event_call() can't always succeed, and the comment/changelog in probe_remove_event_call() (added by the previous change) even tries to document the problems with FL_SOFT_MODE.
> > IOW, if we do not apply the previous "trace_remove_event_call() should > > fail if call/file is in use" patch, then everything is fine: > > > > > write(fd, "0", 1) > > > > this will fail with ENODEV. > > Currently it does not, because the failure in probe_remove_event_call() > due to the event being busy wont remove the event (event_remove() is > never called). Thus the event is still alive and the write will still > have access to it.
Yes, yes. That is why the changelog says "Both trace_kprobe.c/trace_uprobe.c need the additional changes".
IOW, the previous change itself adds the new races fixed by this patch (and the similar change in trace_uprobe.c). Hopefully this is fine because the code is buggy anyway.
> I can update the change log to remove some of the functions that are > being called to be less confusing.
I am fine either way. Just I wanted to be sure that we understand each other and I didn't miss something.
> I agree, this isn't really nice, but for now we have to deal with it.
Yes, yes, this is not for 3.11.
Oleg.
| |