lkml.org 
[lkml]   [2010]   [Mar]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH linux-cr] nested pid namespaces (v2)
    Quoting Oren Laadan (orenl@cs.columbia.edu):
    >
    >
    > Serge E. Hallyn wrote:
    > >Support checkpoint and restart of tasks in nested pid namespaces. At
    > >Oren's request here is an alternative to my previous implementation. In
    > >this one, we keep the original single pids_array to minimize memory
    > >allocations. The pids array entries are augmented with a pidns depth
    >
    > Thanks for adapting the patch.
    >
    > FWIW, not only minimize memory allocations, but also permit a more
    > regular structure of the image data (array of fixed size elements
    > followed by an array of vpids), which simplifies the code that needs
    > to read/write/access this data.
    >
    > >(relative to the container init's pidns, and an "rpid" which is the pid
    > >in the checkpointer's pidns (or 0 if no valid pid exists). The rpid
    > >will be used by userspace to gather more information (like
    > >/proc/$$/mountinfo) after the kernel sys_checkpoint. If any tasks are
    > >in nested pid namespace, another single array holds all of the vpids.
    > >At restart those are used by userspace to determine how to call
    > >eclone(). Kernel ignores them.
    > >
    > >All cr_tests including the new pid_ns testcase pass.
    > >
    > >Signed-off-by: Serge E. Hallyn <serue@us.ibm.com>
    > >---
    >
    > [...]

    Thanks, Oren - all other input is taken into what I'm about to post,
    except:

    > >@@ -293,10 +295,15 @@ static int may_checkpoint_task(struct ckpt_ctx *ctx, struct task_struct *t)
    > > _ckpt_err(ctx, -EPERM, "%(T)Nested net_ns unsupported\n");
    > > ret = -EPERM;
    > > }
    > >- /* no support for >1 private pidns */
    > >- if (nsproxy->pid_ns != ctx->root_nsproxy->pid_ns) {
    > >- _ckpt_err(ctx, -EPERM, "%(T)Nested pid_ns unsupported\n");
    > >- ret = -EPERM;
    > >+ /* pidns must be descendent of root_nsproxy */
    > >+ pidns = nsproxy->pid_ns;
    > >+ while (pidns != ctx->root_nsproxy->pid_ns) {
    > >+ if (pidns == &init_pid_ns) {
    > >+ ret = -EPERM;
    > >+ _ckpt_err(ctx, ret, "%(T)stranger pid_ns\n");
    > >+ break;
    > >+ }
    > >+ pidns = pidns->parent;
    >
    > Currently we do this while() loop twice - once here and once when
    > we collect the vpids. While I doubt if this has any performance
    > impact, is there an advantage to doing it also here ? (a violation
    > will be observed there too).

    With the new logic (ripped verbatim from Louis' email) such a move
    would make the checkpoint_vpids() code a bit uglier. I'm about to
    resend, please let me know if you still want the code moved.

    ...

    > >diff --git a/kernel/nsproxy.c b/kernel/nsproxy.c
    > >index 0da0d83..6d86240 100644
    > >--- a/kernel/nsproxy.c
    > >+++ b/kernel/nsproxy.c
    > >@@ -364,8 +364,13 @@ static struct nsproxy *do_restore_ns(struct ckpt_ctx *ctx)
    > > get_net(net_ns);
    > > nsproxy->net_ns = net_ns;
    > >- get_pid_ns(current->nsproxy->pid_ns);
    > >- nsproxy->pid_ns = current->nsproxy->pid_ns;
    > >+ /*
    > >+ * The pid_ns will get assigned the first time that we
    > >+ * assign the nsproxy to a task. The task had unshared
    > >+ * its pid_ns in userspace before calling restart, and
    > >+ * we want to keep using that pid_ns.
    > >+ */
    > >+ nsproxy->pid_ns = NULL;
    >
    > This doesn't look healthy.
    >
    > If it is (or will be) possible for another process to look at the
    > restarting process, not having a pid-ns may confuse other code in
    > the kernel ?

    No task will have this nproxy attached before we assign a valid
    pid_ns. The NULL pid_ns is only while it is in the objhash but
    not attached to a task.

    thanks,
    -serge


    \
     
     \ /
      Last update: 2010-03-23 05:29    [W:0.055 / U:0.308 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site