Re: Problem of concurrency in arch/ppc/8260_io/uart.c

[Steffen Rumler <Steffen.Rumler@siemens.com>]

[-- Attachment #1: Type: text/plain, Size: 2619 bytes --]

Hi,

I have seen a similar problem for the 2.4.20.

When I force a lot of console output via the following command:

   while true; do cd /; ls -R; done

and type-in some letters in parallel, the console
becomes crazy.

I have added some instrumentation in order to dump the
TX Buffer Descriptor Table. I have found that the
hardware pointer (TBPTR) and the software pointer (tx_cur)
are not more synchronized together:

 >> make new rlogin session <<
/root# cd /proc/driver

/root# cat uart-bdtables; cat mpc82xx/smc1_pram | grep SMC1_PRAM_TBPTR

TX BD table
   (000 at 0xfff005f0) status: 0x1000 len: 0001 addr: 0x001bb084
   (001 at 0xfff005f8) status: 0x1000 len: 0001 addr: 0x001bb0a4
* (002 at 0xfff00600) status: 0x1000 len: 0001 addr: 0x001bb0c4
   (003 at 0xfff00608) status: 0x3000 len: 0004 addr: 0x001bb0e4
    SMC1_PRAM_TBPTR            0x20         2     0600

    --> hardware and software pointer still synchronized

    >> force console to become crazy (see above) <<

/root# cat uart-bdtables; cat mpc82xx/smc1_pram | grep SMC1_PRAM_TBPTR

TX BD table (tbptr: 0x00000088)
   (000 at 0xfff005f0) status: 0x1000 len: 0003 addr: 0x001bb084
* (001 at 0xfff005f8) status: 0x1000 len: 0021 addr: 0x001bb0a4
   (002 at 0xfff00600) status: 0x1000 len: 0001 addr: 0x001bb0c4
   (003 at 0xfff00608) status: 0x3000 len: 0001 addr: 0x001bb0e4
    SMC1_PRAM_TBPTR            0x20         2     0600

    --> hardware and software pointer NOT more synchronized

    >> make additional console output: echo foo >/dev/console <<

/root# cat uart-bdtables; cat mpc82xx/smc1_pram | grep SMC1_PRAM_TBPTR

* (000 at 0xfff005f0) status: 0x1000 len: 0003 addr: 0x001bb084
   (001 at 0xfff005f8) status: 0x9000 len: 0004 addr: 0x001bb0a4
   (002 at 0xfff00600) status: 0x1000 len: 0001 addr: 0x001bb0c4
   (003 at 0xfff00608) status: 0x3000 len: 0001 addr: 0x001bb0e4
    SMC1_PRAM_TBPTR            0x20         2     05f0

    --> hardware pointer hangs at 0x5f0 because R-Bit not set, but
        at 0x5f8

Inside uart.c, there are the following output routines:

   rs_8xx_put_char()
   rs_8xx_write()
   rs_8xx_send_xchar()
   my_console_write()

I think there must be a synchronization accessing the
TX BD table. I suggest the patch attached.


Best Regards
Steffen
--


--------------------------------------------------------------

Steffen Rumler
ICN CP D NT SW 7
Siemens AG
Hofmannstr. 51                 Email: Steffen.Rumler@siemens.com
D-81359 Munich                 Phone: +49 89 722-44061
Germany                        Fax  : +49 89 722-36703

--------------------------------------------------------------



[-- Attachment #2: uart.c.patch --]
[-- Type: text/plain, Size: 3270 bytes --]

diff -Naur old/uart.c new/uart.c
--- old/uart.c	Mon Sep 15 11:52:02 2003
+++ new/uart.c	Mon Sep 15 11:51:32 2003
@@ -44,6 +44,7 @@
 #include <linux/slab.h>
 #include <linux/init.h>
 #include <linux/delay.h>
+#include <linux/spinlock.h>
 #include <asm/uaccess.h>
 #include <asm/immap_8260.h>
 #include <asm/mpc8260.h>
@@ -253,6 +254,11 @@
 	cbd_t			*rx_cur;
 	cbd_t			*tx_bd_base;
 	cbd_t			*tx_cur;
+
+        /*  for output synchronization
+         */
+        spinlock_t output_lock;
+        
 } ser_info_t;
 
 static void change_speed(ser_info_t *info);
@@ -1010,6 +1016,7 @@
 {
 	ser_info_t *info = (ser_info_t *)tty->driver_data;
 	volatile cbd_t	*bdp;
+        unsigned long flags;
 
 	if (serial_paranoia_check(info, tty->device, "rs_put_char"))
 		return;
@@ -1017,6 +1024,8 @@
 	if (!tty)
 		return;
 
+        spin_lock_irqsave(&(info->output_lock), flags);
+
 	bdp = info->tx_cur;
 	while (bdp->cbd_sc & BD_SC_READY);
 
@@ -1033,6 +1042,8 @@
 
 	info->tx_cur = (cbd_t *)bdp;
 
+        spin_unlock_irqrestore(&(info->output_lock), flags);
+
 }
 
 static int rs_8xx_write(struct tty_struct * tty, int from_user,
@@ -1041,6 +1052,7 @@
 	int	c, ret = 0;
 	ser_info_t *info = (ser_info_t *)tty->driver_data;
 	volatile cbd_t *bdp;
+        unsigned long flags;
 
 	if (serial_paranoia_check(info, tty->device, "rs_write"))
 		return 0;
@@ -1048,6 +1060,8 @@
 	if (!tty) 
 		return 0;
 
+        spin_lock_irqsave(&(info->output_lock), flags);
+
 	bdp = info->tx_cur;
 
 	while (1) {
@@ -1086,6 +1100,9 @@
 			bdp++;
 		info->tx_cur = (cbd_t *)bdp;
 	}
+
+        spin_unlock_irqrestore(&(info->output_lock), flags);
+
 	return ret;
 }
 
@@ -1143,12 +1160,15 @@
 static void rs_8xx_send_xchar(struct tty_struct *tty, char ch)
 {
 	volatile cbd_t	*bdp;
+        unsigned long flags;
 
 	ser_info_t *info = (ser_info_t *)tty->driver_data;
 
 	if (serial_paranoia_check(info, tty->device, "rs_send_char"))
 		return;
 
+        spin_lock_irqsave(&(info->output_lock), flags);
+
 	bdp = info->tx_cur;
 	while (bdp->cbd_sc & BD_SC_READY);
 
@@ -1164,6 +1184,8 @@
 		bdp++;
 
 	info->tx_cur = (cbd_t *)bdp;
+
+        spin_unlock_irqrestore(&(info->output_lock), flags);
 }
 
 /*
@@ -2227,9 +2249,11 @@
 	volatile	cbd_t		*bdp, *bdbase;
 	volatile	smc_uart_t	*up;
 	volatile	u_char		*cp;
+        unsigned long   flags;
 
 	ser = rs_table + idx;
 
+
 	/* If the port has been initialized for general use, we have
 	 * to use the buffer descriptors allocated there.  Otherwise,
 	 * we simply use the single buffer allocated.
@@ -2237,6 +2261,7 @@
 	if ((info = (ser_info_t *)ser->info) != NULL) {
 		bdp = info->tx_cur;
 		bdbase = info->tx_bd_base;
+                spin_lock_irqsave(&(info->output_lock), flags);
 	}
 	else {
 		/* Pointer to UART in parameter ram.
@@ -2309,6 +2334,9 @@
 
 	if (info)
 		info->tx_cur = (cbd_t *)bdp;
+
+        if (info)
+            spin_unlock_irqrestore(&(info->output_lock), flags);
 }
 
 static void serial_console_write(struct console *c, const char *s,
@@ -2764,6 +2792,7 @@
 			info->tqueue_hangup.data = info;
 			info->line = i;
 			info->state = state;
+                        spin_lock_init(&(info->output_lock));
 			state->info = (struct async_struct *)info;
 
 			/* We need to allocate a transmit and receive buffer