Hi, Oren.
On Thu, Sep 11, 2008 at 3:36 AM, Oren Laadan <orenl@xxxxxxxxxxxxxxx> wrote:
>
>
> MinChan Kim wrote:
>> On Tue, Sep 9, 2008 at 4:42 PM, Oren Laadan <orenl@xxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linux-foundation.org/mailman/listinfo/containers
