[CRIU] [PATCH] criu: dump filemap as soon as possible
Andrew Vagin
avagin at virtuozzo.com
Thu May 5 17:58:31 PDT 2016
On Thu, May 05, 2016 at 02:16:45PM +0300, Pavel Emelyanov wrote:
> 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.
Ok. Will fix.
>
> > +{
> > + 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.
I amn't sure that I understand you.
if vma_area->file_borrowed is true, it means that vm_area uses the file
descriptor of prev_vfi, so we don't need to close it. We will close this
fd as soon as we meet vma_area with another file descriptor.
>
> > }
> > + close_prev_vma(prev_vfi.vma, false);
> >
> > vma_area = NULL;
> > ret = 0;
> >
>
More information about the CRIU
mailing list