[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH V2] Switch to poll() in cxenstored's IO loop



On Tue, 2013-01-29 at 17:40 +0000, Wei Liu wrote:
> Ian, ping?

It's in my queue but I've been holding off applying until we get a test
pass and push, we've got a weeks worth of stuff in the staging queue at
the moment.

Ian.

> 
> On Tue, 2013-01-22 at 15:18 +0000, Wei Liu wrote:
> > commit 14b82f52d40143019094727b8fa1aa3a111ffab3
> > Author: Wei Liu <wei.liu2@xxxxxxxxxx>
> > Date:   Mon Jan 21 19:07:37 2013 +0000
> > 
> >     Switch to poll() in cxenstored's IO loop
> >     
> >     Poll() can support more file descriptors than select(). We've done this 
> > for
> >     xenconsoled, now do this for cxenstored as well.
> >     
> >     The code is taken from xenconsoled and modified to adapt to cxenstored.
> >     
> >     Updated:
> >     * reopen pipe if polling on reopen_log_pipe fails.
> >     * exit if polling on some critical fds fails.
> >     
> >     Signed-off-by: Wei Liu <wei.liu2@xxxxxxxxxx>
> > 
> > diff --git a/tools/xenstore/xenstored_core.c 
> > b/tools/xenstore/xenstored_core.c
> > index bd44645..b91704e 100644
> > --- a/tools/xenstore/xenstored_core.c
> > +++ b/tools/xenstore/xenstored_core.c
> > @@ -19,7 +19,7 @@
> >  
> >  #include <sys/types.h>
> >  #include <sys/stat.h>
> > -#include <sys/select.h>
> > +#include <poll.h>
> >  #ifndef NO_SOCKETS
> >  #include <sys/socket.h>
> >  #include <sys/un.h>
> > @@ -55,6 +55,12 @@
> >  #include "hashtable.h"
> >  
> >  extern xc_evtchn *xce_handle; /* in xenstored_domain.c */
> > +static struct pollfd *xce_pollfd;
> > +static struct pollfd *fds;
> > +static unsigned int current_array_size;
> > +static unsigned int nr_fds;
> > +
> > +#define ROUNDUP(_x, _w) (((unsigned long)(_x)+(1UL<<(_w))-1) & 
> > ~((1UL<<(_w))-1))
> >  
> >  static bool verbose = false;
> >  LIST_HEAD(connections);
> > @@ -62,6 +68,7 @@ static int tracefd = -1;
> >  static bool recovery = true;
> >  static bool remove_local = true;
> >  static int reopen_log_pipe[2];
> > +static struct pollfd *reopen_log_pipe0_pollfd;
> >  static char *tracefile = NULL;
> >  static TDB_CONTEXT *tdb_ctx = NULL;
> >  
> > @@ -199,7 +206,7 @@ void trace_destroy(const void *data, const char *type)
> >  /**
> >   * Signal handler for SIGHUP, which requests that the trace log is reopened
> >   * (in the main loop).  A single byte is written to reopen_log_pipe, to 
> > awaken
> > - * the select() in the main loop.
> > + * the poll() in the main loop.
> >   */
> >  static void trigger_reopen_log(int signal __attribute__((unused)))
> >  {
> > @@ -279,15 +286,12 @@ static int destroy_conn(void *_conn)
> >  
> >     /* Flush outgoing if possible, but don't block. */
> >     if (!conn->domain) {
> > -           fd_set set;
> > -           struct timeval none;
> > -
> > -           FD_ZERO(&set);
> > -           FD_SET(conn->fd, &set);
> > -           none.tv_sec = none.tv_usec = 0;
> > +           struct pollfd pfd;
> > +           pfd.fd = conn->fd;
> > +           pfd.events = POLLOUT;
> >  
> >             while (!list_empty(&conn->out_list)
> > -                  && select(conn->fd+1, NULL, &set, NULL, &none) == 1)
> > +                  && poll(&pfd, 1, 0) == 1)
> >                     if (!write_messages(conn))
> >                             break;
> >             close(conn->fd);
> > @@ -300,52 +304,74 @@ static int destroy_conn(void *_conn)
> >  }
> >  
> > 
> > -static void set_fd(int fd, fd_set *set, int *max)
> > +static struct pollfd *set_fd(int fd, short events)
> >  {
> > -   if (fd < 0)
> > -           return;
> > -   FD_SET(fd, set);
> > -   if (fd > *max)
> > -           *max = fd;
> > -}
> > +   struct pollfd *ret;
> > +   if (current_array_size < nr_fds + 1) {
> > +           struct pollfd *new_fds = NULL;
> > +           unsigned long newsize;
> > +
> > +           /* Round up to 2^8 boundary, in practice this just
> > +            * make newsize larger than current_array_size.
> > +            */
> > +           newsize = ROUNDUP(nr_fds + 1, 8);
> >  
> > +           new_fds = realloc(fds, sizeof(struct pollfd)*newsize);
> > +           if (!new_fds)
> > +                   goto fail;
> > +           fds = new_fds;
> >  
> > -static int initialize_set(fd_set *inset, fd_set *outset, int sock, int 
> > ro_sock,
> > -                     struct timeval **ptimeout)
> > +           memset(&fds[0] + current_array_size, 0,
> > +                  sizeof(struct pollfd ) * (newsize-current_array_size));
> > +           current_array_size = newsize;
> > +   }
> > +
> > +   fds[nr_fds].fd = fd;
> > +   fds[nr_fds].events = events;
> > +   ret = &fds[nr_fds];
> > +   nr_fds++;
> > +
> > +   return ret;
> > +fail:
> > +   syslog(LOG_ERR, "realloc failed, ignoring fd %d\n", fd);
> > +   return NULL;
> > +}
> > +
> > +static void initialize_fds(int sock, struct pollfd **p_sock_pollfd,
> > +                      int ro_sock, struct pollfd **p_ro_sock_pollfd,
> > +                      int *ptimeout)
> >  {
> > -   static struct timeval zero_timeout = { 0 };
> >     struct connection *conn;
> > -   int max = -1;
> >  
> > -   *ptimeout = NULL;
> > +   memset(fds, 0, sizeof(struct pollfd) * current_array_size);
> > +   nr_fds = 0;
> >  
> > -   FD_ZERO(inset);
> > -   FD_ZERO(outset);
> > +   *ptimeout = -1;
> >  
> >     if (sock != -1)
> > -           set_fd(sock, inset, &max);
> > +           *p_sock_pollfd = set_fd(sock, POLLIN|POLLPRI);
> >     if (ro_sock != -1)
> > -           set_fd(ro_sock, inset, &max);
> > +           *p_ro_sock_pollfd = set_fd(ro_sock, POLLIN|POLLPRI);
> >     if (reopen_log_pipe[0] != -1)
> > -           set_fd(reopen_log_pipe[0], inset, &max);
> > +           reopen_log_pipe0_pollfd =
> > +                   set_fd(reopen_log_pipe[0], POLLIN|POLLPRI);
> >  
> >     if (xce_handle != NULL)
> > -           set_fd(xc_evtchn_fd(xce_handle), inset, &max);
> > +           xce_pollfd = set_fd(xc_evtchn_fd(xce_handle), POLLIN|POLLPRI);
> >  
> >     list_for_each_entry(conn, &connections, list) {
> >             if (conn->domain) {
> >                     if (domain_can_read(conn) ||
> >                         (domain_can_write(conn) &&
> >                          !list_empty(&conn->out_list)))
> > -                           *ptimeout = &zero_timeout;
> > +                           *ptimeout = 0;
> >             } else {
> > -                   set_fd(conn->fd, inset, &max);
> > +                   short events = POLLIN|POLLPRI;
> >                     if (!list_empty(&conn->out_list))
> > -                           FD_SET(conn->fd, outset);
> > +                           events |= POLLOUT;
> > +                   conn->pollfd = set_fd(conn->fd, events);
> >             }
> >     }
> > -
> > -   return max;
> >  }
> >  
> >  /* Is child a subnode of parent, or equal? */
> > @@ -1770,14 +1796,13 @@ int priv_domid = 0;
> >  
> >  int main(int argc, char *argv[])
> >  {
> > -   int opt, *sock, *ro_sock, max;
> > -   fd_set inset, outset;
> > +   int opt, *sock, *ro_sock;
> > +   struct pollfd *sock_pollfd = NULL, *ro_sock_pollfd = NULL;
> >     bool dofork = true;
> >     bool outputpid = false;
> >     bool no_domain_init = false;
> >     const char *pidfile = NULL;
> > -   int evtchn_fd = -1;
> > -   struct timeval *timeout;
> > +   int timeout;
> >  
> >     while ((opt = getopt_long(argc, argv, "DE:F:HNPS:t:T:RLVW:", options,
> >                               NULL)) != -1) {
> > @@ -1880,11 +1905,9 @@ int main(int argc, char *argv[])
> >  
> >     signal(SIGHUP, trigger_reopen_log);
> >  
> > -   if (xce_handle != NULL)
> > -           evtchn_fd = xc_evtchn_fd(xce_handle);
> > -
> >     /* Get ready to listen to the tools. */
> > -   max = initialize_set(&inset, &outset, *sock, *ro_sock, &timeout);
> > +   initialize_fds(*sock, &sock_pollfd, *ro_sock, &ro_sock_pollfd,
> > +                  &timeout);
> >  
> >     /* Tell the kernel we're up and running. */
> >     xenbus_notify_running();
> > @@ -1893,27 +1916,55 @@ int main(int argc, char *argv[])
> >     for (;;) {
> >             struct connection *conn, *next;
> >  
> > -           if (select(max+1, &inset, &outset, NULL, timeout) < 0) {
> > +           if (poll(fds, nr_fds, timeout) < 0) {
> >                     if (errno == EINTR)
> >                             continue;
> > -                   barf_perror("Select failed");
> > +                   barf_perror("Poll failed");
> >             }
> >  
> > -           if (reopen_log_pipe[0] != -1 && FD_ISSET(reopen_log_pipe[0], 
> > &inset)) {
> > -                   char c;
> > -                   if (read(reopen_log_pipe[0], &c, 1) != 1)
> > -                           barf_perror("read failed");
> > -                   reopen_log();
> > +           if (reopen_log_pipe0_pollfd) {
> > +                   if (reopen_log_pipe0_pollfd->revents & 
> > ~(POLLIN|POLLOUT|POLLPRI)) {
> > +                           close(reopen_log_pipe[0]);
> > +                           close(reopen_log_pipe[1]);
> > +                           init_pipe(reopen_log_pipe);
> > +                   } else if (reopen_log_pipe0_pollfd->revents & POLLIN) {
> > +                           char c;
> > +                           if (read(reopen_log_pipe[0], &c, 1) != 1)
> > +                                   barf_perror("read failed");
> > +                           reopen_log();
> > +                   }
> > +                   reopen_log_pipe0_pollfd = NULL;
> >             }
> >  
> > -           if (*sock != -1 && FD_ISSET(*sock, &inset))
> > -                   accept_connection(*sock, true);
> > +           if (sock_pollfd) {
> > +                   if (sock_pollfd->revents & ~(POLLIN|POLLOUT|POLLPRI)) {
> > +                           barf_perror("sock poll failed");
> > +                           break;
> > +                   } else if (sock_pollfd->revents & POLLIN) {
> > +                           accept_connection(*sock, true);
> > +                           sock_pollfd = NULL;
> > +                   }
> > +           }
> >  
> > -           if (*ro_sock != -1 && FD_ISSET(*ro_sock, &inset))
> > -                   accept_connection(*ro_sock, false);
> > +           if (ro_sock_pollfd) {
> > +                   if (ro_sock_pollfd->revents & 
> > ~(POLLIN|POLLOUT|POLLPRI)) {
> > +                           barf_perror("ro sock poll failed");
> > +                           break;
> > +                   } else if (ro_sock_pollfd->revents & POLLIN) {
> > +                           accept_connection(*ro_sock, false);
> > +                           ro_sock_pollfd = NULL;
> > +                   }
> > +           }
> >  
> > -           if (evtchn_fd != -1 && FD_ISSET(evtchn_fd, &inset))
> > -                   handle_event();
> > +           if (xce_pollfd) {
> > +                   if (xce_pollfd->revents & ~(POLLIN|POLLOUT|POLLPRI)) {
> > +                           barf_perror("xce_handle poll failed");
> > +                           break;
> > +                   } else if (xce_pollfd->revents & POLLIN) {
> > +                           handle_event();
> > +                           xce_pollfd = NULL;
> > +                   }
> > +           }
> >  
> >             next = list_entry(connections.next, typeof(*conn), list);
> >             if (&next->list != &connections)
> > @@ -1939,21 +1990,27 @@ int main(int argc, char *argv[])
> >                             if (talloc_free(conn) == 0)
> >                                     continue;
> >                     } else {
> > -                           if (FD_ISSET(conn->fd, &inset))
> > +                           if (conn->pollfd &&
> > +                               !(conn->pollfd->revents & 
> > ~(POLLIN|POLLOUT|POLLPRI)) &&
> > +                               (conn->pollfd->revents & POLLIN))
> >                                     handle_input(conn);
> >                             if (talloc_free(conn) == 0)
> >                                     continue;
> >  
> >                             talloc_increase_ref_count(conn);
> > -                           if (FD_ISSET(conn->fd, &outset))
> > +                           if (conn->pollfd &&
> > +                               !(conn->pollfd->revents & 
> > ~(POLLIN|POLLOUT|POLLPRI)) &&
> > +                               (conn->pollfd->revents & POLLOUT))
> >                                     handle_output(conn);
> >                             if (talloc_free(conn) == 0)
> >                                     continue;
> > +
> > +                           conn->pollfd = NULL;
> >                     }
> >             }
> >  
> > -           max = initialize_set(&inset, &outset, *sock, *ro_sock,
> > -                                &timeout);
> > +           initialize_fds(*sock, &sock_pollfd, *ro_sock, &ro_sock_pollfd,
> > +                          &timeout);
> >     }
> >  }
> >  
> > diff --git a/tools/xenstore/xenstored_core.h 
> > b/tools/xenstore/xenstored_core.h
> > index 492ca0d..f330a87 100644
> > --- a/tools/xenstore/xenstored_core.h
> > +++ b/tools/xenstore/xenstored_core.h
> > @@ -60,6 +60,8 @@ struct connection
> >  
> >     /* The file descriptor we came in on. */
> >     int fd;
> > +   /* The pollfd corresponding to fd. */
> > +   struct pollfd *pollfd;
> >  
> >     /* Who am I? 0 for socket connections. */
> >     unsigned int id;
> > 
> > 
> 
> 



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.