[CRIU] [PATCH 1/5] page-server: Introduce page_server_start_sync_read

[Pavel Emelyanov]

On 06/14/2017 09:52 PM, Pavel Emelyanov wrote:
> On 06/14/2017 02:51 PM, Mike Rapoport wrote:
>> On Wed, Jun 14, 2017 at 01:14:28PM +0300, Pavel Emelyanov wrote:
>>> There's no need in two API calls to read xfer header
>>> and pages themselves, so merge them into one single
>>> call.
>>>
>>> Signed-off-by: Pavel Emelyanov <xemul at virtuozzo.com>
>>> ---
>>>  criu/include/page-xfer.h |  3 +--
>>>  criu/page-xfer.c         | 22 ++++++++++++++++++++--
>>>  criu/pagemap.c           | 14 ++------------
>>>  3 files changed, 23 insertions(+), 16 deletions(-)
>>>
>>> diff --git a/criu/include/page-xfer.h b/criu/include/page-xfer.h
>>> index 0b92ec9..5d8dc81 100644
>>> --- a/criu/include/page-xfer.h
>>> +++ b/criu/include/page-xfer.h
>>> @@ -60,11 +60,10 @@ extern int check_parent_page_xfer(int fd_type, long id);
>>>  
>>>  /* async request/receive of remote pages */
>>>  extern int request_remote_pages(int pid, unsigned long addr, int nr_pages);
>>> -extern int receive_remote_pages_info(int *nr_pages, unsigned long *addr, int *pid);
>>> -extern int receive_remote_pages(int len, void *buf);
>>>  
>>>  typedef int (*ps_async_read_complete)(int pid, unsigned long vaddr, int nr_pages, void *);
>>>  extern int page_server_start_async_read(void *buf, int nr_pages,
>>>  		ps_async_read_complete complete, void *priv);
>>> +extern int page_server_start_sync_read(void *buf, int nr_pages, unsigned long *vaddr);
>>>  
>>>  #endif /* __CR_PAGE_XFER__H__ */
>>> diff --git a/criu/page-xfer.c b/criu/page-xfer.c
>>> index c557407..761985a 100644
>>> --- a/criu/page-xfer.c
>>> +++ b/criu/page-xfer.c
>>> @@ -1174,7 +1174,7 @@ int request_remote_pages(int pid, unsigned long addr, int nr_pages)
>>>  	return 0;
>>>  }
>>>  
>>> -int receive_remote_pages_info(int *nr_pages, unsigned long *addr, int *pid)
>>> +static int receive_remote_pages_info(int *nr_pages, unsigned long *addr, int *pid)
>>>  {
>>>  	struct page_server_iov pi;
>>>  
>>> @@ -1194,7 +1194,7 @@ int receive_remote_pages_info(int *nr_pages, unsigned long *addr, int *pid)
>>>  	return 0;
>>>  }
>>>  
>>> -int receive_remote_pages(int len, void *buf)
>>> +static int receive_remote_pages(int len, void *buf)
>>>  {
>>>  	if (recv(page_server_sk, buf, len, MSG_WAITALL) != len) {
>>>  		pr_perror("Failed to receive page data");
>>> @@ -1203,3 +1203,21 @@ int receive_remote_pages(int len, void *buf)
>>>  
>>>  	return 0;
>>>  }
>>> +
>>> +int page_server_start_sync_read(void *buf, int nr, unsigned long *vaddr)
>>> +{
>>> +	int ret, pid, new_nr;
>>> +
>>> +	/*
>>> +	 * Note, that for async remote page_read, the actual
>>> +	 * transfer happens in the lazy-pages daemon
>>> +	 */
>>> +	ret = receive_remote_pages_info(&new_nr, vaddr, &pid);
>>> +	if (ret == 0) {
>>> +		if (new_nr < 0 || new_nr > nr)
>>> +			return -1;
>>
>> To be completely on the safe side we should also take care of the case when
>> new_nr < nr. 
> 
> True, but isn't it the case for async reader too? I thought it was and planned
> to add the respective checks in the next series.

And BTW, this is just the code-move :) These checks are currently there, so
I should change them in separate patch.

>> With current implementation of remote lazy-pages this should
>> not happen because both dump and restore sides see identical iovecs, but we
>> may extend usage of read_page_remote, e.g. for the image-less case
>> (--remote). I think adding
>>
>> nr = new_nr;
>>
>> here will do the right thing.
>>
>>> +		ret = receive_remote_pages(nr * PAGE_SIZE, buf);
>>> +	}
>>> +
>>> +	return ret;
>>> +}
>>> diff --git a/criu/pagemap.c b/criu/pagemap.c
>>> index d770e81..06264a1 100644
>>> --- a/criu/pagemap.c
>>> +++ b/criu/pagemap.c
>>> @@ -466,7 +466,7 @@ static int read_page_complete(int pid, unsigned long vaddr, int nr_pages, void *
>>>  static int maybe_read_page_remote(struct page_read *pr, unsigned long vaddr,
>>>  				  int nr, void *buf, unsigned flags)
>>>  {
>>> -	int ret, pid, new_nr;
>>> +	int ret;
>>>  
>>>  	/* We always do PR_ASAP mode here (FIXME?) */
>>>  	ret = request_remote_pages(pr->pid, vaddr, nr);
>>> @@ -475,17 +475,7 @@ static int maybe_read_page_remote(struct page_read *pr, unsigned long vaddr,
>>>  	if (flags & PR_ASYNC)
>>>  		return page_server_start_async_read(buf, nr, read_page_complete, pr);
>>>  
>>> -	/*
>>> -	 * Note, that for async remote page_read, the actual
>>> -	 * transfer happens in the lazy-pages daemon
>>> -	 */
>>> -	ret = receive_remote_pages_info(&new_nr, &vaddr, &pid);
>>> -	if (ret == 0) {
>>> -		if (new_nr < 0 || new_nr > nr)
>>> -			return -1;
>>> -		ret = receive_remote_pages(nr * PAGE_SIZE, buf);
>>> -	}
>>> -
>>> +	ret = page_server_start_sync_read(buf, nr, &vaddr);
>>>  	if (ret == 0 && pr->io_complete)
>>>  		ret = pr->io_complete(pr, vaddr, nr);
>>>  
>>> -- 
>>> 2.1.4
>>>
>>> _______________________________________________
>>> CRIU mailing list
>>> CRIU at openvz.org
>>> https://lists.openvz.org/mailman/listinfo/criu
>> .
>>
>