jsm: remove buggy write queue
Thadeu Lima de Souza Cascardo [Wed, 24 Aug 2011 16:14:22 +0000 (13:14 -0300)]
commit 9d898966c4a07e4a5092215b5a2829d0ef02baa2 upstream.

jsm uses a write queue that copies from uart_core circular buffer. This
copying however has some bugs, like not wrapping the head counter. Since
this write queue is also a circular buffer, the consumer function is
ready to use the uart_core circular buffer directly.

This buggy copying function was making some bytes be dropped when
transmitting to a raw tty, doing something like this.

[root@hostname ~]$ cat /dev/ttyn1 > cascardo/dump &
[1] 2658
[root@hostname ~]$ cat /proc/tty/drivers > /dev/ttyn0
[root@hostname ~]$ cat /proc/tty/drivers
/dev/tty             /dev/tty        5       0 system:/dev/tty
/dev/console         /dev/console    5       1 system:console
/dev/ptmx            /dev/ptmx       5       2 system
/dev/vc/0            /dev/vc/0       4       0 system:vtmaster
jsm                  /dev/ttyn     250 0-31 serial
serial               /dev/ttyS       4 64-95 serial
hvc                  /dev/hvc      229 0-7 system
pty_slave            /dev/pts      136 0-1048575 pty:slave
pty_master           /dev/ptm      128 0-1048575 pty:master
unknown              /dev/tty        4 1-63 console
[root@hostname ~]$ cat cascardo/dump
/dev/tty             /dev/tty        5       0 system:/dev/tty
/dev/console         /dev/console    5       1 system:console
/dev/ptmx            /dev/ptmx       5       2 system
/dev/vc/0            /dev/vc/0       4       0 system:vtmaste[root@hostname ~]$

This patch drops the driver write queue entirely, using the circular
buffer from uart_core only.

Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@linux.vnet.ibm.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

drivers/tty/serial/jsm/jsm.h
drivers/tty/serial/jsm/jsm_driver.c
drivers/tty/serial/jsm/jsm_neo.c
drivers/tty/serial/jsm/jsm_tty.c

index b704c8c..5b837e7 100644 (file)
@@ -183,10 +183,8 @@ struct jsm_board
 /* Our Read/Error/Write queue sizes */
 #define RQUEUEMASK     0x1FFF          /* 8 K - 1 */
 #define EQUEUEMASK     0x1FFF          /* 8 K - 1 */
-#define WQUEUEMASK     0x0FFF          /* 4 K - 1 */
 #define RQUEUESIZE     (RQUEUEMASK + 1)
 #define EQUEUESIZE     RQUEUESIZE
-#define WQUEUESIZE     (WQUEUEMASK + 1)
 
 
 /************************************************************************
@@ -226,10 +224,6 @@ struct jsm_channel {
        u16             ch_e_head;      /* Head location of the error queue */
        u16             ch_e_tail;      /* Tail location of the error queue */
 
-       u8              *ch_wqueue;     /* Our write queue buffer - malloc'ed */
-       u16             ch_w_head;      /* Head location of the write queue */
-       u16             ch_w_tail;      /* Tail location of the write queue */
-
        u64             ch_rxcount;     /* total of data received so far */
        u64             ch_txcount;     /* total of data transmitted so far */
 
@@ -378,7 +372,6 @@ extern int  jsm_debug;
  * Prototypes for non-static functions used in more than one module
  *
  *************************************************************************/
-int jsm_tty_write(struct uart_port *port);
 int jsm_tty_init(struct jsm_board *);
 int jsm_uart_port_init(struct jsm_board *);
 int jsm_remove_uart_port(struct jsm_board *);
index 96da178..2aaafa9 100644 (file)
@@ -211,7 +211,6 @@ static void __devexit jsm_remove_one(struct pci_dev *pdev)
                if (brd->channels[i]) {
                        kfree(brd->channels[i]->ch_rqueue);
                        kfree(brd->channels[i]->ch_equeue);
-                       kfree(brd->channels[i]->ch_wqueue);
                        kfree(brd->channels[i]);
                }
        }
