any reason why you want to reference the entire ->fs instead of, as I suggested, make a *copy* of ->fs->root and then reference its contents ? Dave Hansen wrote: > The existing ctx->vfsroot is dangerous. We take it from > the root task via: ctx->root_task->fs->root and don't take > a ref on the 'fs' in there. We also take a ref on the > fs.root which is worthless. > > So, replace ctx->vfsroot with a reference to the 'fs' > instead. This gives us easy access to fs.root later on, > and also lets us do proper refcounting. > > This, of course, eventually needs to be done per-process > but this should at least remove a potential oops. > > --- > > linux-2.6.git-dave/checkpoint/checkpoint.c | 7 ++----- > linux-2.6.git-dave/checkpoint/ckpt_file.c | 2 +- > linux-2.6.git-dave/checkpoint/ckpt_mem.c | 2 +- > linux-2.6.git-dave/checkpoint/sys.c | 4 ++-- > linux-2.6.git-dave/include/linux/checkpoint.h | 2 +- > 5 files changed, 7 insertions(+), 10 deletions(-) > > diff -puN checkpoint/checkpoint.c~fix-cr_ctx_checkpoint-locking checkpoint/checkpoint.c > --- linux-2.6.git/checkpoint/checkpoint.c~fix-cr_ctx_checkpoint-locking 2008-12-02 10:21:52.000000000 -0800 > +++ linux-2.6.git-dave/checkpoint/checkpoint.c 2008-12-02 10:21:52.000000000 -0800 > @@ -511,12 +511,9 @@ static int cr_ctx_checkpoint(struct cr_c > * assume checkpointer is in container's root vfs > * FIXME: this works for now, but will change with real containers > */ > - task_lock(ctx->root_task); > - ctx->vfsroot = &ctx->root_task->fs->root; > - task_unlock(ctx->root_task); > - if (!ctx->vfsroot) > + ctx->fs = get_fs_struct(ctx->root_task); > + if (!ctx->fs) > return -EAGAIN; > - path_get(ctx->vfsroot); > > return 0; > } > diff -puN checkpoint/ckpt_file.c~fix-cr_ctx_checkpoint-locking checkpoint/ckpt_file.c > --- linux-2.6.git/checkpoint/ckpt_file.c~fix-cr_ctx_checkpoint-locking 2008-12-02 10:21:52.000000000 -0800 > +++ linux-2.6.git-dave/checkpoint/ckpt_file.c 2008-12-02 10:46:56.000000000 -0800 > @@ -117,7 +117,7 @@ static int cr_write_fd_data(struct cr_ct > if (ret < 0) > return ret; > > - return cr_write_fname(ctx, &file->f_path, ctx->vfsroot); > + return cr_write_fname(ctx, &file->f_path, &ctx->fs.root); > } > > /** > diff -puN checkpoint/ckpt_mem.c~fix-cr_ctx_checkpoint-locking checkpoint/ckpt_mem.c > --- linux-2.6.git/checkpoint/ckpt_mem.c~fix-cr_ctx_checkpoint-locking 2008-12-02 10:21:52.000000000 -0800 > +++ linux-2.6.git-dave/checkpoint/ckpt_mem.c 2008-12-02 10:21:52.000000000 -0800 > @@ -436,7 +436,7 @@ static int cr_write_vma(struct cr_ctx *c > > /* save the file name, if relevant */ > if (vma->vm_file) { > - ret = cr_write_fname(ctx, &vma->vm_file->f_path, ctx->vfsroot); > + ret = cr_write_fname(ctx, &vma->vm_file->f_path, &ctx->fs.root); > if (ret < 0) > return ret; > } > diff -puN checkpoint/sys.c~fix-cr_ctx_checkpoint-locking checkpoint/sys.c > --- linux-2.6.git/checkpoint/sys.c~fix-cr_ctx_checkpoint-locking 2008-12-02 10:21:52.000000000 -0800 > +++ linux-2.6.git-dave/checkpoint/sys.c 2008-12-02 10:21:52.000000000 -0800 > @@ -152,8 +152,8 @@ static void cr_ctx_free(struct cr_ctx *c > > kfree(ctx->hbuf); > > - if (ctx->vfsroot) > - path_put(ctx->vfsroot); > + if (ctx->fs) > + put_fs_struct(ctx->fs); > > cr_pgarr_free(ctx); > cr_objhash_free(ctx); > diff -puN include/linux/checkpoint.h~fix-cr_ctx_checkpoint-locking include/linux/checkpoint.h > --- linux-2.6.git/include/linux/checkpoint.h~fix-cr_ctx_checkpoint-locking 2008-12-02 10:21:52.000000000 -0800 > +++ linux-2.6.git-dave/include/linux/checkpoint.h 2008-12-02 10:21:52.000000000 -0800 > @@ -41,7 +41,7 @@ struct cr_ctx { > > struct list_head pgarr_list; /* page array to dump VMA contents */ > > - struct path *vfsroot; /* container root (FIXME) */ > + struct fs_struct *fs; /* container root/pwd (FIXME) */ > > /* [multi-process checkpoint] */ > struct task_struct **tasks_arr; /* array of all tasks [checkpoint] */ > _ > _______________________________________________ Containers mailing list Containers@xxxxxxxxxxxxxxxxxxxxxxxxxx https://lists.linux-foundation.org/mailman/listinfo/containers