Linux Serial subsystem development
 help / color / mirror / Atom feed
* Re: [PATCH 1/9] tty: make receive_buf() return the amout of bytes received
From: Alan Cox @ 2012-03-05 10:40 UTC (permalink / raw)
  To: Matthieu CASTET; +Cc: linux-serial, Felipe Balbi, Toby Gray, Stefan Bigler
In-Reply-To: <1330883960-8563-2-git-send-email-castet.matthieu@free.fr>

> For a first step, will always return that we received everything.
> This make sure to we don't break anything when we will check receive_buf
> return value.


What about recursive calls ? We have some of those due to bugs elsewhere
in the code - eg selection.c. I don't wish to throw spanners in the works
of this effort but you can't actually make it work until you fix

selection.c
tty_io.c: tiocsti
n_tty: n_tty_set_room

The first two are unsafe recursive calls, the third one is totally broken
on any port with low latency set and can result in all kinds of
interesting failure cases.

Basically you've got to actually address the fact that code is calling
the ldisc methods wrongly, in parallel, without locking, and in an unsafe
fashion that in some cases can arbitarily recurse.

Until that is done you are never going to be able to debug the changes
because it'll crash anyway if you poke it the right way.

Alan

^ permalink raw reply

* Re: [PATCH 9/9] tty : make receive_room internal to N_TTY line discipline
From: Alan Cox @ 2012-03-05 10:36 UTC (permalink / raw)
  To: Matthieu CASTET; +Cc: linux-serial, Felipe Balbi, Toby Gray, Stefan Bigler
In-Reply-To: <1330883960-8563-10-git-send-email-castet.matthieu@free.fr>

On Sun,  4 Mar 2012 18:59:20 +0100
Matthieu CASTET <castet.matthieu@free.fr> wrote:

> NOTE : capi need fixing

capi should be listening to the tty flow control responses too.

You've made another significant undocumented change in this lot - you've
removed the ability of an ldisc to control the largest chunk of data that
may be thrown at it per call to the ldisc. I don't think anyone cares
about it, but it needs checking and clearly documenting as its a
significant API change.

^ permalink raw reply

* Re: [PATCH 8/9] tty : kill receive_room usage in mxser
From: Alan Cox @ 2012-03-05 10:34 UTC (permalink / raw)
  To: Matthieu CASTET; +Cc: linux-serial, Felipe Balbi, Toby Gray, Stefan Bigler
In-Reply-To: <1330883960-8563-9-git-send-email-castet.matthieu@free.fr>

>  			ch = inb(port->ioaddr + UART_RX);
> -			tty_insert_flip_char(tty, ch, 0);
> +			ret = tty_insert_flip_char(tty, ch, 0);
> +			if (ret != 1) {
> +				if (!port->ldisc_stop_rx)
> +					mxser_stoprx(tty);
> +			}

This can all just go. The tty layer calls down for stop/start events and
the flow control handles it. There is enough asynchronous buffering to
handle the rest. Just insert them and honour stop/start. By the time
tty_insert_flip_char returns 0 you are in an out of memory/64K buffering
full *after* being told to stop situation.

^ permalink raw reply

* Re: [PATCH 7/9] tty : kill receive_room usage in tty_buffer
From: Alan Cox @ 2012-03-05 10:31 UTC (permalink / raw)
  To: Matthieu CASTET; +Cc: linux-serial, Felipe Balbi, Toby Gray, Stefan Bigler
In-Reply-To: <1330883960-8563-8-git-send-email-castet.matthieu@free.fr>

On Sun,  4 Mar 2012 18:59:18 +0100
Matthieu CASTET <castet.matthieu@free.fr> wrote:

