[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



Ian, ping?

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®.