Re: [PATCH 3/3] pids: refactor vnr/nr_ns helpers to make them safe

From: Oleg Nesterov
Date: Fri Jan 16 2009 - 15:48:29 EST

Hi Louis,

On 01/16, Louis Rilling wrote:
> On 16/01/09 6:55 +0100, Oleg Nesterov wrote:
> > + struct pid_namespace *ns)
> > {
> > - return pid_nr_ns(task_pid(tsk), ns);
> > + pid_t nr = 0;
> > +
> > + rcu_read_lock();
> > + if (!ns)
> > + ns = current->nsproxy->pid_ns;
> > + if (likely(pid_alive(task))) {
> I don't see what this pid_alive() check buys you. Since tasklist_lock is not
> enforced, nothing prevents another CPU from detaching the pid right after the
> check.

pid_alive() should be renamed. We use it to make sure the task didn't pass

Yes, you are right, nothing prevents another CPU from detaching the pid right
after the check. But this is fine: we read ->pids[].pid under rcu_read_lock(),
and if it is NULL pid_nr_ns() returns. So, we don't need pid_alive() check at

However, we can not use task->group_leader unless we verify the task is still
alive. That is why we need this check. We do not clear ->group_leader when
the task exits, so we can't do

if (task->group_leader)

Instead we use pid_alive() before using ->group_leader.

> I'm also a bit puzzled by your description with using tasklist_lock when task !=
> current, and not seeing tasklist_lock anywhere in the patch. Does this mean that
> "safe" is for "no access to freed memory is done, but caller has to take
> tasklist_lock or may get 0 as return value"?

I am not sure I understand the question...

This patch doesn't use tasklist, it relies on rcu. With this patch the caller
doesn't need tasklist/rcu to call these helpers (but of course, the caller
must ensure that task_struct is stable).

But, whatever the caller does, it can get 0 as return value anyway if the
task exists, this is correct. Or I misunderstood you?


To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at
Please read the FAQ at