[CRIU] [RFC PATCH 05/12] lazy-pages: track outstanding page faults

[Pavel Emelyanov]

On 01/10/2017 05:58 PM, Mike Rapoport wrote:
> On Tue, Jan 10, 2017 at 04:58:53PM +0300, Pavel Emelyanov wrote:
>> On 01/09/2017 11:23 AM, Mike Rapoport wrote:
>>> Multithreaded applications may concurrently generate page faults at the
>>> same address, which will cause creation of multiple requests for remote
>>> page, and, consequently, confuse the page server on the dump side.
>>> We can keep track on page fault requests in flight and ensure this way that
>>> we request a page from the remote side only once.
>>>
>>> Signed-off-by: Mike Rapoport <rppt at linux.vnet.ibm.com>
>>> ---
>>>  criu/uffd.c | 31 +++++++++++++++++++++++++++++++
>>>  1 file changed, 31 insertions(+)
>>>
>>> diff --git a/criu/uffd.c b/criu/uffd.c
>>> index 8c357b1..a5b990e 100644
>>> --- a/criu/uffd.c
>>> +++ b/criu/uffd.c
>>> @@ -55,10 +55,16 @@ struct lazy_iovec {
>>>  
>>>  struct lazy_pages_info;
>>>  
>>> +struct pf_info {
>>> +	unsigned long addr;
>>> +	struct list_head l;
>>> +};
>>> +
>>>  struct lazy_pages_info {
>>>  	int pid;
>>>  
>>>  	struct list_head iovs;
>>> +	struct list_head pfs;
>>>  
>>>  	struct page_read pr;
>>>  
>>> @@ -85,6 +91,7 @@ static struct lazy_pages_info *lpi_init(void)
>>>  
>>>  	memset(lpi, 0, sizeof(*lpi));
>>>  	INIT_LIST_HEAD(&lpi->iovs);
>>> +	INIT_LIST_HEAD(&lpi->pfs);
>>>  	INIT_LIST_HEAD(&lpi->l);
>>>  	lpi->lpfd.revent = handle_uffd_event;
>>>  
>>> @@ -537,9 +544,19 @@ static int uffd_copy(struct lazy_pages_info *lpi, __u64 address, int nr_pages)
>>>  
>>>  static int complete_page_fault(struct lazy_pages_info *lpi, unsigned long vaddr, int nr)
>>>  {
>>> +	struct pf_info *pf;
>>> +
>>>  	if (uffd_copy(lpi, vaddr, nr))
>>>  		return -1;
>>>  
>>> +	list_for_each_entry(pf, &lpi->pfs, l) {
>>> +		if (pf->addr == vaddr) {
>>> +			list_del(&pf->l);
>>> +			xfree(pf);
>>> +			break;
>>> +		}
>>> +	}
>>> +
>>>  	return update_lazy_iovecs(lpi, vaddr, nr * PAGE_SIZE);
>>>  }
>>>  
>>> @@ -635,6 +652,7 @@ static int handle_remaining_pages(struct lazy_pages_info *lpi)
>>>  
>>>  static int handle_page_fault(struct lazy_pages_info *lpi, struct uffd_msg *msg)
>>>  {
>>> +	struct pf_info *pf;
>>>  	__u64 address;
>>>  	int ret;
>>>  
>>> @@ -657,6 +675,16 @@ static int handle_page_fault(struct lazy_pages_info *lpi, struct uffd_msg *msg)
>>>  	}
>>>  #endif
>>>  
>>> +	list_for_each_entry(pf, &lpi->pfs, l)
>>> +		if (pf->addr == address)
>>> +			return 0;
>>> +
>>> +	pf = xzalloc(sizeof(*pf));
>>> +	if (!pf)
>>> +		return -1;
>>> +	pf->addr = address;
>>> +	list_add(&pf->l, &lpi->pfs);
>>
>> Shouldn't we instead __move__ the iovec from iovs list into pfs one? Splited
>> if being too big.
> 
> Maybe we should... I'm not sure yet, how we would handle remote page faults
> in forking environment. It might change quite a few things...
> 
>> This will allow us to make good asynchronous handling of
>> what's currently called "remaining pages" :)
> 
> Asynchronous meaning pthread_create? ;-)

Nope :) PR_ASYNC :D

>> -- Pavel
>>
>>> +
>>>  	ret = uffd_handle_pages(lpi, address, 1, PR_ASYNC | PR_ASAP);
>>>  	if (ret < 0) {
>>>  		pr_err("Error during regular page copy\n");
>>> @@ -679,6 +707,9 @@ static int handle_uffd_event(struct epoll_rfd *lpfd)
>>>  		return 1;
>>>  
>>>  	if (ret != sizeof(msg)) {
>>> +		/* we've already handled the page fault for another thread */
>>> +		if (errno == EAGAIN)
>>> +			return 0;
>>>  		if (ret < 0)
>>>  			pr_perror("Can't read userfaultfd message");
>>>  		else
>>>
>>
> 
> .
>