[PATCH 2/3] fix utrace_control(DETACH) && utrace->death interaction

[Oleg Nesterov]

On 08/17, Oleg Nesterov wrote:
>
> On 08/16, Roland McGrath wrote:
> >
> > > The problem is, utrace_control(DETACH) does nothing and returns
> > > -EALREADY if utrace->death is set, this is not right. We can and
> > > should detach in this case, we only should skip utrace_reset() to
> > > avoid the race with utrace_report_death()->REPORT_CALLBACKS().
> >
> > This behavior is the original (minimal) synchronization scheme from before
> > we had utrace_barrier.  See "Interlock with final callbacks" in
> > Documentation/DocBook/utrace.tmpl.
>
> OK, I agree my patch breaks this part
>
> 	Normally
> 	<function>utrace_control</function> called with
> 	<constant>UTRACE_DETACH</constant> returns zero, and this means that no
> 	more callbacks will be made.
>
> of documentation (which I never read ;)

Wait. It doesn't break this. It only breaks -EALREADY contract. And
I don't understand why this is bad.

> But in any case, personally I dislike the current behaviour anyway,
> I think this certainly complicates the life for module writers.
> Instead of simple detach + barrier, you always need the nontrivial
> code if report_quiesce/death ever touches engine->data! This can't
> be good.

Yes.

Roland, could you explain once again why do you dislike this patch?


Once again. Currently utrace_control(DETACH) refuses to even try to
detach the engine if utrace->death is set.

Why? What is the point? What makes UTRACE_EVENT(DEATH) so special?
I do not see the logic at all.

If ->report_death() does something which the caller of utrace_control()
should know, then they should take care of synchronization anyway.
With or without this patch, utrace_control(DEATH) can return 0 after
->report_death() was already called.


Currently, utrace_control(DETACH) returns -EALREADY when this callback
was alredy called, or it can be called later, or it may be running. And
utrace_barrier() can't help.

With this patch utrace_control(DETACH) returns either 0 with the same
meaning (will not be called later, but probably was already called
before utrace_control), or -EINPROGRESS which suggests to use
utrace_barrier().


Please correct me, but I think this certainly makes things much simpler.
Otherwise UTRACE_DETACH is never trivial (and iiuc it is better to
avoid utrace_engine_ops->release() hook).

What do you think?

Oleg.