All of lore.kernel.org
 help / color / mirror / Atom feed
From: Peter Hurley <peter@hurleysoftware.com>
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Jiri Slaby <jslaby@suse.cz>,
	One Thousand Gnomes <gnomes@lxorguk.ukuu.org.uk>,
	linux-serial@vger.kernel.org, linux-kernel@vger.kernel.org,
	Peter Hurley <peter@hurleysoftware.com>
Subject: [PATCH 22/26] tty: Serialize tty flow control changes with flow_lock
Date: Tue,  2 Sep 2014 17:39:31 -0400	[thread overview]
Message-ID: <1409693975-1028-23-git-send-email-peter@hurleysoftware.com> (raw)
In-Reply-To: <1409693975-1028-1-git-send-email-peter@hurleysoftware.com>

Without serialization, the flow control state can become inverted
wrt. the actual hardware state. For example,

CPU 0                          | CPU 1
stop_tty()                     |
  lock ctrl_lock               |
  tty->stopped = 1             |
  unlock ctrl_lock             |
                               | start_tty()
                               |   lock ctrl_lock
                               |   tty->stopped = 0
                               |   unlock ctrl_lock
                               |   driver->start()
  driver->stop()               |

In this case, the flow control state now indicates the tty has
been started, but the actual hardware state has actually been stopped.

Introduce tty->flow_lock spinlock to serialize tty flow control changes.
Split out unlocked __start_tty()/__stop_tty() flavors for use by
ioctl(TCXONC) in follow-on patch.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/tty/tty_io.c       | 39 ++++++++++++++++++++++++++++-----------
 include/linux/tty.h        |  5 ++++-
 include/linux/tty_driver.h |  4 ++++
 3 files changed, 36 insertions(+), 12 deletions(-)

diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index 714320b..b898e29 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -933,18 +933,18 @@ void no_tty(void)
  *	but not always.
  *
  *	Locking:
- *		Uses the tty control lock internally
+ *		ctrl_lock
+ *		flow_lock
  */
 
-void stop_tty(struct tty_struct *tty)
+void __stop_tty(struct tty_struct *tty)
 {
 	unsigned long flags;
-	spin_lock_irqsave(&tty->ctrl_lock, flags);
-	if (tty->stopped) {
-		spin_unlock_irqrestore(&tty->ctrl_lock, flags);
+
+	if (tty->stopped)
 		return;
-	}
 	tty->stopped = 1;
+	spin_lock_irqsave(&tty->ctrl_lock, flags);
 	if (tty->link && tty->link->packet) {
 		tty->ctrl_status &= ~TIOCPKT_START;
 		tty->ctrl_status |= TIOCPKT_STOP;
@@ -955,6 +955,14 @@ void stop_tty(struct tty_struct *tty)
 		(tty->ops->stop)(tty);
 }
 
+void stop_tty(struct tty_struct *tty)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&tty->flow_lock, flags);
+	__stop_tty(tty);
+	spin_unlock_irqrestore(&tty->flow_lock, flags);
+}
 EXPORT_SYMBOL(stop_tty);
 
 /**
@@ -968,17 +976,17 @@ EXPORT_SYMBOL(stop_tty);
  *
  *	Locking:
  *		ctrl_lock
+ *		flow_lock
  */
 
-void start_tty(struct tty_struct *tty)
+void __start_tty(struct tty_struct *tty)
 {
 	unsigned long flags;
-	spin_lock_irqsave(&tty->ctrl_lock, flags);
-	if (!tty->stopped || tty->flow_stopped) {
-		spin_unlock_irqrestore(&tty->ctrl_lock, flags);
+
+	if (!tty->stopped || tty->flow_stopped)
 		return;
-	}
 	tty->stopped = 0;
+	spin_lock_irqsave(&tty->ctrl_lock, flags);
 	if (tty->link && tty->link->packet) {
 		tty->ctrl_status &= ~TIOCPKT_STOP;
 		tty->ctrl_status |= TIOCPKT_START;
@@ -991,6 +999,14 @@ void start_tty(struct tty_struct *tty)
 	tty_wakeup(tty);
 }
 
+void start_tty(struct tty_struct *tty)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&tty->flow_lock, flags);
+	__start_tty(tty);
+	spin_unlock_irqrestore(&tty->flow_lock, flags);
+}
 EXPORT_SYMBOL(start_tty);
 
 /* We limit tty time update visibility to every 8 seconds or so. */
