Restore task fs_root and pwd
RFC PATCH 1/1 Restore task fs_root and pwd
Checkpoint and restore task->fs. Tasks sharing task->fs will share them again after restart.
The fs/fs_struct.c part should of course be broken out, but this does the right thing for me.
--- checkpoint/files.c | 211 ++++++++++++++++++++++++++++++++++++++++ checkpoint/objhash.c | 9 ++ checkpoint/process.c | 13 +++ fs/open.c | 53 ++++++---- include/linux/checkpoint.h | 10 ++- include/linux/checkpoint_hdr.h | 10 ++ include/linux/fs.h | 4 + 7 files changed, 287 insertions(+), 23 deletions(-)
checkpoint/files.c +211 -0
@@ -24,6 +24,9 @@#include <linux/checkpoint_hdr.h> #include <linux/eventpoll.h> #include <linux/eventfd.h> +#include <linux/fs.h> +#include <linux/fs_struct.h> +#include <linux/namei.h> #include <net/sock.h>
@@ -449,6 +452,71 @@ int ckpt_collect_file_table(struct ckpt_ctx *ctx, struct task_struct *t)return ret; } +int checkpoint_get_task_fs (struct ckpt_ctx *ctx,struct task_struct *t) +{ +struct fs_struct *fs; +int fs_objref; +int kill; + +task_lock (current); + fs = t->fs; +write_lock (&fs->lock); + fs->users++; +write_unlock (&fs->lock); +task_unlock (current); + + fs_objref =checkpoint_obj (ctx, fs,CKPT_OBJ_TASK_FS ); +write_lock (&fs->lock); + kill = !--fs->users; +write_unlock (&fs->lock); +if (kill) +free_fs_struct (fs); + +return fs_objref; +} + +/* +* called with fs read_lock()d +*/ +int checkpoint_obj_task_fs (struct ckpt_ctx *ctx,struct fs_struct *fs) +{ +struct ckpt_hdr_task_fs *h; +int ret; +struct fs_struct *fscopy; + + h =ckpt_hdr_get_type (ctx,sizeof (*h),CKPT_HDR_TASK_FS ); +if (!h) +return -ENOMEM ; + ret =ckpt_write_obj (ctx, &h->h); +ckpt_hdr_put (ctx, h); +if (ret) +return ret; + + fscopy =copy_fs_struct (fs); +if (!fs) +return -ENOMEM ; + + ret =checkpoint_fname (ctx, &fscopy->root, &ctx->fs_mnt); +if (ret <0 ) { +ckpt_err (ctx, ret,"%(T)writing name of fs root" ); + goto out; + } + ret =checkpoint_fname (ctx, &fscopy->pwd, &ctx->fs_mnt); +if (ret <0 ) { +ckpt_err (ctx, ret,"%(T)writing name of pwd" ); + goto out; + } + ret =0 ; +out: +free_fs_struct (fscopy); +return ret; +} + +int checkpoint_task_fs (struct ckpt_ctx *ctx,void *ptr) +{ +return checkpoint_obj_task_fs (ctx, (struct fs_struct *) ptr); +} +/************************************************************************** * Restart */
@@ -812,3 +880,146 @@ int restore_obj_file_table(struct ckpt_ctx *ctx, int files_objref)return 0 ; } + +static int restore_chroot (struct ckpt_ctx *ctx,struct fs_struct *fs,char *name) +{ +struct nameidata nd; +int ret; + +ckpt_debug ("attempting chroot to %s\n" , name); + ret =path_lookup (name,LOOKUP_FOLLOW |LOOKUP_DIRECTORY , &nd); +if (ret) { +ckpt_err (ctx, ret,"%(T)Opening chroot dir %s" , name); +return ret; + } + ret =do_chroot (fs, &nd.path); +path_put (&nd.path); +if (ret) { +ckpt_err (ctx, ret,"%(T)Setting chroot %s" , name); +return ret; + } +return 0 ; +} + +static int restore_cwd (struct ckpt_ctx *ctx,struct fs_struct *fs,char *name) +{ +struct nameidata nd; +int ret; + +ckpt_debug ("attempting chdir to %s\n" , name); + ret =path_lookup (name,LOOKUP_FOLLOW |LOOKUP_DIRECTORY , &nd); +if (ret) { +ckpt_err (ctx, ret,"%(T)Opening cwd %s" , name); +return ret; + } + ret =do_chdir (fs, &nd.path); +path_put (&nd.path); +if (ret) { +ckpt_err (ctx, ret,"%(T)Setting cwd %s" , name); +return ret; + } +return 0 ; +} + +int obj_task_fs_grab (void *ptr) +{ +struct fs_struct *fs = ptr; + +write_lock (&fs->lock); + fs->users++; +write_unlock (&fs->lock); +return 0 ; +} + +void obj_task_fs_drop (void *ptr,int lastref) +{ +struct fs_struct *fs = ptr; +int kill; + +write_lock (&fs->lock); + kill = !--fs->users; +write_unlock (&fs->lock); +if (kill) +free_fs_struct (fs); +} + +/* this is the fn called by objhash when it runs into a +* CKPT_OBJ_TASK_FS entry. Creates an fs_struct and +* places it in the hash. */ +static struct fs_struct *restore_obj_task_fs (struct ckpt_ctx *ctx) +{ +struct ckpt_hdr_task_fs *h; +struct fs_struct *fs; +int ret =0 ; +char *root, *cwd; +int len; + + h =ckpt_read_obj_type (ctx,sizeof (*h),CKPT_HDR_TASK_FS ); +if (IS_ERR (h)) +return ERR_PTR (PTR_ERR (h)); +ckpt_hdr_put (ctx, h); + + fs =copy_fs_struct (current->fs); +if (!fs) +return ERR_PTR (-ENOMEM ); + + len =ckpt_read_payload (ctx, (void **) &root, +PATH_MAX ,CKPT_HDR_FILE_NAME ); + ret =restore_chroot (ctx, fs, root); +kfree (root); +if (ret) { +free_fs_struct (fs); +return ERR_PTR (ret); + } + + len =ckpt_read_payload (ctx, (void **) &cwd, +PATH_MAX ,CKPT_HDR_FILE_NAME ); + ret =restore_cwd (ctx, fs, cwd); +kfree (cwd); + +if (ret) { +free_fs_struct (fs); +return ERR_PTR (ret); + } +return fs; +} + +void *restore_task_fs (struct ckpt_ctx *ctx) +{ +return (void *)restore_obj_task_fs (ctx); +} + +/* +* Called by task restore code to set the restarted task's +* current->fs to an entry on the hash +*/ +int restore_set_task_fs (struct ckpt_ctx *ctx,int fs_objref) +{ +struct fs_struct *newfs, *oldfs; +int kill; + + newfs =ckpt_obj_fetch (ctx, fs_objref,CKPT_OBJ_TASK_FS ); +if (IS_ERR (newfs)) +return PTR_ERR (newfs); + +task_lock (current); + oldfs = current->fs; +write_lock (&oldfs->lock); +/* tasks run sys_restart in serial, and the restored fs was +* created during restart, so (a) no contention and therefore +* no deadlock is possible by the wait on newfs->lock, and so (b) +* we really shouldn't need this lock at all, but it woudl be +* improper to skip it... */ +write_lock (&newfs->lock); + kill = --oldfs->users; + current->fs = newfs; + newfs->users++; +write_unlock (&newfs->lock); +write_unlock (&oldfs->lock); +task_unlock (current); + +if (kill) +free_fs_struct (oldfs); + +return 0 ; +}
checkpoint/objhash.c +9 -0
@@ -417,6 +417,15 @@ static struct ckpt_obj_ops ckpt_obj_ops[] = { .checkpoint = checkpoint_userns, .restore = restore_userns, }, +/* struct fs_struct */ + { + .obj_name = "TASK_FS", + .obj_type = CKPT_OBJ_TASK_FS, + .ref_drop = obj_task_fs_drop, + .ref_grab = obj_task_fs_grab, + .checkpoint = checkpoint_task_fs, + .restore = restore_task_fs, + },/* struct cred */ { .obj_name = "CRED",
checkpoint/process.c +13 -0
@@ -234,6 +234,7 @@ static int checkpoint_task_objs(struct ckpt_ctx *ctx, struct task_struct *t)int mm_objref;int sighand_objref;int signal_objref; +int fs_objref;int first, ret;/*
@@ -253,6 +254,12 @@ static int checkpoint_task_objs(struct ckpt_ctx *ctx, struct task_struct *t)if (ret <0 )return ret; + fs_objref =checkpoint_get_task_fs (ctx, t); +if (fs_objref <0 ) { +ckpt_err (ctx, fs_objref,"%(T)process fs\n" ); +return fs_objref; + } + files_objref =checkpoint_obj_file_table (ctx, t);ckpt_debug ("files: objref %d\n" , files_objref);if (files_objref <0 ) {
@@ -294,6 +301,7 @@ static int checkpoint_task_objs(struct ckpt_ctx *ctx, struct task_struct *t) h->mm_objref = mm_objref; h->sighand_objref = sighand_objref; h->signal_objref = signal_objref; + h->fs_objref = fs_objref; ret =ckpt_write_obj (ctx, &h->h);ckpt_hdr_put (ctx, h);if (ret <0 )
@@ -628,6 +636,11 @@ static int restore_task_objs(struct ckpt_ctx *ctx)return PTR_ERR (h); } + ret =restore_set_task_fs (ctx, h->fs_objref); +ckpt_debug ("restore_task_fs returned %d\n" , ret); +if (ret <0 ) +return ret; + ret =restore_obj_file_table (ctx, h->files_objref);ckpt_debug ("file_table: ret %d (%p)\n" , ret, current->files);if (ret <0 )
fs/open.c +31 -22
81 lines not on this page · raw message
include/linux/checkpoint.h +9 -1
@@ -10,7 +10,7 @@* distribution for more details. */ -#define CHECKPOINT_VERSION 4 +#define CHECKPOINT_VERSION 5/* checkpoint user flags */ #define CHECKPOINT_SUBTREE 0x1
@@ -229,6 +229,14 @@ extern int checkpoint_file_common(struct ckpt_ctx *ctx, struct file *file,extern int restore_file_common (struct ckpt_ctx *ctx,struct file *file,struct ckpt_hdr_file *h); +extern int checkpoint_task_fs (struct ckpt_ctx *ctx,void *ptr); +extern void *restore_task_fs (struct ckpt_ctx *ctx); +extern void obj_task_fs_drop (void *ptr,int lastref); +extern int obj_task_fs_grab (void *ptr); + +extern int restore_set_task_fs (struct ckpt_ctx *ctx,int fs_objref); +extern int checkpoint_get_task_fs (struct ckpt_ctx *ctx,struct task_struct *t); +/* credentials */ extern int checkpoint_groupinfo (struct ckpt_ctx *ctx,void *ptr);extern int checkpoint_user (struct ckpt_ctx *ctx,void *ptr);
include/linux/checkpoint_hdr.h +10 -0
@@ -89,6 +89,8 @@ enum {#define CKPT_HDR_TASK CKPT_HDR_TASK CKPT_HDR_TASK_NS,#define CKPT_HDR_TASK_NS CKPT_HDR_TASK_NS + CKPT_HDR_TASK_FS, +#define CKPT_HDR_TASK_FS CKPT_HDR_TASK_FS CKPT_HDR_TASK_OBJS,#define CKPT_HDR_TASK_OBJS CKPT_HDR_TASK_OBJS CKPT_HDR_RESTART_BLOCK,
@@ -228,6 +230,8 @@ enum obj_type {#define CKPT_OBJ_IPC_NS CKPT_OBJ_IPC_NS CKPT_OBJ_USER_NS,#define CKPT_OBJ_USER_NS CKPT_OBJ_USER_NS + CKPT_OBJ_TASK_FS, +#define CKPT_OBJ_TASK_FS CKPT_OBJ_TASK_FS CKPT_OBJ_CRED,#define CKPT_OBJ_CRED CKPT_OBJ_CRED CKPT_OBJ_USER,
@@ -365,6 +369,11 @@ struct ckpt_hdr_task_creds {__s32 ecred_ref; } __attribute__((aligned(8))); +struct ckpt_hdr_task_fs { +struct ckpt_hdr h; +/* followed by filenames for fs->root and fs->pwd */ +} __attribute__((aligned (8 ))); +struct ckpt_hdr_cred {struct ckpt_hdr h;__u32 uid, suid, euid, fsuid;
@@ -452,6 +461,7 @@ struct ckpt_hdr_task_objs {__s32 mm_objref;__s32 sighand_objref;__s32 signal_objref; +__s32 fs_objref; } __attribute__((aligned(8)));/* restart blocks */
include/linux/fs.h +4 -0
@@ -1819,9 +1819,13 @@ extern struct vfsmount *collect_mounts(struct path *);extern void drop_collected_mounts (struct vfsmount *);extern int vfs_statfs (struct dentry *,struct kstatfs *); +struct fs_struct ; +extern int do_chdir (struct fs_struct *fs,struct path *path); +extern int do_chroot (struct fs_struct *fs,struct path *path);extern int current_umask (void ); +/* /sys/fs */ extern struct kobject *fs_kobj;
Signature · 1 line
1.6.0.4
A message that is not in the archive (sent off-list or not yet received): 20091210190031.GA19315-r/[email protected]
Quoted 7 lines · Serge E. Hallyn
Checkpoint and restore
task->fs. Tasks sharingtask->fswill share them again after restart.The
fs/fs_struct.cpart should of course be broken out, but this does the right thing for me.Signed-off-by Serge E. Hallyn <serue-r/[email protected]>
Looks good. See a few comments below.
Quoted 29 lines
--- checkpoint/files.c | 211 ++++++++++++++++++++++++++++++++++++++++ checkpoint/objhash.c | 9 ++ checkpoint/process.c | 13 +++ fs/open.c | 53 ++++++---- include/linux/checkpoint.h | 10 ++- include/linux/checkpoint_hdr.h | 10 ++ include/linux/fs.h | 4 + 7 files changed, 287 insertions(+), 23 deletions(-)checkpoint/files.c +4
-0@@ -24,6 +24,9 @@#include <linux/checkpoint_hdr.h> #include <linux/eventpoll.h> #include <linux/eventfd.h> +#include <linux/fs.h> +#include <linux/fs_struct.h> +#include <linux/namei.h> #include <net/sock.h> @@ -449,6 +452,71 @@ int ckpt_collect_file_table(struct ckpt_ctx *ctx, struct task_struct *t)return ret; } +int checkpoint_get_task_fs (struct ckpt_ctx *ctx,struct task_struct *t)
checkpoint/files.c int ckpt_collect_file_table(struct ckpt_ctx *ctx, struct task_struct *t) · line 465
Quoted 10 lines
checkpoint/files.c +10
-0+{ +struct fs_struct *fs; +int fs_objref; +int kill; + +task_lock (current); + fs = t->fs; +write_lock (&fs->lock); + fs->users++; +write_unlock (&fs->lock);
3 lines above are the same in obj_task_fs_grab()...
checkpoint/files.c int ckpt_collect_file_table(struct ckpt_ctx *ctx, struct task_struct *t) · line 473
Quoted 8 lines
checkpoint/files.c +8
-0+task_unlock (current); + + fs_objref =checkpoint_obj (ctx, fs,CKPT_OBJ_TASK_FS ); +write_lock (&fs->lock); + kill = !--fs->users; +write_unlock (&fs->lock); +if (kill) +free_fs_struct (fs);
And last 5 lines are the same in obj_task_fs_drop().
Perhaps put as helpers in fs_struct.h ?
checkpoint/files.c int ckpt_collect_file_table(struct ckpt_ctx *ctx, struct task_struct *t) · line 480
Quoted 7 lines
checkpoint/files.c +7
-0+ +return fs_objref; +} + +/* +* called with fs read_lock()d +*/
Am I right to guess that this comment is stale ?
Quoted 6 lines
checkpoint/files.c +6
-0+int checkpoint_obj_task_fs (struct ckpt_ctx *ctx,struct fs_struct *fs) +{ +struct ckpt_hdr_task_fs *h; +int ret; +struct fs_struct *fscopy; +
...
Quoted 12 lines
checkpoint/files.c +12
-0+/* this is the fn called by objhash when it runs into a +* CKPT_OBJ_TASK_FS entry. Creates an fs_struct and +* places it in the hash. */ +static struct fs_struct *restore_obj_task_fs (struct ckpt_ctx *ctx) +{ +struct ckpt_hdr_task_fs *h; +struct fs_struct *fs; +int ret =0 ; +char *root, *cwd; +int len; + + h =ckpt_read_obj_type (ctx,sizeof (*h),CKPT_HDR_TASK_FS );
checkpoint/files.c int restore_obj_file_table(struct ckpt_ctx *ctx, int files_objref) · line 967
Quoted 10 lines
checkpoint/files.c +10
-0+if (IS_ERR (h)) +return ERR_PTR (PTR_ERR (h)); +ckpt_hdr_put (ctx, h); + + fs =copy_fs_struct (current->fs); +if (!fs) +return ERR_PTR (-ENOMEM ); + + len =ckpt_read_payload (ctx, (void **) &root, +PATH_MAX ,CKPT_HDR_FILE_NAME );
Test for len < 0 ... ?
Since this is another place where we read file-name (also in checkpoint/files.c), perhaps introduce a ckpt_read_fname() ?
Quoted 8 lines
checkpoint/files.c +8
-0+ ret =restore_chroot (ctx, fs, root); +kfree (root); +if (ret) { +free_fs_struct (fs); +return ERR_PTR (ret); + } + + len = ckpt_read_payload(ctx, (void **) &cwd,
checkpoint/files.c int restore_obj_file_table(struct ckpt_ctx *ctx, int files_objref) · line 985
Quoted 10 lines
checkpoint/files.c +10
-0+ PATH_MAX, CKPT_HDR_FILE_NAME); + ret =restore_cwd (ctx, fs, cwd); +kfree (cwd); + +if (ret) { +free_fs_struct (fs); +return ERR_PTR (ret); + } +return fs; +}
...
Oren.
A message that is not in the archive (sent off-list or not yet received): [email protected]
Quoted 66 lines · Oren Laadan
Quoted 7 lines · Serge E. Hallyn
Checkpoint and restore
task->fs. Tasks sharingtask->fswill share them again after restart.The
fs/fs_struct.cpart should of course be broken out, but this does the right thing for me.Signed-off-by Serge E. Hallyn <serue-r/[email protected]>Looks good. See a few comments below.
Quoted 29 lines
--- checkpoint/files.c | 211 ++++++++++++++++++++++++++++++++++++++++ checkpoint/objhash.c | 9 ++ checkpoint/process.c | 13 +++ fs/open.c | 53 ++++++---- include/linux/checkpoint.h | 10 ++- include/linux/checkpoint_hdr.h | 10 ++ include/linux/fs.h | 4 + 7 files changed, 287 insertions(+), 23 deletions(-)checkpoint/files.c +4
-0@@ -24,6 +24,9 @@#include <linux/checkpoint_hdr.h> #include <linux/eventpoll.h> #include <linux/eventfd.h> +#include <linux/fs.h> +#include <linux/fs_struct.h> +#include <linux/namei.h> #include <net/sock.h> @@ -449,6 +452,71 @@ int ckpt_collect_file_table(struct ckpt_ctx *ctx, struct task_struct *t)return ret; } +int checkpoint_get_task_fs (struct ckpt_ctx *ctx,struct task_struct *t)checkpoint/files.c int ckpt_collect_file_table(struct ckpt_ctx *ctx, struct task_struct *t) · line 465
Quoted 10 lines
checkpoint/files.c +10
-0+{ +struct fs_struct *fs; +int fs_objref; +int kill; + +task_lock (current); + fs = t->fs; +write_lock (&fs->lock); + fs->users++; +write_unlock (&fs->lock);3 lines above are the same in
obj_task_fs_grab()...checkpoint/files.c int ckpt_collect_file_table(struct ckpt_ctx *ctx, struct task_struct *t) · line 473
Quoted 8 lines
checkpoint/files.c +8
-0+task_unlock (current); + + fs_objref =checkpoint_obj (ctx, fs,CKPT_OBJ_TASK_FS ); +write_lock (&fs->lock); + kill = !--fs->users; +write_unlock (&fs->lock); +if (kill) +free_fs_struct (fs);And last 5 lines are the same in
obj_task_fs_drop().
Quoted 1 line
Perhaps put as helpers in
fs_struct.h?
Absolutely. Aside from waiting on confirmation on the basics, I was also a little intimidated about adding more generic refcount handling to fs_struct.h, since it appears to me to be a case where generic handlers were not-added on purpose :) But it's not right to have this code here.
Quoted 9 lines
checkpoint/files.c int ckpt_collect_file_table(struct ckpt_ctx *ctx, struct task_struct *t) · line 480
Quoted 7 lines
checkpoint/files.c +7
-0+ +return fs_objref; +} + +/* +* called with fs read_lock()d +*/ Am I right to guess that this comment is stale ?
Sure enough - read_locking it will deadlock checkpoint :) Should be "called with fs refcount bumped".
Quoted 34 lines
Quoted 6 lines
checkpoint/files.c +6
-0+int checkpoint_obj_task_fs (struct ckpt_ctx *ctx,struct fs_struct *fs) +{ +struct ckpt_hdr_task_fs *h; +int ret; +struct fs_struct *fscopy; +...
Quoted 12 lines
checkpoint/files.c +12
-0+/* this is the fn called by objhash when it runs into a +* CKPT_OBJ_TASK_FS entry. Creates an fs_struct and +* places it in the hash. */ +static struct fs_struct *restore_obj_task_fs (struct ckpt_ctx *ctx) +{ +struct ckpt_hdr_task_fs *h; +struct fs_struct *fs; +int ret =0 ; +char *root, *cwd; +int len; + + h =ckpt_read_obj_type (ctx,sizeof (*h),CKPT_HDR_TASK_FS );checkpoint/files.c int restore_obj_file_table(struct ckpt_ctx *ctx, int files_objref) · line 967
Quoted 10 lines
checkpoint/files.c +10
-0+if (IS_ERR (h)) +return ERR_PTR (PTR_ERR (h)); +ckpt_hdr_put (ctx, h); + + fs =copy_fs_struct (current->fs); +if (!fs) +return ERR_PTR (-ENOMEM ); + + len =ckpt_read_payload (ctx, (void **) &root, +PATH_MAX ,CKPT_HDR_FILE_NAME );Test for len < 0 ... ?
Quoted 2 lines
Since this is another place where we read file-name (also in
checkpoint/files.c), perhaps introduce ackpt_read_fname()?
Ok.
Quoted 21 lines
Quoted 18 lines
checkpoint/files.c +18
-0+ ret =restore_chroot (ctx, fs, root); +kfree (root); +if (ret) { +free_fs_struct (fs); +return ERR_PTR (ret); + } + + len =ckpt_read_payload (ctx, (void **) &cwd, +PATH_MAX ,CKPT_HDR_FILE_NAME ); + ret =restore_cwd (ctx, fs, cwd); +kfree (cwd); + +if (ret) { +free_fs_struct (fs); +return ERR_PTR (ret); + } +return fs; +}...
Quoted 1 line
Oren.
Laptop is rebuilding at the moment (plus I'm out) so I may not get this out until Monday. If you wanted to apply this now I'll send a patch to move the refcounting to the right place etc next week, else I'll just send a whole new patch.
Thanks,
-serge