Re: [PATCH v2] connector: add an event for monitoring process tracers

[Oleg Nesterov <oleg@redhat.com>]

On 07/18, Vladimir Zapolskiy wrote:
>
> On Mon, Jul 18, 2011 at 8:54 PM, Oleg Nesterov <oleg@redhat.com> wrote:
>
> > "which_id" doesn't match "ptrace_id" used elsewhere. And PTRACE_ATTACH
> > instead of simple boolean looks as if you are going to add more ptrace
> > events, but I guess this won't happen.
> >
>^
> I'd like to preserve that variant, in my opinion its just a bit more
> undisguised version rather than bare true/false.

OK. Although this "else return" in proc_ptrace_connector() looks like
the "hide the potentional error" to me.

> >> -     if (!retval)
> >> +     if (!retval) {
> >>               wait_on_bit(&task->jobctl, JOBCTL_TRAPPING_BIT,
> >>                           ptrace_trapping_sleep_fn, TASK_UNINTERRUPTIBLE);
> >> +             proc_ptrace_connector(task, PTRACE_ATTACH);
> >> +     }
> >
> > OK, but it is a bit strange we are waiting for STOPPED/TRACED transition
> > before we report PROC_EVENT_PTRACE. Perhaps it makes more sense to
> > call proc_ptrace_connector() first, this also decreases the probability
> > PTRACE_ATTACH will be reported after PROC_EVENT_EXIT.
> >
> Yes, there is a difference. But as far as there is no guaranteed
> serialization in proc connector event reports, user-space process
> trackers should be designed to operate correctly having in mind
> possible event reordering.

Yes, but I didn't really mean the correctness. I meant, this looks
confusing, as if this wait_on_bit() has something to do with attach.
Likewise I do not understand why proc_exec_connector() is called
after ptrace_event() which can sleep unpredictably long.



Nevermind, applied.

Oleg.