* (unknown),
@ 2012-03-04 17:59 Matthieu CASTET
2012-03-04 17:59 ` [PATCH 1/9] tty: make receive_buf() return the amout of bytes received Matthieu CASTET
` (8 more replies)
0 siblings, 9 replies; 15+ messages in thread
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 [flat|nested] 15+ messages in thread* [PATCH 1/9] tty: make receive_buf() return the amout of bytes received 2012-03-04 17:59 (unknown), Matthieu CASTET @ 2012-03-04 17:59 ` Matthieu CASTET 2012-03-05 10:40 ` Alan Cox 2012-03-04 17:59 ` [PATCH 2/9] tty : make n_tty_receive_buf return bytes received in raw mode Matthieu CASTET ` (7 subsequent siblings) 8 siblings, 1 reply; 15+ messages in thread 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 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 [flat|nested] 15+ messages in thread
* Re: [PATCH 1/9] tty: make receive_buf() return the amout of bytes received 2012-03-04 17:59 ` [PATCH 1/9] tty: make receive_buf() return the amout of bytes received Matthieu CASTET @ 2012-03-05 10:40 ` Alan Cox 0 siblings, 0 replies; 15+ messages in thread From: Alan Cox @ 2012-03-05 10:40 UTC (permalink / raw) To: Matthieu CASTET; +Cc: linux-serial, Felipe Balbi, Toby Gray, Stefan Bigler > 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 [flat|nested] 15+ messages in thread
* [PATCH 2/9] tty : make n_tty_receive_buf return bytes received in raw mode 2012-03-04 17:59 (unknown), Matthieu CASTET 2012-03-04 17:59 ` [PATCH 1/9] tty: make receive_buf() return the amout of bytes received Matthieu CASTET @ 2012-03-04 17:59 ` Matthieu CASTET 2012-03-04 17:59 ` [PATCH 3/9] tty: make hci_uart_tty_receive return bytes received Matthieu CASTET ` (6 subsequent siblings) 8 siblings, 0 replies; 15+ messages in thread 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 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 [flat|nested] 15+ messages in thread
* [PATCH 3/9] tty: make hci_uart_tty_receive return bytes received 2012-03-04 17:59 (unknown), Matthieu CASTET 2012-03-04 17:59 ` [PATCH 1/9] tty: make receive_buf() return the amout of bytes received Matthieu CASTET 2012-03-04 17:59 ` [PATCH 2/9] tty : make n_tty_receive_buf return bytes received in raw mode Matthieu CASTET @ 2012-03-04 17:59 ` Matthieu CASTET 2012-03-04 17:59 ` [PATCH 4/9] tty : kill receive_room usage in vt Matthieu CASTET ` (5 subsequent siblings) 8 siblings, 0 replies; 15+ messages in thread 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 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 [flat|nested] 15+ messages in thread
* [PATCH 4/9] tty : kill receive_room usage in vt 2012-03-04 17:59 (unknown), Matthieu CASTET ` (2 preceding siblings ...) 2012-03-04 17:59 ` [PATCH 3/9] tty: make hci_uart_tty_receive return bytes received Matthieu CASTET @ 2012-03-04 17:59 ` Matthieu CASTET 2012-03-05 10:29 ` Alan Cox 2012-03-04 17:59 ` [PATCH 5/9] tty : kill receive_room usage in speakup Matthieu CASTET ` (4 subsequent siblings) 8 siblings, 1 reply; 15+ messages in thread 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 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 [flat|nested] 15+ messages in thread
* Re: [PATCH 4/9] tty : kill receive_room usage in vt 2012-03-04 17:59 ` [PATCH 4/9] tty : kill receive_room usage in vt Matthieu CASTET @ 2012-03-05 10:29 ` Alan Cox 0 siblings, 0 replies; 15+ messages in thread From: Alan Cox @ 2012-03-05 10:29 UTC (permalink / raw) To: Matthieu CASTET; +Cc: linux-serial, Felipe Balbi, Toby Gray, Stefan Bigler 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 [flat|nested] 15+ messages in thread
* [PATCH 5/9] tty : kill receive_room usage in speakup 2012-03-04 17:59 (unknown), Matthieu CASTET ` (3 preceding siblings ...) 2012-03-04 17:59 ` [PATCH 4/9] tty : kill receive_room usage in vt Matthieu CASTET @ 2012-03-04 17:59 ` Matthieu CASTET 2012-03-04 17:59 ` [PATCH 6/9] tty : don't use receive_room in tty_set_ldisc Matthieu CASTET ` (3 subsequent siblings) 8 siblings, 0 replies; 15+ messages in thread 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 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 [flat|nested] 15+ messages in thread
* [PATCH 6/9] tty : don't use receive_room in tty_set_ldisc 2012-03-04 17:59 (unknown), Matthieu CASTET ` (4 preceding siblings ...) 2012-03-04 17:59 ` [PATCH 5/9] tty : kill receive_room usage in speakup Matthieu CASTET @ 2012-03-04 17:59 ` Matthieu CASTET 2012-03-04 17:59 ` [PATCH 7/9] tty : kill receive_room usage in tty_buffer Matthieu CASTET ` (2 subsequent siblings) 8 siblings, 0 replies; 15+ messages in thread 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 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 [flat|nested] 15+ messages in thread
* [PATCH 7/9] tty : kill receive_room usage in tty_buffer 2012-03-04 17:59 (unknown), Matthieu CASTET ` (5 preceding siblings ...) 2012-03-04 17:59 ` [PATCH 6/9] tty : don't use receive_room in tty_set_ldisc Matthieu CASTET @ 2012-03-04 17:59 ` Matthieu CASTET 2012-03-05 10:31 ` Alan Cox 2012-03-04 17:59 ` [PATCH 8/9] tty : kill receive_room usage in mxser Matthieu CASTET 2012-03-04 17:59 ` [PATCH 9/9] tty : make receive_room internal to N_TTY line discipline Matthieu CASTET 8 siblings, 1 reply; 15+ messages in thread 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 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 [flat|nested] 15+ messages in thread
* Re: [PATCH 7/9] tty : kill receive_room usage in tty_buffer 2012-03-04 17:59 ` [PATCH 7/9] tty : kill receive_room usage in tty_buffer Matthieu CASTET @ 2012-03-05 10:31 ` Alan Cox 0 siblings, 0 replies; 15+ messages in thread From: Alan Cox @ 2012-03-05 10:31 UTC (permalink / raw) To: Matthieu CASTET; +Cc: linux-serial, Felipe Balbi, Toby Gray, Stefan Bigler 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 [flat|nested] 15+ messages in thread
* [PATCH 8/9] tty : kill receive_room usage in mxser 2012-03-04 17:59 (unknown), Matthieu CASTET ` (6 preceding siblings ...) 2012-03-04 17:59 ` [PATCH 7/9] tty : kill receive_room usage in tty_buffer Matthieu CASTET @ 2012-03-04 17:59 ` Matthieu CASTET 2012-03-05 10:34 ` Alan Cox 2012-03-04 17:59 ` [PATCH 9/9] tty : make receive_room internal to N_TTY line discipline Matthieu CASTET 8 siblings, 1 reply; 15+ messages in thread 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 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 [flat|nested] 15+ messages in thread
* Re: [PATCH 8/9] tty : kill receive_room usage in mxser 2012-03-04 17:59 ` [PATCH 8/9] tty : kill receive_room usage in mxser Matthieu CASTET @ 2012-03-05 10:34 ` Alan Cox 0 siblings, 0 replies; 15+ messages in thread From: Alan Cox @ 2012-03-05 10:34 UTC (permalink / raw) To: Matthieu CASTET; +Cc: linux-serial, Felipe Balbi, Toby Gray, Stefan Bigler > 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 [flat|nested] 15+ messages in thread
* [PATCH 9/9] tty : make receive_room internal to N_TTY line discipline 2012-03-04 17:59 (unknown), Matthieu CASTET ` (7 preceding siblings ...) 2012-03-04 17:59 ` [PATCH 8/9] tty : kill receive_room usage in mxser Matthieu CASTET @ 2012-03-04 17:59 ` Matthieu CASTET 2012-03-05 10:36 ` Alan Cox 8 siblings, 1 reply; 15+ messages in thread 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 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 [flat|nested] 15+ messages in thread
* Re: [PATCH 9/9] tty : make receive_room internal to N_TTY line discipline 2012-03-04 17:59 ` [PATCH 9/9] tty : make receive_room internal to N_TTY line discipline Matthieu CASTET @ 2012-03-05 10:36 ` Alan Cox 0 siblings, 0 replies; 15+ messages in thread From: Alan Cox @ 2012-03-05 10:36 UTC (permalink / raw) To: Matthieu CASTET; +Cc: linux-serial, Felipe Balbi, Toby Gray, Stefan Bigler 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 [flat|nested] 15+ messages in thread
end of thread, other threads:[~2012-03-05 10:38 UTC | newest] Thread overview: 15+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-03-04 17:59 (unknown), Matthieu CASTET 2012-03-04 17:59 ` [PATCH 1/9] tty: make receive_buf() return the amout of bytes received Matthieu CASTET 2012-03-05 10:40 ` Alan Cox 2012-03-04 17:59 ` [PATCH 2/9] tty : make n_tty_receive_buf return bytes received in raw mode Matthieu CASTET 2012-03-04 17:59 ` [PATCH 3/9] tty: make hci_uart_tty_receive return bytes received Matthieu CASTET 2012-03-04 17:59 ` [PATCH 4/9] tty : kill receive_room usage in vt Matthieu CASTET 2012-03-05 10:29 ` Alan Cox 2012-03-04 17:59 ` [PATCH 5/9] tty : kill receive_room usage in speakup Matthieu CASTET 2012-03-04 17:59 ` [PATCH 6/9] tty : don't use receive_room in tty_set_ldisc Matthieu CASTET 2012-03-04 17:59 ` [PATCH 7/9] tty : kill receive_room usage in tty_buffer Matthieu CASTET 2012-03-05 10:31 ` Alan Cox 2012-03-04 17:59 ` [PATCH 8/9] tty : kill receive_room usage in mxser Matthieu CASTET 2012-03-05 10:34 ` Alan Cox 2012-03-04 17:59 ` [PATCH 9/9] tty : make receive_room internal to N_TTY line discipline Matthieu CASTET 2012-03-05 10:36 ` Alan Cox
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).