[CRIU] [PATCH 1/2] [UGLY PATCH] allow fetching pages from remote page-server

[Pavel Emelyanov]

Adrian, Mike, sorry for such a long delay in review :( Here's my comments, inline.

On 04/26/2016 12:38 PM, Adrian Reber wrote:
> From: Mike Rapoport <rppt at linux.vnet.ibm.com>
> 
> ---
>  criu/cr-restore.c         |   5 +-
>  criu/crtools.c            |   4 ++
>  criu/include/cr_options.h |   1 +
>  criu/include/page-read.h  |   6 ++
>  criu/include/page-xfer.h  |   7 ++
>  criu/page-read.c          |  49 ++++++++++++--
>  criu/page-xfer.c          | 163 +++++++++++++++++++++++++++++++++++++++++++++-
>  7 files changed, 227 insertions(+), 8 deletions(-)
> 
> diff --git a/criu/cr-restore.c b/criu/cr-restore.c
> index 34db7f7..5ae1d28 100644
> --- a/criu/cr-restore.c
> +++ b/criu/cr-restore.c
> @@ -453,10 +453,13 @@ static int restore_priv_vma_content(void)
>  	unsigned int nr_lazy = 0;
>  	unsigned long va;
>  	struct page_read pr;
> +	int pr_flags = PR_TASK;
>  
>  	vma = list_first_entry(vmas, struct vma_area, list);
>  
> -	ret = open_page_read(current->pid.virt, &pr, PR_TASK);
> +	if (opts.use_page_client)
> +		pr_flags |= PR_REMOTE;
> +	ret = open_page_read(current->pid.virt, &pr, pr_flags);

Looking at implementation of PR_REMOTE bits I see that every call to
read_pagemap_page() would block in this case which doesn't fit what
we've discussed so far :) The uffd daemon shouldn't block in page fault
handling.

>  	if (ret <= 0)
>  		return -1;
>  

> @@ -785,3 +938,9 @@ int check_parent_page_xfer(int fd_type, long id)
>  	else
>  		return check_parent_local_xfer(fd_type, id);
>  }
> +
> +int page_xfer_read_pages(struct page_xfer *xfer, unsigned long vaddr,
> +			 int nr, void *buf)

Well, the page_xfer stands for pages transfer and it's not supposed to read any pages :)

Instead, the respective page_read should be created. Now we have legacy page read (for
images we no longer generate, will deprecate) and pagemap reader. So we need to create
the remote page_read reader.

The read_pagemap_page routine can be split in any pieces to facilitate that. E.g. -- the
checks for page-in-parent you seem to need in this patch, can be put into a separate helper.

> +{
> +	return xfer->read_pages ? xfer->read_pages(xfer, vaddr, nr, buf) : -1;
> +}
> 

-- Pavel