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

[MinChan Kim]

Hi, Oren.

On Thu, Sep 11, 2008 at 3:36 AM, Oren Laadan <orenl at cs.columbia.edu> wrote:
>
>
> MinChan Kim wrote:
>> On Tue, Sep 9, 2008 at 4:42 PM, Oren Laadan <orenl at cs.columbia.edu> wrote:
>
> [...]
>
>>> +struct cr_ctx *cr_ctx_alloc(pid_t pid, int fd, unsigned long flags)
>>> +{
>>> +       struct cr_ctx *ctx;
>>> +
>>> +       ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
>>> +       if (!ctx)
>>> +               return ERR_PTR(-ENOMEM);
>>> +
>>> +       ctx->file = fget(fd);
>>> +       if (!ctx->file) {
>>> +               cr_ctx_free(ctx);
>>> +               return ERR_PTR(-EBADF);
>>> +       }
>>> +       get_file(ctx->file);
>>
>> Why do you need get_file?
>> You already called fget.
>> Am I missing something ?
>
> This was meant for when we will restart multiple processes, each would
> have access to the checkpoint-context, such that the checkpoint-context
> may outlives the task that created it and initiated the restart. Thus
> the file-pointer will need to stay around longer than that task.

OK. Thanks for your explanation.
You should have inserted above annotation.

> Of course, restart of multiple processes _can_ be coded such that this
> first task will always terminate last - either after restart completes
> successfully, or after all the other tasks aborted and won't use the
> checkpoint-context anymore.
>
> Because that code is not part of the this patch-set, I considered it
> safer to grab a reference of the file pointer, making it less likely
> that we forget about it later.

What do you mean by that ? Isn't it a your part of your code?
When the last checkpoint-context is ended, who free file ?
I mean how it match pair(fget/fput and get_file) ?

>>
>>> +       ctx->hbuf = (void *) __get_free_pages(GFP_KERNEL, CR_HBUF_ORDER);
>>> +       if (!ctx->hbuf) {
>>> +               cr_ctx_free(ctx);
>>> +               return ERR_PTR(-ENOMEM);
>>> +       }
>>> +
>>> +       ctx->pid = pid;
>>> +       ctx->flags = flags;
>>> +
>>> +       ctx->crid = atomic_inc_return(&cr_ctx_count);
>>> +
>>> +       return ctx;
>>> +}
>
> Oren.
>
>



-- 
Kinds regards,
MinChan Kim
_______________________________________________
Containers mailing list
Containers at lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/containers