netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jiri Slaby <jslaby@suse.cz>
To: gregkh@linuxfoundation.org
Cc: alan@linux.intel.com, linux-kernel@vger.kernel.org,
	jirislaby@gmail.com, Samuel Ortiz <samuel@sortiz.org>,
	netdev@vger.kernel.org
Subject: [PATCH 06/24] TTY: ircomm, revamp locking
Date: Mon,  4 Jun 2012 13:35:20 +0200	[thread overview]
Message-ID: <1338809738-18967-7-git-send-email-jslaby@suse.cz> (raw)
In-Reply-To: <1338809738-18967-1-git-send-email-jslaby@suse.cz>

Use self->spinlock only for ctrl_skb and tx_skb. TTY stuff is now
protected by tty_port->lock. This is needed for further cleanup (and
conversion to tty_port helpers).

This also closes the race in the end of close.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Cc: Samuel Ortiz <samuel@sortiz.org>
Cc: netdev@vger.kernel.org
---
 include/net/irda/ircomm_tty.h |    1 -
 net/irda/ircomm/ircomm_tty.c  |   38 ++++++++++++++++++--------------------
 2 files changed, 18 insertions(+), 21 deletions(-)

diff --git a/include/net/irda/ircomm_tty.h b/include/net/irda/ircomm_tty.h
index e4db3b5..a9027d8 100644
--- a/include/net/irda/ircomm_tty.h
+++ b/include/net/irda/ircomm_tty.h
@@ -96,7 +96,6 @@ struct ircomm_tty_cb {
 	struct work_struct  tqueue;
 
 	/* Protect concurent access to :
-	 *	o self->open_count
 	 *	o self->ctrl_skb
 	 *	o self->tx_skb
 	 * Maybe other things may gain to be protected as well...
diff --git a/net/irda/ircomm/ircomm_tty.c b/net/irda/ircomm/ircomm_tty.c
index 8e61026..7b2152c 100644
--- a/net/irda/ircomm/ircomm_tty.c
+++ b/net/irda/ircomm/ircomm_tty.c
@@ -283,13 +283,12 @@ static int ircomm_tty_block_til_ready(struct ircomm_tty_cb *self,
 	IRDA_DEBUG(2, "%s(%d):block_til_ready before block on %s open_count=%d\n",
 	      __FILE__, __LINE__, tty->driver->name, self->port.count);
 
-	/* As far as I can see, we protect port.count - Jean II */
-	spin_lock_irqsave(&self->spinlock, flags);
+	spin_lock_irqsave(&self->port.lock, flags);
 	if (!tty_hung_up_p(filp)) {
 		extra_count = 1;
 		self->port.count--;
 	}
-	spin_unlock_irqrestore(&self->spinlock, flags);
+	spin_unlock_irqrestore(&self->port.lock, flags);
 	self->port.blocked_open++;
 
 	while (1) {
@@ -340,9 +339,9 @@ static int ircomm_tty_block_til_ready(struct ircomm_tty_cb *self,
 
 	if (extra_count) {
 		/* ++ is not atomic, so this should be protected - Jean II */
-		spin_lock_irqsave(&self->spinlock, flags);
+		spin_lock_irqsave(&self->port.lock, flags);
 		self->port.count++;
-		spin_unlock_irqrestore(&self->spinlock, flags);
+		spin_unlock_irqrestore(&self->port.lock, flags);
 	}
 	self->port.blocked_open--;
 
@@ -409,12 +408,12 @@ static int ircomm_tty_open(struct tty_struct *tty, struct file *filp)
 		hashbin_insert(ircomm_tty, (irda_queue_t *) self, line, NULL);
 	}
 	/* ++ is not atomic, so this should be protected - Jean II */
-	spin_lock_irqsave(&self->spinlock, flags);
+	spin_lock_irqsave(&self->port.lock, flags);
 	self->port.count++;
 
 	tty->driver_data = self;
 	self->tty = tty;
-	spin_unlock_irqrestore(&self->spinlock, flags);
+	spin_unlock_irqrestore(&self->port.lock, flags);
 
 	IRDA_DEBUG(1, "%s(), %s%d, count = %d\n", __func__ , tty->driver->name,
 		   self->line, self->port.count);
@@ -495,10 +494,10 @@ static void ircomm_tty_close(struct tty_struct *tty, struct file *filp)
 	IRDA_ASSERT(self != NULL, return;);
 	IRDA_ASSERT(self->magic == IRCOMM_TTY_MAGIC, return;);
 
-	spin_lock_irqsave(&self->spinlock, flags);
+	spin_lock_irqsave(&self->port.lock, flags);
 
 	if (tty_hung_up_p(filp)) {
-		spin_unlock_irqrestore(&self->spinlock, flags);
+		spin_unlock_irqrestore(&self->port.lock, flags);
 
 		IRDA_DEBUG(0, "%s(), returning 1\n", __func__ );
 		return;
@@ -524,20 +523,15 @@ static void ircomm_tty_close(struct tty_struct *tty, struct file *filp)
 		self->port.count = 0;
 	}
 	if (self->port.count) {
-		spin_unlock_irqrestore(&self->spinlock, flags);
+		spin_unlock_irqrestore(&self->port.lock, flags);
 
 		IRDA_DEBUG(0, "%s(), open count > 0\n", __func__ );
 		return;
 	}
 
-	/* Hum... Should be test_and_set_bit ??? - Jean II */
 	set_bit(ASYNCB_CLOSING, &self->port.flags);
 
-	/* We need to unlock here (we were unlocking at the end of this
-	 * function), because tty_wait_until_sent() may schedule.
-	 * I don't know if the rest should be protected somehow,
-	 * so someone should check. - Jean II */
-	spin_unlock_irqrestore(&self->spinlock, flags);
+	spin_unlock_irqrestore(&self->port.lock, flags);
 
 	/*
 	 * Now we wait for the transmit buffer to clear; and we notify
@@ -552,16 +546,21 @@ static void ircomm_tty_close(struct tty_struct *tty, struct file *filp)
 	tty_driver_flush_buffer(tty);
 	tty_ldisc_flush(tty);
 
+	spin_lock_irqsave(&self->port.lock, flags);
 	tty->closing = 0;
 	self->tty = NULL;
 
 	if (self->port.blocked_open) {
-		if (self->port.close_delay)
+		if (self->port.close_delay) {
+			spin_unlock_irqrestore(&self->port.lock, flags);
 			schedule_timeout_interruptible(self->port.close_delay);
+			spin_lock_irqsave(&self->port.lock, flags);
+		}
 		wake_up_interruptible(&self->port.open_wait);
 	}
 
 	self->port.flags &= ~(ASYNC_NORMAL_ACTIVE|ASYNC_CLOSING);
+	spin_unlock_irqrestore(&self->port.lock, flags);
 	wake_up_interruptible(&self->port.close_wait);
 }
 
@@ -1003,12 +1002,11 @@ static void ircomm_tty_hangup(struct tty_struct *tty)
 	/* ircomm_tty_flush_buffer(tty); */
 	ircomm_tty_shutdown(self);
 
-	/* I guess we need to lock here - Jean II */
-	spin_lock_irqsave(&self->spinlock, flags);
+	spin_lock_irqsave(&self->port.lock, flags);
 	self->port.flags &= ~ASYNC_NORMAL_ACTIVE;
 	self->tty = NULL;
 	self->port.count = 0;
-	spin_unlock_irqrestore(&self->spinlock, flags);
+	spin_unlock_irqrestore(&self->port.lock, flags);
 
 	wake_up_interruptible(&self->port.open_wait);
 }
-- 
1.7.10.3

  parent reply	other threads:[~2012-06-04 11:35 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <1338809738-18967-1-git-send-email-jslaby@suse.cz>
2012-06-04 11:35 ` [PATCH 02/24] TTY: ircomm, add tty_port Jiri Slaby
2012-06-04 11:35 ` [PATCH 03/24] TTY: ircomm, use close times from tty_port Jiri Slaby
2012-06-04 11:35 ` [PATCH 04/24] TTY: ircomm, use open counts " Jiri Slaby
2012-06-04 16:36   ` Alan Cox
2012-06-04 11:35 ` [PATCH 05/24] TTY: ircomm, use flags " Jiri Slaby
2012-06-04 11:35 ` Jiri Slaby [this message]
2012-06-04 16:57   ` [PATCH 06/24] TTY: ircomm, revamp locking Alan Cox
2012-06-04 11:35 ` [PATCH 07/24] TTY: ircomm, use tty from tty_port Jiri Slaby
2012-06-04 11:35 ` [PATCH 08/24] TTY: ircomm, define local tty_port Jiri Slaby
2012-06-04 11:35 ` [PATCH 09/24] TTY: ircomm, define carrier routines Jiri Slaby
2012-06-04 11:35 ` [PATCH 10/24] TTY: ircomm, use tty_port_close_end helper Jiri Slaby
2012-06-04 11:35 ` [PATCH 11/24] TTY: ircomm, use tty_port_close_start helper Jiri Slaby

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=1338809738-18967-7-git-send-email-jslaby@suse.cz \
    --to=jslaby@suse.cz \
    --cc=alan@linux.intel.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=jirislaby@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=samuel@sortiz.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;
as well as URLs for NNTP newsgroup(s).