index 4538c3e..bd6e846 100644 (file)
@@ -496,12 +496,15 @@ static void neo_copy_data_from_queue_to_uart(struct jsm_channel *ch)
        int s;
        int qlen;
        u32 len_written = 0;
+       struct circ_buf *circ;
 
        if (!ch)
                return;
 
+       circ = &ch->uart_port.state->xmit;
+
        /* No data to write to the UART */
-       if (ch->ch_w_tail == ch->ch_w_head)
+       if (uart_circ_empty(circ))
                return;
 
        /* If port is "stopped", don't send any data to the UART */
@@ -517,11 +520,10 @@ static void neo_copy_data_from_queue_to_uart(struct jsm_channel *ch)
                if (ch->ch_cached_lsr & UART_LSR_THRE) {
                        ch->ch_cached_lsr &= ~(UART_LSR_THRE);
 
-                       writeb(ch->ch_wqueue[ch->ch_w_tail], &ch->ch_neo_uart->txrx);
+                       writeb(circ->buf[circ->tail], &ch->ch_neo_uart->txrx);
                        jsm_printk(WRITE, INFO, &ch->ch_bd->pci_dev,
-                                       "Tx data: %x\n", ch->ch_wqueue[ch->ch_w_head]);
-                       ch->ch_w_tail++;
-                       ch->ch_w_tail &= WQUEUEMASK;
+                                       "Tx data: %x\n", circ->buf[circ->head]);
+                       circ->tail = (circ->tail + 1) & (UART_XMIT_SIZE - 1);
                        ch->ch_txcount++;
                }
                return;
@@ -536,36 +538,36 @@ static void neo_copy_data_from_queue_to_uart(struct jsm_channel *ch)
        n = UART_17158_TX_FIFOSIZE - ch->ch_t_tlevel;
 
        /* cache head and tail of queue */
-       head = ch->ch_w_head & WQUEUEMASK;
-       tail = ch->ch_w_tail & WQUEUEMASK;
-       qlen = (head - tail) & WQUEUEMASK;
+       head = circ->head & (UART_XMIT_SIZE - 1);
+       tail = circ->tail & (UART_XMIT_SIZE - 1);
+       qlen = uart_circ_chars_pending(circ);
 
        /* Find minimum of the FIFO space, versus queue length */
        n = min(n, qlen);
 
        while (n > 0) {
 
-               s = ((head >= tail) ? head : WQUEUESIZE) - tail;
+               s = ((head >= tail) ? head : UART_XMIT_SIZE) - tail;
                s = min(s, n);
 
                if (s <= 0)
                        break;
 
-               memcpy_toio(&ch->ch_neo_uart->txrxburst, ch->ch_wqueue + tail, s);
+               memcpy_toio(&ch->ch_neo_uart->txrxburst, circ->buf + tail, s);
                /* Add and flip queue if needed */
-               tail = (tail + s) & WQUEUEMASK;
+               tail = (tail + s) & (UART_XMIT_SIZE - 1);
                n -= s;
                ch->ch_txcount += s;
                len_written += s;
        }
 
        /* Update the final tail */
-       ch->ch_w_tail = tail & WQUEUEMASK;
+       circ->tail = tail & (UART_XMIT_SIZE - 1);
 
        if (len_written >= ch->ch_t_tlevel)
                ch->ch_flags &= ~(CH_TX_FIFO_EMPTY | CH_TX_FIFO_LWM);
 
-       if (!jsm_tty_write(&ch->uart_port))
+       if (uart_circ_empty(circ))
                uart_write_wakeup(&ch->uart_port);
 }
 
@@ -946,7 +948,6 @@ static void neo_param(struct jsm_channel *ch)
        if ((ch->ch_c_cflag & (CBAUD)) == 0) {
                ch->ch_r_head = ch->ch_r_tail = 0;
                ch->ch_e_head = ch->ch_e_tail = 0;
-               ch->ch_w_head = ch->ch_w_tail = 0;
 
                neo_flush_uart_write(ch);
                neo_flush_uart_read(ch);
index 7a4a914..434bd88 100644 (file)
@@ -118,6 +118,19 @@ static void jsm_tty_set_mctrl(struct uart_port *port, unsigned int mctrl)
        udelay(10);
 }
 
