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.