From: Oren Laadan <orenl@cs.columbia.edu>
To: Dave Hansen <dave@linux.vnet.ibm.com>
Cc: Al Viro <viro@ZenIV.linux.org.uk>,
Andrew Morton <akpm@linux-foundation.org>,
Linus Torvalds <torvalds@osdl.org>,
containers@lists.linux-foundation.org,
linux-kernel@vger.kernel.org, linux-mm@kvack.org,
linux-api@vger.kernel.org, Thomas Gleixner <tglx@linutronix.de>,
Serge Hallyn <serue@us.ibm.com>, Ingo Molnar <mingo@elte.hu>,
"H. Peter Anvin" <hpa@zytor.com>
Subject: Re: [RFC v10][PATCH 05/13] Dump memory address space
Date: Mon, 01 Dec 2008 15:57:09 -0500 [thread overview]
Message-ID: <49344FA5.90004@cs.columbia.edu> (raw)
In-Reply-To: <1228154412.2971.44.camel@nimitz>
Dave Hansen wrote:
> On Fri, 2008-11-28 at 10:53 +0000, Al Viro wrote:
>>> +static int cr_ctx_checkpoint(struct cr_ctx *ctx, pid_t pid)
>>> +{
>>> + ctx->root_pid = pid;
>>> +
>>> + /*
>>> + * assume checkpointer is in container's root vfs
>>> + * FIXME: this works for now, but will change with real containers
>>> + */
>>> + ctx->vfsroot = ¤t->fs->root;
>>> + path_get(ctx->vfsroot);
>> This is going to break as soon as you get another thread doing e.g. chroot(2)
>> while you are in there.
>
> Yeah, we do need at least a read_lock(¤t->fs->lock) to keep people
> from chroot()'ing underneath us.
True.
(while adapting older and safer code I omitted these tests with no reason).
>
>> And it's a really, _really_ bad idea to take a
>> pointer to shared object, increment refcount on the current *contents* of
>> said object and assume that dropping refcount on the later contents of the
>> same will balance out.
>
> Absolutely. I assume you mean get_fs_struct(current) instead of
> path_get().
True.
Should change the type of ctx->vfsroot to not be a pointer, and do:
>>> + ctx->vfsroot = *current->fs->root;
>>> + path_get(&ctx->vfsroot);
and adjust accordingly in where the refcount is dropped.
What we need here is a reference point (this will change later when we handle
multiple fs-namespaces), which is the path of the "container root". Assuming
locking is correct so that current->fs does not change under us, it's enough
to get that path and later release that path.
BW, the current->fs is assumed to not change during the checkpoint; if it does,
then it's a mis-use of the checkpoint interface, and the resulting behavior
is undefined - restart is guaranteed to restore the exact old state even if
checkpoint succeeds.
Thanks,
Oren.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
next prev parent reply other threads:[~2008-12-01 20:57 UTC|newest]
Thread overview: 35+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-11-27 1:04 [RFC v10][PATCH 00/13] Kernel based checkpoint/restart Oren Laadan
2008-11-27 1:04 ` [RFC v10][PATCH 01/13] Create syscalls: sys_checkpoint, sys_restart Oren Laadan
2008-11-27 1:04 ` [RFC v10][PATCH 02/13] Checkpoint/restart: initial documentation Oren Laadan
2008-11-28 10:45 ` Al Viro
2008-12-01 18:15 ` Dave Hansen
2008-11-27 1:04 ` [RFC v10][PATCH 03/13] General infrastructure for checkpoint restart Oren Laadan
2008-11-27 1:04 ` [RFC v10][PATCH 04/13] x86 support for checkpoint/restart Oren Laadan
2008-11-27 1:04 ` [RFC v10][PATCH 05/13] Dump memory address space Oren Laadan
2008-11-28 10:53 ` Al Viro
2008-12-01 18:00 ` Dave Hansen
2008-12-01 20:57 ` Oren Laadan [this message]
2008-11-27 1:04 ` [RFC v10][PATCH 06/13] Restore " Oren Laadan
2008-11-27 1:04 ` [RFC v10][PATCH 07/13] Infrastructure for shared objects Oren Laadan
2008-11-27 1:04 ` [RFC v10][PATCH 08/13] Dump open file descriptors Oren Laadan
2008-11-28 10:19 ` Al Viro
2008-12-01 17:47 ` Dave Hansen
2008-12-01 20:23 ` Oren Laadan
2008-12-01 20:51 ` Dave Hansen
2008-12-01 21:02 ` Linus Torvalds
2008-12-01 21:25 ` Dave Hansen
2008-12-01 21:20 ` Oren Laadan
2008-11-27 1:04 ` [RFC v10][PATCH 09/13] Restore open file descriprtors Oren Laadan
2008-11-28 11:27 ` Al Viro
2008-12-01 19:22 ` Dave Hansen
2008-12-01 20:41 ` Oren Laadan
2008-12-01 20:54 ` Dave Hansen
2008-12-01 21:00 ` Oren Laadan
2008-12-01 21:07 ` Dave Hansen
2008-12-02 1:31 ` Dave Hansen
2008-12-02 1:12 ` Dave Hansen
2008-11-27 1:04 ` [RFC v10][PATCH 10/13] External checkpoint of a task other than ourself Oren Laadan
2008-11-27 1:04 ` [RFC v10][PATCH 11/13] Track in-kernel when we expect checkpoint/restart to work Oren Laadan
2008-11-27 1:04 ` [RFC v10][PATCH 12/13] Checkpoint multiple processes Oren Laadan
2008-11-27 1:04 ` [RFC v10][PATCH 13/13] Restart " Oren Laadan
2008-12-03 23:58 ` [RFC v10][PATCH 00/13] Kernel based checkpoint/restart Serge E. Hallyn
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=49344FA5.90004@cs.columbia.edu \
--to=orenl@cs.columbia.edu \
--cc=akpm@linux-foundation.org \
--cc=containers@lists.linux-foundation.org \
--cc=dave@linux.vnet.ibm.com \
--cc=hpa@zytor.com \
--cc=linux-api@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=mingo@elte.hu \
--cc=serue@us.ibm.com \
--cc=tglx@linutronix.de \
--cc=torvalds@osdl.org \
--cc=viro@ZenIV.linux.org.uk \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox