Q: utrace_attach_task && utrace_release_task

[Oleg Nesterov]

On 03/05, Roland McGrath wrote:
>
> > utrace_attach_task() checks ->exit_state == EXIT_DEAD. Why? I mean,
> > how can it help, we don't hold any locks, target can change its
> > ->exit_state right after the check.
>
> Good catch, thanks.  This is a remnant of the utrace-indirect code,
> where utrace_first_engine() had an interlock with reap/release_task.
> (It's one of the several ways that arrangement is superior IMNSHO.)
>
> > I don't understand why utrace_release_task() doesn't set ->reap = 1
> > unconditionally. In that case we could use this flag instead of
> > EXIT_DEAD to verify it is "safe" to attach or get_utrace_lock().
>
> That's what I've made it do now.  In the utrace-indirect setup,
> it was possible to avoid locks for the common case (nobody attached).

Aha, I see the new patches...

what about get_utrace_lock() ? Do we really need the EXI_DEAD check?
And this check looks "racy" too.

> > static inline int utrace_attach_delay(struct task_struct *target)
> [...]
>
> This is the same thing Ananth noticed.  It was an unintended holdover from
> the utrace-indirect code organization.  It's fixed now.

Great, but

	utrace_attach_delay:

		if (signal_pending(current))
			return -ERESTARTNOINTR;

If utrace_attach_delay() fails, utrace_attach_task() returns this error.
This is right, but for example, prepare_ptrace_attach() will convert it
to EPERM?

Oleg.