Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit e673927d authored by Jiri Slaby's avatar Jiri Slaby Committed by Greg Kroah-Hartman
Browse files

TTY: ircomm, revamp locking 



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: default avatarJiri Slaby <jslaby@suse.cz>
Cc: Samuel Ortiz <samuel@sortiz.org>
Cc: netdev@vger.kernel.org
Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
parent 849d5a99

include/net/irda/ircomm_tty.h

+0 −1
Original line number Diff line number Diff line
@@ -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...

net/irda/ircomm/ircomm_tty.c

+18 −20
Original line number Diff line number Diff line
@@ -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);

}