[CRIU] [PATCH 1/3] fsnotify: merge inotify wd image into inotify image (v4)

Andrey Wagin avagin at gmail.com
Tue Sep 2 09:12:20 PDT 2014


On Sep 2, 2014 7:59 PM, "Pavel Emelyanov" <xemul at parallels.com> wrote:
>
> On 09/02/2014 06:49 PM, Andrey Vagin wrote:
> > All watch descriptors are collected in a list and then
> > they are written in inotify image as a repeated field.
> >
> > This images merge reduces the amount of image files criu
> > generates and may simplify the fix of mentioned above issue.
> >
> > v2: use free_inotify_wd_entry() instead of xfree in dump_one_inotify()
> > v3: don't leak ie.wd_entry
> > v4: save the original order of watchers
> > Signed-off-by: Andrey Vagin <avagin at openvz.org>
> > ---
> >  fsnotify.c              | 85
+++++++++++++++++++++++++++++++++++++++----------
> >  include/image-desc.h    |  2 +-
> >  protobuf/fsnotify.proto |  1 +
> >  3 files changed, 70 insertions(+), 18 deletions(-)
> >
> > diff --git a/fsnotify.c b/fsnotify.c
> > index c9373c7..dff548c 100644
> > --- a/fsnotify.c
> > +++ b/fsnotify.c
> > @@ -197,40 +197,68 @@ err:
> >       return -1;
> >  }
> >
> > +struct watch_list {
> > +     struct list_head list;
> > +     int n;
> > +};
> > +
> >  static int dump_inotify_entry(union fdinfo_entries *e, void *arg)
> >  {
> > -     InotifyWdEntry *we = &e->ify.e;
> > -     int ret = -1;
> > +     struct watch_list *wd_list = (struct watch_list *) arg;
> > +     struct inotify_wd_entry *wd_entry = (struct inotify_wd_entry *)
e;;
> > +     InotifyWdEntry *we = &wd_entry->e;
> >
> > -     we->id = *(u32 *)arg;
> >       pr_info("wd: wd 0x%08x s_dev 0x%08x i_ino 0x%16"PRIx64" mask
0x%08x\n",
> >                       we->wd, we->s_dev, we->i_ino, we->mask);
> >       pr_info("\t[fhandle] bytes 0x%08x type 0x%08x __handle
0x%016"PRIx64":0x%016"PRIx64"\n",
> >                       we->f_handle->bytes, we->f_handle->type,
> >                       we->f_handle->handle[0], we->f_handle->handle[1]);
> >
> > -     if (check_open_handle(we->s_dev, we->i_ino, we->f_handle))
> > -             goto out;
> > +     if (check_open_handle(we->s_dev, we->i_ino, we->f_handle)) {
> > +             free_inotify_wd_entry(e);
> > +             return -1;
> > +     }
> >
> > -     ret = pb_write_one(fdset_fd(glob_fdset, CR_FD_INOTIFY_WD), we,
PB_INOTIFY_WD);
> > -out:
> > -     free_inotify_wd_entry(e);
> > -     return ret;
> > +     list_add_tail(&wd_entry->node, &wd_list->list);
> > +     wd_list->n++;
> > +
> > +     return 0;
> >  }
> >
> >  static int dump_one_inotify(int lfd, u32 id, const struct fd_parms *p)
> >  {
> > +     struct watch_list wd_list = {LIST_HEAD_INIT(wd_list.list), 0};
> >       InotifyFileEntry ie = INOTIFY_FILE_ENTRY__INIT;
> > +     union fdinfo_entries *we, *tmp;
> > +     int exit_code = -1, i;
> >
> >       ie.id = id;
> >       ie.flags = p->flags;
> >       ie.fown = (FownEntry *)&p->fown;
> >
> > +     if (parse_fdinfo(lfd, FD_TYPES__INOTIFY, dump_inotify_entry,
&wd_list))
> > +             goto free;
> > +
> > +     ie.wd = xmalloc(sizeof(*ie.wd) * wd_list.n);

wd_list.n is used here

> > +     if (!ie.wd)
> > +             goto free;
> > +
> > +     i = 0;
> > +     list_for_each_entry(we, &wd_list.list, ify.node)
> > +             ie.wd[i++] = &we->ify.e;
> > +     ie.n_wd = wd_list.n;
>
> Yet again. Why do we need this wd_list.n? The i variable has
> the number we need at that place.

It is used for allocating the ie.wd array

>
> > +
> >       pr_info("id 0x%08x flags 0x%08x\n", ie.id, ie.flags);
> >       if (pb_write_one(fdset_fd(glob_fdset, CR_FD_INOTIFY_FILE), &ie,
PB_INOTIFY_FILE))
> > -             return -1;
> > +             goto free;
> >
> > -     return parse_fdinfo(lfd, FD_TYPES__INOTIFY, dump_inotify_entry,
&id);
> > +     exit_code = 0;
> > +free:
> > +     xfree(ie.wd);
> > +     list_for_each_entry_safe(we, tmp, &wd_list.list, ify.node)
> > +             free_inotify_wd_entry(we);
> > +
> > +     return exit_code;
> >  }
> >
> >  static int pre_dump_inotify_entry(union fdinfo_entries *e, void *arg)
> > @@ -613,15 +641,10 @@ static struct fsnotify_file_info
*find_inotify_info(unsigned id)
> >       return NULL;
> >  }
> >
> > -static int collect_inotify_mark(struct fsnotify_mark_info *mark)
> > +static int __collect_inotify_mark(struct fsnotify_file_info *p, struct
fsnotify_mark_info *mark)
> >  {
> > -     struct fsnotify_file_info *p;
> >       struct fsnotify_mark_info *m;
> >
> > -     p = find_inotify_info(mark->iwe->id);
> > -     if (!p)
> > -             return -1;
> > -
> >       /*
> >        * We should put marks in wd ascending order. See comment
> >        * in restore_one_inotify() for explanation.
> > @@ -635,6 +658,17 @@ static int collect_inotify_mark(struct
fsnotify_mark_info *mark)
> >       return 0;
> >  }
> >
> > +static int collect_inotify_mark(struct fsnotify_mark_info *mark)
> > +{
> > +     struct fsnotify_file_info *p;
> > +
> > +     p = find_inotify_info(mark->iwe->id);
> > +     if (!p)
> > +             return -1;
> > +
> > +     return __collect_inotify_mark(p, mark);
> > +}
> > +
> >  static int collect_fanotify_mark(struct fsnotify_mark_info *mark)
> >  {
> >       struct fsnotify_file_info *p;
> > @@ -656,11 +690,28 @@ static int collect_fanotify_mark(struct
fsnotify_mark_info *mark)
> >  static int collect_one_inotify(void *o, ProtobufCMessage *msg)
> >  {
> >       struct fsnotify_file_info *info = o;
> > +     int i;
> >
> >       info->ife = pb_msg(msg, InotifyFileEntry);
> >       INIT_LIST_HEAD(&info->marks);
> >       list_add(&info->list, &inotify_info_head);
> >       pr_info("Collected id 0x%08x flags 0x%08x\n", info->ife->id,
info->ife->flags);
> > +
> > +     for (i = 0; i < info->ife->n_wd; i++) {
> > +             struct fsnotify_mark_info *mark;
> > +
> > +             mark = xmalloc(sizeof(*mark));
> > +             if (!mark)
> > +                     return -1;
> > +
> > +             mark->iwe = info->ife->wd[i];
> > +             INIT_LIST_HEAD(&mark->list);
> > +             mark->remap = NULL;
> > +
> > +             if (__collect_inotify_mark(info, mark))
> > +                     return -1;
> > +     }
> > +
> >       return file_desc_add(&info->d, info->ife->id, &inotify_desc_ops);
> >  }
> >
> > diff --git a/include/image-desc.h b/include/image-desc.h
> > index ab592a1..75e851c 100644
> > --- a/include/image-desc.h
> > +++ b/include/image-desc.h
> > @@ -68,7 +68,6 @@ enum {
> >       CR_FD_EVENTPOLL_TFD,
> >       CR_FD_SIGNALFD,
> >       CR_FD_INOTIFY_FILE,
> > -     CR_FD_INOTIFY_WD,
> >       CR_FD_FANOTIFY_FILE,
> >       CR_FD_FANOTIFY_MARK,
> >       CR_FD_TUNFILE,
> > @@ -93,6 +92,7 @@ enum {
> >
> >       CR_FD_SIGNAL,
> >       CR_FD_PSIGNAL,
> > +     CR_FD_INOTIFY_WD,
> >
> >       CR_FD_MAX
> >  };
> > diff --git a/protobuf/fsnotify.proto b/protobuf/fsnotify.proto
> > index 073fa79..b3fc080 100644
> > --- a/protobuf/fsnotify.proto
> > +++ b/protobuf/fsnotify.proto
> > @@ -15,6 +15,7 @@ message inotify_file_entry {
> >       required uint32         id              = 1;
> >       required uint32         flags           = 2;
> >       required fown_entry     fown            = 4;
> > +     repeated inotify_wd_entry wd            = 5;
> >  }
> >
> >  enum mark_type {
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openvz.org/pipermail/criu/attachments/20140902/b7bc03b4/attachment.html>


More information about the CRIU mailing list