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

[Adrian Reber]

On Tue, Mar 15, 2016 at 01:33:56AM +0300, Pavel Emelyanov wrote:
> On 03/15/2016 12:02 AM, Adrian Reber wrote:
> > On Mon, Mar 14, 2016 at 11:42:11AM +0300, Pavel Emelyanov wrote:
> >> On 03/12/2016 07:54 PM, Adrian Reber wrote:
> >>> On Fri, Mar 11, 2016 at 06:25:52PM +0300, Pavel Emelyanov wrote:
> >>>> On 03/11/2016 06:03 PM, Adrian Reber wrote:
> >>>>> On Fri, Mar 11, 2016 at 04:08:10PM +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.
> >>>>>
> >>>>> I had a quick look, but need to look a bit in more detail.
> >>>>>
> >>>>> If I leave away all those lines I get a segfault, I haven't checked yet
> >>>>> but I think when accessing root_item->pid.virt.
> >>>>
> >>>> Ah! Indeed. You open the page read for init task only. I believe the proper
> >>>> fix would be to pass the pid of the process via socket you use to pass uffd.
> >>>>
> >>>> Since we'll have to do it anyway in the future, I think this is worth doing
> >>>> from the very beginning. And the lazy pages daemon should accept only one
> >>>> such message (for you initial case).
> >>>
> >>> Getting the information about which directory contains the checkpoint
> >>> from the main restore process via the same mechanism as the userfaultfd
> >>> FD was also my initial plan. But, unfortunately, the lazy-pages server
> >>> needs to open the checkpoint directory on its own. Especially as I am
> >>> currently working on the code for remote lazy-restore.
> >>
> >> No no no, I'm not talking about passing the directory with images via socket,
> >> but about finding out the PID of the task to work on. You get this value (the
> >> pid) from root_item, but this value should go via socket as a raw integer,
> >> together with the uffd descriptor.
> >>
> >> This line from patch #3, uffd.c file, uffd_listen() function:
> >>
> >>> +	rc = open_page_read(root_item->pid.virt, &pr, PR_TASK);
> >>
> >> there should not be any root_item-> dereferences, instead, the value of pid.virt
> >> should be sent by the criu restore here:
> > 
> > Ah, okay. I understand now. If I do not use root_item->pid.virt to get
> > the PID but if I get it from somewhere else (hardcoded for a quick
> > test). My code still behaves the same (segfaults, doesn't work, strange
> > error message) as before if I do not call the functions we are
> > discussing. So I do not need it to get the PID but looping over the VMAs
> > of the checkpoint specified with -D just doesn't work.
> > 
> > In my test case, I get for example this output
> > 
> > (02.306731) Opened page read 1 (parent 0)
> > (02.306733) lazy-pages: iov.iov_base 0x400000 (1 pages)
> > (02.306734) lazy-pages: iov.iov_base 0x600000 (2 pages)
> > (02.306736) lazy-pages: iov.iov_base 0x1ead000 (1 pages)
> > (02.306737) lazy-pages: iov.iov_base 0x7fbdaac63000 (8 pages)
> > (02.306738) lazy-pages: iov.iov_base 0x7fbdaac6c000 (1 pages)
> > (02.306739) lazy-pages: iov.iov_base 0x7fbdaae83000 (3 pages)
> > (02.306741) lazy-pages: iov.iov_base 0x7fbdaae8d000 (5 pages)
> > (02.306742) lazy-pages: iov.iov_base 0x7ffc33a55000 (2 pages)
> > (02.306743) lazy-pages: iov.iov_base 0x7ffc33a58000 (1 pages)
> > (02.306744) lazy-pages: iov.iov_base 0x7ffc33a5d000 (2 pages)
> > (02.306747) lazy-pages: Found 0 pages to be handled by UFFD
> > 
> > and not single page has been detected as MAP_ANONYMOUS and MAP_PRIVATE.
> 
> But do you really need to scan vmas in lazy pages daemon? Why not just
> respond with ANY vaddr request on the uffd. For this you'd only need
> the page-read engine.
> 
> I mean the collect_uffd_pages code's list_for_each_entry(vma, &vmas->h, list)
> loop.

I need to know which pages do exist to be able to push unrequested pages
at some point into the restored process. To be able to stop the
lazy-pages daemon at some point because the process is happily running I
need to transfer pages, which have not yet been requested, into the
process. Some restored process might not touch certain pages maybe for a
long time (maybe hours or longer) and it would be good to have the
possibility to be able to end the UFFD daemon at some point. Therefore I
have to scan all existing pages if they are UFFD eligible and I have to
track if they have been requested. If they have not been requested after
some time I have to push the pages into the restored process to be able
to finish the UFFD daemon.

		Adrian