[CRIU] Re: [PATCH 10/10] files: Use sys_kcmp to find file descriptor duplicates

[Pavel Emelyanov]

> +void fd_id_entry_init(void)
> +{
> +       /* Has nothing to do yet */
> +}
> +
> +void fd_id_entry_fini(void)
> +{
> +       /* Clear fd_id_entries */
> +       fd_id_entries_salt = 1;
> +}

These two are useless. Plz, remove.

> +static struct fd_id_entry *
> +lookup_alloc(u64 id, pid_t pid, int fd, bool do_alloc)
> +{
> +       struct rb_node *node = fd_id_root.rb_node;
> +       struct fd_id_entry *e = NULL;
> +
> +       pr_debug("lookup-fd:\tid %16lx pid %6d fd %4d\n", id, pid, fd);
> +       print_fd_id_tree();
> +
> +       while (node) {
> +               struct fd_id_entry *this = rb_entry(node, struct fd_id_entry, node);
> +
> +               pr_debug("lookup-fd:\t\tid %16lx (%6d) -- %16lx (%6d)\n",
> +                        id, pid, this->id, this->pid);
> +
> +               if (id < this->id)
> +                       node = node->rb_left;
> +               else if (id > this->id)
> +                       node = node->rb_right;
> +               else
> +                       return lookup_alloc_subtree(this, id, pid, fd, do_alloc);
> +       }
> +
> +       if (do_alloc) {
> +               struct rb_node **new = &(fd_id_root.rb_node);
> +               struct rb_node *parent = NULL;
> +
> +               e = alloc_fd_id_entry(id, pid, fd);
> +               if (!e)
> +                       goto err;
> +
> +               while (*new) {
> +                       struct fd_id_entry *this = rb_entry(*new, struct fd_id_entry, node);
> +
> +                       pr_debug("lookup-fd:\t\tid %16lx (%6d) -- %16lx (%6d)\n",
> +                                id, pid, this->id, this->pid);
> +
> +                       parent = *new;
> +                       if (id < this->id)
> +                               new = &((*new)->rb_left);
> +                       else if (id > this->id)
> +                               new = &((*new)->rb_right);
> +                       else
> +                               BUG_ON(1);

Double walk -- plz, remove.

> +                       /* Must no be dups here */
> +               }
> +
> +               pr_debug("insert-fd:\tid %16lx pid %6d fd %4d salt %16lx\n",
> +                        e->id, e->pid, e->fd, e->salt);
> +
> +               rb_link_node(&e->node, parent, new);
> +               rb_insert_color(&e->node, &fd_id_root);
> +
> +               print_fd_id_tree();
> +       }
> +
> +err:
> +       return e;
> +}
> +
> +struct fd_id_entry *fd_id_entry_collect(u64 id, pid_t pid, int fd)
> +{
> +       pr_debug("collect-fd:\tid %16lx pid %6d fd %4d\n", id, pid, fd);
> +       return lookup_alloc(id, pid, fd, true);

Plz remove always true argument.

> +}
> +#define GEN_FD_ID(dev, ino, pos)       \
> +       ((u64)(dev) ^ (u64)(ino) ^ (u64)(pos))

u32 is enough for this, no?

> +extern void fd_id_entry_init(void);
> diff --git a/include/image.h b/include/image.h
> index c9b7209..730b84b 100644
> --- a/include/image.h
> +++ b/include/image.h
> @@ -39,15 +39,14 @@
>  #define PAGE_RSS       1
>  #define PAGE_ANON      2
> 
> -#define FD_ID_SIZE     50
> -
>  struct fdinfo_entry {
>         u8      type;
>         u8      len;
>         u16     flags;
>         u32     pos;
>         u64     addr;
> -       u8      id[FD_ID_SIZE];
> +       u64     id;
> +       u64     salt;

Plz, make an ID in image entry be a single field.

>         u8      name[0];
>  } __packed;
> 
> --
> 1.7.7.6
> 
> .
>