From 930b3fc8c9aafdc6723b3addc5a569dedffacc1d Mon Sep 17 00:00:00 2001 From: Ian Campbell Date: Sun, 13 Jan 2013 10:20:40 +0000 Subject: Revert 0b9dfd067b42: Switch from select() to poll() in xenconsoled's IO loop Causes issues with migration etc in test flight 14869 onwards. Signed-off-by: Ian Campbell --- tools/console/daemon/io.c | 209 ++++++++++++++++------------------------------ 1 file changed, 73 insertions(+), 136 deletions(-) (limited to 'tools/console') diff --git a/tools/console/daemon/io.c b/tools/console/daemon/io.c index 6762cbac2a..48fe1519d5 100644 --- a/tools/console/daemon/io.c +++ b/tools/console/daemon/io.c @@ -28,7 +28,7 @@ #include #include #include -#include +#include #include #include #include @@ -66,12 +66,9 @@ extern int discard_overflowed_data; static int log_time_hv_needts = 1; static int log_time_guest_needts = 1; static int log_hv_fd = -1; - -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 evtchn_port_or_error_t log_hv_evtchn = -1; +static xc_interface *xch; /* why does xenconsoled have two xc handles ? */ +static xc_evtchn *xce_handle = NULL; struct buffer { char *data; @@ -84,9 +81,7 @@ struct buffer { struct domain { int domid; int master_fd; - struct pollfd *master_pollfd; int slave_fd; - struct pollfd *slave_pollfd; int log_fd; bool is_dead; unsigned last_seen; @@ -97,7 +92,6 @@ struct domain { evtchn_port_or_error_t local_port; evtchn_port_or_error_t remote_port; xc_evtchn *xce_handle; - struct pollfd *xce_pollfd; struct xencons_interface *interface; int event_count; long long next_period; @@ -775,17 +769,6 @@ static int ring_free_bytes(struct domain *dom) return (sizeof(intf->in) - space); } -static void domain_handle_broken_tty(struct domain *dom, int recreate) -{ - domain_close_tty(dom); - - if (recreate) { - domain_create_tty(dom); - } else { - shutdown_domain(dom); - } -} - static void handle_tty_read(struct domain *dom) { ssize_t len = 0; @@ -811,7 +794,13 @@ static void handle_tty_read(struct domain *dom) * keep the slave open for the duration. */ if (len < 0) { - domain_handle_broken_tty(dom, domain_is_valid(dom->domid)); + domain_close_tty(dom); + + if (domain_is_valid(dom->domid)) { + domain_create_tty(dom); + } else { + shutdown_domain(dom); + } } else if (domain_is_valid(dom->domid)) { prod = intf->in_prod; for (i = 0; i < len; i++) { @@ -839,7 +828,14 @@ static void handle_tty_write(struct domain *dom) if (len < 1) { dolog(LOG_DEBUG, "Write failed on domain %d: %zd, %d\n", dom->domid, len, errno); - domain_handle_broken_tty(dom, domain_is_valid(dom->domid)); + + domain_close_tty(dom); + + if (domain_is_valid(dom->domid)) { + domain_create_tty(dom); + } else { + shutdown_domain(dom); + } } else { buffer_advance(&dom->buffer, len); } @@ -887,7 +883,7 @@ static void handle_xs(void) free(vec); } -static void handle_hv_logs(xc_evtchn *xce_handle) +static void handle_hv_logs(void) { char buffer[1024*16]; char *bufptr = buffer; @@ -898,7 +894,7 @@ static void handle_hv_logs(xc_evtchn *xce_handle) if ((port = xc_evtchn_pending(xce_handle)) == -1) return; - if (xc_readconsolering(xc, bufptr, &size, 0, 1, &index) == 0 && size > 0) { + if (xc_readconsolering(xch, bufptr, &size, 0, 1, &index) == 0 && size > 0) { int logret; if (log_time_hv) logret = write_with_timestamp(log_hv_fd, buffer, size, @@ -932,54 +928,18 @@ static void handle_log_reload(void) } } -static struct pollfd *set_fds(int fd, short events) -{ - 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; - - 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: - dolog(LOG_ERR, "realloc failed, ignoring fd %d\n", fd); - return NULL; -} - -static void reset_fds(void) -{ - nr_fds = 0; - memset(fds, 0, sizeof(struct pollfd) * current_array_size); -} - void handle_io(void) { + fd_set readfds, writefds; int ret; - evtchn_port_or_error_t log_hv_evtchn = -1; - struct pollfd *xce_pollfd = NULL; - struct pollfd *xs_pollfd = NULL; - xc_evtchn *xce_handle = NULL; if (log_hv) { + xch = xc_interface_open(0,0,0); + if (!xch) { + dolog(LOG_ERR, "Failed to open xc handle: %d (%s)", + errno, strerror(errno)); + goto out; + } xce_handle = xc_evtchn_open(NULL, 0); if (xce_handle == NULL) { dolog(LOG_ERR, "Failed to open xce handle: %d (%s)", @@ -999,17 +959,21 @@ void handle_io(void) for (;;) { struct domain *d, *n; - int poll_timeout; /* timeout in milliseconds */ + int max_fd = -1; + struct timeval timeout; struct timespec ts; long long now, next_timeout = 0; - reset_fds(); + FD_ZERO(&readfds); + FD_ZERO(&writefds); - xs_pollfd = set_fds(xs_fileno(xs), POLLIN|POLLPRI); + FD_SET(xs_fileno(xs), &readfds); + max_fd = MAX(xs_fileno(xs), max_fd); - if (log_hv) - xce_pollfd = set_fds(xc_evtchn_fd(xce_handle), - POLLIN|POLLPRI); + if (log_hv) { + FD_SET(xc_evtchn_fd(xce_handle), &readfds); + max_fd = MAX(xc_evtchn_fd(xce_handle), max_fd); + } if (clock_gettime(CLOCK_MONOTONIC, &ts) < 0) return; @@ -1018,12 +982,10 @@ void handle_io(void) /* Re-calculate any event counter allowances & unblock domains with new allowance */ for (d = dom_head; d; d = d->next) { - /* CS 16257:955ee4fa1345 introduces a 5ms fuzz - * for select(), it is not clear poll() has - * similar behavior (returning a couple of ms - * sooner than requested) as well. Just leave - * the fuzz here. Remove it with a separate - * patch if necessary */ + /* Add 5ms of fuzz since select() often returns + a couple of ms sooner than requested. Without + the fuzz we typically do an extra spin in select() + with a 1/2 ms timeout every other iteration */ if ((now+5) > d->next_period) { d->next_period = now + RATE_LIMIT_PERIOD; if (d->event_count >= RATE_LIMIT_ALLOWANCE) { @@ -1044,101 +1006,75 @@ void handle_io(void) !d->buffer.max_capacity || d->buffer.size < d->buffer.max_capacity) { int evtchn_fd = xc_evtchn_fd(d->xce_handle); - d->xce_pollfd = set_fds(evtchn_fd, - POLLIN|POLLPRI); + FD_SET(evtchn_fd, &readfds); + max_fd = MAX(evtchn_fd, max_fd); } } if (d->master_fd != -1) { - short events = 0; if (!d->is_dead && ring_free_bytes(d)) - events |= POLLIN; + FD_SET(d->master_fd, &readfds); if (!buffer_empty(&d->buffer)) - events |= POLLOUT; - - if (events) - d->master_pollfd = - set_fds(d->master_fd, - events|POLLPRI); + FD_SET(d->master_fd, &writefds); + max_fd = MAX(d->master_fd, max_fd); } } /* If any domain has been rate limited, we need to work - out what timeout to supply to poll */ + out what timeout to supply to select */ if (next_timeout) { long long duration = (next_timeout - now); if (duration <= 0) /* sanity check */ duration = 1; - poll_timeout = (int)duration; + timeout.tv_sec = duration / 1000; + timeout.tv_usec = ((duration - (timeout.tv_sec * 1000)) + * 1000); } - ret = poll(fds, nr_fds, next_timeout ? poll_timeout : -1); + ret = select(max_fd + 1, &readfds, &writefds, 0, + next_timeout ? &timeout : NULL); if (log_reload) { handle_log_reload(); log_reload = 0; } - /* Abort if poll failed, except for EINTR cases + /* Abort if select failed, except for EINTR cases which indicate a possible log reload */ if (ret == -1) { if (errno == EINTR) continue; - dolog(LOG_ERR, "Failure in poll: %d (%s)", + dolog(LOG_ERR, "Failure in select: %d (%s)", errno, strerror(errno)); break; } - if (log_hv && xce_pollfd) { - if (xce_pollfd->revents & ~(POLLIN|POLLOUT|POLLPRI)) { - dolog(LOG_ERR, - "Failure in poll xce_handle: %d (%s)", - errno, strerror(errno)); - break; - } else if (xce_pollfd->revents & POLLIN) - handle_hv_logs(xce_handle); - } + if (log_hv && FD_ISSET(xc_evtchn_fd(xce_handle), &readfds)) + handle_hv_logs(); if (ret <= 0) continue; - if (xs_pollfd) { - if (xs_pollfd->revents & ~(POLLIN|POLLOUT|POLLPRI)) { - dolog(LOG_ERR, - "Failure in poll xs_handle: %d (%s)", - errno, strerror(errno)); - break; - } else if (xs_pollfd->revents & POLLIN) - handle_xs(); - } + if (FD_ISSET(xs_fileno(xs), &readfds)) + handle_xs(); for (d = dom_head; d; d = n) { n = d->next; if (d->event_count < RATE_LIMIT_ALLOWANCE) { if (d->xce_handle != NULL && - d->xce_pollfd && - !(d->xce_pollfd->revents & - ~(POLLIN|POLLOUT|POLLPRI)) && - (d->xce_pollfd->revents & - POLLIN)) - handle_ring_read(d); + FD_ISSET(xc_evtchn_fd(d->xce_handle), + &readfds)) + handle_ring_read(d); } - if (d->master_fd != -1 && d->master_pollfd) { - if (d->master_pollfd->revents & - ~(POLLIN|POLLOUT|POLLPRI)) - domain_handle_broken_tty(d, - domain_is_valid(d->domid)); - else { - if (d->master_pollfd->revents & - POLLIN) - handle_tty_read(d); - if (d->master_pollfd->revents & - POLLOUT) - handle_tty_write(d); - } - } + if (d->master_fd != -1 && FD_ISSET(d->master_fd, + &readfds)) + handle_tty_read(d); + + if (d->master_fd != -1 && FD_ISSET(d->master_fd, + &writefds)) + handle_tty_write(d); if (d->last_seen != enum_pass) shutdown_domain(d); @@ -1148,14 +1084,15 @@ void handle_io(void) } } - free(fds); - current_array_size = 0; - out: if (log_hv_fd != -1) { close(log_hv_fd); log_hv_fd = -1; } + if (xch) { + xc_interface_close(xch); + xch = 0; + } if (xce_handle != NULL) { xc_evtchn_close(xce_handle); xce_handle = NULL; -- cgit v1.2.3