Re: [PATCH] Switch to poll in xenconsoled's io loop.

[Wei Liu <Wei.Liu2@citrix.com>]

On Thu, 2013-01-03 at 18:22 +0000, Mats Petersson wrote:
> On 03/01/13 17:14, Wei Liu wrote:
> > The original implementation utilies select(). In Linux select() typically
> > supports up to 1024 file descriptors. This can be a problem when user tries to
> > boot up many guests. Switching to poll() has minimum impact on existing code
> > and has better scalibility.
> >
> > Up to 8192 file descriptors are supported in the current implementation.
> >
> > Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> > ---
> >   tools/console/daemon/io.c |   90 +++++++++++++++++++++++++--------------------
> >   1 file changed, 50 insertions(+), 40 deletions(-)
> >
> > diff --git a/tools/console/daemon/io.c b/tools/console/daemon/io.c
> > index 48fe151..4e3c55c 100644
> > --- a/tools/console/daemon/io.c
> > +++ b/tools/console/daemon/io.c
> > @@ -28,7 +28,7 @@
> >   #include <stdlib.h>
> >   #include <errno.h>
> >   #include <string.h>
> > -#include <sys/select.h>
> > +#include <poll.h>
> >   #include <fcntl.h>
> >   #include <unistd.h>
> >   #include <termios.h>
> > @@ -930,7 +930,6 @@ static void handle_log_reload(void)
> >   
> >   void handle_io(void)
> >   {
> > -	fd_set readfds, writefds;
> >   	int ret;
> >   
> >   	if (log_hv) {
> > @@ -959,21 +958,33 @@ void handle_io(void)
> >   
> >   	for (;;) {
> >   		struct domain *d, *n;
> > -		int max_fd = -1;
> > -		struct timeval timeout;
> > +		int poll_timeout; /* timeout in milliseconds */
> >   		struct timespec ts;
> >   		long long now, next_timeout = 0;
> >   
> > -		FD_ZERO(&readfds);
> > -		FD_ZERO(&writefds);
> > -
> > -		FD_SET(xs_fileno(xs), &readfds);
> > -		max_fd = MAX(xs_fileno(xs), max_fd);
> > -
> > -		if (log_hv) {
> > -			FD_SET(xc_evtchn_fd(xce_handle), &readfds);
> > -			max_fd = MAX(xc_evtchn_fd(xce_handle), max_fd);
> > -		}
> > +#define MAX_POLL_FDS 8192
> > +		static struct pollfd fds[MAX_POLL_FDS];
> > +		static struct pollfd *fd_to_pollfd[MAX_POLL_FDS];
> > +		int nr_fds;
> > +#define SET_FDS(_fd, _events) do {				\
> > +			if (_fd >= MAX_POLL_FDS)		\
> > +				break;				\
> > +			fds[nr_fds].fd = (_fd);			\
> > +			fds[nr_fds].events = (_events);		\
> > +			fd_to_pollfd[(_fd)] = &fds[nr_fds];	\
> > +			nr_fds++;				\
> > +		} while (0)
> > +#define FD_REVENTS(_fd) (((_fd) < MAX_POLL_FDS && fd_to_pollfd[(_fd)]) ? \
> > +			 fd_to_pollfd[(_fd)]->revents : 0)
> > +
> > +		nr_fds = 0;
> > +		memset(fds, 0, sizeof(fds));
> > +		memset(fd_to_pollfd, 0, sizeof(fd_to_pollfd));
> > +
> > +		SET_FDS(xs_fileno(xs), POLLIN);
> > +
> > +		if (log_hv)
> > +			SET_FDS(xc_evtchn_fd(xce_handle), POLLIN);
> >   
> Would it not make sense to use dynamically allocated memory instead - 
> that way, when we run out of 8192, there is nothing to change.

Writing a new version to use dynamically allocated memory.


Wei.