[CRIU] [PATCH 05/11] lazy-pages: use poll instead of select

[Mike Rapoport]

On Tue, Apr 12, 2016 at 02:03:21PM +0300, Pavel Emelyanov wrote:
> On 04/11/2016 09:19 AM, Mike Rapoport wrote:
> 
> Why not epoll? AFAIK this is right now the best (from the scalability
> POV) way to poll file descriptors.

Frankly, I was too lazy to read epoll manuals because for tens of
descriptors poll should be fast enough :)
We anyway will have to do some optimization rounds for uffd.c... 
 
> > Signed-off-by: Mike Rapoport <rppt at linux.vnet.ibm.com>
> > ---
> >  criu/uffd.c | 47 ++++++++++++++++++++++++++++++++---------------
> >  1 file changed, 32 insertions(+), 15 deletions(-)
> > 
> > diff --git a/criu/uffd.c b/criu/uffd.c
> > index 254a5b3..14de015 100644
> > --- a/criu/uffd.c
> > +++ b/criu/uffd.c
> > @@ -500,15 +500,17 @@ out:
> >  	return ret;
> >  }
> >  
> > +#define POLL_TIMEOUT 5000
> > +
> >  static int handle_requests(int fd)
> >  {
> > -	fd_set set;
> > +	struct pollfd *pollfd;
> > +	int nr_pollfd;
> >  	int ret = -1;
> >  	struct uffd_msg msg;
> >  	__u64 flags;
> >  	__u64 address;
> >  	unsigned long ps;
> > -	struct timeval timeout;
> >  	struct uffd_pages_struct *uffd_pages;
> >  	void *dest;
> >  
> > @@ -521,34 +523,46 @@ static int handle_requests(int fd)
> >  	if ((total_pages = find_vmas(&uffd_list)) == -1)
> >  		return -1;
> >  
> > -	/* Initialize FD sets for read() with timeouts (using select()) */
> > -	FD_ZERO(&set);
> > -	FD_SET(fd, &set);
> > -
> >  	/* All operations will be done on page size */
> >  	ps = page_size();
> >  	dest = malloc(ps);
> >  
> > +	/* allocate pollfd per task + pollfd for listen */
> > +	nr_pollfd = task_entries->nr_tasks + 1;
> > +	pollfd = malloc(sizeof(*pollfd) * nr_pollfd);
> > +	if (!pollfd || !dest) {
> > +		pr_err("memory allocation failed\n");
> > +		ret = -1;
> > +		goto out;
> > +	}
> > +
> > +	/*
> > +	 * use nr_pollfd to set number of pollfd's being polled
> > +	 * FIXME: eventually nr_pollfd should be nr_tasks + 1, and the
> > +	 * first fd to poll on should that of listen
> > +	 */
> > +	nr_pollfd = 1;
> > +	pollfd[0].fd = fd;
> > +	pollfd[0].events = POLLIN;
> > +
> >  	while (1) {
> >  		bool page_sent = false;
> >  		/*
> >  		 * Setting the timeout to 5 seconds. If after this time
> >  		 * no uffd pages are requested the code switches to
> >  		 * copying the remaining pages.
> > -		 *
> > -		 * Timeout is re-defined every time select() is run as
> > -		 * select(2) says:
> > -		 *  Consider timeout to be undefined after select() returns.
> >  		 */
> > -		timeout.tv_sec = 5;
> > -		timeout.tv_usec = 0;
> > -		ret = select(fd + 1, &set, NULL, NULL, &timeout);
> > -		pr_debug("select() rc: 0x%x\n", ret);
> > -		if (ret == 0) {
> > +		ret = poll(pollfd, nr_pollfd, POLL_TIMEOUT);
> > +		pr_debug("poll() rc: 0x%x\n", ret);
> > +		if (ret < 0) {
> > +			pr_perror("polling failed");
> > +			goto out;
> > +		} else if (ret == 0) {
> >  			pr_debug("read timeout\n");
> >  			pr_debug("switching from request to copy mode\n");
> >  			break;
> >  		}
> > +
> >  		ret = read(fd, &msg, sizeof(msg));
> >  		pr_debug("read() ret: 0x%x\n", ret);
> >  		if (!ret)
> > @@ -613,6 +627,7 @@ static int handle_requests(int fd)
> >  	ret = 0;
> >  
> >  out:
> > +	free(pollfd);
> >  	free(dest);
> >  	close(fd);
> >  	return ret;
> > @@ -674,6 +689,8 @@ int cr_lazy_pages()
> >  	pr_debug("uffd is 0x%d\n", uffd);
> >  	uffd_flags = fcntl(uffd, F_GETFD, NULL);
> >  	pr_debug("uffd_flags are 0x%x\n", uffd_flags);
> > +	if (fcntl(uffd, F_SETFD, uffd_flags | O_NONBLOCK))
> > +		return -1;
> >  
> >  	ret = handle_requests(uffd);
> >  	close(listen);
> > 
>