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
[EMAIL PROTECTED]
https://lists.linux-foundation.org/mailman/listinfo/containers

_______________________________________________
Devel mailing list
Devel@openvz.org
https://openvz.org/mailman/listinfo/devel

Reply via email to