From: "George Spelvin" <linux@horizon.com>
To: linux-kernel@vger.kernel.org
Cc: linux@horizon.com
Subject: [RFC 2/2] serial/8250.c: Use self-adjusting list for port poll order.
Date: Fri, 14 Nov 2008 16:34:49 -0500 [thread overview]
Message-ID: <20081114213449.14960.qmail@science.horizon.com> (raw)
In-Reply-To: <20081114053314.12093.qmail@science.horizon.com>
I posted this to linux-serial yesterday, but it has attracted no comment
yet, so I'm submitting it to a wider audience. It boots and runs on
amd64 with a standard (non-shared) serial port, but I haven't subjected
it to detailed testing. Comments about the code style and general idea
are requested.
This is the interesting patch. It changes the uart_8250_port list to
a self-adjusting one. Busy ports move to the front, and idle ones
migrate to the end. This reduces overall polling effort when there is
a mix of busy and idle ports.
Any time you're dealing with an edge-sensitive interrupt, it is essential
that the interrupt is turned off before returning from the handler.
With a level-sensitive one, you'll just loop back into the handler,
but an edge-sensitive one will never respond again.
When there are multiple independent sources, that means that you have to
poll them all repeatedly until everyone reports idle. Since interrupts don't
go away by themselves, that means that there was an instant, just before
the final pass started, when there were no active interrupt sources.
Now on modern computers even PCI bus reads are painfully slow, and
super-I/O chips connected via the "LPC bus" low-pin count emulated ISA
bus (basically a 4-bit/33 MHz bus which acts like a 16-bit/8.33 MHz
ISA bus) are truly agonizing.
We can't get around the need to poll every port sharing the same IRQ line,
but if we can predict which ports will be busy and poll them first,
we can start the final verification pass as early as possible.
This code does that by using the previous poll cycles as a hint.
If a port is idle, it will migrate to the end of the list and
only have to be checked once.
Part 1 changed the list to singly-linked to make the list shuffling easier.
Comments? Please?
---
drivers/serial/8250.c | 49 ++++++++++++++++++++++++++++++++++---------------
1 files changed, 34 insertions(+), 15 deletions(-)
diff --git a/drivers/serial/8250.c b/drivers/serial/8250.c
index 7f66335..96e784f 100644
--- a/drivers/serial/8250.c
+++ b/drivers/serial/8250.c
@@ -1469,18 +1469,27 @@ serial8250_handle_port(struct uart_8250_port *up)
* "end" points to the beginning of the most recent run of idle ports.
* It is NULL if the current port is not idle. The loop ends when we
* are about to check end again.
+ *
+ * Optimization: this will finish sooner if we can check all busy
+ * ports first, starting the final verify-idle pass as soon as possible.
+ * To achieve this, busy ports are moved to the front of the list.
+ * *tailp marks the boundary between the front and the back of the list;
+ * when a port is found to be busy, it is removed from between *upp
+ * and upp->next, and reinserted at *tailp.
*/
static irqreturn_t serial8250_interrupt(int irq, void *dev_id)
{
struct irq_info *i = dev_id;
- struct uart_8250_port *up, *end = NULL;
+ struct uart_8250_port *up, **upp, *end = NULL, **tailp;
int pass_counter = 0, handled = 0;
DEBUG_INTR("serial8250_interrupt(%d)...", irq);
+ tailp = upp = &i->head;
+
spin_lock(&i->lock);
- up = i->head;
+ up = *upp;
do {
unsigned int iir;
@@ -1491,7 +1500,7 @@ static irqreturn_t serial8250_interrupt(int irq, void *dev_id)
"irq%d\n", irq);
break;
}
- up = i->head;
+ tailp = upp = &i->head;
continue;
}
@@ -1499,11 +1508,8 @@ static irqreturn_t serial8250_interrupt(int irq, void *dev_id)
if (!(iir & UART_IIR_NO_INT)) {
serial8250_handle_port(up);
- handled = 1;
-
- end = NULL;
- } else if (up->port.iotype == UPIO_DWAPB &&
- (iir & UART_IIR_BUSY) == UART_IIR_BUSY) {
+ } else if ((iir & UART_IIR_BUSY) == UART_IIR_BUSY &&
+ up->port.iotype == UPIO_DWAPB) {
/* The DesignWare APB UART has an Busy Detect (0x07)
* interrupt meaning an LCR write attempt occured
* while the UART was busy. The interrupt must
@@ -1513,14 +1519,27 @@ static irqreturn_t serial8250_interrupt(int irq, void *dev_id)
status = *(volatile u32 *)up->port.private_data;
serial_out(up, UART_LCR, up->lcr);
- handled = 1;
-
- end = NULL;
- } else if (end == NULL)
- end = up;
+ } else {
+ /* Port does not have interrupt asserted. */
+ if (end == NULL)
+ end = up; /* Start of run of idle ports */
+ upp = &up->next; /* Do not move anything */
+ continue;
+ }
- up = up->next;
- } while (up != end);
+ /* up had activity */
+ handled = 1;
+ end = NULL;
+ /* Move to front of list; append after *tailp */
+ if (tailp != upp) {
+ *upp = up->next;
+ up->next = *tailp;
+ *tailp = up;
+ tailp = &up->next;
+ } else {
+ tailp = upp = &up->next;
+ }
+ } while ((up = *upp) != end);
spin_unlock(&i->lock);
--
1.5.6.5
next prev parent reply other threads:[~2008-11-14 21:37 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20081113150308.3590.qmail@science.horizon.com>
[not found] ` <20081114053042.11532.qmail@science.horizon.com>
2008-11-14 21:17 ` [RFC 1/2] serial/8250.c: Change to singly linked handler chain George Spelvin
[not found] ` <20081114053314.12093.qmail@science.horizon.com>
2008-11-14 21:34 ` George Spelvin [this message]
2008-11-14 21:47 ` [RFC 2/2] serial/8250.c: Use self-adjusting list for port poll order Alan Cox
2008-11-15 0:10 ` Theodore Tso
2008-11-16 15:23 ` George Spelvin
2008-11-16 15:51 ` Alan Cox
2008-11-16 16:57 ` George Spelvin
2008-11-16 16:23 ` Theodore Tso
2008-11-16 18:02 ` George Spelvin
2008-11-16 16:49 ` Theodore Tso
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20081114213449.14960.qmail@science.horizon.com \
--to=linux@horizon.com \
--cc=linux-kernel@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox