lkml.org 
[lkml]   [2007]   [Jul]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [PATCH 10/16] Changes in copy_process() to work with pid namespaces
    Pavel Emelianov [xemul@openvz.org] wrote:
    | We must pass the namespace pointer to the alloc_pid() to
    | show what namespace to allocate the pid from and we should
    | call this *after* the namespace is copied.
    |
    | Essentially, the task->pid etc initialization is done after
    | the alloc_pid().
    |
    | To do so I move the alloc_pid() inside copy_process() and
    | introduce an argument to the alloc_pid() function.
    |
    | Signed-off-by: Pavel Emelianov <xemul@openvz.org>
    |
    | ---
    |
    | include/linux/pid.h | 2 +-
    | kernel/fork.c | 29 +++++++++++++++++------------
    | kernel/pid.c | 2 +-
    | 3 files changed, 19 insertions(+), 14 deletions(-)
    |
    | --- ./include/linux/pid.h.ve9 2007-07-06 11:03:55.000000000 +0400
    | +++ ./include/linux/pid.h 2007-07-06 11:03:55.000000000 +0400
    | @@ -116,7 +116,7 @@ extern struct pid *FASTCALL(find_pid_ns(
    | extern struct pid *find_get_pid(int nr);
    | extern struct pid *find_ge_pid(int nr, struct pid_namespace *);
    |
    | -extern struct pid *alloc_pid(void);
    | +extern struct pid *alloc_pid(struct pid_namespace *ns);
    | extern void FASTCALL(free_pid(struct pid *pid));
    |
    | /*
    | --- ./kernel/fork.c.ve9 2007-07-06 11:03:55.000000000 +0400
    | +++ ./kernel/fork.c 2007-07-06 11:04:07.000000000 +0400
    | @@ -50,6 +50,7 @@
    | #include <linux/taskstats_kern.h>
    | #include <linux/random.h>
    | #include <linux/tty.h>
    | +#include <linux/pid.h>
    |
    | #include <asm/pgtable.h>
    | #include <asm/pgalloc.h>
    | @@ -1032,7 +1033,6 @@ static struct task_struct *copy_process(
    | p->did_exec = 0;
    | delayacct_tsk_init(p); /* Must remain after dup_task_struct() */
    | copy_flags(clone_flags, p);
    | - p->pid = pid_nr(pid);
    | INIT_LIST_HEAD(&p->children);
    | INIT_LIST_HEAD(&p->sibling);
    | p->vfork_done = NULL;
    | @@ -1107,10 +1107,6 @@ static struct task_struct *copy_process(
    | p->blocked_on = NULL; /* not blocked yet */
    | #endif
    |
    | - p->tgid = p->pid;
    | - if (clone_flags & CLONE_THREAD)
    | - p->tgid = current->tgid;
    | -
    | if ((retval = security_task_alloc(p)))
    | goto bad_fork_cleanup_policy;
    | if ((retval = audit_alloc(p)))
    | @@ -1132,9 +1128,14 @@ static struct task_struct *copy_process(
    | goto bad_fork_cleanup_mm;
    | if ((retval = copy_namespaces(clone_flags, p)))
    | goto bad_fork_cleanup_keys;
    | + if (likely(pid == NULL)) {
    | + pid = alloc_pid(p->nsproxy->pid_ns);
    | + if (pid == NULL)
    | + goto bad_fork_cleanup_namespaces;
    | + }
    | retval = copy_thread(0, clone_flags, stack_start, stack_size, p, regs);
    | if (retval)
    | - goto bad_fork_cleanup_namespaces;
    | + goto bad_fork_cleanup_pid;
    |
    | p->set_child_tid = (clone_flags & CLONE_CHILD_SETTID) ? child_tidptr : NULL;
    | /*
    | @@ -1255,6 +1256,11 @@ static struct task_struct *copy_process(
    | }
    | }
    |
    | + p->pid = pid_nr(pid);
    | + p->tgid = p->pid;
    | + if (clone_flags & CLONE_THREAD)
    | + p->tgid = current->tgid;
    | +
    | if (likely(p->pid)) {
    | add_parent(p);
    | if (unlikely(p->ptrace & PT_PTRACED))
    | @@ -1288,6 +1294,8 @@ static struct task_struct *copy_process(
    | proc_fork_connector(p);
    | return p;
    |
    | +bad_fork_cleanup_pid:
    | + free_pid(pid);

    If copy_process() was called from fork_idle(), pid would refer
    to &init_struct_pid. Don't you need something like:

    if (pid == &init_struct_pid)
    return;

    in free_pid() to not do anything in that case ?


    | bad_fork_cleanup_namespaces:
    | exit_task_namespaces(p);
    | bad_fork_cleanup_keys:
    | @@ -1380,19 +1388,16 @@ long do_fork(unsigned long clone_flags,
    | {
    | struct task_struct *p;
    | int trace = 0;
    | - struct pid *pid = alloc_pid();
    | long nr;
    |
    | - if (!pid)
    | - return -EAGAIN;
    | - nr = pid->nr;
    | if (unlikely(current->ptrace)) {
    | trace = fork_traceflag (clone_flags);
    | if (trace)
    | clone_flags |= CLONE_PTRACE;
    | }
    |
    | - p = copy_process(clone_flags, stack_start, regs, stack_size, parent_tidptr, child_tidptr, pid);
    | + p = copy_process(clone_flags, stack_start, regs, stack_size,
    | + parent_tidptr, child_tidptr, NULL);
    | /*
    | * Do this prior waking up the new thread - the thread pointer
    | * might get invalid after that point, if the thread exits quickly.
    | @@ -1418,6 +1423,7 @@ long do_fork(unsigned long clone_flags,
    | else
    | p->state = TASK_STOPPED;
    |
    | + nr = pid_vnr(task_pid(p));
    | if (unlikely (trace)) {
    | current->ptrace_message = nr;
    | ptrace_notify ((trace << 8) | SIGTRAP);
    | @@ -1433,7 +1439,6 @@ long do_fork(unsigned long clone_flags,
    | }
    | }
    | } else {
    | - free_pid(pid);
    | nr = PTR_ERR(p);
    | }
    | return nr;
    | --- ./kernel/pid.c.ve9 2007-07-06 11:03:55.000000000 +0400
    | +++ ./kernel/pid.c 2007-07-06 11:03:55.000000000 +0400
    | @@ -206,7 +206,7 @@ fastcall void free_pid(struct pid *pid)
    | call_rcu(&pid->rcu, delayed_put_pid);
    | }
    |
    | -struct pid *alloc_pid(void)
    | +struct pid *alloc_pid(struct pid_namespace *pid_ns)
    | {
    | struct pid *pid;
    | enum pid_type type;
    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2007-07-12 02:35    [W:3.796 / U:0.008 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site