> Signed-off-by: Matthieu CASTET <castet.matthieu@free.fr>
> ---
>  drivers/tty/tty_buffer.c |   10 ++++------
>  1 files changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/tty/tty_buffer.c b/drivers/tty/tty_buffer.c
> index 0e0f0fa..8cc7ceb 100644
> --- a/drivers/tty/tty_buffer.c
> +++ b/drivers/tty/tty_buffer.c
> @@ -437,17 +437,15 @@ static void flush_to_ldisc(struct work_struct *work)
>  			 */
>  			if (test_bit(TTY_LDISC_CHANGING, &tty->flags))
>  				break;
> -			if (!tty->receive_room)
> -				break;
> -			if (count > tty->receive_room)
> -				count = tty->receive_room;
>  			char_buf = head->char_buf_ptr + head->read;
>  			flag_buf = head->flag_buf_ptr + head->read;
> -			head->read += count;
>  			spin_unlock_irqrestore(&tty->buf.lock, flags);
> -			disc->ops->receive_buf(tty, char_buf,
> +			count = disc->ops->receive_buf(tty, char_buf,
>  							flag_buf, count);
>  			spin_lock_irqsave(&tty->buf.lock, flags);
> +			if (count == 0)
> +				break;
> +			head->read += count;

If you've not fixed the parallel calls bugs head->read can now be
invalid. Previously it was at least vaguely self consistent because the
lock was held for the entire calculation and head->read set up before the
buffer want to the ldisc, now its updated some arbitary time in the
future by which point one of the buggy parallel callers may have messed
it up.

^ permalink raw reply

* Re: [PATCH 4/9] tty : kill receive_room usage in vt
From: Alan Cox @ 2012-03-05 10:29 UTC (permalink / raw)
  To: Matthieu CASTET; +Cc: linux-serial, Felipe Balbi, Toby Gray, Stefan Bigler
In-Reply-To: <1330883960-8563-5-git-send-email-castet.matthieu@free.fr>

On Sun,  4 Mar 2012 18:59:15 +0100
Matthieu CASTET <castet.matthieu@free.fr> wrote:

> Signed-off-by: Matthieu CASTET <castet.matthieu@free.fr>
> ---
>  drivers/tty/vt/selection.c |    3 +--
>  1 files changed, 1 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/tty/vt/selection.c b/drivers/tty/vt/selection.c
> index 7a0a12a..3e9a914 100644
> --- a/drivers/tty/vt/selection.c
> +++ b/drivers/tty/vt/selection.c
> @@ -332,8 +332,7 @@ int paste_selection(struct tty_struct *tty)
>  			continue;
>  		}
>  		count = sel_buffer_lth - pasted;
> -		count = min(count, tty->receive_room);
> -		tty->ldisc->ops->receive_buf(tty, sel_buffer + pasted,
> +		count = tty->ldisc->ops->receive_buf(tty, sel_buffer + pasted,
>  								NULL, count);

This is unsafe in your model - it's not valid to call it as selection
does in the first place. You have two parallel consumers here so any
result is nonsense (and it'll make messes elsewhere)

It's unsafe in the existing model too, but the existing one will recover
whereas once you switch behaviour the new one won't.


^ permalink raw reply

* [PATCH 0/9] [RFC] tty : make receive_room internal to N_TTY line discipline
From: matthieu castet @ 2012-03-04 18:05 UTC (permalink / raw)
  To: Alan Cox, linux-serial, Felipe Balbi, Toby Gray, Stefan Bigler

These patches try to solve issue exposed on
https://lkml.org/lkml/2011/6/7/700

On Tue, Jun 07, 2011 at 07:44:48PM -0700, Linus Torvalds wrote
> I'd love to get rid of receive_room entirely - and just letting the
> tty line discipline handler say how much it actually received. in
> other words, having receive_buf() just tell us how much it used, and
> not looking at receive_room in the caller is absolutely the right
> thing.

GIT: [PATCH 1/9] tty: make receive_buf() return the amout of bytes
GIT: [PATCH 2/9] tty : make n_tty_receive_buf return bytes received in
GIT: [PATCH 3/9] tty: make hci_uart_tty_receive return bytes received
GIT: [PATCH 4/9] tty : kill receive_room usage in vt
GIT: [PATCH 5/9] tty : kill receive_room usage in speakup
GIT: [PATCH 6/9] tty : don't use receive_room in tty_set_ldisc
GIT: [PATCH 7/9] tty : kill receive_room usage in tty_buffer
GIT: [PATCH 8/9] tty : kill receive_room usage in mxser
GIT: [PATCH 9/9] tty : make receive_room internal to N_TTY line

^ permalink raw reply

* [PATCH 9/9] tty : make receive_room internal to N_TTY line discipline
From: Matthieu CASTET @ 2012-03-04 17:59 UTC (permalink / raw)
  To: Alan Cox, linux-serial, Felipe Balbi, Toby Gray, Stefan Bigler
  Cc: Matthieu CASTET
In-Reply-To: <1330883960-8563-1-git-send-email-castet.matthieu@free.fr>

NOTE : capi need fixing

Signed-off-by: Matthieu CASTET <castet.matthieu@free.fr>
---
 drivers/bluetooth/hci_ldisc.c  |    1 -
 drivers/input/serio/serport.c  |    1 -
 drivers/isdn/capi/capi.c       |    1 +
 drivers/misc/ti-st/st_core.c   |    3 ---
 drivers/net/caif/caif_serial.c |    1 -
 drivers/net/can/slcan.c        |    1 -
 drivers/net/hamradio/6pack.c   |    1 -
 drivers/net/hamradio/mkiss.c   |    1 -
 drivers/net/irda/irtty-sir.c   |    1 -
 drivers/net/ppp/ppp_async.c    |    1 -
 drivers/net/ppp/ppp_synctty.c  |    1 -
 drivers/net/slip/slip.c        |    1 -
 drivers/net/wan/x25_asy.c      |    1 -
 drivers/tty/n_gsm.c            |    1 -
 drivers/tty/n_hdlc.c           |    1 -
 drivers/tty/n_r3964.c          |    1 -
 drivers/tty/n_tracerouter.c    |    7 -------
 drivers/tty/n_tty.c            |    9 ++++++---
 include/linux/tty.h            |    2 +-
 19 files changed, 8 insertions(+), 28 deletions(-)

diff --git a/drivers/bluetooth/hci_ldisc.c b/drivers/bluetooth/hci_ldisc.c
index 588656e..24872dc 100644
--- a/drivers/bluetooth/hci_ldisc.c
+++ b/drivers/bluetooth/hci_ldisc.c
@@ -273,7 +273,6 @@ static int hci_uart_tty_open(struct tty_struct *tty)
 
 	tty->disc_data = hu;
 	hu->tty = tty;
-	tty->receive_room = 65536;
 
 	spin_lock_init(&hu->rx_lock);
 
diff --git a/drivers/input/serio/serport.c b/drivers/input/serio/serport.c
index 657043a..1bf1e71 100644
--- a/drivers/input/serio/serport.c
+++ b/drivers/input/serio/serport.c
@@ -97,7 +97,6 @@ static int serport_ldisc_open(struct tty_struct *tty)
 	init_waitqueue_head(&serport->wait);
 
 	tty->disc_data = serport;
-	tty->receive_room = 256;
 	set_bit(TTY_DO_WRITE_WAKEUP, &tty->flags);
 
 	return 0;
diff --git a/drivers/isdn/capi/capi.c b/drivers/isdn/capi/capi.c
index e44933d..70f892f 100644
--- a/drivers/isdn/capi/capi.c
+++ b/drivers/isdn/capi/capi.c
@@ -437,6 +437,7 @@ static int handle_recv_skb(struct capiminor *mp, struct sk_buff *skb)
 		goto deref_ldisc;
 	}
 
+	/* FIXME this is internal to N_TTY */
 	if (tty->receive_room < datalen) {
 		pr_debug("capi: no room in tty\n");
 		goto deref_ldisc;
diff --git a/drivers/misc/ti-st/st_core.c b/drivers/misc/ti-st/st_core.c
index bc3f61c..c4259fb 100644
--- a/drivers/misc/ti-st/st_core.c
+++ b/drivers/misc/ti-st/st_core.c
@@ -705,9 +705,6 @@ static int st_tty_open(struct tty_struct *tty)
 	/* don't do an wakeup for now */
 	clear_bit(TTY_DO_WRITE_WAKEUP, &tty->flags);
 
-	/* mem already allocated
-	 */
-	tty->receive_room = 65536;
 	/* Flush any pending characters in the driver and discipline. */
 	tty_ldisc_flush(tty);
 	tty_driver_flush_buffer(tty);
diff --git a/drivers/net/caif/caif_serial.c b/drivers/net/caif/caif_serial.c
index d801d6e..5bb1411 100644
--- a/drivers/net/caif/caif_serial.c
+++ b/drivers/net/caif/caif_serial.c
@@ -331,7 +331,6 @@ static int ldisc_open(struct tty_struct *tty)
 	ser->tty = tty_kref_get(tty);
 	ser->dev = dev;
 	debugfs_init(ser, tty);
-	tty->receive_room = N_TTY_BUF_SIZE;
 	tty->disc_data = ser;
 	set_bit(TTY_DO_WRITE_WAKEUP, &tty->flags);
 	rtnl_lock();
diff --git a/drivers/net/can/slcan.c b/drivers/net/can/slcan.c
index 3229e30..f3ef7ba 100644
--- a/drivers/net/can/slcan.c
+++ b/drivers/net/can/slcan.c
@@ -546,7 +546,6 @@ static int slcan_open(struct tty_struct *tty)
 
 	/* Done.  We have linked the TTY line to a channel. */
 	rtnl_unlock();
-	tty->receive_room = 65536;	/* We don't flow control */
 
 	/* TTY layer expects 0 on success */
 	return 0;
diff --git a/drivers/net/hamradio/6pack.c b/drivers/net/hamradio/6pack.c
index 39ad8fe..ae3f24d 100644
--- a/drivers/net/hamradio/6pack.c
+++ b/drivers/net/hamradio/6pack.c
@@ -661,7 +661,6 @@ static int sixpack_open(struct tty_struct *tty)
 
 	/* Done.  We have linked the TTY line to a channel. */
 	tty->disc_data = sp;
-	tty->receive_room = 65536;
 
 	/* Now we're ready to register. */
 	if (register_netdev(dev))
diff --git a/drivers/net/hamradio/mkiss.c b/drivers/net/hamradio/mkiss.c
index 75173ca..85f3682 100644
--- a/drivers/net/hamradio/mkiss.c
+++ b/drivers/net/hamradio/mkiss.c
@@ -751,7 +751,6 @@ static int mkiss_open(struct tty_struct *tty)
 
 	ax->tty = tty;
 	tty->disc_data = ax;
-	tty->receive_room = 65535;
 
 	tty_driver_flush_buffer(tty);
 
diff --git a/drivers/net/irda/irtty-sir.c b/drivers/net/irda/irtty-sir.c
index 3166e91..4a5a3af 100644
--- a/drivers/net/irda/irtty-sir.c
+++ b/drivers/net/irda/irtty-sir.c
@@ -473,7 +473,6 @@ static int irtty_open(struct tty_struct *tty)
 
 	dev->priv = priv;
 	tty->disc_data = priv;
-	tty->receive_room = 65536;
 
 	mutex_unlock(&irtty_mutex);
 
diff --git a/drivers/net/ppp/ppp_async.c b/drivers/net/ppp/ppp_async.c
index aefd499..213d9a2 100644
--- a/drivers/net/ppp/ppp_async.c
+++ b/drivers/net/ppp/ppp_async.c
@@ -198,7 +198,6 @@ ppp_asynctty_open(struct tty_struct *tty)
 		goto out_free;
 
 	tty->disc_data = ap;
-	tty->receive_room = 65536;
 	return 0;
 
  out_free:
diff --git a/drivers/net/ppp/ppp_synctty.c b/drivers/net/ppp/ppp_synctty.c
index 248ece3..c2dff73 100644
--- a/drivers/net/ppp/ppp_synctty.c
+++ b/drivers/net/ppp/ppp_synctty.c
@@ -245,7 +245,6 @@ ppp_sync_open(struct tty_struct *tty)
 		goto out_free;
 
 	tty->disc_data = ap;
-	tty->receive_room = 65536;
 	return 0;
 
  out_free:
diff --git a/drivers/net/slip/slip.c b/drivers/net/slip/slip.c
index b9e7156..466c0e8 100644
--- a/drivers/net/slip/slip.c
+++ b/drivers/net/slip/slip.c
@@ -837,7 +837,6 @@ static int slip_open(struct tty_struct *tty)
 
 	/* Done.  We have linked the TTY line to a channel. */
 	rtnl_unlock();
-	tty->receive_room = 65536;	/* We don't flow control */
 
 	/* TTY layer expects 0 on success */
 	return 0;
diff --git a/drivers/net/wan/x25_asy.c b/drivers/net/wan/x25_asy.c
index 3b07835..7adb510 100644
--- a/drivers/net/wan/x25_asy.c
+++ b/drivers/net/wan/x25_asy.c
@@ -567,7 +567,6 @@ static int x25_asy_open_tty(struct tty_struct *tty)
 
 	sl->tty = tty;
 	tty->disc_data = sl;
-	tty->receive_room = 65536;
 	tty_driver_flush_buffer(tty);
 	tty_ldisc_flush(tty);
 
diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c
index f8133a4..f605163 100644
--- a/drivers/tty/n_gsm.c
+++ b/drivers/tty/n_gsm.c
@@ -2356,7 +2356,6 @@ static int gsmld_open(struct tty_struct *tty)
 		return -ENOMEM;
 
 	tty->disc_data = gsm;
-	tty->receive_room = 65536;
 
 	/* Attach the initial passive connection */
 	gsm->encoding = 1;
diff --git a/drivers/tty/n_hdlc.c b/drivers/tty/n_hdlc.c
index f74ef15..747f38a 100644
--- a/drivers/tty/n_hdlc.c
+++ b/drivers/tty/n_hdlc.c
@@ -358,7 +358,6 @@ static int n_hdlc_tty_open (struct tty_struct *tty)
 		
 	tty->disc_data = n_hdlc;
 	n_hdlc->tty    = tty;
-	tty->receive_room = 65536;
 	
 #if defined(TTY_NO_WRITE_SPLIT)
 	/* change tty_io write() to not split large writes into 8K chunks */
diff --git a/drivers/tty/n_r3964.c b/drivers/tty/n_r3964.c
index 43384b3..2519200 100644
--- a/drivers/tty/n_r3964.c
+++ b/drivers/tty/n_r3964.c
@@ -993,7 +993,6 @@ static int r3964_open(struct tty_struct *tty)
 	pInfo->nRetry = 0;
 
 	tty->disc_data = pInfo;
-	tty->receive_room = 65536;
 
 	setup_timer(&pInfo->tmr, on_timeout, (unsigned long)pInfo);
 
diff --git a/drivers/tty/n_tracerouter.c b/drivers/tty/n_tracerouter.c
index 22130db..5882264 100644
--- a/drivers/tty/n_tracerouter.c
+++ b/drivers/tty/n_tracerouter.c
@@ -37,12 +37,6 @@
 #include <asm-generic/bug.h>
 #include "n_tracesink.h"
 
-/*
- * Other ldisc drivers use 65536 which basically means,
- * 'I can always accept 64k' and flow control is off.
- * This number is deemed appropriate for this driver.
- */
-#define RECEIVE_ROOM	65536
 #define DRIVERNAME	"n_tracerouter"
 
 /*
@@ -81,7 +75,6 @@ static int n_tracerouter_open(struct tty_struct *tty)
 		} else {
 			tr_data->opencalled = 1;
 			tty->disc_data      = tr_data;
-			tty->receive_room   = RECEIVE_ROOM;
 			tty_driver_flush_buffer(tty);
 			retval = 0;
 		}
diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index 5ef5a22..cda8fd9 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -91,7 +91,7 @@ static inline int tty_put_user(struct tty_struct *tty, unsigned char x,
  *	"instant".
  */
 
-static void n_tty_set_room(struct tty_struct *tty)
+static int n_tty_set_room(struct tty_struct *tty)
 {
 	/* tty->read_cnt is not read locked ? */
 	int	left = N_TTY_BUF_SIZE - tty->read_cnt - 1;
@@ -111,6 +111,8 @@ static void n_tty_set_room(struct tty_struct *tty)
 	/* Did this open up the receive buffer? We may need to flip */
 	if (left && !old_left)
 		schedule_work(&tty->buf.work);
+
+	return left;
 }
 
 static int put_tty_queue_nolock(unsigned char c, struct tty_struct *tty)
@@ -1380,6 +1382,7 @@ static int n_tty_receive_buf(struct tty_struct *tty, const unsigned char *cp,
 	char	buf[64];
 	unsigned long cpuflags;
 	int received = 0;
+	int left;
 
 	if (!tty->read_buf)
 		return count;
@@ -1436,7 +1439,7 @@ static int n_tty_receive_buf(struct tty_struct *tty, const unsigned char *cp,
 			tty->ops->flush_chars(tty);
 	}
 
-	n_tty_set_room(tty);
+	left = n_tty_set_room(tty);
 
 	if ((!tty->icanon && (tty->read_cnt >= tty->minimum_to_wake)) ||
 		L_EXTPROC(tty)) {
@@ -1450,7 +1453,7 @@ static int n_tty_receive_buf(struct tty_struct *tty, const unsigned char *cp,
 	 * mode.  We don't want to throttle the driver if we're in
 	 * canonical mode and don't have a newline yet!
 	 */
-	if (tty->receive_room < TTY_THRESHOLD_THROTTLE)
+	if (left < TTY_THRESHOLD_THROTTLE)
 		tty_throttle(tty);
 
 	return received;
diff --git a/include/linux/tty.h b/include/linux/tty.h
index 5dbb3cb..42accf6 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -282,7 +282,6 @@ struct tty_struct {
 	unsigned char stopped:1, hw_stopped:1, flow_stopped:1, packet:1;
 	unsigned char low_latency:1, warned:1;
 	unsigned char ctrl_status;	/* ctrl_lock */
-	unsigned int receive_room;	/* Bytes free for queue */
 
 	struct tty_struct *link;
 	struct fasync_struct *fasync;
@@ -310,6 +309,7 @@ struct tty_struct {
 	unsigned long overrun_time;
 	int num_overrun;
 	unsigned long process_char_map[256/(8*sizeof(unsigned long))];
+	unsigned int receive_room;	/* Bytes free for queue */
 	char *read_buf;
 	int read_head;
 	int read_tail;
-- 
1.7.9.1


^ permalink raw reply related

* [PATCH 8/9] tty : kill receive_room usage in mxser
From: Matthieu CASTET @ 2012-03-04 17:59 UTC (permalink / raw)
  To: Alan Cox, linux-serial, Felipe Balbi, Toby Gray, Stefan Bigler
  Cc: Matthieu CASTET
In-Reply-To: <1330883960-8563-1-git-send-email-castet.matthieu@free.fr>

Note this is need checking, the logic is a bit different.

Signed-off-by: Matthieu CASTET <castet.matthieu@free.fr>
---
 drivers/tty/mxser.c |   19 ++++++++-----------
 1 files changed, 8 insertions(+), 11 deletions(-)

diff --git a/drivers/tty/mxser.c b/drivers/tty/mxser.c
index 8998d52..f349160 100644
--- a/drivers/tty/mxser.c
+++ b/drivers/tty/mxser.c
@@ -2056,12 +2056,9 @@ static void mxser_receive_chars(struct tty_struct *tty,
 	unsigned char ch, gdl;
 	int ignored = 0;
 	int cnt = 0;
-	int recv_room;
 	int max = 256;
+	int ret;
 
-	recv_room = tty->receive_room;
-	if (recv_room == 0 && !port->ldisc_stop_rx)
-		mxser_stoprx(tty);
 	if (port->board->chip_flag != MOXA_OTHER_UART) {
 
 		if (*status & UART_LSR_SPECIAL)
@@ -2076,13 +2073,13 @@ static void mxser_receive_chars(struct tty_struct *tty,
 
 		if (port->board->chip_flag == MOXA_MUST_MU150_HWID)
 			gdl &= MOXA_MUST_GDL_MASK;
-		if (gdl >= recv_room) {
-			if (!port->ldisc_stop_rx)
-				mxser_stoprx(tty);
-		}
 		while (gdl--) {
 			ch = inb(port->ioaddr + UART_RX);
-			tty_insert_flip_char(tty, ch, 0);
+			ret = tty_insert_flip_char(tty, ch, 0);
+			if (ret != 1) {
+				if (!port->ldisc_stop_rx)
+					mxser_stoprx(tty);
+			}
 			cnt++;
 		}
 		goto end_intr;
@@ -2121,9 +2118,9 @@ intr_old:
 				} else
 					flag = TTY_BREAK;
 			}
-			tty_insert_flip_char(tty, ch, flag);
+			ret = tty_insert_flip_char(tty, ch, flag);
 			cnt++;
-			if (cnt >= recv_room) {
+			if (ret != 1) {
 				if (!port->ldisc_stop_rx)
 					mxser_stoprx(tty);
 				break;
-- 
1.7.9.1


^ permalink raw reply related

* [PATCH 7/9] tty : kill receive_room usage in tty_buffer
From: Matthieu CASTET @ 2012-03-04 17:59 UTC (permalink / raw)
  To: Alan Cox, linux-serial, Felipe Balbi, Toby Gray, Stefan Bigler
  Cc: Matthieu CASTET
In-Reply-To: <1330883960-8563-1-git-send-email-castet.matthieu@free.fr>

Signed-off-by: Matthieu CASTET <castet.matthieu@free.fr>
---
 drivers/tty/tty_buffer.c |   10 ++++------
 1 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/tty/tty_buffer.c b/drivers/tty/tty_buffer.c
index 0e0f0fa..8cc7ceb 100644
--- a/drivers/tty/tty_buffer.c
+++ b/drivers/tty/tty_buffer.c
@@ -437,17 +437,15 @@ static void flush_to_ldisc(struct work_struct *work)
 			 */
 			if (test_bit(TTY_LDISC_CHANGING, &tty->flags))
 				break;
-			if (!tty->receive_room)
-				break;
-			if (count > tty->receive_room)
-				count = tty->receive_room;
 			char_buf = head->char_buf_ptr + head->read;
 			flag_buf = head->flag_buf_ptr + head->read;
-			head->read += count;
 			spin_unlock_irqrestore(&tty->buf.lock, flags);
-			disc->ops->receive_buf(tty, char_buf,
+			count = disc->ops->receive_buf(tty, char_buf,
 							flag_buf, count);
 			spin_lock_irqsave(&tty->buf.lock, flags);
+			if (count == 0)
+				break;
+			head->read += count;
 		}
 		clear_bit(TTY_FLUSHING, &tty->flags);
 	}
-- 
1.7.9.1


^ permalink raw reply related

* [PATCH 6/9] tty : don't use receive_room in tty_set_ldisc
From: Matthieu CASTET @ 2012-03-04 17:59 UTC (permalink / raw)
  To: Alan Cox, linux-serial, Felipe Balbi, Toby Gray, Stefan Bigler
  Cc: Matthieu CASTET
In-Reply-To: <1330883960-8563-1-git-send-email-castet.matthieu@free.fr>

Signed-off-by: Matthieu CASTET <castet.matthieu@free.fr>
---
 drivers/tty/n_tty.c      |    1 +
 drivers/tty/tty_buffer.c |    5 +++++
 drivers/tty/tty_ldisc.c  |    7 -------
 3 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index ed5a976..5ef5a22 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -190,6 +190,7 @@ static void reset_buffer_flags(struct tty_struct *tty)
 
 	tty->canon_head = tty->canon_data = tty->erasing = 0;
 	memset(&tty->read_flags, 0, sizeof tty->read_flags);
+	tty->receive_room = 0;
 	n_tty_set_room(tty);
 }
 
diff --git a/drivers/tty/tty_buffer.c b/drivers/tty/tty_buffer.c
index 6c9b7cd..0e0f0fa 100644
--- a/drivers/tty/tty_buffer.c
+++ b/drivers/tty/tty_buffer.c
@@ -432,6 +432,11 @@ static void flush_to_ldisc(struct work_struct *work)
 			   line discipline as we want to empty the queue */
 			if (test_bit(TTY_FLUSHPENDING, &tty->flags))
 				break;
+			/*
+			 *	No more input please, we are switching.
+			 */
+			if (test_bit(TTY_LDISC_CHANGING, &tty->flags))
+				break;
 			if (!tty->receive_room)
 				break;
 			if (count > tty->receive_room)
diff --git a/drivers/tty/tty_ldisc.c b/drivers/tty/tty_ldisc.c
index 24b95db..ab0a180 100644
--- a/drivers/tty/tty_ldisc.c
+++ b/drivers/tty/tty_ldisc.c
@@ -613,13 +613,6 @@ int tty_set_ldisc(struct tty_struct *tty, int ldisc)
 
 	set_bit(TTY_LDISC_CHANGING, &tty->flags);
 
-	/*
-	 *	No more input please, we are switching. The new ldisc
-	 *	will update this value in the ldisc open function
-	 */
-
-	tty->receive_room = 0;
-
 	o_ldisc = tty->ldisc;
 
 	tty_unlock();
-- 
1.7.9.1


^ permalink raw reply related

* [PATCH 5/9] tty : kill receive_room usage in speakup
From: Matthieu CASTET @ 2012-03-04 17:59 UTC (permalink / raw)
  To: Alan Cox, linux-serial, Felipe Balbi, Toby Gray, Stefan Bigler
  Cc: Matthieu CASTET
In-Reply-To: <1330883960-8563-1-git-send-email-castet.matthieu@free.fr>

Signed-off-by: Matthieu CASTET <castet.matthieu@free.fr>
---
 drivers/staging/speakup/selection.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/speakup/selection.c b/drivers/staging/speakup/selection.c
index fe1f405..5df5b25 100644
--- a/drivers/staging/speakup/selection.c
+++ b/drivers/staging/speakup/selection.c
@@ -139,8 +139,7 @@ int speakup_paste_selection(struct tty_struct *tty)
 			continue;
 		}
 		count = sel_buffer_lth - pasted;
-		count = min_t(int, count, tty->receive_room);
-		tty->ldisc->ops->receive_buf(tty, sel_buffer + pasted,
+		count = tty->ldisc->ops->receive_buf(tty, sel_buffer + pasted,
 			0, count);
 		pasted += count;
 	}
-- 
1.7.9.1


^ permalink raw reply related

* [PATCH 4/9] tty : kill receive_room usage in vt
From: Matthieu CASTET @ 2012-03-04 17:59 UTC (permalink / raw)
  To: Alan Cox, linux-serial, Felipe Balbi, Toby Gray, Stefan Bigler
  Cc: Matthieu CASTET
In-Reply-To: <1330883960-8563-1-git-send-email-castet.matthieu@free.fr>

Signed-off-by: Matthieu CASTET <castet.matthieu@free.fr>
---
 drivers/tty/vt/selection.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/vt/selection.c b/drivers/tty/vt/selection.c
index 7a0a12a..3e9a914 100644
--- a/drivers/tty/vt/selection.c
+++ b/drivers/tty/vt/selection.c
@@ -332,8 +332,7 @@ int paste_selection(struct tty_struct *tty)
 			continue;
 		}
 		count = sel_buffer_lth - pasted;
-		count = min(count, tty->receive_room);
-		tty->ldisc->ops->receive_buf(tty, sel_buffer + pasted,
+		count = tty->ldisc->ops->receive_buf(tty, sel_buffer + pasted,
 								NULL, count);
 		pasted += count;
 	}
-- 
1.7.9.1


^ permalink raw reply related

* [PATCH 3/9] tty: make hci_uart_tty_receive return bytes received
From: Matthieu CASTET @ 2012-03-04 17:59 UTC (permalink / raw)
  To: Alan Cox, linux-serial, Felipe Balbi, Toby Gray, Stefan Bigler
  Cc: Matthieu CASTET
In-Reply-To: <1330883960-8563-1-git-send-email-castet.matthieu@free.fr>

Signed-off-by: Matthieu CASTET <castet.matthieu@free.fr>
---
 drivers/bluetooth/hci_ldisc.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/bluetooth/hci_ldisc.c b/drivers/bluetooth/hci_ldisc.c
index 47b97a8..588656e 100644
--- a/drivers/bluetooth/hci_ldisc.c
+++ b/drivers/bluetooth/hci_ldisc.c
@@ -369,7 +369,7 @@ static int hci_uart_tty_receive(struct tty_struct *tty,
 		return count;
 
 	spin_lock(&hu->rx_lock);
-	hu->proto->recv(hu, (void *) data, count);
+	count = hu->proto->recv(hu, (void *) data, count);
 	hu->hdev->stat.byte_rx += count;
 	spin_unlock(&hu->rx_lock);
 
-- 
1.7.9.1


^ permalink raw reply related

* [PATCH 2/9] tty : make n_tty_receive_buf return bytes received in raw mode
From: Matthieu CASTET @ 2012-03-04 17:59 UTC (permalink / raw)
  To: Alan Cox, linux-serial, Felipe Balbi, Toby Gray, Stefan Bigler
  Cc: Matthieu CASTET
In-Reply-To: <1330883960-8563-1-git-send-email-castet.matthieu@free.fr>

Signed-off-by: Matthieu CASTET <castet.matthieu@free.fr>
---
 drivers/tty/n_tty.c |   60 +++++++++++++++++++++++++++++++++-----------------
 1 files changed, 39 insertions(+), 21 deletions(-)

diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index c47cd7e..ed5a976 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -113,13 +113,15 @@ static void n_tty_set_room(struct tty_struct *tty)
 		schedule_work(&tty->buf.work);
 }
 
-static void put_tty_queue_nolock(unsigned char c, struct tty_struct *tty)
+static int put_tty_queue_nolock(unsigned char c, struct tty_struct *tty)
 {
 	if (tty->read_cnt < N_TTY_BUF_SIZE) {
 		tty->read_buf[tty->read_head] = c;
 		tty->read_head = (tty->read_head + 1) & (N_TTY_BUF_SIZE-1);
 		tty->read_cnt++;
+		return 1;
 	}
+	return 0;
 }
 
 /**
@@ -127,21 +129,25 @@ static void put_tty_queue_nolock(unsigned char c, struct tty_struct *tty)
  *	@c: character
  *	@tty: tty device
  *
+ *  Report the number of character written
+ *
  *	Add a character to the tty read_buf queue. This is done under the
  *	read_lock to serialize character addition and also to protect us
  *	against parallel reads or flushes
  */
 
-static void put_tty_queue(unsigned char c, struct tty_struct *tty)
+static int put_tty_queue(unsigned char c, struct tty_struct *tty)
 {
 	unsigned long flags;
+	int ret;
 	/*
 	 *	The problem of stomping on the buffers ends here.
 	 *	Why didn't anyone see this one coming? --AJK
 	*/
 	spin_lock_irqsave(&tty->read_lock, flags);
-	put_tty_queue_nolock(c, tty);
+	ret = put_tty_queue_nolock(c, tty);
 	spin_unlock_irqrestore(&tty->read_lock, flags);
+	return ret;
 }
 
 /**
@@ -1082,26 +1088,12 @@ static inline void n_tty_receive_parity_error(struct tty_struct *tty,
 	wake_up_interruptible(&tty->read_wait);
 }
 
-/**
- *	n_tty_receive_char	-	perform processing
- *	@tty: terminal device
- *	@c: character
- *
- *	Process an individual character of input received from the driver.
- *	This is serialized with respect to itself by the rules for the
- *	driver above.
- */
-
-static inline void n_tty_receive_char(struct tty_struct *tty, unsigned char c)
+static inline void n_tty_receive_char_no_raw(struct tty_struct *tty,
+					      unsigned char c)
 {
 	unsigned long flags;
 	int parmrk;
 
-	if (tty->raw) {
-		put_tty_queue(c, tty);
-		return;
-	}
-
 	if (I_ISTRIP(tty))
 		c &= 0x7f;
 	if (I_IUCLC(tty) && L_IEXTEN(tty))
@@ -1330,6 +1322,25 @@ handle_newline:
 	put_tty_queue(c, tty);
 }
 
+/**
+ *	n_tty_receive_char	-	perform processing
+ *	@tty: terminal device
+ *	@c: character
+ *
+ *	Process an individual character of input received from the driver.
+ *	This is serialized with respect to itself by the rules for the
+ *	driver above.
+ */
+
+static inline int n_tty_receive_char(struct tty_struct *tty, unsigned char c)
+{
+	if (tty->raw) {
+		return put_tty_queue(c, tty);
+	}
+
+	n_tty_receive_char_no_raw(tty, c);
+	return 1;
+}
 
 /**
  *	n_tty_write_wakeup	-	asynchronous I/O notifier
@@ -1367,6 +1378,7 @@ static int n_tty_receive_buf(struct tty_struct *tty, const unsigned char *cp,
 	int	i;
 	char	buf[64];
 	unsigned long cpuflags;
+	int received = 0;
 
 	if (!tty->read_buf)
 		return count;
@@ -1381,6 +1393,7 @@ static int n_tty_receive_buf(struct tty_struct *tty, const unsigned char *cp,
 		tty->read_cnt += i;
 		cp += i;
 		count -= i;
+		received += i;
 
 		i = min(N_TTY_BUF_SIZE - tty->read_cnt,
 			N_TTY_BUF_SIZE - tty->read_head);
@@ -1388,14 +1401,16 @@ static int n_tty_receive_buf(struct tty_struct *tty, const unsigned char *cp,
 		memcpy(tty->read_buf + tty->read_head, cp, i);
 		tty->read_head = (tty->read_head + i) & (N_TTY_BUF_SIZE-1);
 		tty->read_cnt += i;
+		received += i;
 		spin_unlock_irqrestore(&tty->read_lock, cpuflags);
 	} else {
 		for (i = count, p = cp, f = fp; i; i--, p++) {
+			int ret = 1;
 			if (f)
 				flags = *f++;
 			switch (flags) {
 			case TTY_NORMAL:
-				n_tty_receive_char(tty, *p);
+				ret = n_tty_receive_char(tty, *p);
 				break;
 			case TTY_BREAK:
 				n_tty_receive_break(tty);
@@ -1412,6 +1427,9 @@ static int n_tty_receive_buf(struct tty_struct *tty, const unsigned char *cp,
 				       tty_name(tty, buf), flags);
 				break;
 			}
+			if (ret == 0)
+				break;
+			received++;
 		}
 		if (tty->ops->flush_chars)
 			tty->ops->flush_chars(tty);
@@ -1434,7 +1452,7 @@ static int n_tty_receive_buf(struct tty_struct *tty, const unsigned char *cp,
 	if (tty->receive_room < TTY_THRESHOLD_THROTTLE)
 		tty_throttle(tty);
 
-	return count;
+	return received;
 }
 
 int is_ignored(int sig)
-- 
1.7.9.1


^ permalink raw reply related

* [PATCH 1/9] tty: make receive_buf() return the amout of bytes received
From: Matthieu CASTET @ 2012-03-04 17:59 UTC (permalink / raw)
  To: Alan Cox, linux-serial, Felipe Balbi, Toby Gray, Stefan Bigler
  Cc: Matthieu CASTET
In-Reply-To: <1330883960-8563-1-git-send-email-castet.matthieu@free.fr>

it makes it simpler to keep track of the amount of bytes received.

For a first step, will always return that we received everything.
This make sure to we don't break anything when we will check receive_buf
return value.

Some code is based on b1c43f82c5aa265442f82dba31ce985ebb7aa71c reverted
commit.

Signed-off-by: Matthieu CASTET <castet.matthieu@free.fr>
---
 drivers/bluetooth/hci_ldisc.c      |   11 +++++++----
 drivers/input/serio/serport.c      |    9 +++++++--
 drivers/isdn/gigaset/ser-gigaset.c |    8 +++++---
 drivers/misc/ti-st/st_core.c       |    6 ++++--
 drivers/net/caif/caif_serial.c     |   10 ++++++----
 drivers/net/can/slcan.c            |    9 ++++++---
 drivers/net/hamradio/6pack.c       |   11 ++++++-----
 drivers/net/hamradio/mkiss.c       |   11 +++++++----
 drivers/net/irda/irtty-sir.c       |   16 +++++++++-------
 drivers/net/ppp/ppp_async.c        |    6 ++++--
 drivers/net/ppp/ppp_synctty.c      |    6 ++++--
 drivers/net/slip/slip.c            |    7 +++++--
 drivers/net/wan/x25_asy.c          |    9 ++++++---
 drivers/tty/n_gsm.c                |    6 ++++--
 drivers/tty/n_hdlc.c               |   18 ++++++++++--------
 drivers/tty/n_r3964.c              |   10 ++++++----
 drivers/tty/n_tracerouter.c        |    4 +++-
 drivers/tty/n_tty.c                |    6 ++++--
 include/linux/tty_ldisc.h          |    9 +++++----
 sound/soc/omap/ams-delta.c         |    7 ++++---
 20 files changed, 112 insertions(+), 67 deletions(-)

diff --git a/drivers/bluetooth/hci_ldisc.c b/drivers/bluetooth/hci_ldisc.c
index 0711448..47b97a8 100644
--- a/drivers/bluetooth/hci_ldisc.c
+++ b/drivers/bluetooth/hci_ldisc.c
@@ -355,17 +355,18 @@ static void hci_uart_tty_wakeup(struct tty_struct *tty)
  *             flags        pointer to flags for data
  *             count        count of received data in bytes
  *     
- * Return Value:    None
+ * Return Value:    bytes received.
  */
-static void hci_uart_tty_receive(struct tty_struct *tty, const u8 *data, char *flags, int count)
+static int hci_uart_tty_receive(struct tty_struct *tty,
+		const u8 *data, char *flags, int count)
 {
 	struct hci_uart *hu = (void *)tty->disc_data;
 
 	if (!hu || tty != hu->tty)
-		return;
+		return count;
 
 	if (!test_bit(HCI_UART_PROTO_SET, &hu->flags))
-		return;
+		return count;
 
 	spin_lock(&hu->rx_lock);
 	hu->proto->recv(hu, (void *) data, count);
@@ -373,6 +374,8 @@ static void hci_uart_tty_receive(struct tty_struct *tty, const u8 *data, char *f
 	spin_unlock(&hu->rx_lock);
 
 	tty_unthrottle(tty);
+
+	return count;
 }
 
 static int hci_uart_register_dev(struct hci_uart *hu)
diff --git a/drivers/input/serio/serport.c b/drivers/input/serio/serport.c
index 8755f5f..657043a 100644
--- a/drivers/input/serio/serport.c
+++ b/drivers/input/serio/serport.c
@@ -120,17 +120,20 @@ static void serport_ldisc_close(struct tty_struct *tty)
  * 'interrupt' routine.
  */
 
-static void serport_ldisc_receive(struct tty_struct *tty, const unsigned char *cp, char *fp, int count)
+static int serport_ldisc_receive(struct tty_struct *tty,
+		const unsigned char *cp, char *fp, int count)
 {
 	struct serport *serport = (struct serport*) tty->disc_data;
 	unsigned long flags;
 	unsigned int ch_flags;
+	int ret = 0;
 	int i;
 
 	spin_lock_irqsave(&serport->lock, flags);
 
-	if (!test_bit(SERPORT_ACTIVE, &serport->flags))
+	if (!test_bit(SERPORT_ACTIVE, &serport->flags)) {
 		goto out;
+	}
 
 	for (i = 0; i < count; i++) {
 		switch (fp[i]) {
@@ -152,6 +155,8 @@ static void serport_ldisc_receive(struct tty_struct *tty, const unsigned char *c
 
 out:
 	spin_unlock_irqrestore(&serport->lock, flags);
+
+	return count;
 }
 
 /*
diff --git a/drivers/isdn/gigaset/ser-gigaset.c b/drivers/isdn/gigaset/ser-gigaset.c
index 86a5c4f..569712a 100644
--- a/drivers/isdn/gigaset/ser-gigaset.c
+++ b/drivers/isdn/gigaset/ser-gigaset.c
@@ -674,7 +674,7 @@ gigaset_tty_ioctl(struct tty_struct *tty, struct file *file,
  *	cflags	buffer containing error flags for received characters (ignored)
  *	count	number of received characters
  */
-static void
+static int
 gigaset_tty_receive(struct tty_struct *tty, const unsigned char *buf,
 		    char *cflags, int count)
 {
@@ -683,12 +683,12 @@ gigaset_tty_receive(struct tty_struct *tty, const unsigned char *buf,
 	struct inbuf_t *inbuf;
 
 	if (!cs)
-		return;
+		return count;
 	inbuf = cs->inbuf;
 	if (!inbuf) {
 		dev_err(cs->dev, "%s: no inbuf\n", __func__);
 		cs_put(cs);
-		return;
+		return count;
 	}
 
 	tail = inbuf->tail;
@@ -725,6 +725,8 @@ gigaset_tty_receive(struct tty_struct *tty, const unsigned char *buf,
 	gig_dbg(DEBUG_INTR, "%s-->BH", __func__);
 	gigaset_schedule_event(cs);
 	cs_put(cs);
+
+	return count;
 }
 
 /*
diff --git a/drivers/misc/ti-st/st_core.c b/drivers/misc/ti-st/st_core.c
index 2b62232..bc3f61c 100644
--- a/drivers/misc/ti-st/st_core.c
+++ b/drivers/misc/ti-st/st_core.c
@@ -765,8 +765,8 @@ static void st_tty_close(struct tty_struct *tty)
 	pr_debug("%s: done ", __func__);
 }
 
-static void st_tty_receive(struct tty_struct *tty, const unsigned char *data,
-			   char *tty_flags, int count)
+static int st_tty_receive(struct tty_struct *tty,
+		const unsigned char *data, char *tty_flags, int count)
 {
 #ifdef VERBOSE
 	print_hex_dump(KERN_DEBUG, ">in>", DUMP_PREFIX_NONE,
@@ -779,6 +779,8 @@ static void st_tty_receive(struct tty_struct *tty, const unsigned char *data,
 	 */
 	st_recv(tty->disc_data, data, count);
 	pr_debug("done %s", __func__);
+
+	return count;
 }
 
 /* wake-up function called in from the TTY layer
diff --git a/drivers/net/caif/caif_serial.c b/drivers/net/caif/caif_serial.c
index 8a3054b..d801d6e 100644
--- a/drivers/net/caif/caif_serial.c
+++ b/drivers/net/caif/caif_serial.c
@@ -167,8 +167,8 @@ static inline void debugfs_tx(struct ser_device *ser, const u8 *data, int size)
 
 #endif
 
-static void ldisc_receive(struct tty_struct *tty, const u8 *data,
-			char *flags, int count)
+static int ldisc_receive(struct tty_struct *tty,
+		const u8 *data, char *flags, int count)
 {
 	struct sk_buff *skb = NULL;
 	struct ser_device *ser;
@@ -191,7 +191,7 @@ static void ldisc_receive(struct tty_struct *tty, const u8 *data,
 		dev_info(&ser->dev->dev,
 			"Bytes received before initial transmission -"
 			"bytes discarded.\n");
-		return;
+		return count;
 	}
 
 	BUG_ON(ser->dev == NULL);
@@ -199,7 +199,7 @@ static void ldisc_receive(struct tty_struct *tty, const u8 *data,
 	/* Get a suitable caif packet and copy in data. */
 	skb = netdev_alloc_skb(ser->dev, count+1);
 	if (skb == NULL)
-		return;
+		return count;
 	p = skb_put(skb, count);
 	memcpy(p, data, count);
 
@@ -215,6 +215,8 @@ static void ldisc_receive(struct tty_struct *tty, const u8 *data,
 	} else
 		++ser->dev->stats.rx_dropped;
 	update_tty_status(ser);
+
+	return count;
 }
 
 static int handle_tx(struct ser_device *ser)
diff --git a/drivers/net/can/slcan.c b/drivers/net/can/slcan.c
index 3f1ebcc..3229e30 100644
--- a/drivers/net/can/slcan.c
+++ b/drivers/net/can/slcan.c
@@ -403,16 +403,17 @@ static void slc_setup(struct net_device *dev)
  * in parallel
  */
 
-static void slcan_receive_buf(struct tty_struct *tty,
+static int slcan_receive_buf(struct tty_struct *tty,
 			      const unsigned char *cp, char *fp, int count)
 {
 	struct slcan *sl = (struct slcan *) tty->disc_data;
+	int bytes = count;
 
 	if (!sl || sl->magic != SLCAN_MAGIC || !netif_running(sl->dev))
-		return;
+		return count;
 
 	/* Read the characters out of the buffer */
-	while (count--) {
+	while (bytes--) {
 		if (fp && *fp++) {
 			if (!test_and_set_bit(SLF_ERROR, &sl->flags))
 				sl->dev->stats.rx_errors++;
@@ -421,6 +422,8 @@ static void slcan_receive_buf(struct tty_struct *tty,
 		}
 		slcan_unesc(sl, *cp++);
 	}
+
+	return count;
 }
 
 /************************************
diff --git a/drivers/net/hamradio/6pack.c b/drivers/net/hamradio/6pack.c
index 2a5a34d..39ad8fe 100644
--- a/drivers/net/hamradio/6pack.c
+++ b/drivers/net/hamradio/6pack.c
@@ -456,25 +456,24 @@ out:
  * a block of 6pack data has been received, which can now be decapsulated
  * and sent on to some IP layer for further processing.
  */
-static void sixpack_receive_buf(struct tty_struct *tty,
+static int sixpack_receive_buf(struct tty_struct *tty,
 	const unsigned char *cp, char *fp, int count)
 {
 	struct sixpack *sp;
 	unsigned char buf[512];
-	int count1;
+	int received = count;
 
 	if (!count)
-		return;
+		return 0;
 
 	sp = sp_get(tty);
 	if (!sp)
-		return;
+		return count;
 
 	memcpy(buf, cp, count < sizeof(buf) ? count : sizeof(buf));
 
 	/* Read the characters out of the buffer */
 
-	count1 = count;
 	while (count) {
 		count--;
 		if (fp && *fp++) {
@@ -487,6 +486,8 @@ static void sixpack_receive_buf(struct tty_struct *tty,
 
 	sp_put(sp);
 	tty_unthrottle(tty);
+
+	return received;
 }
 
 /*
diff --git a/drivers/net/hamradio/mkiss.c b/drivers/net/hamradio/mkiss.c
index bc02968..75173ca 100644
--- a/drivers/net/hamradio/mkiss.c
+++ b/drivers/net/hamradio/mkiss.c
@@ -923,13 +923,14 @@ static long mkiss_compat_ioctl(struct tty_struct *tty, struct file *file,
  * a block of data has been received, which can now be decapsulated
  * and sent on to the AX.25 layer for further processing.
  */
-static void mkiss_receive_buf(struct tty_struct *tty, const unsigned char *cp,
-	char *fp, int count)
+static int mkiss_receive_buf(struct tty_struct *tty,
+		const unsigned char *cp, char *fp, int count)
 {
 	struct mkiss *ax = mkiss_get(tty);
+	int bytes = count;
 
 	if (!ax)
-		return;
+		return count;
 
 	/*
 	 * Argh! mtu change time! - costs us the packet part received
@@ -939,7 +940,7 @@ static void mkiss_receive_buf(struct tty_struct *tty, const unsigned char *cp,
 		ax_changedmtu(ax);
 
 	/* Read the characters out of the buffer */
-	while (count--) {
+	while (bytes--) {
 		if (fp != NULL && *fp++) {
 			if (!test_and_set_bit(AXF_ERROR, &ax->flags))
 				ax->dev->stats.rx_errors++;
@@ -952,6 +953,8 @@ static void mkiss_receive_buf(struct tty_struct *tty, const unsigned char *cp,
 
 	mkiss_put(ax);
 	tty_unthrottle(tty);
+
+	return count;
 }
 
 /*
diff --git a/drivers/net/irda/irtty-sir.c b/drivers/net/irda/irtty-sir.c
index 3352b24..3166e91 100644
--- a/drivers/net/irda/irtty-sir.c
+++ b/drivers/net/irda/irtty-sir.c
@@ -216,23 +216,23 @@ static int irtty_do_write(struct sir_dev *dev, const unsigned char *ptr, size_t
  * usbserial:	urb-complete-interrupt / softint
  */
 
-static void irtty_receive_buf(struct tty_struct *tty, const unsigned char *cp,
-			      char *fp, int count) 
+static int irtty_receive_buf(struct tty_struct *tty,
+		const unsigned char *cp, char *fp, int count)
 {
 	struct sir_dev *dev;
 	struct sirtty_cb *priv = tty->disc_data;
 	int	i;
 
-	IRDA_ASSERT(priv != NULL, return;);
-	IRDA_ASSERT(priv->magic == IRTTY_MAGIC, return;);
+	IRDA_ASSERT(priv != NULL, return count;);
+	IRDA_ASSERT(priv->magic == IRTTY_MAGIC, return count;);
 
 	if (unlikely(count==0))		/* yes, this happens */
-		return;
+		return 0;
 
 	dev = priv->dev;
 	if (!dev) {
 		IRDA_WARNING("%s(), not ready yet!\n", __func__);
-		return;
+		return count;
 	}
 
 	for (i = 0; i < count; i++) {
@@ -242,11 +242,13 @@ static void irtty_receive_buf(struct tty_struct *tty, const unsigned char *cp,
  		if (fp && *fp++) { 
 			IRDA_DEBUG(0, "Framing or parity error!\n");
 			sirdev_receive(dev, NULL, 0);	/* notify sir_dev (updating stats) */
-			return;
+			return count;
  		}
 	}
 
 	sirdev_receive(dev, cp, count);
+
+	return count;
 }
 
 /*
diff --git a/drivers/net/ppp/ppp_async.c b/drivers/net/ppp/ppp_async.c
index c6ba643..aefd499 100644
--- a/drivers/net/ppp/ppp_async.c
+++ b/drivers/net/ppp/ppp_async.c
@@ -341,7 +341,7 @@ ppp_asynctty_poll(struct tty_struct *tty, struct file *file, poll_table *wait)
 }
 
 /* May sleep, don't call from interrupt level or with interrupts disabled */
-static void
+static int
 ppp_asynctty_receive(struct tty_struct *tty, const unsigned char *buf,
 		  char *cflags, int count)
 {
@@ -349,7 +349,7 @@ ppp_asynctty_receive(struct tty_struct *tty, const unsigned char *buf,
 	unsigned long flags;
 
 	if (!ap)
-		return;
+		return count;
 	spin_lock_irqsave(&ap->recv_lock, flags);
 	ppp_async_input(ap, buf, cflags, count);
 	spin_unlock_irqrestore(&ap->recv_lock, flags);
@@ -357,6 +357,8 @@ ppp_asynctty_receive(struct tty_struct *tty, const unsigned char *buf,
 		tasklet_schedule(&ap->tsk);
 	ap_put(ap);
 	tty_unthrottle(tty);
+
+	return count;
 }
 
 static void
diff --git a/drivers/net/ppp/ppp_synctty.c b/drivers/net/ppp/ppp_synctty.c
index 736a39e..248ece3 100644
--- a/drivers/net/ppp/ppp_synctty.c
+++ b/drivers/net/ppp/ppp_synctty.c
@@ -382,7 +382,7 @@ ppp_sync_poll(struct tty_struct *tty, struct file *file, poll_table *wait)
 }
 
 /* May sleep, don't call from interrupt level or with interrupts disabled */
-static void
+static int
 ppp_sync_receive(struct tty_struct *tty, const unsigned char *buf,
 		  char *cflags, int count)
 {
@@ -390,7 +390,7 @@ ppp_sync_receive(struct tty_struct *tty, const unsigned char *buf,
 	unsigned long flags;
 
 	if (!ap)
-		return;
+		return count;
 	spin_lock_irqsave(&ap->recv_lock, flags);
 	ppp_sync_input(ap, buf, cflags, count);
 	spin_unlock_irqrestore(&ap->recv_lock, flags);
@@ -398,6 +398,8 @@ ppp_sync_receive(struct tty_struct *tty, const unsigned char *buf,
 		tasklet_schedule(&ap->tsk);
 	sp_put(ap);
 	tty_unthrottle(tty);
+
+	return count;
 }
 
 static void
diff --git a/drivers/net/slip/slip.c b/drivers/net/slip/slip.c
index ba08341..b9e7156 100644
--- a/drivers/net/slip/slip.c
+++ b/drivers/net/slip/slip.c
@@ -668,13 +668,14 @@ static void sl_setup(struct net_device *dev)
  * in parallel
  */
 
-static void slip_receive_buf(struct tty_struct *tty, const unsigned char *cp,
+static int slip_receive_buf(struct tty_struct *tty, const unsigned char *cp,
 							char *fp, int count)
 {
 	struct slip *sl = tty->disc_data;
+	int received = count;
 
 	if (!sl || sl->magic != SLIP_MAGIC || !netif_running(sl->dev))
-		return;
+		return count;
 
 	/* Read the characters out of the buffer */
 	while (count--) {
@@ -691,6 +692,8 @@ static void slip_receive_buf(struct tty_struct *tty, const unsigned char *cp,
 #endif
 			slip_unesc(sl, *cp++);
 	}
+
+	return received;
 }
 
 /************************************
diff --git a/drivers/net/wan/x25_asy.c b/drivers/net/wan/x25_asy.c
index 8a10bb7..3b07835 100644
--- a/drivers/net/wan/x25_asy.c
+++ b/drivers/net/wan/x25_asy.c
@@ -516,17 +516,18 @@ static int x25_asy_close(struct net_device *dev)
  * and sent on to some IP layer for further processing.
  */
 
-static void x25_asy_receive_buf(struct tty_struct *tty,
+static int x25_asy_receive_buf(struct tty_struct *tty,
 				const unsigned char *cp, char *fp, int count)
 {
 	struct x25_asy *sl = tty->disc_data;
+	int bytes = count;
 
 	if (!sl || sl->magic != X25_ASY_MAGIC || !netif_running(sl->dev))
-		return;
+		return count;
 
 
 	/* Read the characters out of the buffer */
-	while (count--) {
+	while (bytes--) {
 		if (fp && *fp++) {
 			if (!test_and_set_bit(SLF_ERROR, &sl->flags))
 				sl->dev->stats.rx_errors++;
@@ -535,6 +536,8 @@ static void x25_asy_receive_buf(struct tty_struct *tty,
 		}
 		x25_asy_unesc(sl, *cp++);
 	}
+
+	return count;
 }
 
 /*
diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c
index fc7bbba..f8133a4 100644
--- a/drivers/tty/n_gsm.c
+++ b/drivers/tty/n_gsm.c
@@ -2246,8 +2246,8 @@ static void gsmld_detach_gsm(struct tty_struct *tty, struct gsm_mux *gsm)
 	gsm->tty = NULL;
 }
 
-static void gsmld_receive_buf(struct tty_struct *tty, const unsigned char *cp,
-			      char *fp, int count)
+static int gsmld_receive_buf(struct tty_struct *tty,
+		const unsigned char *cp, char *fp, int count)
 {
 	struct gsm_mux *gsm = tty->disc_data;
 	const unsigned char *dp;
@@ -2280,6 +2280,8 @@ static void gsmld_receive_buf(struct tty_struct *tty, const unsigned char *cp,
 	}
 	/* FASYNC if needed ? */
 	/* If clogged call tty_throttle(tty); */
+
+	return count;
 }
 
 /**
diff --git a/drivers/tty/n_hdlc.c b/drivers/tty/n_hdlc.c
index a09ce3e..f74ef15 100644
--- a/drivers/tty/n_hdlc.c
+++ b/drivers/tty/n_hdlc.c
@@ -188,8 +188,8 @@ static unsigned int n_hdlc_tty_poll(struct tty_struct *tty, struct file *filp,
 				    poll_table *wait);
 static int n_hdlc_tty_open(struct tty_struct *tty);
 static void n_hdlc_tty_close(struct tty_struct *tty);
-static void n_hdlc_tty_receive(struct tty_struct *tty, const __u8 *cp,
-			       char *fp, int count);
+static int n_hdlc_tty_receive(struct tty_struct *tty,
+		const __u8 *cp, char *fp, int count);
 static void n_hdlc_tty_wakeup(struct tty_struct *tty);
 
 #define bset(p,b)	((p)[(b) >> 5] |= (1 << ((b) & 0x1f)))
@@ -509,8 +509,8 @@ static void n_hdlc_tty_wakeup(struct tty_struct *tty)
  * Called by tty low level driver when receive data is available. Data is
  * interpreted as one HDLC frame.
  */
-static void n_hdlc_tty_receive(struct tty_struct *tty, const __u8 *data,
-			       char *flags, int count)
+static int n_hdlc_tty_receive(struct tty_struct *tty,
+		const __u8 *data, char *flags, int count)
 {
 	register struct n_hdlc *n_hdlc = tty2n_hdlc (tty);
 	register struct n_hdlc_buf *buf;
@@ -521,20 +521,20 @@ static void n_hdlc_tty_receive(struct tty_struct *tty, const __u8 *data,
 		
 	/* This can happen if stuff comes in on the backup tty */
 	if (!n_hdlc || tty != n_hdlc->tty)
-		return;
+		return count;
 		
 	/* verify line is using HDLC discipline */
 	if (n_hdlc->magic != HDLC_MAGIC) {
 		printk("%s(%d) line not using HDLC discipline\n",
 			__FILE__,__LINE__);
-		return;
+		return count;
 	}
 	
 	if ( count>maxframe ) {
 		if (debuglevel >= DEBUG_LEVEL_INFO)	
 			printk("%s(%d) rx count>maxframesize, data discarded\n",
 			       __FILE__,__LINE__);
-		return;
+		return count;
 	}
 
 	/* get a free HDLC buffer */	
@@ -550,7 +550,7 @@ static void n_hdlc_tty_receive(struct tty_struct *tty, const __u8 *data,
 		if (debuglevel >= DEBUG_LEVEL_INFO)	
 			printk("%s(%d) no more rx buffers, data discarded\n",
 			       __FILE__,__LINE__);
-		return;
+		return count;
 	}
 		
 	/* copy received data to HDLC buffer */
@@ -565,6 +565,8 @@ static void n_hdlc_tty_receive(struct tty_struct *tty, const __u8 *data,
 	if (n_hdlc->tty->fasync != NULL)
 		kill_fasync (&n_hdlc->tty->fasync, SIGIO, POLL_IN);
 
+	return count;
+
 }	/* end of n_hdlc_tty_receive() */
 
 /**
diff --git a/drivers/tty/n_r3964.c b/drivers/tty/n_r3964.c
index 5c6c314..43384b3 100644
--- a/drivers/tty/n_r3964.c
+++ b/drivers/tty/n_r3964.c
@@ -139,8 +139,8 @@ static int r3964_ioctl(struct tty_struct *tty, struct file *file,
 static void r3964_set_termios(struct tty_struct *tty, struct ktermios *old);
 static unsigned int r3964_poll(struct tty_struct *tty, struct file *file,
 		struct poll_table_struct *wait);
-static void r3964_receive_buf(struct tty_struct *tty, const unsigned char *cp,
-		char *fp, int count);
+static int r3964_receive_buf(struct tty_struct *tty,
+		const unsigned char *cp, char *fp, int count);
 
 static struct tty_ldisc_ops tty_ldisc_N_R3964 = {
 	.owner = THIS_MODULE,
@@ -1239,8 +1239,8 @@ static unsigned int r3964_poll(struct tty_struct *tty, struct file *file,
 	return result;
 }
 
-static void r3964_receive_buf(struct tty_struct *tty, const unsigned char *cp,
-			char *fp, int count)
+static int r3964_receive_buf(struct tty_struct *tty,
+		const unsigned char *cp, char *fp, int count)
 {
 	struct r3964_info *pInfo = tty->disc_data;
 	const unsigned char *p;
@@ -1257,6 +1257,8 @@ static void r3964_receive_buf(struct tty_struct *tty, const unsigned char *cp,
 		}
 
 	}
+
+	return count;
 }
 
 MODULE_LICENSE("GPL");
diff --git a/drivers/tty/n_tracerouter.c b/drivers/tty/n_tracerouter.c
index 1f063d3..22130db 100644
--- a/drivers/tty/n_tracerouter.c
+++ b/drivers/tty/n_tracerouter.c
@@ -168,13 +168,15 @@ static ssize_t n_tracerouter_write(struct tty_struct *tty, struct file *file,
  * This function takes the input buffer, cp, and passes it to
  * an external API function for processing.
  */
-static void n_tracerouter_receivebuf(struct tty_struct *tty,
+static int n_tracerouter_receivebuf(struct tty_struct *tty,
 					const unsigned char *cp,
 					char *fp, int count)
 {
 	mutex_lock(&routelock);
 	n_tracesink_datadrain((u8 *) cp, count);
 	mutex_unlock(&routelock);
+
+	return count;
 }
 
 /*
diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index d2256d0..c47cd7e 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -1359,7 +1359,7 @@ static void n_tty_write_wakeup(struct tty_struct *tty)
  *	calls one at a time and in order (or using flush_to_ldisc)
  */
 
-static void n_tty_receive_buf(struct tty_struct *tty, const unsigned char *cp,
+static int n_tty_receive_buf(struct tty_struct *tty, const unsigned char *cp,
 			      char *fp, int count)
 {
 	const unsigned char *p;
@@ -1369,7 +1369,7 @@ static void n_tty_receive_buf(struct tty_struct *tty, const unsigned char *cp,
 	unsigned long cpuflags;
 
 	if (!tty->read_buf)
-		return;
+		return count;
 
 	if (tty->real_raw) {
 		spin_lock_irqsave(&tty->read_lock, cpuflags);
@@ -1433,6 +1433,8 @@ static void n_tty_receive_buf(struct tty_struct *tty, const unsigned char *cp,
 	 */
 	if (tty->receive_room < TTY_THRESHOLD_THROTTLE)
 		tty_throttle(tty);
+
+	return count;
 }
 
 int is_ignored(int sig)
diff --git a/include/linux/tty_ldisc.h b/include/linux/tty_ldisc.h
index ff7dc08..e251028 100644
--- a/include/linux/tty_ldisc.h
+++ b/include/linux/tty_ldisc.h
@@ -76,7 +76,7 @@
  * 	tty device.  It is solely the responsibility of the line
  * 	discipline to handle poll requests.
  *
- * void	(*receive_buf)(struct tty_struct *, const unsigned char *cp,
+ * int (*receive_buf)(struct tty_struct *, const unsigned char *cp,
  * 		       char *fp, int count);
  *
  * 	This function is called by the low-level tty driver to send
@@ -84,7 +84,8 @@
  * 	processing.  <cp> is a pointer to the buffer of input
  * 	character received by the device.  <fp> is a pointer to a
  * 	pointer of flag bytes which indicate whether a character was
- * 	received with a parity error, etc.
+ * 	received with a parity error, etc. Returns the amount of bytes
+ * 	received.
  * 
  * void	(*write_wakeup)(struct tty_struct *);
  *
@@ -140,8 +141,8 @@ struct tty_ldisc_ops {
 	/*
 	 * The following routines are called from below.
 	 */
-	void	(*receive_buf)(struct tty_struct *, const unsigned char *cp,
-			       char *fp, int count);
+	int (*receive_buf)(struct tty_struct *,
+			const unsigned char *cp, char *fp, int count);
 	void	(*write_wakeup)(struct tty_struct *);
 	void	(*dcd_change)(struct tty_struct *, unsigned int,
 				struct pps_event_time *);
diff --git a/sound/soc/omap/ams-delta.c b/sound/soc/omap/ams-delta.c
index a67f437..559dc29 100644
--- a/sound/soc/omap/ams-delta.c
+++ b/sound/soc/omap/ams-delta.c
@@ -332,7 +332,7 @@ static int cx81801_hangup(struct tty_struct *tty)
 }
 
 /* Line discipline .receive_buf() */
-static void cx81801_receive(struct tty_struct *tty,
+static int cx81801_receive(struct tty_struct *tty,
 				const unsigned char *cp, char *fp, int count)
 {
 	struct snd_soc_codec *codec = tty->disc_data;
@@ -340,7 +340,7 @@ static void cx81801_receive(struct tty_struct *tty,
 	int apply, ret;
 
 	if (!codec)
-		return;
+		return count;
 
 	if (!codec->hw_write) {
 		/* First modem response, complete setup procedure */
@@ -359,7 +359,7 @@ static void cx81801_receive(struct tty_struct *tty,
 				"Failed to link hook switch to DAPM pins, "
 				"will continue with hook switch unlinked.\n");
 
-		return;
+		return count;
 	}
 
 	v253_ops.receive_buf(tty, cp, fp, count);
@@ -382,6 +382,7 @@ static void cx81801_receive(struct tty_struct *tty,
 						AMS_DELTA_LATCH2_MODEM_CODEC);
 		break;
 	}
+	return count;
 }
 
 /* Line discipline .write_wakeup() */
-- 
1.7.9.1


^ permalink raw reply related

* (unknown), 
From: Matthieu CASTET @ 2012-03-04 17:59 UTC (permalink / raw)
  To: Alan Cox, linux-serial, Felipe Balbi, Toby Gray, Stefan Bigler

Subject: [RFC] tty : make receive_room internal to N_TTY line discipline
In-Reply-To:

This patch try to solve issue exposed on https://lkml.org/lkml/2011/6/7/700

On Tue, Jun 07, 2011 at 07:44:48PM -0700, Linus Torvalds wrote
> I'd love to get rid of receive_room entirely - and just letting the
> tty line discipline handler say how much it actually received. in
> other words, having receive_buf() just tell us how much it used, and
> not looking at receive_room in the caller is absolutely the right
> thing.



^ permalink raw reply

* [PATCH 6/6] vt: push the tty_lock down into the map handling
From: Alan Cox @ 2012-03-02 15:00 UTC (permalink / raw)
  To: linux-kernel, linux-serial
In-Reply-To: <20120302145628.32320.53944.stgit@bob.linux.org.uk>

From: Alan Cox <alan@linux.intel.com>

When we do this it becomes clear the lock we should be holding is the vc
lock, and in fact many of our other helpers are properly invoked this way.

We don't at this point guarantee not to race the keyboard code but the results
of that appear harmless and that was true before we started as well.

We now have no users of tty_lock in the console driver...

Signed-off-by: Alan Cox <alan@linux.intel.com>
---

 drivers/tty/vt/consolemap.c |  119 ++++++++++++++++++++++++++++++++-----------
 drivers/tty/vt/vt_ioctl.c   |   25 ++-------
 include/linux/vt_kern.h     |    1 
 3 files changed, 93 insertions(+), 52 deletions(-)


diff --git a/drivers/tty/vt/consolemap.c b/drivers/tty/vt/consolemap.c
index a0f3d6c..299fa08 100644
--- a/drivers/tty/vt/consolemap.c
+++ b/drivers/tty/vt/consolemap.c
@@ -312,6 +312,7 @@ int con_set_trans_old(unsigned char __user * arg)
 	if (!access_ok(VERIFY_READ, arg, E_TABSZ))
 		return -EFAULT;
 
+	console_lock();
 	for (i=0; i<E_TABSZ ; i++) {
 		unsigned char uc;
 		__get_user(uc, arg+i);
@@ -319,6 +320,7 @@ int con_set_trans_old(unsigned char __user * arg)
 	}
 
 	update_user_maps();
+	console_unlock();
 	return 0;
 }
 
@@ -330,11 +332,13 @@ int con_get_trans_old(unsigned char __user * arg)
 	if (!access_ok(VERIFY_WRITE, arg, E_TABSZ))
 		return -EFAULT;
 
+	console_lock();
 	for (i=0; i<E_TABSZ ; i++)
-	  {
-	    ch = conv_uni_to_pc(vc_cons[fg_console].d, p[i]);
-	    __put_user((ch & ~0xff) ? 0 : ch, arg+i);
-	  }
+	{
+		ch = conv_uni_to_pc(vc_cons[fg_console].d, p[i]);
+		__put_user((ch & ~0xff) ? 0 : ch, arg+i);
+	}
+	console_unlock();
 	return 0;
 }
 
@@ -346,6 +350,7 @@ int con_set_trans_new(ushort __user * arg)
 	if (!access_ok(VERIFY_READ, arg, E_TABSZ*sizeof(unsigned short)))
 		return -EFAULT;
 
+	console_lock();
 	for (i=0; i<E_TABSZ ; i++) {
 		unsigned short us;
 		__get_user(us, arg+i);
@@ -353,6 +358,7 @@ int con_set_trans_new(ushort __user * arg)
 	}
 
 	update_user_maps();
+	console_unlock();
 	return 0;
 }
 
@@ -364,8 +370,10 @@ int con_get_trans_new(ushort __user * arg)
 	if (!access_ok(VERIFY_WRITE, arg, E_TABSZ*sizeof(unsigned short)))
 		return -EFAULT;
 
+	console_lock();
 	for (i=0; i<E_TABSZ ; i++)
 	  __put_user(p[i], arg+i);
+	console_unlock();
 	
 	return 0;
 }
@@ -407,6 +415,7 @@ static void con_release_unimap(struct uni_pagedir *p)
 	}
 }
 
+/* Caller must hold the console lock */
 void con_free_unimap(struct vc_data *vc)
 {
 	struct uni_pagedir *p;
@@ -487,17 +496,21 @@ con_insert_unipair(struct uni_pagedir *p, u_short unicode, u_short fontpos)
 	return 0;
 }
 
-/* ui is a leftover from using a hashtable, but might be used again */
-int con_clear_unimap(struct vc_data *vc, struct unimapinit *ui)
+/* ui is a leftover from using a hashtable, but might be used again
+   Caller must hold the lock */
+static int con_do_clear_unimap(struct vc_data *vc, struct unimapinit *ui)
 {
 	struct uni_pagedir *p, *q;
-  
+
 	p = (struct uni_pagedir *)*vc->vc_uni_pagedir_loc;
-	if (p && p->readonly) return -EIO;
+	if (p && p->readonly)
+		return -EIO;
+
 	if (!p || --p->refcount) {
 		q = kzalloc(sizeof(*p), GFP_KERNEL);
 		if (!q) {
-			if (p) p->refcount++;
+			if (p)
+				p->refcount++;
 			return -ENOMEM;
 		}
 		q->refcount=1;
@@ -511,22 +524,41 @@ int con_clear_unimap(struct vc_data *vc, struct unimapinit *ui)
 	return 0;
 }
 
+int con_clear_unimap(struct vc_data *vc, struct unimapinit *ui)
+{
+	int ret;
+	console_lock();
+	ret = con_do_clear_unimap(vc, ui);
+	console_unlock();
+	return ret;
+}
+	
 int con_set_unimap(struct vc_data *vc, ushort ct, struct unipair __user *list)
 {
 	int err = 0, err1, i;
 	struct uni_pagedir *p, *q;
 
+	console_lock();
 	p = (struct uni_pagedir *)*vc->vc_uni_pagedir_loc;
-	if (p->readonly) return -EIO;
+	if (p->readonly) {
+		console_unlock();
+		return -EIO;
+	}
 	
-	if (!ct) return 0;
+	if (!ct) {
+		console_unlock();
+		return 0;
+	}
 	
 	if (p->refcount > 1) {
 		int j, k;
 		u16 **p1, *p2, l;
 		
-		err1 = con_clear_unimap(vc, NULL);
-		if (err1) return err1;
+		err1 = con_do_clear_unimap(vc, NULL);
+		if (err1) {
+			console_unlock();
+			return err1;
+		}
 		
 		q = (struct uni_pagedir *)*vc->vc_uni_pagedir_loc;
 		for (i = 0, l = 0; i < 32; i++)
@@ -541,6 +573,7 @@ int con_set_unimap(struct vc_data *vc, ushort ct, struct unipair __user *list)
 						*vc->vc_uni_pagedir_loc = (unsigned long)p;
 						con_release_unimap(q);
 						kfree(q);
+						console_unlock();
 						return err1; 
 					}
               			}
@@ -557,21 +590,30 @@ int con_set_unimap(struct vc_data *vc, ushort ct, struct unipair __user *list)
 		list++;
 	}
 	
-	if (con_unify_unimap(vc, p))
+	if (con_unify_unimap(vc, p)) {
+		console_unlock();
 		return err;
+	}
 
 	for (i = 0; i <= 3; i++)
 		set_inverse_transl(vc, p, i); /* Update all inverse translations */
 	set_inverse_trans_unicode(vc, p);
-  
+
+	console_unlock();
 	return err;
 }
 
-/* Loads the unimap for the hardware font, as defined in uni_hash.tbl.
-   The representation used was the most compact I could come up
-   with.  This routine is executed at sys_setup time, and when the
-   PIO_FONTRESET ioctl is called. */
-
+/**
+ *	con_set_default_unimap	-	set default unicode map
+ *	@vc: the console we are updating
+ *
+ *	Loads the unimap for the hardware font, as defined in uni_hash.tbl.
+ *	The representation used was the most compact I could come up
+ *	with.  This routine is executed at video setup, and when the
+ *	PIO_FONTRESET ioctl is called. 
+ *
+ *	The caller must hold the console lock
+ */
 int con_set_default_unimap(struct vc_data *vc)
 {
 	int i, j, err = 0, err1;
@@ -582,6 +624,7 @@ int con_set_default_unimap(struct vc_data *vc)
 		p = (struct uni_pagedir *)*vc->vc_uni_pagedir_loc;
 		if (p == dflt)
 			return 0;
+
 		dflt->refcount++;
 		*vc->vc_uni_pagedir_loc = (unsigned long)dflt;
 		if (p && !--p->refcount) {
@@ -593,8 +636,9 @@ int con_set_default_unimap(struct vc_data *vc)
 	
 	/* The default font is always 256 characters */
 
-	err = con_clear_unimap(vc, NULL);
-	if (err) return err;
+	err = con_do_clear_unimap(vc, NULL);
+	if (err)
+		return err;
     
 	p = (struct uni_pagedir *)*vc->vc_uni_pagedir_loc;
 	q = dfont_unitable;
@@ -619,6 +663,13 @@ int con_set_default_unimap(struct vc_data *vc)
 }
 EXPORT_SYMBOL(con_set_default_unimap);
 
+/**
+ *	con_copy_unimap		-	copy unimap between two vts
+ *	@dst_vc: target
+ *	@src_vt: source
+ *
+ *	The caller must hold the console lock when invoking this method
+ */
 int con_copy_unimap(struct vc_data *dst_vc, struct vc_data *src_vc)
 {
 	struct uni_pagedir *q;
@@ -633,13 +684,23 @@ int con_copy_unimap(struct vc_data *dst_vc, struct vc_data *src_vc)
 	*dst_vc->vc_uni_pagedir_loc = (long)q;
 	return 0;
 }
+EXPORT_SYMBOL(con_copy_unimap);
 
+/**
+ *	con_get_unimap		-	get the unicode map
+ *	@vc: the console to read from
+ *
+ *	Read the console unicode data for this console. Called from the ioctl
+ *	handlers.
+ */
 int con_get_unimap(struct vc_data *vc, ushort ct, ushort __user *uct, struct unipair __user *list)
 {
 	int i, j, k, ect;
 	u16 **p1, *p2;
 	struct uni_pagedir *p;
 
+	console_lock();
+
 	ect = 0;
 	if (*vc->vc_uni_pagedir_loc) {
 		p = (struct uni_pagedir *)*vc->vc_uni_pagedir_loc;
@@ -659,22 +720,19 @@ int con_get_unimap(struct vc_data *vc, ushort ct, ushort __user *uct, struct uni
 				}
 	}
 	__put_user(ect, uct);
+	console_unlock();
 	return ((ect <= ct) ? 0 : -ENOMEM);
 }
 
-void con_protect_unimap(struct vc_data *vc, int rdonly)
-{
-	struct uni_pagedir *p = (struct uni_pagedir *)*vc->vc_uni_pagedir_loc;
-	
-	if (p)
-		p->readonly = rdonly;
-}
-
 /*
  * Always use USER_MAP. These functions are used by the keyboard,
  * which shouldn't be affected by G0/G1 switching, etc.
  * If the user map still contains default values, i.e. the
  * direct-to-font mapping, then assume user is using Latin1.
+ *
+ * FIXME: at some point we need to decide if we want to lock the table
+ * update element itself via the keyboard_event_lock for consistency with the
+ * keyboard driver as well as the consoles
  */
 /* may be called during an interrupt */
 u32 conv_8bit_to_uni(unsigned char c)
@@ -742,4 +800,3 @@ console_map_init(void)
 			con_set_default_unimap(vc_cons[i].d);
 }
 
-EXPORT_SYMBOL(con_copy_unimap);
diff --git a/drivers/tty/vt/vt_ioctl.c b/drivers/tty/vt/vt_ioctl.c
index ede2ef1..6461854 100644
--- a/drivers/tty/vt/vt_ioctl.c
+++ b/drivers/tty/vt/vt_ioctl.c
@@ -910,7 +910,9 @@ int vt_ioctl(struct tty_struct *tty,
 		ret = con_font_op(vc_cons[fg_console].d, &op);
 		if (ret)
 			break;
+		console_lock();
 		con_set_default_unimap(vc_cons[fg_console].d);
+		console_unlock();
 		break;
 		}
 #endif
@@ -934,33 +936,23 @@ int vt_ioctl(struct tty_struct *tty,
 	case PIO_SCRNMAP:
 		if (!perm)
 			ret = -EPERM;
-		else {
-			tty_lock();
+		else
 			ret = con_set_trans_old(up);
-			tty_unlock();
-		}
 		break;
 
 	case GIO_SCRNMAP:
-		tty_lock();
 		ret = con_get_trans_old(up);
-		tty_unlock();
 		break;
 
 	case PIO_UNISCRNMAP:
 		if (!perm)
 			ret = -EPERM;
-		else {
-			tty_lock();
+		else
 			ret = con_set_trans_new(up);
-			tty_unlock();
-		}
 		break;
 
 	case GIO_UNISCRNMAP:
-		tty_lock();
 		ret = con_get_trans_new(up);
-		tty_unlock();
 		break;
 
 	case PIO_UNIMAPCLR:
@@ -970,19 +962,14 @@ int vt_ioctl(struct tty_struct *tty,
 		ret = copy_from_user(&ui, up, sizeof(struct unimapinit));
 		if (ret)
 			ret = -EFAULT;
-		else {
-			tty_lock();
+		else
 			con_clear_unimap(vc, &ui);
-			tty_unlock();
-		}
 		break;
 	      }
 
 	case PIO_UNIMAP:
 	case GIO_UNIMAP:
-		tty_lock();
 		ret = do_unimap_ioctl(cmd, up, perm, vc);
-		tty_unlock();
 		break;
 
 	case VT_LOCKSWITCH:
@@ -1196,9 +1183,7 @@ long vt_compat_ioctl(struct tty_struct *tty,
 
 	case PIO_UNIMAP:
 	case GIO_UNIMAP:
-		tty_lock();
 		ret = compat_unimap_ioctl(cmd, up, perm, vc);
-		tty_unlock();
 		break;
 
 	/*
diff --git a/include/linux/vt_kern.h b/include/linux/vt_kern.h
index e33d77f..50ae7d0 100644
--- a/include/linux/vt_kern.h
+++ b/include/linux/vt_kern.h
@@ -70,7 +70,6 @@ int con_set_unimap(struct vc_data *vc, ushort ct, struct unipair __user *list);
 int con_get_unimap(struct vc_data *vc, ushort ct, ushort __user *uct, struct unipair __user *list);
 int con_set_default_unimap(struct vc_data *vc);
 void con_free_unimap(struct vc_data *vc);
-void con_protect_unimap(struct vc_data *vc, int rdonly);
 int con_copy_unimap(struct vc_data *dst_vc, struct vc_data *src_vc);
 
 #define vc_translate(vc, c) ((vc)->vc_translate[(c) |			\

^ permalink raw reply related

* [PATCH 4/6] vt: waitevent is self locked so drop the tty_lock
From: Alan Cox @ 2012-03-02 14:59 UTC (permalink / raw)
  To: linux-kernel, linux-serial
In-Reply-To: <20120302145628.32320.53944.stgit@bob.linux.org.uk>

From: Alan Cox <alan@linux.intel.com>

Signed-off-by: Alan Cox <alan@linux.intel.com>
---

 drivers/tty/vt/vt_ioctl.c |   11 ++---------
 1 files changed, 2 insertions(+), 9 deletions(-)


diff --git a/drivers/tty/vt/vt_ioctl.c b/drivers/tty/vt/vt_ioctl.c
index c6720be..ede2ef1 100644
--- a/drivers/tty/vt/vt_ioctl.c
+++ b/drivers/tty/vt/vt_ioctl.c
@@ -130,7 +130,7 @@ static void vt_event_wait(struct vt_event_wait *vw)
 	list_add(&vw->list, &vt_events);
 	spin_unlock_irqrestore(&vt_event_lock, flags);
 	/* Wait for it to pass */
-	wait_event_interruptible_tty(vt_event_waitqueue, vw->done);
+	wait_event_interruptible(vt_event_waitqueue, vw->done);
 	/* Dequeue it */
 	spin_lock_irqsave(&vt_event_lock, flags);
 	list_del(&vw->list);
@@ -671,11 +671,8 @@ int vt_ioctl(struct tty_struct *tty,
 			return -EPERM;
 		if (arg == 0 || arg > MAX_NR_CONSOLES)
 			ret = -ENXIO;
-		else {
-			tty_lock();
+		else
 			ret = vt_waitactive(arg);
-			tty_unlock();
-		}
 		break;
 
 	/*
@@ -1426,14 +1423,10 @@ int vt_move_to_console(unsigned int vt, int alloc)
 		return -EIO;
 	}
 	console_unlock();
-	/* Review: I don't see why we need tty_lock here FIXME */
-	tty_lock();
 	if (vt_waitactive(vt + 1)) {
 		pr_debug("Suspend: Can't switch VCs.");
-		tty_unlock();
 		return -EINTR;
 	}
-	tty_unlock();
 	return prev;
 }
 

^ permalink raw reply related

* [PATCH 3/6] vt: push down tioclinux cases
From: Alan Cox @ 2012-03-02 14:59 UTC (permalink / raw)
  To: linux-kernel, linux-serial
In-Reply-To: <20120302145628.32320.53944.stgit@bob.linux.org.uk>

From: Alan Cox <alan@linux.intel.com>

Some of this ventures into selection which is still a complete lost cause. We
are not making it any worse. It's completely busted anyway.

Signed-off-by: Alan Cox <alan@linux.intel.com>
---

 drivers/tty/vt/selection.c |   12 ++++++------
 drivers/tty/vt/vt.c        |   12 ++++++++++++
 drivers/tty/vt/vt_ioctl.c  |    2 --
 3 files changed, 18 insertions(+), 8 deletions(-)


diff --git a/drivers/tty/vt/selection.c b/drivers/tty/vt/selection.c
index 738e45a..2a50916 100644
--- a/drivers/tty/vt/selection.c
+++ b/drivers/tty/vt/selection.c
@@ -75,7 +75,7 @@ clear_selection(void) {
 
 /*
  * User settable table: what characters are to be considered alphabetic?
- * 256 bits
+ * 256 bits. FIXME: Needs a locking model.
  */
 static u32 inwordLut[8]={
   0x00000000, /* control chars     */
@@ -307,7 +307,8 @@ int set_selection(const struct tiocl_selection __user *sel, struct tty_struct *t
  * queue of the tty associated with the current console.
  * Invoked by ioctl().
  *
- * Locking: always called with BTM from vt_ioctl
+ * Locking: called without locks. Calls the ldisc wrongly with
+ * unsafe methods,
  */
 int paste_selection(struct tty_struct *tty)
 {
@@ -322,13 +323,12 @@ int paste_selection(struct tty_struct *tty)
 	poke_blanked_console();
 	console_unlock();
 
+	/* FIXME: wtf is this supposed to achieve ? */
 	ld = tty_ldisc_ref(tty);
-	if (!ld) {
-		tty_unlock();
+	if (!ld)
 		ld = tty_ldisc_ref_wait(tty);
-		tty_lock();
-	}
 
+	/* FIXME: this is completely unsafe */
 	add_wait_queue(&vc->paste_wait, &wait);
 	while (sel_buffer && sel_buffer_lth > pasted) {
 		set_current_state(TASK_INTERRUPTIBLE);
diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
index 8439303..280a4c4 100644
--- a/drivers/tty/vt/vt.c
+++ b/drivers/tty/vt/vt.c
@@ -2637,11 +2637,15 @@ int tioclinux(struct tty_struct *tty, unsigned long arg)
 			ret = __put_user(data, p);
 			break;
 		case TIOCL_GETMOUSEREPORTING:
+			console_lock();	/* May be overkill */
 			data = mouse_reporting();
+			console_unlock();
 			ret = __put_user(data, p);
 			break;
 		case TIOCL_SETVESABLANK:
+			console_lock();
 			ret = set_vesa_blanking(p);
+			console_unlock();
 			break;
 		case TIOCL_GETKMSGREDIRECT:
 			data = vt_get_kmsg_redirect();
@@ -2658,13 +2662,21 @@ int tioclinux(struct tty_struct *tty, unsigned long arg)
 			}
 			break;
 		case TIOCL_GETFGCONSOLE:
+			/* No locking needed as this is a transiently
+			   correct return anyway if the caller hasn't
+			   disabled switching */
 			ret = fg_console;
 			break;
 		case TIOCL_SCROLLCONSOLE:
 			if (get_user(lines, (s32 __user *)(p+4))) {
 				ret = -EFAULT;
 			} else {
+				/* Need the console lock here. Note that lots
+				   of other calls need fixing before the lock
+				   is actually useful ! */
+				console_lock();
 				scrollfront(vc_cons[fg_console].d, lines);
+				console_unlock();
 				ret = 0;
 			}
 			break;
diff --git a/drivers/tty/vt/vt_ioctl.c b/drivers/tty/vt/vt_ioctl.c
index e05094d..c6720be 100644
--- a/drivers/tty/vt/vt_ioctl.c
+++ b/drivers/tty/vt/vt_ioctl.c
@@ -298,9 +298,7 @@ int vt_ioctl(struct tty_struct *tty,
  
 	switch (cmd) {
 	case TIOCLINUX:
-		tty_lock();
 		ret = tioclinux(tty, arg);
-		tty_unlock();
 		break;
 	case KIOCSOUND:
 		if (!perm)

^ permalink raw reply related

* [PATCH 2/6] vt: push down the tty lock so we can see what is left to tackle
From: Alan Cox @ 2012-03-02 14:59 UTC (permalink / raw)
  To: linux-kernel, linux-serial
In-Reply-To: <20120302145628.32320.53944.stgit@bob.linux.org.uk>

From: Alan Cox <alan@linux.intel.com>

At this point we have the tty_lock guarding a couple of oddities, plus the
translation and unimap still.

We also extend the console_lock in a couple of spots where coverage is wrong
and switch vcs_open to use the right lock !

[Fixed the locking issue Jiri reported]

Signed-off-by: Alan Cox <alan@linux.intel.com>
---

 drivers/tty/vt/vc_screen.c |    4 +-
 drivers/tty/vt/vt_ioctl.c  |   92 +++++++++++++++++++++++++++-----------------
 2 files changed, 59 insertions(+), 37 deletions(-)


diff --git a/drivers/tty/vt/vc_screen.c b/drivers/tty/vt/vc_screen.c
index 7a367ff..fa7268a 100644
--- a/drivers/tty/vt/vc_screen.c
+++ b/drivers/tty/vt/vc_screen.c
@@ -608,10 +608,10 @@ vcs_open(struct inode *inode, struct file *filp)
 	unsigned int currcons = iminor(inode) & 127;
 	int ret = 0;
 	
-	tty_lock();
+	console_lock();
 	if(currcons && !vc_cons_allocated(currcons-1))
 		ret = -ENXIO;
-	tty_unlock();
+	console_unlock();
 	return ret;
 }
 
diff --git a/drivers/tty/vt/vt_ioctl.c b/drivers/tty/vt/vt_ioctl.c
index 28ca0aa..e05094d 100644
--- a/drivers/tty/vt/vt_ioctl.c
+++ b/drivers/tty/vt/vt_ioctl.c
@@ -281,7 +281,6 @@ int vt_ioctl(struct tty_struct *tty,
 
 	console = vc->vc_num;
 
-	tty_lock();
 
 	if (!vc_cons_allocated(console)) { 	/* impossible? */
 		ret = -ENOIOCTLCMD;
@@ -299,16 +298,18 @@ int vt_ioctl(struct tty_struct *tty,
  
 	switch (cmd) {
 	case TIOCLINUX:
+		tty_lock();
 		ret = tioclinux(tty, arg);
+		tty_unlock();
 		break;
 	case KIOCSOUND:
 		if (!perm)
-			goto eperm;
+			return -EPERM;
 		/*
 		 * The use of PIT_TICK_RATE is historic, it used to be
 		 * the platform-dependent CLOCK_TICK_RATE between 2.6.12
 		 * and 2.6.36, which was a minor but unfortunate ABI
-		 * change.
+		 * change. kd_mksound is locked by the input layer.
 		 */
 		if (arg)
 			arg = PIT_TICK_RATE / arg;
@@ -317,7 +318,7 @@ int vt_ioctl(struct tty_struct *tty,
 
 	case KDMKTONE:
 		if (!perm)
-			goto eperm;
+			return -EPERM;
 	{
 		unsigned int ticks, count;
 		
@@ -335,7 +336,7 @@ int vt_ioctl(struct tty_struct *tty,
 
 	case KDGKBTYPE:
 		/*
-		 * this is naive.
+		 * this is naïve.
 		 */
 		ucval = KB_101;
 		ret = put_user(ucval, (char __user *)arg);
@@ -353,6 +354,8 @@ int vt_ioctl(struct tty_struct *tty,
 		/*
 		 * KDADDIO and KDDELIO may be able to add ports beyond what
 		 * we reject here, but to be safe...
+		 *
+		 * These are locked internally via sys_ioperm
 		 */
 		if (arg < GPFIRST || arg > GPLAST) {
 			ret = -EINVAL;
@@ -375,7 +378,7 @@ int vt_ioctl(struct tty_struct *tty,
 		struct kbd_repeat kbrep;
 		
 		if (!capable(CAP_SYS_TTY_CONFIG))
-			goto eperm;
+			return -EPERM;
 
 		if (copy_from_user(&kbrep, up, sizeof(struct kbd_repeat))) {
 			ret =  -EFAULT;
@@ -399,7 +402,7 @@ int vt_ioctl(struct tty_struct *tty,
 		 * need to restore their engine state. --BenH
 		 */
 		if (!perm)
-			goto eperm;
+			return -EPERM;
 		switch (arg) {
 		case KD_GRAPHICS:
 			break;
@@ -412,6 +415,7 @@ int vt_ioctl(struct tty_struct *tty,
 			ret = -EINVAL;
 			goto out;
 		}
+		/* FIXME: this needs the console lock extending */
 		if (vc->vc_mode == (unsigned char) arg)
 			break;
 		vc->vc_mode = (unsigned char) arg;
@@ -443,7 +447,7 @@ int vt_ioctl(struct tty_struct *tty,
 
 	case KDSKBMODE:
 		if (!perm)
-			goto eperm;
+			return -EPERM;
 		ret = vt_do_kdskbmode(console, arg);
 		if (ret == 0)
 			tty_ldisc_flush(tty);
@@ -512,7 +516,7 @@ int vt_ioctl(struct tty_struct *tty,
 	case KDSIGACCEPT:
 	{
 		if (!perm || !capable(CAP_KILL))
-			goto eperm;
+			return -EPERM;
 		if (!valid_signal(arg) || arg < 1 || arg == SIGKILL)
 			ret = -EINVAL;
 		else {
@@ -530,7 +534,7 @@ int vt_ioctl(struct tty_struct *tty,
 		struct vt_mode tmp;
 
 		if (!perm)
-			goto eperm;
+			return -EPERM;
 		if (copy_from_user(&tmp, up, sizeof(struct vt_mode))) {
 			ret = -EFAULT;
 			goto out;
@@ -576,6 +580,7 @@ int vt_ioctl(struct tty_struct *tty,
 		struct vt_stat __user *vtstat = up;
 		unsigned short state, mask;
 
+		/* Review: FIXME: Console lock ? */
 		if (put_user(fg_console + 1, &vtstat->v_active))
 			ret = -EFAULT;
 		else {
@@ -593,6 +598,7 @@ int vt_ioctl(struct tty_struct *tty,
 	 * Returns the first available (non-opened) console.
 	 */
 	case VT_OPENQRY:
+		/* FIXME: locking ? - but then this is a stupid API */
 		for (i = 0; i < MAX_NR_CONSOLES; ++i)
 			if (! VT_IS_IN_USE(i))
 				break;
@@ -606,7 +612,7 @@ int vt_ioctl(struct tty_struct *tty,
 	 */
 	case VT_ACTIVATE:
 		if (!perm)
-			goto eperm;
+			return -EPERM;
 		if (arg == 0 || arg > MAX_NR_CONSOLES)
 			ret =  -ENXIO;
 		else {
@@ -625,7 +631,7 @@ int vt_ioctl(struct tty_struct *tty,
 		struct vt_setactivate vsa;
 
 		if (!perm)
-			goto eperm;
+			return -EPERM;
 
 		if (copy_from_user(&vsa, (struct vt_setactivate __user *)arg,
 					sizeof(struct vt_setactivate))) {
@@ -653,6 +659,7 @@ int vt_ioctl(struct tty_struct *tty,
 			if (ret)
 				break;
 			/* Commence switch and lock */
+			/* Review set_console locks */
 			set_console(vsa.console);
 		}
 		break;
@@ -663,11 +670,14 @@ int vt_ioctl(struct tty_struct *tty,
 	 */
 	case VT_WAITACTIVE:
 		if (!perm)
-			goto eperm;
+			return -EPERM;
 		if (arg == 0 || arg > MAX_NR_CONSOLES)
 			ret = -ENXIO;
-		else
+		else {
+			tty_lock();
 			ret = vt_waitactive(arg);
+			tty_unlock();
+		}
 		break;
 
 	/*
@@ -682,16 +692,17 @@ int vt_ioctl(struct tty_struct *tty,
 	 */
 	case VT_RELDISP:
 		if (!perm)
-			goto eperm;
+			return -EPERM;
 
+		console_lock();
 		if (vc->vt_mode.mode != VT_PROCESS) {
+			console_unlock();
 			ret = -EINVAL;
 			break;
 		}
 		/*
 		 * Switching-from response
 		 */
-		console_lock();
 		if (vc->vt_newvt >= 0) {
 			if (arg == 0)
 				/*
@@ -768,7 +779,7 @@ int vt_ioctl(struct tty_struct *tty,
 
 		ushort ll,cc;
 		if (!perm)
-			goto eperm;
+			return -EPERM;
 		if (get_user(ll, &vtsizes->v_rows) ||
 		    get_user(cc, &vtsizes->v_cols))
 			ret = -EFAULT;
@@ -779,6 +790,7 @@ int vt_ioctl(struct tty_struct *tty,
 
 				if (vc) {
 					vc->vc_resize_user = 1;
+					/* FIXME: review v tty lock */
 					vc_resize(vc_cons[i].d, cc, ll);
 				}
 			}
@@ -792,7 +804,7 @@ int vt_ioctl(struct tty_struct *tty,
 		struct vt_consize __user *vtconsize = up;
 		ushort ll,cc,vlin,clin,vcol,ccol;
 		if (!perm)
-			goto eperm;
+			return -EPERM;
 		if (!access_ok(VERIFY_READ, vtconsize,
 				sizeof(struct vt_consize))) {
 			ret = -EFAULT;
@@ -848,7 +860,7 @@ int vt_ioctl(struct tty_struct *tty,
 
 	case PIO_FONT: {
 		if (!perm)
-			goto eperm;
+			return -EPERM;
 		op.op = KD_FONT_OP_SET;
 		op.flags = KD_FONT_FLAG_OLD | KD_FONT_FLAG_DONT_RECALC;	/* Compatibility */
 		op.width = 8;
@@ -889,7 +901,7 @@ int vt_ioctl(struct tty_struct *tty,
 	case PIO_FONTRESET:
 	{
 		if (!perm)
-			goto eperm;
+			return -EPERM;
 
 #ifdef BROKEN_GRAPHICS_PROGRAMS
 		/* With BROKEN_GRAPHICS_PROGRAMS defined, the default
@@ -915,7 +927,7 @@ int vt_ioctl(struct tty_struct *tty,
 			break;
 		}
 		if (!perm && op.op != KD_FONT_OP_GET)
-			goto eperm;
+			return -EPERM;
 		ret = con_font_op(vc, &op);
 		if (ret)
 			break;
@@ -927,50 +939,65 @@ int vt_ioctl(struct tty_struct *tty,
 	case PIO_SCRNMAP:
 		if (!perm)
 			ret = -EPERM;
-		else
+		else {
+			tty_lock();
 			ret = con_set_trans_old(up);
+			tty_unlock();
+		}
 		break;
 
 	case GIO_SCRNMAP:
+		tty_lock();
 		ret = con_get_trans_old(up);
+		tty_unlock();
 		break;
 
 	case PIO_UNISCRNMAP:
 		if (!perm)
 			ret = -EPERM;
-		else
+		else {
+			tty_lock();
 			ret = con_set_trans_new(up);
+			tty_unlock();
+		}
 		break;
 
 	case GIO_UNISCRNMAP:
+		tty_lock();
 		ret = con_get_trans_new(up);
+		tty_unlock();
 		break;
 
 	case PIO_UNIMAPCLR:
 	      { struct unimapinit ui;
 		if (!perm)
-			goto eperm;
+			return -EPERM;
 		ret = copy_from_user(&ui, up, sizeof(struct unimapinit));
 		if (ret)
 			ret = -EFAULT;
-		else
+		else {
+			tty_lock();
 			con_clear_unimap(vc, &ui);
+			tty_unlock();
+		}
 		break;
 	      }
 
 	case PIO_UNIMAP:
 	case GIO_UNIMAP:
+		tty_lock();
 		ret = do_unimap_ioctl(cmd, up, perm, vc);
+		tty_unlock();
 		break;
 
 	case VT_LOCKSWITCH:
 		if (!capable(CAP_SYS_TTY_CONFIG))
-			goto eperm;
+			return -EPERM;
 		vt_dont_switch = 1;
 		break;
 	case VT_UNLOCKSWITCH:
 		if (!capable(CAP_SYS_TTY_CONFIG))
-			goto eperm;
+			return -EPERM;
 		vt_dont_switch = 0;
 		break;
 	case VT_GETHIFONTMASK:
@@ -984,11 +1011,7 @@ int vt_ioctl(struct tty_struct *tty,
 		ret = -ENOIOCTLCMD;
 	}
 out:
-	tty_unlock();
 	return ret;
-eperm:
-	ret = -EPERM;
-	goto out;
 }
 
 void reset_vc(struct vc_data *vc)
@@ -1150,8 +1173,6 @@ long vt_compat_ioctl(struct tty_struct *tty,
 
 	console = vc->vc_num;
 
-	tty_lock();
-
 	if (!vc_cons_allocated(console)) { 	/* impossible? */
 		ret = -ENOIOCTLCMD;
 		goto out;
@@ -1180,7 +1201,9 @@ long vt_compat_ioctl(struct tty_struct *tty,
 
 	case PIO_UNIMAP:
 	case GIO_UNIMAP:
+		tty_lock();
 		ret = compat_unimap_ioctl(cmd, up, perm, vc);
+		tty_unlock();
 		break;
 
 	/*
@@ -1217,11 +1240,9 @@ long vt_compat_ioctl(struct tty_struct *tty,
 		goto fallback;
 	}
 out:
-	tty_unlock();
 	return ret;
 
 fallback:
-	tty_unlock();
 	return vt_ioctl(tty, cmd, arg);
 }
 
@@ -1407,6 +1428,7 @@ int vt_move_to_console(unsigned int vt, int alloc)
 		return -EIO;
 	}
 	console_unlock();
+	/* Review: I don't see why we need tty_lock here FIXME */
 	tty_lock();
 	if (vt_waitactive(vt + 1)) {
 		pr_debug("Suspend: Can't switch VCs.");

^ permalink raw reply related

* [PATCH 1/6] vt: sort out locking for font handling
From: Alan Cox @ 2012-03-02 14:59 UTC (permalink / raw)
  To: linux-kernel, linux-serial
In-Reply-To: <20120302145628.32320.53944.stgit@bob.linux.org.uk>

From: Alan Cox <alan@linux.intel.com>

The font methods are console_lock covered. Unfortunately they don't extend
the lock over all the needed tests.

Signed-off-by: Alan Cox <alan@linux.intel.com>
---

 drivers/tty/vt/vt.c |   23 +++++++++++++----------
 1 files changed, 13 insertions(+), 10 deletions(-)


diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
index 8205578..8439303 100644
--- a/drivers/tty/vt/vt.c
+++ b/drivers/tty/vt/vt.c
@@ -3972,9 +3972,6 @@ static int con_font_get(struct vc_data *vc, struct console_font_op *op)
 	int rc = -EINVAL;
 	int c;
 
-	if (vc->vc_mode != KD_TEXT)
-		return -EINVAL;
-
 	if (op->data) {
 		font.data = kmalloc(max_font_size, GFP_KERNEL);
 		if (!font.data)
@@ -3983,7 +3980,9 @@ static int con_font_get(struct vc_data *vc, struct console_font_op *op)
 		font.data = NULL;
 
 	console_lock();
-	if (vc->vc_sw->con_font_get)
+	if (vc->vc_mode != KD_TEXT)
+		rc = -EINVAL;
+	else if (vc->vc_sw->con_font_get)
 		rc = vc->vc_sw->con_font_get(vc, &font);
 	else
 		rc = -ENOSYS;
@@ -4065,7 +4064,9 @@ static int con_font_set(struct vc_data *vc, struct console_font_op *op)
 	if (IS_ERR(font.data))
 		return PTR_ERR(font.data);
 	console_lock();
-	if (vc->vc_sw->con_font_set)
+	if (vc->vc_mode != KD_TEXT)
+		rc = -EINVAL;
+	else if (vc->vc_sw->con_font_set)
 		rc = vc->vc_sw->con_font_set(vc, &font, op->flags);
 	else
 		rc = -ENOSYS;
@@ -4081,8 +4082,6 @@ static int con_font_default(struct vc_data *vc, struct console_font_op *op)
 	char *s = name;
 	int rc;
 
-	if (vc->vc_mode != KD_TEXT)
-		return -EINVAL;
 
 	if (!op->data)
 		s = NULL;
@@ -4092,6 +4091,10 @@ static int con_font_default(struct vc_data *vc, struct console_font_op *op)
 		name[MAX_FONT_NAME - 1] = 0;
 
 	console_lock();
+	if (vc->vc_mode != KD_TEXT) {
+		console_unlock();
+		return -EINVAL;
+	}
 	if (vc->vc_sw->con_font_default)
 		rc = vc->vc_sw->con_font_default(vc, &font, s);
 	else
@@ -4109,11 +4112,11 @@ static int con_font_copy(struct vc_data *vc, struct console_font_op *op)
 	int con = op->height;
 	int rc;
 
-	if (vc->vc_mode != KD_TEXT)
-		return -EINVAL;
 
 	console_lock();
-	if (!vc->vc_sw->con_font_copy)
+	if (vc->vc_mode != KD_TEXT)
+		rc = -EINVAL;
+	else if (!vc->vc_sw->con_font_copy)
 		rc = -ENOSYS;
 	else if (con < 0 || !vc_cons_allocated(con))
 		rc = -ENOTTY;

^ permalink raw reply related

* [PATCH 5/6] vt: tackle the main part of the selection logic
From: Alan Cox @ 2012-03-02 15:00 UTC (permalink / raw)
  To: linux-kernel, linux-serial
In-Reply-To: <20120302145628.32320.53944.stgit@bob.linux.org.uk>

From: Alan Cox <alan@linux.intel.com>

We leave the existing paste mess alone and just fix up the vt side of
things.

Signed-off-by: Alan Cox <alan@linux.intel.com>
---

 drivers/tty/vt/selection.c |   39 +++++++++++++++++++++++++++++++--------
 drivers/tty/vt/vt.c        |    2 ++
 2 files changed, 33 insertions(+), 8 deletions(-)


diff --git a/drivers/tty/vt/selection.c b/drivers/tty/vt/selection.c
index 2a50916..8e9b4be 100644
--- a/drivers/tty/vt/selection.c
+++ b/drivers/tty/vt/selection.c
@@ -62,10 +62,14 @@ sel_pos(int n)
 				use_unicode);
 }
 
-/* remove the current selection highlight, if any,
-   from the console holding the selection. */
-void
-clear_selection(void) {
+/**
+ *	clear_selection		-	remove current selection
+ *
+ *	Remove the current selection highlight, if any from the console
+ *	holding the selection. The caller must hold the console lock.
+ */
+void clear_selection(void)
+{
 	highlight_pointer(-1); /* hide the pointer */
 	if (sel_start != -1) {
 		highlight(sel_start, sel_end);
@@ -75,7 +79,7 @@ clear_selection(void) {
 
 /*
  * User settable table: what characters are to be considered alphabetic?
- * 256 bits. FIXME: Needs a locking model.
+ * 256 bits. Locked by the console lock.
  */
 static u32 inwordLut[8]={
   0x00000000, /* control chars     */
@@ -92,10 +96,20 @@ static inline int inword(const u16 c) {
 	return c > 0xff || (( inwordLut[c>>5] >> (c & 0x1F) ) & 1);
 }
 
-/* set inwordLut contents. Invoked by ioctl(). */
+/**
+ *	set loadlut		-	load the LUT table
+ *	@p: user table
+ *
+ *	Load the LUT table from user space. The caller must hold the console
+ *	lock. Make a temporary copy so a partial update doesn't make a mess.
+ */
 int sel_loadlut(char __user *p)
 {
-	return copy_from_user(inwordLut, (u32 __user *)(p+4), 32) ? -EFAULT : 0;
+	u32 tmplut[8];
+	if (copy_from_user(tmplut, (u32 __user *)(p+4), 32))
+		return -EFAULT;
+	memcpy(inwordLut, tmplut, 32);
+	return 0;
 }
 
 /* does screen address p correspond to character at LH/RH edge of screen? */
@@ -131,7 +145,16 @@ static int store_utf8(u16 c, char *p)
     	}
 }
 
-/* set the current selection. Invoked by ioctl() or by kernel code. */
+/**
+ *	set_selection		- 	set the current selection.
+ *	@sel: user selection info
+ *	@tty: the console tty
+ *
+ *	Invoked by the ioctl handle for the vt layer.
+ *
+ *	The entire selection process is managed under the console_lock. It's
+ *	 a lot under the lock but its hardly a performance path
+ */
 int set_selection(const struct tiocl_selection __user *sel, struct tty_struct *tty)
 {
 	struct vc_data *vc = vc_cons[fg_console].d;
diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
index 280a4c4..7d79ca8 100644
--- a/drivers/tty/vt/vt.c
+++ b/drivers/tty/vt/vt.c
@@ -2623,7 +2623,9 @@ int tioclinux(struct tty_struct *tty, unsigned long arg)
 			console_unlock();
 			break;
 		case TIOCL_SELLOADLUT:
+			console_lock();
 			ret = sel_loadlut(p);
+			console_unlock();
 			break;
 		case TIOCL_GETSHIFTSTATE:
 


^ permalink raw reply related

* [PATCH 0/6] Series short description
From: Alan Cox @ 2012-03-02 14:58 UTC (permalink / raw)
  To: linux-kernel, linux-serial

With corrections noted. This is going to cause problems somewhere I'm sure
but I don't see any way to avoid that apart from trying it.

Alan

---

Alan Cox (6):
      vt: push the tty_lock down into the map handling
      vt: tackle the main part of the selection logic
      vt: waitevent is self locked so drop the tty_lock
      vt: push down tioclinux cases
      vt: push down the tty lock so we can see what is left to tackle
      vt: sort out locking for font handling


 drivers/tty/vt/consolemap.c |  119 ++++++++++++++++++++++++++++++++-----------
 drivers/tty/vt/selection.c  |   49 +++++++++++++-----
 drivers/tty/vt/vc_screen.c  |    4 +
 drivers/tty/vt/vt.c         |   37 ++++++++++---
 drivers/tty/vt/vt_ioctl.c   |   68 ++++++++++++-------------
 include/linux/vt_kern.h     |    1 
 6 files changed, 186 insertions(+), 92 deletions(-)

-- 
 "And the nice thing is you don't need to be able to type, because every
  possible keystroke does {something} for/to you. Anything from point out
  off-by-one errors to perform a sex change on your gerbil."
			-- Bill Davidsen describing emacs


^ permalink raw reply

* Re: [PATCH 5/7] serial: remove back and forth conversions in serial_out_sync
From: Paul Gortmaker @ 2012-03-02 14:13 UTC (permalink / raw)
  To: Alan Cox; +Cc: gregkh, linux-serial
In-Reply-To: <20120302112204.4f990c78@bob.linux.org.uk>

On 12-03-02 06:22 AM, Alan Cox wrote:
> On Thu,  1 Mar 2012 21:33:21 -0500
> Paul Gortmaker <paul.gortmaker@windriver.com> wrote:
> 
>> The two callers to serial_out_sync() have a struct port right
>> there in scope, but then pass in a struct 8250_port which then
>> is locally resolved to a struct port.
> 
> This change I don't like. It means serial_out_sync and serial_out end
> up with different parameters which seems odd to say the least. You then
> extend this oddity by adding helpers for port_in/out but not the sync
> one.

I can't argue that.  I'll reorder it so the generic serial_port_X
helpers are introduced 1st, and then amend this to rename the fcn
from serial_out_sync to serial_port_out_sync.   Or I could just
drop is commit entirely.  Any preference one way or the other?

Thanks for the review.
Paul.

> 
> Otherwise it all looks good to me.
> 
> Alan
> 
> 

^ permalink raw reply

* Re: [PATCH 5/7] serial: remove back and forth conversions in serial_out_sync
From: Alan Cox @ 2012-03-02 11:22 UTC (permalink / raw)
  To: Paul Gortmaker; +Cc: gregkh, linux-serial
In-Reply-To: <1330655603-5268-6-git-send-email-paul.gortmaker@windriver.com>

On Thu,  1 Mar 2012 21:33:21 -0500
Paul Gortmaker <paul.gortmaker@windriver.com> wrote:

> The two callers to serial_out_sync() have a struct port right
> there in scope, but then pass in a struct 8250_port which then
> is locally resolved to a struct port.

This change I don't like. It means serial_out_sync and serial_out end
up with different parameters which seems odd to say the least. You then
extend this oddity by adding helpers for port_in/out but not the sync
one.

Otherwise it all looks good to me.

Alan



^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox