[Devel] Re: [PATCH 8/15] Helpers to find the task by its numerical ids

Oleg Nesterov oleg at tv-sign.ru
Sun Jul 29 05:40:45 PDT 2007


On 07/26, Pavel Emelyanov wrote:
>
> +#define find_pid(pid)	find_pid_ns(pid, &init_pid_ns)

Again, I think find_pid() should use current's active ns, not
init_pid_ns. Just grep for find_pid/find_task_by_pid.

> --- linux-2.6.23-rc1-mm1.orig/kernel/pid.c	2007-07-26 16:34:45.000000000 +0400
> +++ linux-2.6.23-rc1-mm1-7/kernel/pid.c	2007-07-26 16:36:37.000000000 +0400
> @@ -204,19 +221,20 @@ static void delayed_put_pid(struct rcu_h
>  	goto out;
>  }
>
> -struct pid * fastcall find_pid(int nr)
> +struct pid * fastcall find_pid_ns(int nr, struct pid_namespace *ns)
>  {
>  	struct hlist_node *elem;
> -	struct pid *pid;
> +	struct upid *pnr;
> +
> +	hlist_for_each_entry_rcu(pnr, elem,
> +			&pid_hash[pid_hashfn(nr, ns)], pid_chain)
> +		if (pnr->nr == nr && pnr->ns == ns)
                                     ^^^^^^^^^^^^^
Aha, that is why we need upid->ns.

I am a bit surprised we don't move the global pid_hash into the
"struct pid_namespace", this could speedup the search, and we
don't need upid->ns.

> -struct pid *find_ge_pid(int nr)
> +struct pid *find_ge_pid(int nr, struct pid_namespace *ns)
>  {
>  	struct pid *pid;
>
>  	do {
> -		pid = find_pid(nr);
> +		pid = find_pid_ns(nr, ns);
>  		if (pid)
>  			break;
> -		nr = next_pidmap(task_active_pid_ns(current), nr);
> +		nr = next_pidmap(ns, nr);
>  	} while (nr > 0);
>
>  	return pid;

This means we should fix the caller, next_tgid(), but this is done
in 15/15.

Oleg.




More information about the Devel mailing list