[CRIU] [PATCH 2/2] files: Fix breaking of fds list ordering by splice

[Kirill Tkhai]

Hi, Pavel,

On Fri, Jun 30, 2017 at 22:56, Pavel Begunkov wrote:
> Kirill, could you pls take a look?
> 
> The basic idea is to preserve the order of the final list. The
> 'completed' list is split into 2 separate lists by #1a945c9, but the
> final splicing may break the order. As a simplest solution, I just
> returned it to the single list version.
> Should they really be explicitly separated? Then there should be some
> actual merge. Or maybe it's already sorted?

*completed* list is not ordered, because we add fles there
after they are restored. But they restore asynchronous
and it's impossible to guess, which time the certain fle will
be completed:

if you have 3 fles in the list hashed in the order: A, B, C;
you may have B restored first, then A, and then C.

So, if we kill fake list, fles still won't be ordered. And
I don't think we should have them be so, as we do not
use fles after the function is finished.

> On Fri, Jun 30, 2017 at 10:29 PM, Pavel Begunkov <asml.silence at gmail.com> wrote:
> > fds lists should be sorted in ascending order, splicing of 2 (ordered)
> > lists violates this invariant.
> >
> > Signed-off-by: Pavel Begunkov <asml.silence at gmail.com>
> > ---
> >  criu/files.c | 14 +++++---------
> >  1 file changed, 5 insertions(+), 9 deletions(-)
> >
> > diff --git a/criu/files.c b/criu/files.c
> > index 4a263c438..a0df8f531 100644
> > --- a/criu/files.c
> > +++ b/criu/files.c
> > @@ -1161,12 +1161,13 @@ static int receive_fd(struct fdinfo_list_entry *fle)
> >         return 0;
> >  }
> >
> > -static void close_fdinfos(struct list_head *list)
> > +static void close_fake_fdinfos(struct list_head *list)
> >  {
> >         struct fdinfo_list_entry *fle;
> >
> >         list_for_each_entry(fle, list, ps_list)
> > -               close(fle->fe->fd);
> > +               if (fle->fake)
> > +                       close(fle->fe->fd);
> >  }
> >
> >  static int open_fdinfos(struct pstree_item *me)
> > @@ -1174,7 +1175,6 @@ static int open_fdinfos(struct pstree_item *me)
> >         struct list_head *list = &rsti(me)->fds;
> >         struct fdinfo_list_entry *fle, *tmp;
> >         LIST_HEAD(completed);
> > -       LIST_HEAD(fake);
> >         bool progress, again;
> >         int st, ret = 0;
> >
> > @@ -1197,10 +1197,7 @@ static int open_fdinfos(struct pstree_item *me)
> >                                  * and reduce number of fles in their checks.
> >                                  */
> >                                 list_del(&fle->ps_list);
> > -                               if (!fle->fake)
> > -                                       list_add_tail(&fle->ps_list, &completed);
> > -                               else
> > -                                       list_add_tail(&fle->ps_list, &fake);
> > +                               list_add_tail(&fle->ps_list, &completed);
> >                         }
> >                         if (ret == 1)
> >                                again = true;
> > @@ -1214,9 +1211,8 @@ static int open_fdinfos(struct pstree_item *me)
> >          * Fake fles may be used for restore other
> >          * file types, so their closing is delayed.
> >          */
> > -       close_fdinfos(&fake);
> > +       close_fake_fdinfos(&completed);
> >  splice:
> > -       list_splice(&fake, list);
> >         list_splice(&completed, list);
> >
> >         return ret;
> > --
> > 2.11.1
> >