[CRIU] [PATCH] Add test case for code page sharing

[Christopher Covington]

Hi Cyrill,

On 04/23/2015 01:22 PM, Pavel Emelyanov wrote:
> On 04/23/2015 08:10 PM, Christopher Covington wrote:
>> On 04/08/2015 09:54 AM, Christopher Covington wrote:
>>> On 04/07/2015 09:27 AM, Pavel Emelyanov wrote:
>>>>
>>>>>>> and restore only changed pages.
>>>>>>>
>>>>>>> static inline bool should_dump_page(VmaEntry *vmae, u64 pme)
>>>>>>> {
>>>>>>> ...
>>>>>>>         /*
>>>>>>>          * Optimisation for private mapping pages, that haven't
>>>>>>>          * yet being COW-ed
>>>>>>>          */
>>>>>>>         if (vma_entry_is(vmae, VMA_FILE_PRIVATE) && (pme & PME_FILE))
>>>>>>>                 return false;
>>>>>>
>>>>>> But that's the dump routine. For COW restore different code is used:
>>>>>
>>>>> No. It's another case. We are talking about private file mappings. If
>>>>> processes don't change content, they access shared pages which are
>>>>> associated with a file. If a process tries to change a page, the kernel
>>>>> allocates a new page and copies the file page into the new page.
>>>>
>>>> Ah, I see. Well, yes, CRIU touches one private executable page to inject
>>>> parasite routine into it. Then this page gets dumped into the image.
>>>>
>>>> Are we talking about a single page here?
>>>
>>> A single page losing sharing because it was ptrace poked is a small concern.
>>> The sharing being maintained for that single page would be "nice to have" but
>>> I don't see it as a high priority.
>>>
>>> The greater concern is that all of the ELF's read-only pages, even those never
>>> poked by ptrace, may be losing sharing. We're going to extend the test case to
>>> span several pages to confirm or disprove our reading of the code. I'm not
>>> sure Andrew's extension of the test to look at getpagesize() disproves the
>>> concern, because that library function is in a separate .so file rather than
>>> part of the main ELF file.
>>
>> We've confirmed that only a single page is affected. Some spot-checking shows
>> that for 4K pages it may not be much of an issue but when using 64K pages it
>> can impact a significant number of memory accesses. I think we'll investigate
>> possible solutions as time allows. 
> 
> Cool :) Thanks for the investigation!
> 
>> Possibilities to explore that come to mind are:
>>
>> A) Don't dump the poked page into the image but leave it as file-backed, if it
>> hasn't been modified since being mapped
> 
> If we read (at least part of) pagemap file before injecting a parasite, this
> should give us correct bits for this particular page.
> 
>> B) Poking the VDSO code page instead of a file-backed code page (was this
>> mentioned recently in another thread?)
> 
> Yup, this is what Oleg Nesterov suggested -- keep the needed code snippets in the
> VDSO page i.e. -- submit a patch to the kernel mailing list. The solution A) can
> be done w/o messing with the kernel :)
> 
>> Right now our test case is multiple processes. Ideally a solution would work
>> for multiple threads as well, but I'm not sure off the top of my head what
>> else might need to be considered--perhaps an additional test case is in order.

I thought I heard mentioned at Plumbers that you might have experimented with
such a change. If you have and it's no trouble, I'd be keen to see those
changes. If I misunderstood or it'd be a lot of work, then don't worry about it.

Thanks,
Christopher Covington

-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project