[PATCH] drivers/tty/serial/8250 : Add throttling capability to aspeed serial
Balbir Singh
bsingharora at gmail.com
Tue Nov 14 01:04:36 AEDT 2017
This patch implements a throttle mechanism to the vuart aspeed serial
driver. The logic is contained in the file itself and should probably
move to 8250_port.c, so that it provides the necessary API to disable
IER and move to a timer. Right now we have some built in constants
that decide how many interrupts are too many, what the time period
for the backup timer and interrupt check is.
The logic then uses a kernel thread to trigger disabling IER as needed
and move to the timer. When the code detects that the interrupt storm
has slowed down, it restores interrupts.
Without a throttle mechanism, the system itself can spend a whole lot
of time handling interrupts from the serial port and this can affect
responsiveness and usability of the openbmc system itself.
Signed-off-by: Balbir Singh <bsingharora at gmail.com>
---
NOTES: There's probably an alternative way to solve this problem
see https://github.com/jk-ozlabs/linux/commit/b1cec3eafb60c7e815932cd674cc082c1acaa01e
I am posting this approach, in my tests I found that the throughput
was higher (measured externally from the system). The tty throttle
was a little more aggressive. In fact some of the IER enable/disable
logic is common.
Nick Piggin recommended that we poll until port->handle_irq returned 0.
I'll probably do a follow up patch for that, doing that now requires
a global variable to communicate the state of handle_irq between the
thread and timer context. I can do that in timer_list's data member,
but I'll follow up with changes
drivers/tty/serial/8250/8250_aspeed_vuart.c | 93 +++++++++++++++++++++++++++++
1 file changed, 93 insertions(+)
diff --git a/drivers/tty/serial/8250/8250_aspeed_vuart.c b/drivers/tty/serial/8250/8250_aspeed_vuart.c
index 822be49..5a67887 100644
--- a/drivers/tty/serial/8250/8250_aspeed_vuart.c
+++ b/drivers/tty/serial/8250/8250_aspeed_vuart.c
@@ -15,6 +15,10 @@
#include <linux/of_irq.h>
#include <linux/of_platform.h>
#include <linux/clk.h>
+#include <linux/kthread.h>
+#include <linux/kernel_stat.h>
+#include <linux/freezer.h>
+#include <linux/timer.h>
#include "8250.h"
@@ -32,6 +36,9 @@ struct aspeed_vuart {
void __iomem *regs;
struct clk *clk;
int line;
+ int irq;
+ struct task_struct *irq_storm_task;
+ struct timer_list backup_timer;
};
/*
@@ -183,6 +190,87 @@ static void aspeed_vuart_shutdown(struct uart_port *uart_port)
serial8250_do_shutdown(uart_port);
}
+/*
+ * Ideally this belongs to 8250_port.c, but our requirements
+ * might be specifically limited to our driver, not sure if
+ * pushing it to the layer above makes a lot of sense, but
+ * we do end up using routines that ideally should be avoided.
+ */
+static void aspeed_vuart_backup_timeout(unsigned long data)
+{
+ struct aspeed_vuart *vuart = (struct aspeed_vuart *)data;
+ struct uart_8250_port *up = serial8250_get_port(vuart->line);
+
+ up->port.handle_irq(&up->port);
+ mod_timer(&vuart->backup_timer, jiffies + HZ / 10);
+}
+
+static void aspeed_vuart_irqs_to_timeout(struct aspeed_vuart *vuart,
+ bool disable_irqs)
+{
+ struct uart_8250_port *up = serial8250_get_port(vuart->line);
+ unsigned long flags;
+
+ if (!up)
+ return;
+
+ spin_lock_irqsave(&up->port.lock, flags);
+ if (!disable_irqs) {
+ /*
+ * Unthrottle interrupts
+ */
+ up->ier |= (UART_IER_RLSI | UART_IER_RDI);
+ serial_out(up, UART_IER, up->ier);
+ del_timer_sync(&vuart->backup_timer);
+ } else {
+ /*
+ * Must disable interrupts and move to timer.
+ */
+ up->ier &= ~(UART_IER_RLSI | UART_IER_RDI);
+ serial_out(up, UART_IER, up->ier);
+ init_timer(&vuart->backup_timer);
+ vuart->backup_timer.data = (unsigned long)vuart;
+ vuart->backup_timer.function = aspeed_vuart_backup_timeout;
+ mod_timer(&vuart->backup_timer, jiffies + HZ / 10);
+ }
+ spin_unlock_irqrestore(&up->port.lock, flags);
+}
+
+static int aspeed_vuart_irq_storm_thread(void *data)
+{
+ static unsigned long irq_count; /* ignore overflows for now */
+ static bool irq_registered = true;
+ unsigned long current_irq_count;
+ struct aspeed_vuart *vuart = (struct aspeed_vuart *)data;
+
+ /* This value needs to be tuned as needed */
+ const int max_irqs = 15000;
+
+ set_freezable();
+ do {
+ try_to_freeze();
+
+ current_irq_count = kstat_irqs(vuart->irq);
+
+ if ((current_irq_count - irq_count) > max_irqs) {
+ if (irq_registered) {
+ aspeed_vuart_irqs_to_timeout(vuart, true);
+ irq_registered = false;
+ }
+ } else if (!irq_registered) {
+ aspeed_vuart_irqs_to_timeout(vuart, false);
+ irq_registered = true;
+ }
+
+ irq_count = current_irq_count;
+ set_current_state(TASK_INTERRUPTIBLE);
+ schedule_timeout(15 * HZ);
+ __set_current_state(TASK_RUNNING);
+ } while (!kthread_should_stop());
+
+ return 0;
+}
+
static int aspeed_vuart_probe(struct platform_device *pdev)
{
struct uart_8250_port port;
@@ -276,11 +364,14 @@ static int aspeed_vuart_probe(struct platform_device *pdev)
goto err_clk_disable;
vuart->line = rc;
+ vuart->irq = port.port.irq;
aspeed_vuart_set_enabled(vuart, true);
aspeed_vuart_set_host_tx_discard(vuart, true);
platform_set_drvdata(pdev, vuart);
+ vuart->irq_storm_task = kthread_run(aspeed_vuart_irq_storm_thread,
+ vuart, "vuart_irq_storm");
return 0;
err_clk_disable:
@@ -294,6 +385,8 @@ static int aspeed_vuart_remove(struct platform_device *pdev)
struct aspeed_vuart *vuart = platform_get_drvdata(pdev);
aspeed_vuart_set_enabled(vuart, false);
+ kthread_stop(vuart->irq_storm_task);
+ del_timer_sync(&vuart->backup_timer);
serial8250_unregister_port(vuart->line);
sysfs_remove_group(&vuart->dev->kobj, &aspeed_vuart_attr_group);
clk_disable_unprepare(vuart->clk);
--
2.7.4
More information about the openbmc
mailing list