Re: [PATCH V2] Switch from select() to poll() in xenconsoled's IO loop.

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

On Fri, 2013-01-04 at 16:08 +0000, Ian Campbell wrote:
> > +static int initialize_pollfd_arrays(void)
> > +{
> > +       fds = (struct pollfd *)
> > +               malloc(sizeof(struct pollfd) * DEFAULT_ARRAY_SIZE);
> > +       if (!fds)
> > +               goto fail;
> > +       fd_to_pollfd = (struct pollfd **)
> > +               malloc(sizeof(struct pollfd *) * DEFAULT_ARRAY_SIZE);
> > +       if (!fd_to_pollfd)
> > +               goto fail;
> > +       memset(fds, 0, sizeof(struct pollfd) * DEFAULT_ARRAY_SIZE);
> > +       memset(fd_to_pollfd, 0, sizeof(struct pollfd *) * DEFAULT_ARRAY_SIZE);
> > +       current_array_size = DEFAULT_ARRAY_SIZE;
> > +       return 0;
> > +fail:
> > +       free(fds);
> > +       free(fd_to_pollfd);
> > +       return -ENOMEM;
> > +}
> > +
> > +static void destroy_pollfd_arrays(void)
> > +{
> > +       free(fds);
> > +       free(fd_to_pollfd);
> > +       current_array_size = 0;
> > +}
> > +
> > +static void set_fds(int fd, short events)
> > +{
> > +       if (current_array_size < fd+1) {
> > +               struct pollfd  *p1 = NULL;
> > +               struct pollfd **p2 = NULL;
> > +               unsigned int newsize = current_array_size;
> > +
> > +               do { newsize += GROWTH_LENGTH; } while (newsize < fd+1);
> 
> Steal #define ROUNDUP from tools/libxc/xc_linux_osdep.c.
> > +
> > +               p1 = realloc(fds, sizeof(struct pollfd)*newsize);
> > +               if (!p1)
> > +                       goto fail;
> > +               fds = p1;
> > +
> > +               p2 = realloc(fd_to_pollfd, sizeof(struct pollfd *)*newsize);
> 
> realloc(NULL, ...) is the same as malloc() so I think you can initialise
> current_array_size to 0 and the various pointers to NULL and avoid the
> need for initialize_pollfd_arrays -- i.e. it will just grow from 0 on
> the first use here.
> 

Huh, good idea.

> >         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);
> > +               reset_fds();
> > 
> > -               FD_SET(xs_fileno(xs), &readfds);
> > -               max_fd = MAX(xs_fileno(xs), max_fd);
> > +               set_fds(xs_fileno(xs), POLLIN);
> 
> Do you know, or can you track, the maximum fd over time?
> If you can then you could likely make use of automatic stack allocations
> (struct pollfd fds[max_fd]) and therefore avoid the pain of manual
> memory management.

Stack size is subject to system setting. Typically it is limited to 8MB
in Linux as shown by `ulimit -s`. This is actually very small compared
to heap space.

Of course we can make xenconsoled special among all the processes... But
that's not ideal.


Wei.

> Not sure what the semantics of those are inside a for loop where max_fd
> can change but worst case you could put the content of the loop into a
> function.
> 
> Ian.
>