Re: [RFC v2][PATCH 2/9] General infrastructure for checkpoint restart

Louis Rilling
2008-08-21T09:35:42Z

[-- Attachment #1: Type: text/plain, Size: 4410 bytes --]

On Wed, Aug 20, 2008 at 11:04:13PM -0400, Oren Laadan wrote:
>
> Add those interfaces, as well as helpers needed to easily manage the
> file format. The code is roughly broken out as follows:
>
> ckpt/sys.c - user/kernel data transfer, as well as setup of the
> checkpoint/restart context (a per-checkpoint data structure for
> housekeeping)
>
> ckpt/checkpoint.c - output wrappers and basic checkpoint handling
>
> ckpt/restart.c - input wrappers and basic restart handling
>
> Patches to add the per-architecture support as well as the actual
> work to do the memory checkpoint follow in subsequent patches.
>

[...]

> diff --git a/checkpoint/sys.c b/checkpoint/sys.c
> new file mode 100644
> index 0000000..2891c48
> --- /dev/null
> +++ b/checkpoint/sys.c

[...]

> +/*
> + * helpers to manage CR contexts: allocated for each checkpoint and/or
> + * restart operation, and persists until the operation is completed.
> + */
> +
> +static atomic_t cr_ctx_count; /* unique checkpoint identifier */

I thought we agreed that this counter should be per-container. Perhaps add a
TODO here?

> +
> +void cr_ctx_free(struct cr_ctx *ctx)
> +{
> +
> + if (ctx->file)
> + fput(ctx->file);
> + if (ctx->vfsroot)
> + path_put(ctx->vfsroot);
> +
> + free_pages((unsigned long) ctx->tbuf, CR_TBUF_ORDER);
> + free_pages((unsigned long) ctx->hbuf, CR_HBUF_ORDER);
> +
> + kfree(ctx);
> +}
> +
> +struct cr_ctx *cr_ctx_alloc(pid_t pid, struct file *file, unsigned long flags)
> +{
> + struct cr_ctx *ctx;
> +
> + ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
> + if (!ctx)
> + return NULL;
> +
> + ctx->tbuf = (void *) __get_free_pages(GFP_KERNEL, CR_TBUF_ORDER);
> + ctx->hbuf = (void *) __get_free_pages(GFP_KERNEL, CR_HBUF_ORDER);
> + if (!ctx->tbuf || !ctx->hbuf)
> + goto nomem;
> +
> + ctx->pid = pid;
> + ctx->flags = flags;
> +
> + ctx->file = file;
> + get_file(file);
> +
> + /* assume checkpointer is in container's root vfs */

I'm a bit puzzled by this assumption. I would say: either this is a
self-checkpoint (only current process), or this is a container checkpoint. In
the latter case, I expect that in the general case the checkpointer lives
outside the container (and the interface of sys_checkpoint() below confirms
this), so it's root fs is probably not the container's one.

Does it differ from what you're planning?

Thanks,

Louis

> + ctx->vfsroot = &current->fs->root;
> + path_get(ctx->vfsroot);
> +
> + ctx->crid = atomic_inc_return(&cr_ctx_count);
> +
> + return ctx;
> +
> + nomem:
> + cr_ctx_free(ctx);
> + return NULL;
> +}
> +
> +/**
> + * sys_checkpoint - checkpoint a container
> + * @pid: pid of the container init(1) process
> + * @fd: file to which dump the checkpoint image
> + * @flags: checkpoint operation flags
> + */
> +asmlinkage long sys_checkpoint(pid_t pid, int fd, unsigned long flags)
> +{
> + struct cr_ctx *ctx;
> + struct file *file;
> + int fput_needed;
> + int ret;
> +
> + file = fget_light(fd, &fput_needed);
> + if (!file)
> + return -EBADF;
> +
> + /* no flags for now */
> + if (flags)
> + return -EINVAL;
> +
> + ctx = cr_ctx_alloc(pid, file, flags | CR_CTX_CKPT);
> + if (!ctx) {
> + fput_light(file, fput_needed);
> + return -ENOMEM;
> + }
> +
> + ret = do_checkpoint(ctx);
> +
> + cr_ctx_free(ctx);
> + fput_light(file, fput_needed);
> +
> + return ret;
> +}
> +
> +/**
> + * sys_restart - restart a container
> + * @crid: checkpoint image identifier
> + * @fd: file from which read the checkpoint image
> + * @flags: restart operation flags
> + */
> +asmlinkage long sys_restart(int crid, int fd, unsigned long flags)
> +{
> + struct cr_ctx *ctx;
> + struct file *file;
> + int fput_needed;
> + int ret;
> +
> + file = fget_light(fd, &fput_needed);
> + if (!file)
> + return -EBADF;
> +
> + /* no flags for now */
> + if (flags)
> + return -EINVAL;
> +
> + ctx = cr_ctx_alloc(crid, file, flags | CR_CTX_RSTR);
> + if (!ctx) {
> + fput_light(file, fput_needed);
> + return -ENOMEM;
> + }
> +
> + ret = do_restart(ctx);
> +
> + cr_ctx_free(ctx);
> + fput_light(file, fput_needed);
> +
> + return ret;
> +}

--
Dr Louis Rilling Kerlabs
Skype: louis.rilling Batiment Germanium
Phone: (+33|0) 6 80 89 08 23 80 avenue des Buttes de Coesmes
http://www.kerlabs.com/ 35700 Rennes

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]