[CRIU] [PATCH 1/3] page-pipe: allow to handle pipes in batch mode

[Andrew Vagin]

On Thu, Jan 30, 2014 at 03:07:12PM +0400, Pavel Emelyanov wrote:
> On 01/30/2014 12:21 PM, Andrey Wagin wrote:
> > 2014-01-30 Pavel Emelyanov <xemul at parallels.com>:
> >> On 01/30/2014 12:12 AM, Andrew Vagin wrote:
> >>> On Wed, Jan 29, 2014 at 05:13:23PM +0400, Pavel Emelyanov wrote:
> >>>> On 01/27/2014 06:22 PM, Andrey Vagin wrote:
> >>>>> The problem is that vmsplice() to a big pipe fails very often.
> >>>>>
> >>>>> The kernel allocates the linear chunk of memory for pipe buffer
> >>>>> descriptos, but big allocation in kernel can fail.
> >>>>>
> >>>>> So we need to restrict maximal capacity of pipes. But the number of
> >>>>> pipes is restricted too, so we need to dump memory in a batch mode.
> >>>>
> >>>> Batch mode means "several at once". Do you mean "... using one pipe"?
> >>>
> >>> I mean match mode "several at once" ;).
> >>
> >> Then current mode is batch also -- it pulls several pages into several
> >> pipes and dumps them all at once.
> > 
> > The current mode does one iteration per process. The number of pipes
> > is not limited.
> > 
> > Pavel, pls look at the next patch, it starts to use this code.
> 
> I did. Let's try to follow the code with patches applied.
> 
> __parasite_dump_pages_seized();
>  `- create_page_pipe();
>  |    `- pp = xmalloc();
>  |         `- pp->pipe_nr = INT_MAX;
>  |            /* WTF? pp->nr_pipes initialization is removed */
>  |            page_pipe_grow();

Probably we don't need to call page_pipe_grow() here

>  |              `- pp->nr_pipes++; /* FTW? Why nr_pipes contains a garbage? */
>  |                 open_pipe();
>  |                   `- pp->pipe_nr--; /* For the record. INT_MAX - 1 is now here. */
>  `- page_pipe_start_batching();
>  |   `- pp->pipe_nr = NR_PIPES_PER_BATCH; /* FTF? INT_MAX - 1 doesn't matter any longer?

And check here that we don't create any pipe yet

>  |                                           Any why it's NR_..._BATCH, while there's the whole 1 pipe in there? */
>  `- generate_iovs()
>      `- page_pipe_add_page()
>          `- if (pp->pipe_nr == 0)
>          |    `- pp->batch_cb();
>          |       page_pipe_cleanup();        /* WTF? The pre-dump's pages get all flushed! */
>          |         `- page_pipe_close_buf(); /* WTF? Why do we start from the very beginning, there's
>          |                                      NR_..._BATCH plus 1 pipe in there already. */
>          |
>          `- page_pipe_grow();
>              `- pp->nr_pipes++; /* WTF? This is increase-only counter. */
> 
> I'm afraid that the WTF-per-KLOC value is too high to apply this set.

Don't worry, here is a bug, but even in this case this code is working
more stable than a previous one;). It creates NR_PIPES_PER_BATCH + 1
pipes instead of NR_PIPES_PER_BATCH and nothing more.

I'm going to resend this series. Thank you for the comments.

> 
> Thanks,
> Pavel