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.