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/console-server.c b/console-server.c
index e8f0352..50ee82a 100644
--- a/console-server.c
+++ b/console-server.c
@@ -394,6 +394,19 @@
free(poller);
}
+void console_poller_set_events(struct console *console, struct poller *poller,
+ int events)
+{
+ int i;
+
+ /* find the entry in our pollers array */
+ for (i = 0; i < console->n_pollers; i++)
+ if (console->pollers[i] == poller)
+ break;
+
+ console->pollfds[i].events = events;
+}
+
static int call_pollers(struct console *console)
{
struct poller *poller;
diff --git a/console-server.h b/console-server.h
index 0831c57..9571ff9 100644
--- a/console-server.h
+++ b/console-server.h
@@ -75,6 +75,9 @@
void console_poller_unregister(struct console *console, struct poller *poller);
+void console_poller_set_events(struct console *console, struct poller *poller,
+ int events);
+
/* ringbuffer API */
enum ringbuffer_poll_ret {
RINGBUFFER_POLL_OK = 0,
diff --git a/socket-handler.c b/socket-handler.c
index cf5a2de..84cd55a 100644
--- a/socket-handler.c
+++ b/socket-handler.c
@@ -37,6 +37,7 @@
struct poller *poller;
struct ringbuffer_consumer *rbc;
int fd;
+ bool blocked;
};
struct socket_handler {
@@ -82,11 +83,30 @@
sizeof(*sh->clients) * sh->n_clients);
}
-static ssize_t send_all(int fd, void *buf, size_t len, bool block)
+static void client_set_blocked(struct client *client, bool blocked)
{
- int rc, flags;
+ int events;
+
+ if (client->blocked == blocked)
+ return;
+
+ client->blocked = blocked;
+
+ events = POLLIN;
+ if (client->blocked)
+ events |= POLLOUT;
+
+ console_poller_set_events(client->sh->console, client->poller, events);
+}
+
+static ssize_t send_all(struct client *client, void *buf,
+ size_t len, bool block)
+{
+ int fd, rc, flags;
size_t pos;
+ fd = client->fd;
+
flags = MSG_NOSIGNAL;
if (!block)
flags |= MSG_DONTWAIT;
@@ -94,8 +114,11 @@
for (pos = 0; pos < len; pos += rc) {
rc = send(fd, buf + pos, len - pos, flags);
if (rc < 0) {
- if (!block && (errno == EAGAIN || errno == EWOULDBLOCK))
+ if (!block && (errno == EAGAIN ||
+ errno == EWOULDBLOCK)) {
+ client_set_blocked(client, true);
break;
+ }
if (errno == EINTR)
continue;
@@ -123,12 +146,16 @@
wlen = 0;
block = !!force_len;
+ /* if we're already blocked, no need for the write */
+ if (!block && client->blocked)
+ return 0;
+
for (;;) {
len = ringbuffer_dequeue_peek(client->rbc, total_len, &buf);
if (!len)
break;
- wlen = send_all(client->fd, buf, len, block);
+ wlen = send_all(client, buf, len, block);
if (wlen <= 0)
break;
@@ -187,6 +214,7 @@
}
if (events & POLLOUT) {
+ client_set_blocked(client, false);
rc = client_drain_queue(client, 0);
if (rc)
goto err_close;
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)