mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
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,



  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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

Powered by JetHome