[CRIU] [PATCH v2 3/3] aio: Restore aio ring content

[Pavel Emelyanov]

On 03/22/2016 12:25 PM, Kirill Tkhai wrote:
> 
> 
> On 21.03.2016 16:01, Pavel Emelyanov wrote:
>> On 03/21/2016 03:56 PM, Kirill Tkhai wrote:
>>>
>>>
>>> On 21.03.2016 15:42, Pavel Emelyanov wrote:
>>>> On 03/21/2016 03:22 PM, Kirill Tkhai wrote:
>>>>>
>>>>>
>>>>> On 21.03.2016 15:10, Pavel Emelyanov wrote:
>>>>>> On 03/21/2016 03:02 PM, Kirill Tkhai wrote:
>>>>>>>
>>>>>>>
>>>>>>> On 21.03.2016 13:15, Pavel Emelyanov wrote:
>>>>>>>> On 03/21/2016 01:10 PM, Kirill Tkhai wrote:
>>>>>>>>
>>>>>>>>>>> I'm going to add plugin to parasite_check_aios(), and to wait inflight requests
>>>>>>>>>>> from there.
>>>>>>>>>>>  
>>>>>>>>>>>> Anyway, I don't think treating aio ring buffer as regular anonymous memory
>>>>>>>>>>>> is good idea.
>>>>>>>>>>>
>>>>>>>>>>> What do you suggest? Add vma_entry_is_private(entry) | vma_entry_is(entry, VMA_AREA_AIORING)
>>>>>>>>>>> every place we used it?
>>>>>>>>>>
>>>>>>>>>> Not every. My current opinion is that soft-dirty tracking should NOT
>>>>>>>>>> be done for AIO rings.
>>>>>>>>>
>>>>>>>>> So, if we skip AIO on pre-dump, but we will dump it like any other private memory,
>>>>>>>>> is this OK for you?
>>>>>>>>
>>>>>>>> I tend to think it's not, since these are objects with structure. However
>>>>>>>> I don't have string arguments against dumping them as raw blobs _provided_
>>>>>>>> we check for ring head magic beforehand.
>>>>>>>
>>>>>>> I don't completely understand you. What is dumping as raw blobs, but not as like memory?
>>>>>>
>>>>>> By "as raw blobs" I mean just read the contents of the memory byte-by-byte w/o
>>>>>> understanding its structure.
>>>>>>
>>>>>>> Checking of ring head magic make a sense at a restore, but which way should it be used
>>>>>>> at dump?
>>>>>>
>>>>>> I'd say the same -- check magic and go ahead and read ring memory contents starting
>>>>>> from the events array. Reading the header as memory doesn't make sense from my pov.
>>>>>
>>>>> Since header shares page[0] with events, if we do not dump it, this saves 32 byte only.
>>>>>
>>>>> So, anyway, we dump aio ring content in a file separate from private memory? (I'm just
>>>>> clarifying before I started rework).
>>>>
>>>> Good question. If we parsed the contents of it, we'd have to save it into separate
>>>> image file. But we treat it as 'memory contents'... Let's keep it in pages.img.
>>>
>>> Ok, but this case again it's need to choose it's need EITHER to modify vma_entry_is_private(),
>>> like I did, and add a check to ignore AIO at pre-dump,
>>> OR add to add
>>> 	vma_entry_is_private(entry) | vma_entry_is(entry, VMA_AREA_AIORING)
>>> where it's need.
>>>
>>> Do I understand right, we choose the second variant?
>>
>> Where it's appropriate -- yes.
> 
> I've tried. It looks awful. We have many functions with "_private_" prefixes,
> where vma_entry_is_private() and vma_area_is_private are used. It will be necessary
> to rename all of them. This introduces ugly very ugly names.
> 
> Maybe we choose the first variant?

OK, but there should be clear exclusion of rings from memory changes tracker code.

-- Pavel