Eliminate excessive CPU consumption when redirecting UART over SSH
Redirecting the external UART via SSH caused the console-server,
console-client, and dropbear to consume ~30% of the available CPU each
when a large amount of data was being written to the UART output.
Buffering all of the small 16550 FIFO bytes into a larger packet and
then sending that to the SSH SW allows more efficient transmission
over the ethernet connection.
Tested this by "ssh root@<bmc.ip.addr> -p 2200" on a system running a
CentOS distribution. Using a BASH console run a large binary file
through "od -t x1 <fname>" to create a large amount of traffic. At
the BMC console run "top" to review the CPU usage. My experience is
after this change is applied:
console-server: ~25% CPU
dropbear: ~3% CPU
console-client: ~1% CPU
Change-Id: Ibabfd285e97a487e7ff040e1cb3159fbff360328
Signed-off-by: Johnathan Mantey <johnathanx.mantey@intel.com>
diff --git a/Makefile.am b/Makefile.am
index f61fddf..059ac1b 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -18,7 +18,7 @@
tty-handler.c \
console-socket.c
-obmc_console_server_LDFLAGS = $(SYSTEMD_LIBS)
+obmc_console_server_LDFLAGS = $(SYSTEMD_LIBS) -lrt
obmc_console_server_CFLAGS = $(SYSTEMD_CFLAGS)
obmc_console_client_SOURCES = \
diff --git a/console-server.c b/console-server.c
index cde4bfc..d814dff 100644
--- a/console-server.c
+++ b/console-server.c
@@ -31,9 +31,11 @@
#include <string.h>
#include <getopt.h>
#include <limits.h>
+#include <time.h>
#include <termios.h>
#include <sys/types.h>
+#include <sys/time.h>
#include <poll.h>
#include "console-server.h"
@@ -66,7 +68,9 @@
struct poller {
struct handler *handler;
void *data;
- poller_fn_t fn;
+ poller_event_fn_t event_fn;
+ poller_timeout_fn_t timeout_fn;
+ struct timeval timeout;
bool remove;
};
@@ -456,6 +460,26 @@
}
}
+static int get_current_time(struct timeval *tv)
+{
+ struct timespec t;
+ int rc;
+
+ /*
+ * We use clock_gettime(CLOCK_MONOTONIC) so we're immune to
+ * local time changes. However, a struct timeval is more
+ * convenient for calculations, so convert to that.
+ */
+ rc = clock_gettime(CLOCK_MONOTONIC, &t);
+ if (rc)
+ return rc;
+
+ tv->tv_sec = t.tv_sec;
+ tv->tv_usec = t.tv_nsec / 1000;
+
+ return 0;
+}
+
struct ringbuffer_consumer *console_ringbuffer_consumer_register(
struct console *console,
ringbuffer_poll_fn_t poll_fn, void *data)
@@ -464,8 +488,9 @@
}
struct poller *console_poller_register(struct console *console,
- struct handler *handler, poller_fn_t poller_fn,
- int fd, int events, void *data)
+ struct handler *handler, poller_event_fn_t poller_fn,
+ poller_timeout_fn_t timeout_fn, int fd,
+ int events, void *data)
{
struct poller *poller;
int n;
@@ -473,7 +498,8 @@
poller = malloc(sizeof(*poller));
poller->remove = false;
poller->handler = handler;
- poller->fn = poller_fn;
+ poller->event_fn = poller_fn;
+ poller->timeout_fn = timeout_fn;
poller->data = data;
/* add one to our pollers array */
@@ -547,7 +573,57 @@
console->pollfds[i].events = events;
}
-static int call_pollers(struct console *console)
+void console_poller_set_timeout(struct console *console, struct poller *poller,
+ const struct timeval *tv)
+{
+ struct timeval now;
+ int rc;
+
+ rc = get_current_time(&now);
+ if (rc)
+ return;
+
+ timeradd(&now, tv, &poller->timeout);
+}
+
+static int get_poll_timeout(struct console *console, struct timeval *cur_time)
+{
+ struct timeval *earliest, interval;
+ struct poller *poller;
+ int i;
+
+ earliest = NULL;
+
+ for (i = 0; i < console->n_pollers; i++) {
+ poller = console->pollers[i];
+
+ if (poller->timeout_fn && timerisset(&poller->timeout) &&
+ (!earliest ||
+ (earliest && timercmp(&poller->timeout, earliest, <)))){
+ // poller is buffering data and needs the poll
+ // function to timeout.
+ earliest = &poller->timeout;
+ }
+ }
+
+ if (earliest) {
+ if (timercmp(earliest, cur_time, >)) {
+ /* recalculate the timeout period, time period has
+ * not elapsed */
+ timersub(earliest, cur_time, &interval);
+ return ((interval.tv_sec * 1000) +
+ (interval.tv_usec / 1000));
+ } else {
+ /* return from poll immediately */
+ return 0;
+ }
+ } else {
+ /* poll indefinitely */
+ return -1;
+ }
+}
+
+static int call_pollers(struct console *console, struct timeval *cur_time)
{
struct poller *poller;
struct pollfd *pollfd;
@@ -563,16 +639,33 @@
for (i = 0; i < console->n_pollers; i++) {
poller = console->pollers[i];
pollfd = &console->pollfds[i];
+ prc = POLLER_OK;
- if (!pollfd->revents)
- continue;
+ /* process pending events... */
+ if (pollfd->revents) {
+ prc = poller->event_fn(poller->handler, pollfd->revents,
+ poller->data);
+ if (prc == POLLER_EXIT)
+ rc = -1;
+ else if (prc == POLLER_REMOVE)
+ poller->remove = true;
+ }
- prc = poller->fn(poller->handler, pollfd->revents,
- poller->data);
- if (prc == POLLER_EXIT)
- rc = -1;
- else if (prc == POLLER_REMOVE)
- poller->remove = true;
+ if ((prc == POLLER_OK) && poller->timeout_fn &&
+ timerisset(&poller->timeout) &&
+ timercmp(&poller->timeout, cur_time, <=)) {
+ /* One of the ringbuffer consumers is buffering the
+ data stream. The amount of idle time the consumer
+ desired has expired. Process the buffered data for
+ transmission. */
+ timerclear(&poller->timeout);
+ prc = poller->timeout_fn(poller->handler, poller->data);
+ if (prc == POLLER_EXIT) {
+ rc = -1;
+ } else if (prc == POLLER_REMOVE) {
+ poller->remove = true;
+ }
+ }
}
/**
@@ -606,7 +699,8 @@
int run_console(struct console *console)
{
sighandler_t sighandler_save;
- int rc;
+ struct timeval tv;
+ int rc, timeout;
sighandler_save = signal(SIGINT, sighandler);
@@ -622,8 +716,18 @@
break;
}
+ rc = get_current_time(&tv);
+ if (rc) {
+ warn("Failed to read current time");
+ break;
+ }
+
+ timeout = get_poll_timeout(console, &tv);
+
rc = poll(console->pollfds,
- console->n_pollers + MAX_INTERNAL_POLLFD, -1);
+ console->n_pollers + MAX_INTERNAL_POLLFD,
+ timeout);
+
if (rc < 0) {
if (errno == EINTR) {
continue;
@@ -651,7 +755,7 @@
}
/* ... and then the pollers */
- rc = call_pollers(console);
+ rc = call_pollers(console, &tv);
if (rc)
break;
}
diff --git a/console-server.h b/console-server.h
index cac88d1..d6822c5 100644
--- a/console-server.h
+++ b/console-server.h
@@ -20,7 +20,9 @@
#include <stdbool.h>
#include <stdint.h>
#include <termios.h> /* for speed_t */
+#include <time.h>
#include <systemd/sd-bus.h>
+#include <sys/time.h>
struct console;
struct config;
@@ -72,18 +74,24 @@
POLLER_EXIT,
};
-typedef enum poller_ret (*poller_fn_t)(struct handler *handler,
+typedef enum poller_ret (*poller_event_fn_t)(struct handler *handler,
int revents, void *data);
+typedef enum poller_ret (*poller_timeout_fn_t)(struct handler *handler,
+ void *data);
struct poller *console_poller_register(struct console *console,
- struct handler *handler, poller_fn_t poller_fn,
- int fd, int events, void *data);
+ struct handler *handler, poller_event_fn_t event_fn,
+ poller_timeout_fn_t timeout_fn, int fd, int events,
+ void *data);
void console_poller_unregister(struct console *console, struct poller *poller);
void console_poller_set_events(struct console *console, struct poller *poller,
int events);
+void console_poller_set_timeout(struct console *console, struct poller *poller,
+ const struct timeval *interval);
+
/* ringbuffer API */
enum ringbuffer_poll_ret {
RINGBUFFER_POLL_OK = 0,
@@ -111,6 +119,8 @@
int ringbuffer_dequeue_commit(struct ringbuffer_consumer *rbc, size_t len);
+size_t ringbuffer_len(struct ringbuffer_consumer *rbc);
+
/* console wrapper around ringbuffer consumer registration */
struct ringbuffer_consumer *console_ringbuffer_consumer_register(
struct console *console,
diff --git a/ringbuffer.c b/ringbuffer.c
index 3edab0d..964ab43 100644
--- a/ringbuffer.c
+++ b/ringbuffer.c
@@ -108,7 +108,7 @@
free(rbc);
}
-static size_t ringbuffer_len(struct ringbuffer_consumer *rbc)
+size_t ringbuffer_len(struct ringbuffer_consumer *rbc)
{
if (rbc->pos <= rbc->rb->tail)
return rbc->rb->tail - rbc->pos;
@@ -148,6 +148,9 @@
if (len >= rb->size)
return -1;
+ if (len == 0)
+ return 0;
+
/* Ensure there is at least len bytes of space available.
*
* If a client doesn't have sufficient space, perform a blocking write
@@ -176,6 +179,7 @@
memcpy(rb->buf, data, len);
rb->tail += len;
+
/* Inform consumers of new data in non-blocking mode, by calling
* ->poll_fn with 0 force_len */
for (i = 0; i < rb->n_consumers; i++) {
diff --git a/socket-handler.c b/socket-handler.c
index 1b2cb4f..be7daa4 100644
--- a/socket-handler.c
+++ b/socket-handler.c
@@ -32,6 +32,10 @@
#include "console-server.h"
+#define SOCKET_HANDLER_PKT_SIZE 512
+/* Set poll() timeout to 4000 uS, or 4 mS */
+#define SOCKET_HANDLER_PKT_US_TIMEOUT 4000
+
struct client {
struct socket_handler *sh;
struct poller *poller;
@@ -50,6 +54,11 @@
int n_clients;
};
+static struct timeval const socket_handler_timeout = {
+ .tv_sec = 0,
+ .tv_usec = SOCKET_HANDLER_PKT_US_TIMEOUT
+};
+
static struct socket_handler *to_socket_handler(struct handler *handler)
{
return container_of(handler, struct socket_handler, handler);
@@ -179,7 +188,17 @@
size_t force_len)
{
struct client *client = arg;
- int rc;
+ int rc, len;
+
+ len = ringbuffer_len(client->rbc);
+ if (!force_len && (len < SOCKET_HANDLER_PKT_SIZE)) {
+ /* Do nothing until many small requests have accumulated, or
+ * the UART is idle for awhile (as determined by the timeout
+ * value supplied to the poll function call in console_server.c. */
+ console_poller_set_timeout(client->sh->console, client->poller,
+ &socket_handler_timeout);
+ return RINGBUFFER_POLL_OK;
+ }
rc = client_drain_queue(client, force_len);
if (rc) {
@@ -191,6 +210,26 @@
return RINGBUFFER_POLL_OK;
}
+static enum poller_ret client_timeout(struct handler *handler, void *data)
+{
+ struct client *client = data;
+ int rc = 0;
+
+ if (client->blocked) {
+ /* nothing to do here, we'll call client_drain_queue when
+ * we become unblocked */
+ return POLLER_OK;
+ }
+
+ rc = client_drain_queue(client, 0);
+ if (rc) {
+ client_close(client);
+ return POLLER_REMOVE;
+ }
+
+ return POLLER_OK;
+}
+
static enum poller_ret client_poll(struct handler *handler,
int events, void *data)
{
@@ -248,7 +287,8 @@
client->sh = sh;
client->fd = fd;
client->poller = console_poller_register(sh->console, handler,
- client_poll, client->fd, POLLIN, client);
+ client_poll, client_timeout, client->fd, POLLIN,
+ client);
client->rbc = console_ringbuffer_consumer_register(sh->console,
client_ringbuffer_poll, client);
@@ -297,7 +337,7 @@
}
sh->poller = console_poller_register(console, handler, socket_poll,
- sh->sd, POLLIN, NULL);
+ NULL, sh->sd, POLLIN, NULL);
return 0;
}
diff --git a/tty-handler.c b/tty-handler.c
index a6e622d..3546aca 100644
--- a/tty-handler.c
+++ b/tty-handler.c
@@ -269,7 +269,7 @@
if (make_terminal_raw(th, tty_name) != 0)
fprintf(stderr, "Couldn't make %s a raw terminal\n", tty_name);
- th->poller = console_poller_register(console, handler, tty_poll,
+ th->poller = console_poller_register(console, handler, tty_poll, NULL,
th->fd, POLLIN, NULL);
th->console = console;
th->rbc = console_ringbuffer_consumer_register(console,