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

[Pavel Emelyanov]

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.

> 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);
>