linux-kernel - Re: [RFC v4][PATCH 2/9] General infrastructure for checkpoint restart

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <28c262360809101554l1faab2a9na6b9066a07747554@mail.gmail.com>
Date:	Thu, 11 Sep 2008 07:54:16 +0900
From:	"MinChan Kim" <minchan.kim@...il.com>
To:	"Oren Laadan" <orenl@...columbia.edu>
Cc:	dave@...ux.vnet.ibm.com, arnd@...db.de, jeremy@...p.org,
	linux-kernel@...r.kernel.org, containers@...ts.linux-foundation.org
Subject: Re: [RFC v4][PATCH 2/9] General infrastructure for checkpoint restart

Hi, Oren.

On Thu, Sep 11, 2008 at 3:36 AM, Oren Laadan <orenl@...columbia.edu> wrote:
>
>
> MinChan Kim wrote:
>> On Tue, Sep 9, 2008 at 4:42 PM, Oren Laadan <orenl@...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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/