[CRIU] [PATCH v5 3/5] Try to include userfaultfd with criu (part 1)

[Pavel Emelyanov]

On 03/15/2016 07:22 PM, Adrian Reber wrote:
> On Tue, Mar 15, 2016 at 12:03:07PM +0300, Pavel Emelyanov wrote:
>>>>>>>>>>> +static void criu_init()
>>>>>>>>>>> +{
>>>>>>>>>>> +	/* TODO: return code checking */
>>>>>>>>>>> +	check_img_inventory();
>>>>>>>>>>> +	prepare_task_entries();
>>>>>>>>>>> +	prepare_pstree();
>>>>>>>>>>> +	collect_remaps_and_regfiles();
>>>>>>>>>>> +	prepare_shared_reg_files();
>>>>>>>>>>> +	prepare_remaps();
>>>>>>>>>>> +	prepare_mm_pid(root_item);
>>>>>>>>>>> +
>>>>>>>>>>> +	/* We found a PID */
>>>>>>>>>>> +	pr_debug("root_item->pid.virt %d\n", root_item->pid.virt);
>>>>>>>>>>> +	pr_debug("root_item->pid.real %d\n", root_item->pid.real);
>>>>>>>>>>> +}
>>>>>>>>>>
>>>>>>>>>> This portion should be really resolved before merging. All of the above
>>>>>>>>>> has nothing to do with the page_read, so please, find the reason for
>>>>>>>>>> page read engine non working due to absence of this. If you need help
>>>>>>>>>> with the code, just drop me an e-mail, I'll help.
> [...]
>> The second suggestion is to just open the mm.img file and pull the vma entries
>> into the lazy daemon. There's no such helper in criu code yet, so introducing
>> one would be required.
> 
> I tried to re-implement prepare_mm_pid() (which basically parses the
> mm.img file).What I currently have is
> 
> check_img_inventory();
> prepare_task_entries();
> prepare_pstree();
> collect_remaps_and_regfiles();
> ri = rsti(root_item);
> 
> and then parts of prepare_mm_pid() re-implemented manually to get the
> VMA entries. So most of the code is still there and it seems difficult
> to reduce the amount of common code used. The common code already has
> all the knowledge to correctly handle the files and read the protocol
> buffers with all necessary sanity checks.
> 
> It would probably help me to understand the situation if I knew why this
> common code should not be used. 

Common code can and should be used, but only the one that's really required.

In particular, the call to collect_remaps_and_regfiles() should not be
here as it picks up info about files, not memory.

Collecting pstree looks legit, since lazy pages daemon should know which
processes it serves.

Collecting task_entries doesn't seem relevant, since this thing is used to
synchronize process tree restore and lazy daemon doesn't do it.

Checking the inventory is ... harmless here I would say.

> With the implementation in my latest
> patch I am re-using the existing the CRIU infrastructure to access the
> mm.img correctly as a normal restore would do? It is not clear to me why
> I rather shouldn't use the existing code. Some clarification would be
> nice, thanks.
> 
> 		Adrian
> .
>