[CRIU] [PATCH] criu: dump filemap as soon as possible
Pavel Emelyanov
xemul at virtuozzo.com
Thu May 5 04:16:45 PDT 2016
On 04/27/2016 12:42 AM, Andrey Vagin wrote:
> From: Andrew Vagin <avagin at virtuozzo.com>
>
> A process can have many vma-s and we can hit rlimit for files.
>
> I suggest to dump vma files as soon as possible and close them.
>
> https://jira.sw.ru/browse/PSBM-46355
>
> Cc: Cyrill Gorcunov <gorcunov at openvz.org>
> Signed-off-by: Andrew Vagin <avagin at virtuozzo.com>
> ---
> criu/cr-dump.c | 15 ++++++---------
> criu/cr-exec.c | 2 +-
> criu/include/proc_parse.h | 4 +++-
> criu/include/vma.h | 5 ++++-
> criu/proc_parse.c | 26 +++++++++++++++++++++++++-
> 5 files changed, 39 insertions(+), 13 deletions(-)
>
> diff --git a/criu/cr-dump.c b/criu/cr-dump.c
> index 5ac9fd0..b7e30ba 100644
> --- a/criu/cr-dump.c
> +++ b/criu/cr-dump.c
> @@ -114,7 +114,8 @@ void free_mappings(struct vm_area_list *vma_area_list)
> vma_area_list->nr = 0;
> }
>
> -int collect_mappings(pid_t pid, struct vm_area_list *vma_area_list)
> +int collect_mappings(pid_t pid, struct vm_area_list *vma_area_list,
> + dump_filemap_t dump_file)
> {
> int ret = -1;
>
> @@ -122,7 +123,7 @@ int collect_mappings(pid_t pid, struct vm_area_list *vma_area_list)
> pr_info("Collecting mappings (pid: %d)\n", pid);
> pr_info("----------------------------------------\n");
>
> - ret = parse_smaps(pid, vma_area_list);
> + ret = parse_smaps(pid, vma_area_list, dump_file);
> if (ret < 0)
> goto err;
>
> @@ -364,8 +365,7 @@ static int dump_pid_misc(pid_t pid, TaskCoreEntry *tc)
> return 0;
> }
>
> -static int dump_filemap(pid_t pid, struct vma_area *vma_area,
> - const struct cr_imgset *imgset)
> +static int dump_filemap(struct vma_area *vma_area)
> {
> struct fd_parms p = FD_PARMS_INIT;
> VmaEntry *vma = vma_area->e;
> @@ -469,9 +469,6 @@ static int dump_task_mm(pid_t pid, const struct proc_pid_stat *stat,
> ret = check_sysvipc_map_dump(pid, vma);
> else if (vma_entry_is(vma, VMA_ANON_SHARED))
> ret = add_shmem_area(pid, vma);
> - else if (vma_entry_is(vma, VMA_FILE_PRIVATE) ||
> - vma_entry_is(vma, VMA_FILE_SHARED))
> - ret = dump_filemap(pid, vma_area, imgset);
> else if (vma_entry_is(vma, VMA_AREA_SOCKET))
> ret = dump_socket_map(vma_area);
> else
> @@ -1125,7 +1122,7 @@ static int pre_dump_one_task(struct pstree_item *item, struct list_head *ctls)
> if (item->pid.state == TASK_DEAD)
> return 0;
>
> - ret = collect_mappings(pid, &vmas);
> + ret = collect_mappings(pid, &vmas, NULL);
> if (ret) {
> pr_err("Collect mappings (pid: %d) failed with %d\n", pid, ret);
> goto err;
> @@ -1205,7 +1202,7 @@ static int dump_one_task(struct pstree_item *item)
> if (ret < 0)
> goto err;
>
> - ret = collect_mappings(pid, &vmas);
> + ret = collect_mappings(pid, &vmas, dump_filemap);
> if (ret) {
> pr_err("Collect mappings (pid: %d) failed with %d\n", pid, ret);
> goto err;
> diff --git a/criu/cr-exec.c b/criu/cr-exec.c
> index e595ec1..a4c07e2 100644
> --- a/criu/cr-exec.c
> +++ b/criu/cr-exec.c
> @@ -147,7 +147,7 @@ int cr_exec(int pid, char **opt)
> */
> free(creds);
>
> - ret = collect_mappings(pid, &vmas);
> + ret = collect_mappings(pid, &vmas, NULL);
> if (ret) {
> pr_err("Can't collect vmas for %d\n", pid);
> goto out_unseize;
> diff --git a/criu/include/proc_parse.h b/criu/include/proc_parse.h
> index 5de5c86..5ef6989 100644
> --- a/criu/include/proc_parse.h
> +++ b/criu/include/proc_parse.h
> @@ -128,7 +128,9 @@ extern int parse_pid_stat(pid_t pid, struct proc_pid_stat *s);
> extern unsigned int parse_pid_loginuid(pid_t pid, int *err, bool ignore_noent);
> extern int parse_pid_oom_score_adj(pid_t pid, int *err);
> extern int prepare_loginuid(unsigned int value, unsigned int loglevel);
> -extern int parse_smaps(pid_t pid, struct vm_area_list *vma_area_list);
> +struct vma_area;
> +typedef int (*dump_filemap_t)(struct vma_area *vma_area);
> +extern int parse_smaps(pid_t pid, struct vm_area_list *vma_area_list, dump_filemap_t cb);
> extern int parse_self_maps_lite(struct vm_area_list *vms);
> extern int parse_pid_status(pid_t pid, struct proc_status_creds *);
>
> diff --git a/criu/include/vma.h b/criu/include/vma.h
> index d69f5f0..15db4ea 100644
> --- a/criu/include/vma.h
> +++ b/criu/include/vma.h
> @@ -71,8 +71,11 @@ struct vma_area {
> };
> };
>
> +typedef int (*dump_filemap_t)(struct vma_area *vma_area);
> +
> extern struct vma_area *alloc_vma_area(void);
> -extern int collect_mappings(pid_t pid, struct vm_area_list *vma_area_list);
> +extern int collect_mappings(pid_t pid,
> + struct vm_area_list *vma_area_list, dump_filemap_t cb);
> extern void free_mappings(struct vm_area_list *vma_area_list);
>
> #define vma_area_is(vma_area, s) vma_entry_is((vma_area)->e, s)
> diff --git a/criu/proc_parse.c b/criu/proc_parse.c
> index 4522ce6..10badfe 100644
> --- a/criu/proc_parse.c
> +++ b/criu/proc_parse.c
> @@ -592,7 +592,22 @@ static int vma_list_add(struct vma_area *vma_area,
> return 0;
> }
>
> -int parse_smaps(pid_t pid, struct vm_area_list *vma_area_list)
> +static void close_prev_vma(struct vma_area *vma, bool borrowed)
There's a close_vma_file() routine that becomes no-op after this change.
Also, the vm_file_fd field becomes unneeded on vm area struct as it's
opened, dumped and closed in the same call stack.
> +{
> + if (!vma)
> + return;
> +
> + if (!vma_entry_is(vma->e, VMA_FILE_PRIVATE) &&
> + !vma_entry_is(vma->e, VMA_FILE_SHARED))
> + return;
> +
> + if (!borrowed)
> + close_safe(&vma->vm_file_fd);
> + vma->vm_file_fd = -1;
> +}
> +
> +int parse_smaps(pid_t pid, struct vm_area_list *vma_area_list,
> + dump_filemap_t dump_filemap)
> {
> struct vma_area *vma_area = NULL;
> unsigned long start, end, pgoff, prev_end = 0;
> @@ -694,7 +709,16 @@ int parse_smaps(pid_t pid, struct vm_area_list *vma_area_list)
> if (handle_vma(pid, vma_area, str + path_off, map_files_dir,
> &vfi, &prev_vfi, vma_area_list))
> goto err;
> +
> + if (vma_entry_is(vma_area->e, VMA_FILE_PRIVATE) ||
> + vma_entry_is(vma_area->e, VMA_FILE_SHARED)) {
> + if (dump_filemap && dump_filemap(vma_area))
> + goto err;
> + }
> +
> + close_prev_vma(prev_vfi.vma, vma_area->file_borrowed);
This doesn't look correct. What if we have 3 vmas in a row, that shared the vm_file_fd?
You will end up with the fd in question not closed at all.
> }
> + close_prev_vma(prev_vfi.vma, false);
>
> vma_area = NULL;
> ret = 0;
>
More information about the CRIU
mailing list