@@ -3031,6 +3047,7 @@ void initialize_tty_struct(struct tty_struct *tty,
 	INIT_WORK(&tty->hangup_work, do_tty_hangup);
 	mutex_init(&tty->atomic_write_lock);
 	spin_lock_init(&tty->ctrl_lock);
+	spin_lock_init(&tty->flow_lock);
 	INIT_LIST_HEAD(&tty->tty_files);
 	INIT_WORK(&tty->SAK_work, do_SAK_work);
 
diff --git a/include/linux/tty.h b/include/linux/tty.h
index 7cf61cb..19ce455 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -252,6 +252,7 @@ struct tty_struct {
 	struct rw_semaphore termios_rwsem;
 	struct mutex winsize_mutex;
 	spinlock_t ctrl_lock;
+	spinlock_t flow_lock;
 	/* Termios values are protected by the termios rwsem */
 	struct ktermios termios, termios_locked;
 	struct termiox *termiox;	/* May be NULL for unsupported */
@@ -261,7 +262,7 @@ struct tty_struct {
 	unsigned long flags;
 	int count;
 	struct winsize winsize;		/* winsize_mutex */
-	bool stopped;
+	bool stopped;			/* flow_lock */
 	bool hw_stopped;
 	bool flow_stopped;
 	bool packet;
@@ -400,7 +401,9 @@ extern int tty_paranoia_check(struct tty_struct *tty, struct inode *inode,
 extern char *tty_name(struct tty_struct *tty, char *buf);
 extern void tty_wait_until_sent(struct tty_struct *tty, long timeout);
 extern int tty_check_change(struct tty_struct *tty);
+extern void __stop_tty(struct tty_struct *tty);
 extern void stop_tty(struct tty_struct *tty);
+extern void __start_tty(struct tty_struct *tty);
 extern void start_tty(struct tty_struct *tty);
 extern int tty_register_driver(struct tty_driver *driver);
 extern int tty_unregister_driver(struct tty_driver *driver);
diff --git a/include/linux/tty_driver.h b/include/linux/tty_driver.h
index e48c608..92e337c 100644
--- a/include/linux/tty_driver.h
+++ b/include/linux/tty_driver.h
@@ -152,6 +152,8 @@
  * 	This routine notifies the tty driver that it should stop
  * 	outputting characters to the tty device.  
  *
+ *	Called with ->flow_lock held. Serialized with start() method.
+ *
  *	Optional:
  *
  *	Note: Call stop_tty not this method.
@@ -161,6 +163,8 @@
  * 	This routine notifies the tty driver that it resume sending
  *	characters to the tty device.
  *
+ *	Called with ->flow_lock held. Serialized with stop() method.
+ *
  *	Optional:
  *
  *	Note: Call start_tty not this method.
-- 
2.1.0

  parent reply	other threads:[~2014-09-02 21:39 UTC|newest]

Thread overview: 76+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-09-02 21:39 [PATCH 00/26] tty/serial flow control fixes Peter Hurley
2014-09-02 21:39 ` [PATCH 01/26] Revert "serial: uart: add hw flow control support configuration" Peter Hurley
2014-09-02 21:50   ` Murali Karicheri
2014-09-02 21:50     ` Murali Karicheri
2014-09-02 21:58     ` Peter Hurley
2014-09-02 22:17       ` Murali Karicheri
2014-09-02 22:17         ` Murali Karicheri
2014-09-02 21:39 ` [PATCH 02/26] serial: Style fix Peter Hurley
2014-09-02 21:39 ` [PATCH 03/26] serial: imx: Fix x_char handling and tx flow control Peter Hurley
2014-09-02 21:39 ` [PATCH 04/26] serial: core: Fix x_char race Peter Hurley
2014-09-02 21:39 ` [PATCH 05/26] serial: core: Remove unsafe x_char optimization Peter Hurley
2014-09-02 21:39 ` [PATCH 06/26] serial: Fix send_xchar() handlers Peter Hurley
2014-09-02 21:39   ` Peter Hurley
2014-09-02 21:39   ` Peter Hurley
2014-09-02 21:39 ` [PATCH 07/26] serial: mpc52xx: Use default serial core x_char handler Peter Hurley
2014-09-02 21:39 ` [PATCH 08/26] serial: sunsab: Don't enable tx if tx stopped Peter Hurley
2014-09-02 21:39   ` Peter Hurley
2014-09-02 21:39   ` Peter Hurley
2014-09-02 21:39 ` [PATCH 09/26] serial: blackfin: Fix missing gpio.h Peter Hurley
2014-09-02 21:39 ` [PATCH 10/26] serial: core: Document lock requirement for UPF_* flags updates Peter Hurley
2014-09-02 21:39 ` [PATCH 11/26] serial: 8250: Document serial8250_modem_status() locking Peter Hurley
2014-09-02 21:39 ` [PATCH 12/26] serial: core: Unwrap tertiary assignment in uart_handle_dcd_change() Peter Hurley
2014-09-02 21:39 ` [PATCH 13/26] locking: Add non-fatal spin lock assert Peter Hurley
2014-09-03  9:27   ` Peter Zijlstra
2014-09-03 11:20     ` Peter Hurley
2014-09-03 14:40       ` Peter Zijlstra
2014-09-03 14:50         ` Peter Hurley
2014-09-03 15:07           ` Peter Zijlstra
2014-09-04  5:14             ` Ingo Molnar
2014-09-10 11:02               ` Peter Hurley
2014-09-10 13:08                 ` Peter Zijlstra
2014-09-10 14:45                   ` Peter Hurley
2014-09-10 18:34                     ` Greg Kroah-Hartman
2014-09-02 21:39 ` [PATCH 14/26] serial: core: Document and assert lock requirements for irq helpers Peter Hurley
2014-09-02 21:39 ` [PATCH 15/26] serial: core: Privatize modem status enable flags Peter Hurley
2014-09-02 21:39 ` [PATCH 16/26] isdn: i4l: Remove ASYNC_CTS_FLOW Peter Hurley
2014-09-02 21:39   ` Peter Hurley
2014-09-02 21:39 ` [PATCH 17/26] serial: core: Privatize tty->hw_stopped Peter Hurley
2014-09-09  0:29   ` Peter Hurley
2014-09-02 21:39 ` [PATCH 18/26] usb: serial: Remove unused tty->hw_stopped Peter Hurley
2014-09-02 21:39 ` [PATCH 19/26] serial: bfin-uart: Fix auto CTS Peter Hurley
2014-09-02 21:39 ` [PATCH 20/26] serial: core: Use spin_lock_irq() in uart_set_termios() Peter Hurley
2014-09-02 21:39 ` [PATCH 21/26] tty: Convert tty_struct bitfield to bools Peter Hurley
2014-09-03 10:58   ` One Thousand Gnomes
2014-09-03 12:14     ` Peter Hurley
2014-09-03 12:19       ` One Thousand Gnomes
2014-09-03 15:10         ` Peter Hurley
2014-09-03 17:56           ` Greg Kroah-Hartman
2014-09-04 16:08             ` Peter Hurley
2014-09-02 21:39 ` Peter Hurley [this message]
2014-09-02 21:39 ` [PATCH 23/26] tty: Move packet mode flow control notifications to pty driver Peter Hurley
2014-09-02 21:39 ` [PATCH 24/26] tty: Serialize tcflow() with other tty flow control changes Peter Hurley
2014-09-02 21:39 ` [PATCH 25/26] tty: Move and rename send_prio_char() as tty_send_xchar() Peter Hurley
2014-09-02 21:39 ` [PATCH 26/26] tty: Hold termios_rwsem for tcflow(TCIxxx) Peter Hurley
2014-09-08 23:24 ` [PATCH 00/26] tty/serial flow control fixes Greg Kroah-Hartman
2014-09-08 23:33   ` Peter Hurley
2014-09-08 23:43     ` Greg Kroah-Hartman
2014-09-08 23:46       ` Peter Hurley
2014-09-09  0:01         ` Greg Kroah-Hartman
2014-09-10 19:06 ` [PATCH v2 00/14] " Peter Hurley
2014-09-10 19:06   ` [PATCH v2 01/14] serial: core: Document and assert lock requirements for irq helpers Peter Hurley
2014-09-10 19:06   ` [PATCH v2 02/14] serial: core: Privatize modem status enable flags Peter Hurley
2014-09-10 19:06   ` [PATCH v2 03/14] isdn: i4l: Remove ASYNC_CTS_FLOW Peter Hurley
2014-09-10 19:06     ` Peter Hurley
2014-09-10 19:06   ` [PATCH v2 04/14] serial: core: Privatize tty->hw_stopped Peter Hurley
2014-09-10 19:06   ` [PATCH v2 05/14] usb: serial: Remove unused tty->hw_stopped Peter Hurley
2014-09-23 15:33     ` Johan Hovold
2014-09-10 19:06   ` [PATCH v2 06/14] serial: bfin-uart: Fix auto CTS Peter Hurley
2014-09-10 19:06   ` [PATCH v2 07/14] serial: core: Use spin_lock_irq() in uart_set_termios() Peter Hurley
2014-09-10 19:06   ` [PATCH v2 08/14] tty: Convert tty_struct bitfield to ints Peter Hurley
2014-09-10 19:06   ` [PATCH v2 09/14] tty: Serialize tty flow control changes with flow_lock Peter Hurley
2014-09-10 19:06   ` [PATCH v2 10/14] tty: Move packet mode flow control notifications to pty driver Peter Hurley
2014-09-10 19:06   ` [PATCH v2 11/14] tty: Serialize tcflow() with other tty flow control changes Peter Hurley
2014-09-10 19:06   ` [PATCH v2 12/14] tty: Move and rename send_prio_char() as tty_send_xchar() Peter Hurley
2014-09-10 19:06   ` [PATCH v2 13/14] tty: Hold termios_rwsem for tcflow(TCIxxx) Peter Hurley
2014-09-10 19:06   ` [PATCH v2 14/14] tty: Workaround Alpha non-atomic byte storage in tty_struct Peter Hurley

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=1409693975-1028-23-git-send-email-peter@hurleysoftware.com \
    --to=peter@hurleysoftware.com \
    --cc=gnomes@lxorguk.ukuu.org.uk \
    --cc=gregkh@linuxfoundation.org \
    --cc=jslaby@suse.cz \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-serial@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 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.