utrace_set_events/utrace_control && death/reap checks

[Oleg Nesterov]

On 03/15, Roland McGrath wrote:
>
> > utrace_set_events:
> >
> > 	(utrace->death && ((old_flags & ~events) & DEATH_EVENTS))
> >
> > "(old_flags & ~events) & DEATH_EVENTS)" means the caller tries to
> > clear DEATH/QUIESCE. Why this is not allowed? And why this is not
> > allowed _only_ when the target runs utrace_report_death()->REPORT()?
>
> This is specifically documented for -EALREADY, and in the DocBook section
> "Interlock with final callbacks".  The idea is this:

Aha, I didn't know.

> > And I don't understand why do we need utrace->death at all.
> ...
> > it is only used by
> > utrace_control(UTRACE_DETACH).
> ...
> that it will (or it's already happening).
>
> utrace_control as the synchronizing step of
> asynchronous tear-down.  If it returns 0, then report_death will not and it
> is safe to destroy data structures the callback code would use.

Yes, with your explanation above this is clear.


But can't we simplify this check a little bit?

	utrace_control:

		else if (unlikely(target->utrace_flags & DEATH_EVENTS) ||
			   unlikely(utrace->death)) {
			return -EALREADY;

can't we just do

		else if (unlikely(utrace->death)) {
			return -EALREADY;

I guess I missed something, but can't understand why do we need to
check ->utrace_flags. We are going to call mark_engine_detached()
below which clears engine->flags, and we hold utrace->lock.

If utrace_flags & DEATH_EVENTS is true, the subsequent
utrace_report_death() must see engine->flags == 0 (it takes
utrace->lock before REPORT_CALLBACKS), so it won't call any
callback. Yes, it can play with engine itself, but this should
be safe because "struct utrace" has a reference to attached
engine.

No?

Oleg.