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

[Oleg Nesterov <oleg@redhat.com>]

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.