server: improve blocked-write behaviour for handlers
We currently don't implement POLLOUT properly; we never set this for
polled events, and will repeat calls to write() if we see EAGAIN.
This change improves the behaviour when writes start to block, by
tracking when a fd is blocked. Once we detect blocking behaviour, we
supress future (non-forced) writes, and wait for POLLOUT so we know when
we can write again.
Change-Id: I809bde4e1c7c78a58ea296d5c076b3d93c272558
Signed-off-by: Jeremy Kerr <jk@ozlabs.org>
diff --git a/tty-handler.c b/tty-handler.c
index 22f9a75..cd1e0e8 100644
--- a/tty-handler.c
+++ b/tty-handler.c
@@ -34,6 +34,8 @@
struct ringbuffer_consumer *rbc;
struct poller *poller;
int fd;
+ int fd_flags;
+ bool blocked;
};
struct terminal_speed_name {
@@ -46,21 +48,58 @@
return container_of(handler, struct tty_handler, handler);
}
+static void tty_set_fd_blocking(struct tty_handler *th, bool fd_blocking)
+{
+ int flags;
+
+ flags = th->fd_flags & ~O_NONBLOCK;
+ if (!fd_blocking)
+ flags |= O_NONBLOCK;
+
+ if (flags != th->fd_flags) {
+ fcntl(th->fd, F_SETFL, flags);
+ th->fd_flags = flags;
+ }
+}
+
+/*
+ * A "blocked" handler indicates that the last write returned EAGAIN
+ * (==EWOULDBLOCK), so we know not to continue writing (for non-forced output),
+ * as it'll just return EAGAIN again.
+ *
+ * Once we detect this, we watch for POLLOUT in the poller events. A
+ * POLLOUT indicates that the fd is no longer blocking, so we clear
+ * blocked mode and can continue writing.
+ */
+static void tty_set_blocked(struct tty_handler *th, bool blocked)
+{
+ int events;
+
+ if (blocked == th->blocked)
+ return;
+
+ th->blocked = blocked;
+ events = POLLIN;
+
+ if (th->blocked)
+ events |= POLLOUT;
+
+ console_poller_set_events(th->console, th->poller, events);
+}
+
static int tty_drain_queue(struct tty_handler *th, size_t force_len)
{
size_t len, total_len;
ssize_t wlen;
uint8_t *buf;
- int flags;
/* if we're forcing data, we need to clear non-blocking mode */
- if (force_len) {
- flags = fcntl(th->fd, F_GETFL, 0);
- if (flags & O_NONBLOCK) {
- flags &= ~O_NONBLOCK;
- fcntl(th->fd, F_SETFL, flags);
- }
- }
+ if (force_len)
+ tty_set_fd_blocking(th, true);
+
+ /* no point writing, we'll just see -EAGAIN */
+ else if (th->blocked)
+ return 0;
total_len = 0;
@@ -78,8 +117,10 @@
if (errno == EINTR)
continue;
if ((errno == EAGAIN || errno == EWOULDBLOCK)
- && !force_len)
+ && !force_len) {
+ tty_set_blocked(th, true);
break;
+ }
warn("failed writing to local tty; disabling");
return -1;
}
@@ -93,7 +134,7 @@
ringbuffer_dequeue_commit(th->rbc, total_len);
if (force_len)
- fcntl(th->fd, F_SETFL, flags | O_NONBLOCK);
+ tty_set_fd_blocking(th, false);
return 0;
}
@@ -132,6 +173,7 @@
}
if (events & POLLOUT) {
+ tty_set_blocked(th, false);
rc = tty_drain_queue(th, 0);
if (rc) {
ringbuffer_consumer_unregister(th->rbc);
@@ -265,6 +307,7 @@
}
free(tty_path);
+ th->fd_flags = fcntl(th->fd, F_GETFL, 0);
tty_baud = config_get_value(config, "local-tty-baud");
if (tty_baud != NULL)