[CRIU] Re: [PATCH cr 6/6] restore: restore pocesses which share one
fdtable
Pavel Emelyanov
xemul at parallels.com
Tue Oct 2 11:56:54 EDT 2012
On 10/02/2012 06:07 PM, Andrey Vagin wrote:
> Currently crtools supports a case when children are shared fd table
> with parent.
>
> Here is only two interesting thing.
> * Service descriptors should be cloned for each process
> who shared one fd table.
> * One task should restore files and other tasks should sleep in this time.
>
> Signed-off-by: Andrey Vagin <avagin at openvz.org>
> ---
> cr-dump.c | 12 +++++++-----
> cr-restore.c | 35 +++++++++++++++++++++++++++++++----
> files.c | 6 +-----
> include/crtools.h | 8 ++++++++
> include/files.h | 1 +
> include/lock.h | 4 ++++
> include/pstree.h | 5 +++++
> pstree.c | 5 ++++-
> 8 files changed, 61 insertions(+), 15 deletions(-)
>
> diff --git a/cr-dump.c b/cr-dump.c
> index 1b689a3..13ce5db 100644
> --- a/cr-dump.c
> +++ b/cr-dump.c
> @@ -1483,12 +1483,14 @@ static int dump_one_task(struct pstree_item *item)
>
> ret = dump_task_kobj_ids(item);
> if (ret)
> - goto err_cure;
> -
> - ret = dump_task_files_seized(parasite_ctl, cr_fdset, dfds);
> - if (ret) {
> - pr_err("Dump files (pid: %d) failed with %d\n", pid, ret);
> goto err_cure;
> +
> + if (!shared_fdtable(item)) {
> + ret = dump_task_files_seized(parasite_ctl, cr_fdset, dfds);
> + if (ret) {
> + pr_err("Dump files (pid: %d) failed with %d\n", pid, ret);
> + goto err_cure;
> + }
> }
>
> ret = parasite_dump_pages_seized(parasite_ctl, &vma_area_list, cr_fdset);
> diff --git a/cr-restore.c b/cr-restore.c
> index 007e13d..69372a0 100644
> --- a/cr-restore.c
> +++ b/cr-restore.c
> @@ -324,13 +324,25 @@ static int pstree_wait_helpers()
>
> static int restore_one_alive_task(int pid, CoreEntry *core)
> {
> + struct pstree_item *child;
> pr_info("Restoring resources\n");
>
> if (pstree_wait_helpers())
> return -1;
>
> - if (prepare_fds(current))
> - return -1;
> + list_for_each_entry(child, ¤t->children, list) {
> + if (child->state == TASK_HELPER)
> + continue;
> + futex_wait_while_lt(&child->rst->fdt_lock, FDT_LOCK_SYNC);
> + }
> + futex_set_and_wake(¤t->rst->fdt_lock, FDT_LOCK_SYNC);
> +
> + if (!shared_fdtable(current)) {
> + if (prepare_fds(current))
> + return -1;
> + } else
> + futex_wait_while_lt(¤t->parent->rst->fdt_lock, FDT_LOCK_DONE);
> + futex_set_and_wake(¤t->rst->fdt_lock, FDT_LOCK_DONE);
This looks too complicated and not correct. Plz, explain what is going on here.
> if (prepare_fs(pid))
> return -1;
> @@ -407,6 +419,7 @@ static int restore_one_zombie(int pid, int exit_code)
> {
> pr_info("Restoring zombie with %d code\n", exit_code);
>
> + futex_set_and_wake(¤t->rst->fdt_lock, FDT_LOCK_DONE);
> if (task_entries != NULL) {
> futex_dec_and_wake(&task_entries->nr_in_progress);
> futex_wait_while(&task_entries->start, CR_STATE_RESTORE);
> @@ -524,6 +537,9 @@ static inline int fork_with_pid(struct pstree_item *item, unsigned long ns_clone
> ca.item = item;
> ca.clone_flags = ns_clone_flags;
>
> + if (shared_fdtable(item))
> + ca.clone_flags |= CLONE_FILES;
> +
> if (!(ca.clone_flags & CLONE_NEWPID)) {
> char buf[32];
>
> @@ -734,9 +750,17 @@ static int restore_task_with_children(void *_arg)
> int ret;
> sigset_t blockmask;
>
> - close_safe(&ca->fd);
> -
> current = ca->item;
> + if (!shared_fdtable(current))
> + close_safe(&ca->fd);
> +
> + if (ca->clone_flags & CLONE_FILES)
This if () can be merged with the above one.
> + ret = clone_service_fd(false);
> + else
> + ret = clone_service_fd(true);
> +
> + if (ret != 0)
> + return -1;
>
> pid = getpid();
> if (current->pid.virt != pid) {
> @@ -768,6 +792,9 @@ static int restore_task_with_children(void *_arg)
> exit(-1);
> }
>
> + if (!shared_fdtable(current))
> + close_old_fds(current);
> +
> /*
> * The block mask will be restored in sigresturn.
> *
> diff --git a/files.c b/files.c
> index cb34c48..bb53f10 100644
> --- a/files.c
> +++ b/files.c
> @@ -496,7 +496,7 @@ static int open_fdinfos(int pid, struct list_head *list, int state)
> return ret;
> }
>
> -static int close_old_fds(struct pstree_item *me)
> +int close_old_fds(struct pstree_item *me)
> {
> DIR *dir;
> struct dirent *de;
> @@ -533,10 +533,6 @@ int prepare_fds(struct pstree_item *me)
> u32 ret;
> int state;
>
> - ret = close_old_fds(me);
> - if (ret)
> - return ret;
> -
> pr_info("Opening fdinfo-s\n");
>
> for (state = 0; state < ARRAY_SIZE(states); state++) {
> diff --git a/include/crtools.h b/include/crtools.h
> index c72c0b0..7a56e98 100644
> --- a/include/crtools.h
> +++ b/include/crtools.h
> @@ -218,10 +218,18 @@ struct vma_area {
> #define vma_area_is(vma_area, s) vma_entry_is(&((vma_area)->vma), s)
> #define vma_area_len(vma_area) vma_entry_len(&((vma_area)->vma))
>
> +enum {
> + FDT_LOCK_INIT = 0,
> + FDT_LOCK_SYNC,
> + FDT_LOCK_DONE,
> +};
> +
> struct rst_info {
> struct list_head fds;
> struct list_head eventpoll;
> struct list_head tty_slaves;
> +
> + futex_t fdt_lock;
> };
>
> struct pid
> diff --git a/include/files.h b/include/files.h
> index cc0e46f..7199212 100644
> --- a/include/files.h
> +++ b/include/files.h
> @@ -91,6 +91,7 @@ extern int get_filemap_fd(int pid, VmaEntry *vma_entry);
> extern int prepare_fs(int pid);
> extern int set_fd_flags(int fd, int flags);
>
> +extern int close_old_fds(struct pstree_item *me);
> #ifndef AT_EMPTY_PATH
> #define AT_EMPTY_PATH 0x1000
> #endif
> diff --git a/include/lock.h b/include/lock.h
> index e699a06..c83d4ad 100644
> --- a/include/lock.h
> +++ b/include/lock.h
> @@ -84,6 +84,10 @@ static inline void futex_wait_until(futex_t *f, u32 v)
> static inline void futex_wait_while_gt(futex_t *f, u32 v)
> { futex_wait_if_cond(f, v, <=); }
>
> +/* Wait while futex @f value is greater than @v */
> +static inline void futex_wait_while_lt(futex_t *f, u32 v)
> +{ futex_wait_if_cond(f, v, >=); }
> +
> /* Wait while futex @f value is @v */
> static inline void futex_wait_while(futex_t *f, u32 v)
> {
> diff --git a/include/pstree.h b/include/pstree.h
> index 2da7176..2663cfe 100644
> --- a/include/pstree.h
> +++ b/include/pstree.h
> @@ -23,6 +23,11 @@ struct pstree_item {
> struct rst_info *rst;
> };
>
> +static inline int shared_fdtable(struct pstree_item *item) {
> + return (item->parent && item->parent->state != TASK_HELPER &&
> + item->files_id && item->files_id == item->parent->files_id);
> +}
Formatting.
> +
> extern void free_pstree(struct pstree_item *root_item);
> extern struct pstree_item *__alloc_pstree_item(bool rst);
> #define alloc_pstree_item() __alloc_pstree_item(false)
> diff --git a/pstree.c b/pstree.c
> index 1672b26..057cad3 100644
> --- a/pstree.c
> +++ b/pstree.c
> @@ -5,6 +5,7 @@
> #include "pstree.h"
> #include "restorer.h"
> #include "util.h"
> +#include "lock.h"
>
> #include "protobuf.h"
> #include "protobuf/pstree.pb-c.h"
> @@ -41,6 +42,8 @@ struct pstree_item *__alloc_pstree_item(bool rst)
> item->rst = shmalloc(sizeof(item->rst[0]));
> if (!item->rst)
> return NULL;
> +
> + futex_init(&item->rst->fdt_lock);
> }
>
> INIT_LIST_HEAD(&item->children);
> @@ -250,7 +253,7 @@ int prepare_pstree_ids(void)
> if (item->sid == root_item->sid || item->sid == item->pid.virt)
> continue;
>
> - helper = alloc_pstree_item();
> + helper = alloc_pstree_item_with_rst();
Huh?
> if (helper == NULL)
> return -1;
> helper->sid = item->sid;
>
More information about the CRIU
mailing list