[CRIU] [PATCH] vmsplice user-buffer(grabbed by process_vm_readv) to page-pipe

[abhishek dubey]

Hi Andrei,

On 12/07/19 3:01 AM, abhishek dubey wrote:
> Hi Andrei,
>
> On 09/07/19 9:38 PM, Andrei Vagin wrote:
>> On Tue, Jul 09, 2019 at 01:50:57AM +0530, abhishek dubey wrote:
>>> Hi Andrei,
>>>
>>> Please find inline replies.
>>>
>>> On 08/07/19 9:35 AM, Andrei Vagin wrote:
>>>> On Fri, Jul 05, 2019 at 05:34:26PM +0530, Abhishek Dubey wrote:
>>>>> This patch implements usage of process_vm_readv
>>>>> syscall to collect memory pages from target process during
>>>>> pre-dump. process_vm_readv collects pages in user-buffer,
>>>>> which is later vmspliced to page-pipes.
>>>> You have to be more detailed in the commit message.
>>>>
>>>> Frist of all, you have to explain why we need this patch.
>>> I will take care of same from next time.
>>>> If you improve performance, you need to give some numbers.
>>> Still I am handling issue of "missing entry in parent pagemap". Next 
>>> patch
>>> will have comparative figures included.
>>>> If you implement a new feature, you need to provide tests for it.
>>>>
>>>> If I understand this patch right, criu freezes all processes, collects
>>>> page maps, unfreezes process, dump pages? If this is right,
>>> Yes, this is right.
>>>> you need to
>>>> explain how it handles in this case:
>>>>
>>>> addr = map(NULL, 4 * PAGE_SIZE)    |
>>>> addr[0] = 1            |
>>>> addr[PAGE_SIZE]  = 2        |
>>>> addr[2 * PAGE_SIZE] = 3        |
>>>> addr[3 * PAGE_SIZE] = 4        |
>>>>                 | criu freezes the process
>>>>                 | criu collects page maps
>>>>                 | criu unfreezes the process
>>>> unmap(addr, PAGE_SIZE * 2)    |
>>>>                 | criu dumps pages
>>> Need to explain this example somewhere along with code(as in 
>>> page-pipe.h) or
>>> some other place?
>> here :)
>>
>> I think this can be non-trivial case. process_vm_readv will return
>> ENOMEM for this segment, but a part of it still exists and we need to
>> find what part is here and dump it.
>
> Pagemap cache won't be useful here.
>
> For now, I can only think of setting a threshold. If size of mapping 
> is below threshold, we can process page-by-page(which is costly).
>
> Beyond threshold, we can leave such mapping to be handled by dump stage.
>
>
Just Ignore following query. I thought there might be other regions like 
regions lacking PROT_READ, but my issue was due to implementation error.

Got resolved.

> Implementation query:
>
> During "pre-dump", some of the iovs in iovec, fail to be processed by 
> process_vm_readv(). Hence pagemap entry is skipped for same.
>
> Now, memory belonging to these failed iovs is not dirtied. During 
> "dump" on generating iovs, such non-dirty memory areas are marked as
>
> holes and check for pages(in parent) backing these holes is done in 
> page-xfer. Ultimately, holes are pointing to pages that are not 
> pre-dumped.
>
> This check must be shifted to "dump stage", so that pages(iovs) 
> skipped by process_vm_readv can't be marked as holes in incremental 
> steps.
>
> Can I add new data structure to list down all the iovs failed by 
> process_vm_readv. This list will be checked while generating holes in 
> dump stage.
>
> is that a right approach?
>
>
> Please suggest if something more efficient or direct solution is 
> possible.
>
>
> -Abhishek
>
-Abhishek