+/*
+ * jsm_tty_write()
+ *
+ * Take data from the user or kernel and send it out to the FEP.
+ * In here exists all the Transparent Print magic as well.
+ */
+static void jsm_tty_write(struct uart_port *port)
+{
+       struct jsm_channel *channel;
+       channel = container_of(port, struct jsm_channel, uart_port);
+       channel->ch_bd->bd_ops->copy_data_from_queue_to_uart(channel);
+}
+
 static void jsm_tty_start_tx(struct uart_port *port)
 {
        struct jsm_channel *channel = (struct jsm_channel *)port;
@@ -216,14 +229,6 @@ static int jsm_tty_open(struct uart_port *port)
                        return -ENOMEM;
                }
        }
-       if (!channel->ch_wqueue) {
-               channel->ch_wqueue = kzalloc(WQUEUESIZE, GFP_KERNEL);
-               if (!channel->ch_wqueue) {
-                       jsm_printk(INIT, ERR, &channel->ch_bd->pci_dev,
-                               "unable to allocate write queue buf");
-                       return -ENOMEM;
-               }
-       }
 
        channel->ch_flags &= ~(CH_OPENING);
        /*
@@ -237,7 +242,6 @@ static int jsm_tty_open(struct uart_port *port)
         */
        channel->ch_r_head = channel->ch_r_tail = 0;
        channel->ch_e_head = channel->ch_e_tail = 0;
-       channel->ch_w_head = channel->ch_w_tail = 0;
 
        brd->bd_ops->flush_uart_write(channel);
        brd->bd_ops->flush_uart_read(channel);
@@ -836,75 +840,3 @@ void jsm_check_queue_flow_control(struct jsm_channel *ch)
                }
        }
 }
-
-/*
- * jsm_tty_write()
- *
- * Take data from the user or kernel and send it out to the FEP.
- * In here exists all the Transparent Print magic as well.
- */
-int jsm_tty_write(struct uart_port *port)
-{
-       int bufcount;
-       int data_count = 0,data_count1 =0;
-       u16 head;
-       u16 tail;
-       u16 tmask;
-       u32 remain;
-       int temp_tail = port->state->xmit.tail;
-       struct jsm_channel *channel = (struct jsm_channel *)port;
-
-       tmask = WQUEUEMASK;
-       head = (channel->ch_w_head) & tmask;
-       tail = (channel->ch_w_tail) & tmask;
-
-       if ((bufcount = tail - head - 1) < 0)
-               bufcount += WQUEUESIZE;
-
-       bufcount = min(bufcount, 56);
-       remain = WQUEUESIZE - head;
-
-       data_count = 0;
-       if (bufcount >= remain) {
-               bufcount -= remain;
-               while ((port->state->xmit.head != temp_tail) &&
-               (data_count < remain)) {
-                       channel->ch_wqueue[head++] =
-                       port->state->xmit.buf[temp_tail];
-
-                       temp_tail++;
-                       temp_tail &= (UART_XMIT_SIZE - 1);
-                       data_count++;
-               }
-               if (data_count == remain) head = 0;
-       }
-
-       data_count1 = 0;
-       if (bufcount > 0) {
-               remain = bufcount;
-               while ((port->state->xmit.head != temp_tail) &&
-                       (data_count1 < remain)) {
-                       channel->ch_wqueue[head++] =
-                               port->state->xmit.buf[temp_tail];
-
-                       temp_tail++;
-                       temp_tail &= (UART_XMIT_SIZE - 1);
-                       data_count1++;
-
-               }
-       }
-
-       port->state->xmit.tail = temp_tail;
-
-       data_count += data_count1;
-       if (data_count) {
-               head &= tmask;
-               channel->ch_w_head = head;
-       }
-
-       if (data_count) {
-               channel->ch_bd->bd_ops->copy_data_from_queue_to_uart(channel);
-       }
-
-       return data_count;
-}