[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