All of lore.kernel.org
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@suse.de>
To: linux-serial@vger.kernel.org
Cc: Al Cooper <alcooperx@gmail.com>, stable <stable@kernel.org>,
	Greg Kroah-Hartman <gregkh@suse.de>
Subject: [PATCH 09/12] 8250: Fix race condition in serial8250_backup_timeout().
Date: Fri, 26 Aug 2011 13:58:18 -0700	[thread overview]
Message-ID: <1314392301-25140-9-git-send-email-gregkh@suse.de> (raw)
In-Reply-To: <1314392301-25140-1-git-send-email-gregkh@suse.de>

From: Al Cooper <alcooperx@gmail.com>

This is to fix an issue where output will suddenly become very slow.
The problem occurs on 8250 UARTS with the hardware bug UART_BUG_THRE.

BACKGROUND
For normal UARTs (without UART_BUG_THRE): When the serial core layer
gets new transmit data and the transmitter is idle, it buffers the
data and calls the 8250s' serial8250_start_tx() routine which will
simply enable the TX interrupt in the IER register and return. This
should immediately fire a THRE interrupt and begin transmitting the
data.
For buggy UARTs (with UART_BUG_THRE): merely enabling the TX interrupt
in IER does not necessarily generate a new THRE interrupt.
Therefore, a background timer periodically checks to see if there is
pending data, and starts transmission if that is the case.

The bug happens on SMP systems when the system has nothing to transmit,
the transmit interrupt is disabled and the following sequence occurs:
- CPU0: The background timer routine serial8250_backup_timeout()
  starts and saves the state of the interrupt enable register (IER)
  and then disables all interrupts in IER. NOTE: The transmit interrupt
  (TI) bit is saved as disabled.
- CPU1: The serial core gets data to transmit, grabs the port lock and
  calls serial8250_start_tx() which enables the TI in IER.
- CPU0: serial8250_backup_timeout() waits for the port lock.
- CPU1: finishes (with TI enabled) and releases the port lock.
- CPU0: serial8250_backup_timeout() calls the interrupt routine which
  will transmit the next fifo's worth of data and then restores the
  IER from the previously saved value (TI disabled).
At this point, as long as the serial core has more transmit data
buffered, it will not call serial8250_start_tx() again and the
background timer routine will slowly transmit the data.

The fix is to have serial8250_start_tx() get the port lock before
it saves the IER state and release it after restoring IER. This will
prevent serial8250_start_tx() from running in parallel.

Signed-off-by: Al Cooper <alcooperx@gmail.com>
Cc: stable <stable@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
---
 drivers/tty/serial/8250.c |    8 +++++---
 1 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/tty/serial/8250.c b/drivers/tty/serial/8250.c
index f2dfec8..7f50999 100644
--- a/drivers/tty/serial/8250.c
+++ b/drivers/tty/serial/8250.c
@@ -1819,6 +1819,8 @@ static void serial8250_backup_timeout(unsigned long data)
 	unsigned int iir, ier = 0, lsr;
 	unsigned long flags;
 
+	spin_lock_irqsave(&up->port.lock, flags);
+
 	/*
 	 * Must disable interrupts or else we risk racing with the interrupt
 	 * based handler.
@@ -1836,10 +1838,8 @@ static void serial8250_backup_timeout(unsigned long data)
 	 * the "Diva" UART used on the management processor on many HP
 	 * ia64 and parisc boxes.
 	 */
-	spin_lock_irqsave(&up->port.lock, flags);
 	lsr = serial_in(up, UART_LSR);
 	up->lsr_saved_flags |= lsr & LSR_SAVE_FLAGS;
-	spin_unlock_irqrestore(&up->port.lock, flags);
 	if ((iir & UART_IIR_NO_INT) && (up->ier & UART_IER_THRI) &&
 	    (!uart_circ_empty(&up->port.state->xmit) || up->port.x_char) &&
 	    (lsr & UART_LSR_THRE)) {
@@ -1848,11 +1848,13 @@ static void serial8250_backup_timeout(unsigned long data)
 	}
 
 	if (!(iir & UART_IIR_NO_INT))
-		serial8250_handle_port(up);
+		transmit_chars(up);
 
 	if (is_real_interrupt(up->port.irq))
 		serial_out(up, UART_IER, ier);
 
+	spin_unlock_irqrestore(&up->port.lock, flags);
+
 	/* Standard timer interval plus 0.2s to keep the port running */
 	mod_timer(&up->timer,
 		jiffies + uart_poll_timeout(&up->port) + HZ / 5);
-- 
1.7.6.1


  parent reply	other threads:[~2011-08-26 20:59 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-08-26 19:11 [GIT PATCH] TTY/serial driver fixes for 3.1 Greg KH
2011-08-26 20:58 ` [PATCH 01/12] serial: samsung: Fix build error Greg Kroah-Hartman
2011-08-26 20:58   ` [PATCH 02/12] pch_uart: Set PCIe bus number using probe parameter Greg Kroah-Hartman
2011-08-26 20:58   ` [PATCH 03/12] drivers/serial/ucc_uart.c: Fix compiler warning Greg Kroah-Hartman
2011-08-26 20:58   ` [PATCH 04/12] serial: 8250_pnp: add Intermec CV60 touchscreen device Greg Kroah-Hartman
2011-08-26 20:58   ` [PATCH 05/12] atmel_serial: fix atmel_default_console_device Greg Kroah-Hartman
2011-08-26 20:58   ` [PATCH 06/12] tty: Add "spi:" prefix for spi modalias Greg Kroah-Hartman
2011-08-26 20:58   ` [PATCH 07/12] 8250_pci: add support for Rosewill RC-305 4x serial port card Greg Kroah-Hartman
2011-08-26 20:58   ` [PATCH 08/12] serial/8250_pci: delete duplicate data definition Greg Kroah-Hartman
2011-08-26 20:58   ` Greg Kroah-Hartman [this message]
2011-08-26 20:58   ` [PATCH 10/12] TTY: pty, fix pty counting Greg Kroah-Hartman
2011-08-26 20:58   ` [PATCH 11/12] TTY: serial, document ignoring of uart->ops->startup error Greg Kroah-Hartman
2011-08-26 20:58   ` [PATCH 12/12] omap-serial: Allow IXON and IXOFF to be disabled Greg Kroah-Hartman

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=1314392301-25140-9-git-send-email-gregkh@suse.de \
    --to=gregkh@suse.de \
    --cc=alcooperx@gmail.com \
    --cc=linux-serial@vger.kernel.org \
    --cc=stable@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 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.