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

From: Oleg Nesterov
Date: Thu Aug 01 2013 - 10:38:48 EST


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.

--
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/