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

[Pavel Emelyanov]

On 01/30/2014 03:20 PM, Andrew Vagin wrote:
> 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.

It also ruins the pre-dump functionality ;)

> I'm going to resend this series. Thank you for the comments.
> 
>>
>> Thanks,
>> Pavel
> .
>