[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20110718193938.GA17629@redhat.com>
Date: Mon, 18 Jul 2011 21:39:38 +0200
From: Oleg Nesterov <oleg@...hat.com>
To: Vladimir Zapolskiy <vzapolskiy@...il.com>
Cc: netdev@...r.kernel.org, Evgeniy Polyakov <zbr@...emap.net>,
"David S. Miller" <davem@...emloft.net>
Subject: Re: [PATCH v2] connector: add an event for monitoring process
tracers
On 07/18, Vladimir Zapolskiy wrote:
>
> On Mon, Jul 18, 2011 at 8:54 PM, Oleg Nesterov <oleg@...hat.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.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists