From: Peter Zijlstra <a.p.zijlstra@chello.nl>
To: Linus Torvalds <torvalds@linux-foundation.org>,
"Ted Ts'o" <tytso@mit.edu>, Greg KH <greg@kroah.com>,
Ingo Molnar <mingo@elte.hu>, Thomas Gleixner <tglx@linutronix.de>,
akpm@linux-foundation.org
Cc: linux-kernel <linux-kernel@vger.kernel.org>,
Peter Zijlstra <a.p.zijlstra@chello.nl>
Subject: [RFC][PATCH 7/7] serial, 8250: Mostly avoid wakeups from under port->lock
Date: Wed, 21 Dec 2011 11:57:46 +0100 [thread overview]
Message-ID: <20111221111143.678537446@chello.nl> (raw)
In-Reply-To: 20111221105739.798864333@chello.nl
[-- Attachment #1: 8250.patch --]
[-- Type: text/plain, Size: 4882 bytes --]
While not a strict requirement (anymore), it is nice not to issue
wakeups (or have them connected by locks) from console output.
This patch avoids doing most wakeups from under port->lock, which is
connected to 8250console_write(). Only seriously challenged UARTs will
still do the wakeup, but since I don't actually have such a creature,
I'm less inclined to fix it up.
[ INFO: possible circular locking dependency detected ]
3.2.0-rc5-tip+ #185 Tainted: G W
-------------------------------------------------------
watchdog/0/10 is trying to acquire lock:
((console_sem).lock){-.-...}, at:
but task is already holding lock:
(&rt_rq->rt_runtime_lock){-.-...}, at:
which lock already depends on the new lock.
the existing dependency chain (in reverse order) is:
-> #5 (&rt_rq->rt_runtime_lock){-.-...}:
-> #4 (&rq->lock){-.-.-.}:
-> #3 (&p->pi_lock){-.-.-.}:
-> #2 (&tty->write_wait){-.-...}:
-> #1 (&port_lock_key){-.-...}:
-> #0 ((console_sem).lock){-.-...}:
Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
---
drivers/tty/serial/8250.c | 62 +++++++++++++++++++++++++++++++++++++---------
1 file changed, 50 insertions(+), 12 deletions(-)
--- a/drivers/tty/serial/8250.c
+++ b/drivers/tty/serial/8250.c
@@ -1326,7 +1326,7 @@ static void serial8250_stop_tx(struct ua
}
}
-static void transmit_chars(struct uart_8250_port *up);
+static int transmit_chars(struct uart_8250_port *up);
static void serial8250_start_tx(struct uart_port *port)
{
@@ -1343,8 +1343,15 @@ static void serial8250_start_tx(struct u
up->lsr_saved_flags |= lsr & LSR_SAVE_FLAGS;
if ((up->port.type == PORT_RM9000) ?
(lsr & UART_LSR_THRE) :
- (lsr & UART_LSR_TEMT))
- transmit_chars(up);
+ (lsr & UART_LSR_TEMT)) {
+ /*
+ * This will nest a wakeup inside port->lock
+ * and is thus less reliable than a well
+ * functioning uart.
+ */
+ if (transmit_chars(up))
+ uart_write_wakeup(&up->port);
+ }
}
}
@@ -1484,24 +1491,25 @@ receive_chars(struct uart_8250_port *up,
*status = lsr;
}
-static void transmit_chars(struct uart_8250_port *up)
+static int transmit_chars(struct uart_8250_port *up)
{
struct circ_buf *xmit = &up->port.state->xmit;
int count;
+ int ret = 0;
if (up->port.x_char) {
serial_outp(up, UART_TX, up->port.x_char);
up->port.icount.tx++;
up->port.x_char = 0;
- return;
+ goto out;
}
if (uart_tx_stopped(&up->port)) {
serial8250_stop_tx(&up->port);
- return;
+ goto out;
}
if (uart_circ_empty(xmit)) {
__stop_tx(up);
- return;
+ goto out;
}
count = up->tx_loadsz;
@@ -1514,12 +1522,15 @@ static void transmit_chars(struct uart_8
} while (--count > 0);
if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
- uart_write_wakeup(&up->port);
+ ret = 1;
DEBUG_INTR("THRE...");
if (uart_circ_empty(xmit))
__stop_tx(up);
+
+out:
+ return ret;
}
static unsigned int check_modem_status(struct uart_8250_port *up)
@@ -1546,12 +1557,38 @@ static unsigned int check_modem_status(s
}
/*
+ * Unlocks port->lock and issues a uart_write_wakeup() outside of the lock.
+ *
+ * Duplicates parts of uart_write_wakeup().
+ */
+static void
+port_unlock_and_wake(struct uart_8250_port *up, unsigned long flags, int wake)
+{
+ struct uart_state *state;
+ struct tty_struct *tty;
+
+ if (!wake) {
+ spin_unlock_irqrestore(&up->port.lock, flags);
+ return;
+ }
+
+ state = up->port.state;
+ BUG_ON(!state);
+ tty = state->port.tty;
+ tty_kref_get(tty);
+ spin_unlock_irqrestore(&up->port.lock, flags);
+ tty_wakeup(tty);
+ tty_kref_put(tty);
+}
+
+/*
* This handles the interrupt from one port.
*/
static void serial8250_handle_port(struct uart_8250_port *up)
{
unsigned int status;
unsigned long flags;
+ int wakeup = 0;
spin_lock_irqsave(&up->port.lock, flags);
@@ -1563,9 +1600,9 @@ static void serial8250_handle_port(struc
receive_chars(up, &status);
check_modem_status(up);
if (status & UART_LSR_THRE)
- transmit_chars(up);
+ wakeup = transmit_chars(up);
- spin_unlock_irqrestore(&up->port.lock, flags);
+ port_unlock_and_wake(up, flags, wakeup);
}
int serial8250_handle_irq(struct uart_port *port, unsigned int iir)
@@ -1771,6 +1808,7 @@ static void serial8250_backup_timeout(un
struct uart_8250_port *up = (struct uart_8250_port *)data;
unsigned int iir, ier = 0, lsr;
unsigned long flags;
+ int wakeup = 0;
spin_lock_irqsave(&up->port.lock, flags);
@@ -1801,12 +1839,12 @@ static void serial8250_backup_timeout(un
}
if (!(iir & UART_IIR_NO_INT))
- transmit_chars(up);
+ wakeup = transmit_chars(up);
if (is_real_interrupt(up->port.irq))
serial_out(up, UART_IER, ier);
- spin_unlock_irqrestore(&up->port.lock, flags);
+ port_unlock_and_wake(up, flags, wakeup);
/* Standard timer interval plus 0.2s to keep the port running */
mod_timer(&up->timer,
next prev parent reply other threads:[~2011-12-21 11:19 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-12-21 10:57 [RFC][PATCH 0/7] improve printk reliability Peter Zijlstra
2011-12-21 10:57 ` [RFC][PATCH 1/7] arch, early_printk: Consolidate early_printk() implementations Peter Zijlstra
2011-12-21 17:01 ` Mike Frysinger
2011-12-21 17:03 ` Peter Zijlstra
2011-12-21 19:23 ` David Miller
2011-12-21 10:57 ` [RFC][PATCH 2/7] lockdep: Provide early_printk() support Peter Zijlstra
2011-12-21 10:57 ` [RFC][PATCH 3/7] printk, lockdep: Remove lockdep_off() usage Peter Zijlstra
2011-12-21 10:57 ` [RFC][PATCH 4/7] printk: Rework printk recursion Peter Zijlstra
2011-12-21 10:57 ` [RFC][PATCH 5/7] semaphore: Pull wakeup out from under sem->lock Peter Zijlstra
2011-12-21 10:57 ` [RFC][PATCH 6/7] printk: Poke printk extra hard Peter Zijlstra
2011-12-22 1:17 ` Linus Torvalds
2011-12-22 7:02 ` Ingo Molnar
2011-12-22 8:43 ` Peter Zijlstra
2011-12-22 9:03 ` Ingo Molnar
2011-12-22 9:14 ` Peter Zijlstra
2011-12-22 10:15 ` Ingo Molnar
2011-12-22 10:19 ` Peter Zijlstra
2011-12-21 10:57 ` Peter Zijlstra [this message]
2011-12-21 16:03 ` [RFC][PATCH 7/7] serial, 8250: Mostly avoid wakeups from under port->lock Alan Cox
2011-12-21 16:22 ` Peter Zijlstra
2011-12-21 16:30 ` Peter Zijlstra
2011-12-21 18:51 ` Alan Cox
2011-12-21 11:23 ` [RFC][PATCH 0/7] improve printk reliability Peter Zijlstra
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=20111221111143.678537446@chello.nl \
--to=a.p.zijlstra@chello.nl \
--cc=akpm@linux-foundation.org \
--cc=greg@kroah.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@elte.hu \
--cc=tglx@linutronix.de \
--cc=torvalds@linux-foundation.org \
--cc=tytso@mit.edu \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.