[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20081215194911.GA20166@us.ibm.com>
Date: Mon, 15 Dec 2008 13:49:11 -0600
From: "Serge E. Hallyn" <serue@...ibm.com>
To: Sukadev Bhattiprolu <sukadev@...ux.vnet.ibm.com>
Cc: Andrew Morton <akpm@...l.org>, bastian@...di.eu.org,
linux-kernel@...r.kernel.org,
"Eric W. Biederman" <ebiederm@...ssion.com>,
Containers <containers@...ts.osdl.org>,
Oleg Nesterov <oleg@...sign.ru>, roland@...hat.com,
Pavel Emelyanov <xemul@...nvz.org>
Subject: Re: [PATCH 2/2] pid: Generalize task_active_pid_ns
Quoting Sukadev Bhattiprolu (sukadev@...ux.vnet.ibm.com):
> From: Sukadev Bhattiprolu <sukadev@...ux.vnet.ibm.com>
> Date: Fri, 12 Dec 2008 11:49:27 -0800
> Subject: [PATCH 2/2] pid: Generalize task_active_pid_ns
>
> Currently task_active_pid_ns is not safe to call after a
> task becomes a zombie and exit_task_namespaces is called,
> as nsproxy becomes NULL. By reading the pid namespace from
> the pid of the task we can trivially solve this problem at
> the cost of one extra memory read in what should be the
> same cacheline as we read the namespace from.
>
> When moving things around I have made task_active_pid_ns
> out of line because keeping it in pid_namespace.h would
> require adding includes of pid.h and sched.h that I
> don't think we want.
>
> This change does make task_active_pid_ns unsafe to call during
> copy_process until we attach a pid on the task_struct which
> seems to be a reasonable trade off.
Yeah that's not really much of a tradeoff... the text for those
lines gets shorter! :)
Acked-by: Serge Hallyn <serue@...ibm.com>
(to all three)
thanks,
-serge
> Signed-off-by: Eric W. Biederman <ebiederm@...ssion.com>
> ---
> include/linux/pid_namespace.h | 6 +-----
> kernel/fork.c | 4 ++--
> kernel/pid.c | 6 ++++++
> 3 files changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/include/linux/pid_namespace.h b/include/linux/pid_namespace.h
> index d82fe82..38d1032 100644
> --- a/include/linux/pid_namespace.h
> +++ b/include/linux/pid_namespace.h
> @@ -79,11 +79,7 @@ static inline void zap_pid_ns_processes(struct pid_namespace *ns)
> }
> #endif /* CONFIG_PID_NS */
>
> -static inline struct pid_namespace *task_active_pid_ns(struct task_struct *tsk)
> -{
> - return tsk->nsproxy->pid_ns;
> -}
> -
> +extern struct pid_namespace *task_active_pid_ns(struct task_struct *tsk);
> void pidhash_init(void);
> void pidmap_init(void);
>
> diff --git a/kernel/fork.c b/kernel/fork.c
> index dba2d3f..9fa1d91 100644
> --- a/kernel/fork.c
> +++ b/kernel/fork.c
> @@ -1100,12 +1100,12 @@ static struct task_struct *copy_process(unsigned long clone_flags,
>
> if (pid != &init_struct_pid) {
> retval = -ENOMEM;
> - pid = alloc_pid(task_active_pid_ns(p));
> + pid = alloc_pid(p->nsproxy->pid_ns);
> if (!pid)
> goto bad_fork_cleanup_io;
>
> if (clone_flags & CLONE_NEWPID) {
> - retval = pid_ns_prepare_proc(task_active_pid_ns(p));
> + retval = pid_ns_prepare_proc(p->nsproxy->pid_ns);
> if (retval < 0)
> goto bad_fork_free_pid;
> }
> diff --git a/kernel/pid.c b/kernel/pid.c
> index 064e76a..c5513fe 100644
> --- a/kernel/pid.c
> +++ b/kernel/pid.c
> @@ -474,6 +474,12 @@ pid_t task_session_nr_ns(struct task_struct *tsk, struct pid_namespace *ns)
> }
> EXPORT_SYMBOL(task_session_nr_ns);
>
> +struct pid_namespace *task_active_pid_ns(struct task_struct *tsk)
> +{
> + return ns_of_pid(task_pid(tsk));
> +}
> +EXPORT_SYMBOL_GPL(task_active_pid_ns);
> +
> /*
> * Used by proc to find the first pid that is greater then or equal to nr.
> *
> --
> 1.5.2.5
>
> _______________________________________________
> Containers mailing list
> Containers@...ts.linux-foundation.org
> https://lists.linux-foundation.org/mailman/listinfo/containers
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists