MOXA: mxser_new lockup

MOXA: mxser_new lockup

[Jan Kasprzak]

	Hello,

I have a MOXA C168H card, and after the upgrade from 2.6.19 to 2.6.21-rc6
(and from moxa.c to mxser_new.c) I get a system lockup after few seconds
of communicating over the MOXA serial line. Noting is printed on the
serial console at all. The system is SMP x86_64 (Fedora 5). I may be
able to do a limited testing on request, but the system is in production
use most of the time. For now, I will probably move back to the older
driver.

	Are you aware of any SMP or 64-bit issues in mxser_new.c?
Thanks,

-Yenya

-- 
| Jan "Yenya" Kasprzak  <kas at {fi.muni.cz - work | yenya.net - private}> |
| GPG: ID 1024/D3498839      Fingerprint 0D99A7FB206605D7 8B35FCDE05B18A5E |
| http://www.fi.muni.cz/~kas/    Journal: http://www.fi.muni.cz/~kas/blog/ |
> I will never go to meetings again because I think  face to face meetings <
> are the biggest waste of time you can ever have.        --Linus Torvalds <

Re: MOXA: mxser_new lockup

[Jiri Slaby]

Jan Kasprzak napsal(a):
> 	Hello,

Hello.

> I have a MOXA C168H card, and after the upgrade from 2.6.19 to 2.6.21-rc6
> (and from moxa.c to mxser_new.c) I get a system lockup after few seconds
> of communicating over the MOXA serial line. Noting is printed on the
> serial console at all. The system is SMP x86_64 (Fedora 5). I may be
> able to do a limited testing on request, but the system is in production
> use most of the time. For now, I will probably move back to the older
> driver.
> 
> 	Are you aware of any SMP or 64-bit issues in mxser_new.c?

No :(. I went through the locking again just now, but I can't see anything
wrong. Could you please enable CONFIG_PROVE_LOCKING and CONFIG_DEBUG_LOCKDEP and
retest?

And also post stty -a -F /dev/ttyMIXX or a setup passed to
tcsetattr/ioctl(TCSETA/S) if available. What do you use for communication?

thanks,
-- 
http://www.fi.muni.cz/~xslaby/            Jiri Slaby
faculty of informatics, masaryk university, brno, cz
e-mail: jirislaby gmail com, gpg pubkey fingerprint:
B674 9967 0407 CE62 ACC8  22A0 32CC 55C3 39D4 7A7E

Re: MOXA: mxser_new lockup

[Jan Kasprzak]

Jiri Slaby wrote:
: I went through the locking again just now, but I can't see anything
: wrong. Could you please enable CONFIG_PROVE_LOCKING and CONFIG_DEBUG_LOCKDEP and
: retest?
: 
: And also post stty -a -F /dev/ttyMIXX or a setup passed to
: tcsetattr/ioctl(TCSETA/S) if available. What do you use for communication?

	I use cu(1) from the uucp package. This MOXA card is used as a
serial console for 8 other linux boxes (mgetty is on the other side).

	I have recompiled the kernel with debugging options you have
mentioned, connected to the remote serial console using "cu -l ttyMI0 -s 38400",
logged to the remote system, and ran "find / -print" to generate some
serial traffic. It went for a minute or so, but as soon as I have pressed
enter on my keyboard (generating traffic in an opposite direction),
I've got the recursive locking message (I have tried it twice, two
dumps are attached).

	Looking at the code, maybe the &port->slock in mxser_interrupt()
should be dropped before calling mxser_receive_chars() (and probably
also befor calling other mxser_* functions too). A proof-of-concept
patch attached - I am not able to reproduce the lockup with this patch.
Another approach would be to use in_interrupt() all over the code to
determine whether the lock should be re-aquired or not.

-Yenya

[ now the two "recursive locking" dumps, and the patch itself ]

=============================================
[ INFO: possible recursive locking detected ]
2.6.21-rc6 #8
---------------------------------------------
cu/1793 is trying to acquire lock:
 (&info->slock){++..}, at: [<ffffffff88003afd>] mxser_flush_chars+0x4d/0x83 [mxser_new]

but task is already holding lock:
 (&info->slock){++..}, at: [<ffffffff88003ead>] mxser_interrupt+0xc2/0x237 [mxser_new]

other info that might help us debug this:
2 locks held by cu/1793:
 #0:  (&tty->atomic_write_lock){--..}, at: [<ffffffff802252dd>] tty_write+0x9e/0x211
 #1:  (&info->slock){++..}, at: [<ffffffff88003ead>] mxser_interrupt+0xc2/0x237 [mxser_new]

stack backtrace:

Call Trace:
 <IRQ>  [<ffffffff8028c6e4>] __lock_acquire+0x155/0xbe1
 [<ffffffff88003afd>] :mxser_new:mxser_flush_chars+0x4d/0x83
 [<ffffffff8028d1eb>] lock_acquire+0x7b/0x9f
 [<ffffffff88003afd>] :mxser_new:mxser_flush_chars+0x4d/0x83
 [<ffffffff8025947c>] _spin_lock_irqsave+0x2d/0x3e
 [<ffffffff88003afd>] :mxser_new:mxser_flush_chars+0x4d/0x83
 [<ffffffff80212a9b>] n_tty_receive_buf+0xd26/0xddc
 [<ffffffff8028d0db>] __lock_acquire+0xb4c/0xbe1
 [<ffffffff803377b1>] flush_to_ldisc+0x3d/0x191
 [<ffffffff8033788e>] flush_to_ldisc+0x11a/0x191
 [<ffffffff803378a5>] flush_to_ldisc+0x131/0x191
 [<ffffffff880025aa>] :mxser_new:mxser_receive_chars+0x25f/0x26e
 [<ffffffff88003f83>] :mxser_new:mxser_interrupt+0x198/0x237
 [<ffffffff8020ed84>] handle_IRQ_event+0x1e/0x51
 [<ffffffff8029ccd4>] handle_fasteoi_irq+0x9a/0xd9
 [<ffffffff80260ccd>] do_IRQ+0x6f/0xd6
 [<ffffffff80253aa6>] ret_from_intr+0x0/0xf
 <EOI>  [<ffffffff80259406>] _spin_unlock_irqrestore+0x40/0x44
 [<ffffffff802175dc>] write_chan+0x2e0/0x2ff
 [<ffffffff80275d26>] default_wake_function+0x0/0xe
 [<ffffffff802253b6>] tty_write+0x177/0x211
 [<ffffffff802172fc>] write_chan+0x0/0x2ff
 [<ffffffff802142a3>] vfs_write+0xce/0x157
 [<ffffffff80214baa>] sys_write+0x45/0x6e
 [<ffffffff8025355e>] system_call+0x7e/0x83

BUG: spinlock lockup on CPU#1, cu/1793, ffffffff880082a8

Call Trace:
 <IRQ>  [<ffffffff80207632>] _raw_spin_lock+0xcc/0xf3
 [<ffffffff80259484>] _spin_lock_irqsave+0x35/0x3e
 [<ffffffff88003afd>] :mxser_new:mxser_flush_chars+0x4d/0x83
 [<ffffffff80212a9b>] n_tty_receive_buf+0xd26/0xddc
 [<ffffffff8028d0db>] __lock_acquire+0xb4c/0xbe1
 [<ffffffff803377b1>] flush_to_ldisc+0x3d/0x191
 [<ffffffff8033788e>] flush_to_ldisc+0x11a/0x191
 [<ffffffff803378a5>] flush_to_ldisc+0x131/0x191
 [<ffffffff880025aa>] :mxser_new:mxser_receive_chars+0x25f/0x26e
 [<ffffffff88003f83>] :mxser_new:mxser_interrupt+0x198/0x237
 [<ffffffff8020ed84>] handle_IRQ_event+0x1e/0x51
 [<ffffffff8029ccd4>] handle_fasteoi_irq+0x9a/0xd9
 [<ffffffff80260ccd>] do_IRQ+0x6f/0xd6
 [<ffffffff80253aa6>] ret_from_intr+0x0/0xf
 <EOI>  [<ffffffff80259406>] _spin_unlock_irqrestore+0x40/0x44
 [<ffffffff802175dc>] write_chan+0x2e0/0x2ff
 [<ffffffff80275d26>] default_wake_function+0x0/0xe
 [<ffffffff802253b6>] tty_write+0x177/0x211
 [<ffffffff802172fc>] write_chan+0x0/0x2ff
 [<ffffffff802142a3>] vfs_write+0xce/0x157
 [<ffffffff80214baa>] sys_write+0x45/0x6e
 [<ffffffff8025355e>] system_call+0x7e/0x83


-----------------------------------------------------------------------
Another one:

=============================================
[ INFO: possible recursive locking detected ]
����ӡ���2.6.21-rc6 #8
---------------------------------------------
swapper/0 is trying to acquire lock:
 (&info->slock){++..}, at: [<ffffffff88003c6d>] mxser_stop+0x1c/0x43 [mxser_new]

but task is already holding lock:
 (&info->slock){++..}, at: [<ffffffff88003ead>] mxser_interrupt+0xc2/0x237 [mxser_new]

other info that might help us debug this:
1 lock held by swapper/0:
 #0:  (&info->slock){++..}, at: [<ffffffff88003ead>] mxser_interrupt+0xc2/0x237 [mxser_new]

stack backtrace:

Call Trace:
 <IRQ>  [<ffffffff8028c6e4>] __lock_acquire+0x155/0xbe1
 [<ffffffff88003c6d>] :mxser_new:mxser_stop+0x1c/0x43
 [<ffffffff8028d1eb>] lock_acquire+0x7b/0x9f
 [<ffffffff88003c6d>] :mxser_new:mxser_stop+0x1c/0x43
 [<ffffffff8025947c>] _spin_lock_irqsave+0x2d/0x3e
 [<ffffffff88003c6d>] :mxser_new:mxser_stop+0x1c/0x43
 [<ffffffff8021207a>] n_tty_receive_buf+0x305/0xddc
 [<ffffffff8028d0db>] __lock_acquire+0xb4c/0xbe1
 [<ffffffff803377b1>] flush_to_ldisc+0x3d/0x191
 [<ffffffff8033788e>] flush_to_ldisc+0x11a/0x191
 [<ffffffff803378a5>] flush_to_ldisc+0x131/0x191
 [<ffffffff880025aa>] :mxser_new:mxser_receive_chars+0x25f/0x26e
 [<ffffffff88003f83>] :mxser_new:mxser_interrupt+0x198/0x237
 [<ffffffff8020ed84>] handle_IRQ_event+0x1e/0x51
 [<ffffffff8029ccd4>] handle_fasteoi_irq+0x9a/0xd9
 [<ffffffff80260ccd>] do_IRQ+0x6f/0xd6
 [<ffffffff8025f6ae>] default_idle+0x35/0x51
 [<ffffffff8025f679>] default_idle+0x0/0x51
 [<ffffffff80253aa6>] ret_from_intr+0x0/0xf
 <EOI>  [<ffffffff8025f679>] default_idle+0x0/0x51
 [<ffffffff8025f6ae>] default_idle+0x35/0x51
 [<ffffffff8025f6b0>] default_idle+0x37/0x51
 [<ffffffff8025f6ae>] default_idle+0x35/0x51
 [<ffffffff802420fc>] cpu_idle+0x57/0x76


BUG: spinlock lockup on CPU#1, swapper/0, ffffffff880082a8

Call Trace:
 <IRQ>  [<ffffffff80207632>] _raw_spin_lock+0xcc/0xf3
 [<ffffffff80259484>] _spin_lock_irqsave+0x35/0x3e
 [<ffffffff88003c6d>] :mxser_new:mxser_stop+0x1c/0x43
 [<ffffffff8021207a>] n_tty_receive_buf+0x305/0xddc
 [<ffffffff8028d0db>] __lock_acquire+0xb4c/0xbe1
 [<ffffffff803377b1>] flush_to_ldisc+0x3d/0x191
 [<ffffffff8033788e>] flush_to_ldisc+0x11a/0x191
 [<ffffffff803378a5>] flush_to_ldisc+0x131/0x191
 [<ffffffff880025aa>] :mxser_new:mxser_receive_chars+0x25f/0x26e
 [<ffffffff88003f83>] :mxser_new:mxser_interrupt+0x198/0x237
 [<ffffffff8020ed84>] handle_IRQ_event+0x1e/0x51
 [<ffffffff8029ccd4>] handle_fasteoi_irq+0x9a/0xd9
 [<ffffffff80260ccd>] do_IRQ+0x6f/0xd6
 [<ffffffff8025f6ae>] default_idle+0x35/0x51
 [<ffffffff8025f679>] default_idle+0x0/0x51
 [<ffffffff80253aa6>] ret_from_intr+0x0/0xf
 <EOI>  [<ffffffff8025f679>] default_idle+0x0/0x51
 [<ffffffff8025f6ae>] default_idle+0x35/0x51
 [<ffffffff8025f6b0>] default_idle+0x37/0x51
[...]

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

Signed-off-by: Jan "Yenya" Kasprzak <kas@fi.muni.cz>

--- linux-2.6.21-rc6/drivers/char/mxser_new.c.orig	2007-04-14 11:50:54.000000000 +0200
+++ linux-2.6.21-rc6/drivers/char/mxser_new.c	2007-04-14 12:14:28.000000000 +0200
@@ -2352,27 +2352,43 @@
 					if (iir == MOXA_MUST_IIR_GDA ||
 					    iir == MOXA_MUST_IIR_RDA ||
 					    iir == MOXA_MUST_IIR_RTO ||
-					    iir == MOXA_MUST_IIR_LSR)
+					    iir == MOXA_MUST_IIR_LSR) {
+						spin_unlock(&port->slock);
 						mxser_receive_chars(port,
 								&status);
+						spin_lock(&port->slock);
+					}
 
 				} else {
 					status &= port->read_status_mask;
-					if (status & UART_LSR_DR)
+					if (status & UART_LSR_DR) {
+						spin_unlock(&port->slock);
 						mxser_receive_chars(port,
 								&status);
+						spin_lock(&port->slock);
+					}
 				}
 				msr = inb(port->ioaddr + UART_MSR);
-				if (msr & UART_MSR_ANY_DELTA)
+				if (msr & UART_MSR_ANY_DELTA) {
+					spin_unlock(&port->slock);
 					mxser_check_modem_status(port, msr);
+					spin_lock(&port->slock);
+				}
 
 				if (port->board->chip_flag) {
 					if (iir == 0x02 && (status &
-								UART_LSR_THRE))
+							UART_LSR_THRE)) {
+						spin_unlock(&port->slock);
+						
 						mxser_transmit_chars(port);
+						spin_lock(&port->slock);
+					}
 				} else {
-					if (status & UART_LSR_THRE)
+					if (status & UART_LSR_THRE) {
+						spin_unlock(&port->slock);
 						mxser_transmit_chars(port);
+						spin_lock(&port->slock);
+					}
 				}
 			} while (int_cnt++ < MXSER_ISR_PASS_LIMIT);
 			spin_unlock(&port->slock);
-- 
| Jan "Yenya" Kasprzak  <kas at {fi.muni.cz - work | yenya.net - private}> |
| GPG: ID 1024/D3498839      Fingerprint 0D99A7FB206605D7 8B35FCDE05B18A5E |
| http://www.fi.muni.cz/~kas/    Journal: http://www.fi.muni.cz/~kas/blog/ |
> I will never go to meetings again because I think  face to face meetings <
> are the biggest waste of time you can ever have.        --Linus Torvalds <

[RFC 1/1] Char: mxser_new, fix recursive locking, Re: MOXA: mxser_new lockup

[Jiri Slaby]

Jan Kasprzak wrote:
> Jiri Slaby wrote:
> : wrong. Could you please enable CONFIG_PROVE_LOCKING and CONFIG_DEBUG_LOCKDEP and
> 
> 	Looking at the code, maybe the &port->slock in mxser_interrupt()
> should be dropped before calling mxser_receive_chars() (and probably
> also befor calling other mxser_* functions too). A proof-of-concept
> patch attached - I am not able to reproduce the lockup with this patch.
> Another approach would be to use in_interrupt() all over the code to
> determine whether the lock should be re-aquired or not.

I would rather incline to the second variant as it is the original approach
from moxa driver and I have no idea, what might happen if we drop the lock
before calling all involved mxser_ routines.

Could you both (if possible) test the attached patch and drop a message,
please?

--

mxser_new, fix recursive locking

Acquire a port lock only if not in_interrupt, because ISR holds the lock
yet (and ldisc calls some of driver's routines which tries to acquire it
again due to tty->low_latency).

Thanks to Jan "Yenya" Kasprzak for revealing this issue.

Cc: Jan "Yenya" Kasprzak <kas@fi.muni.cz>
Signed-off-by: Jiri Slaby <jirislaby@gmail.com>

---
commit a00e6d6d5f44defe9c01be2c6a3fdf1c890917c8
tree c4f2698596ec7eeb3290da616b64c2fb36797032
parent 54f13ae12fa0a3e3fc02dda6225baa3436f1e93a
author Jiri Slaby <jirislaby@gmail.com> Sat, 14 Apr 2007 14:00:01 +0200
committer Jiri Slaby <jirislaby@gmail.com> Sat, 14 Apr 2007 14:00:01 +0200

 drivers/char/mxser_new.c |  132 +++++++++++++++++++++++++---------------------
 1 files changed, 71 insertions(+), 61 deletions(-)

diff --git a/drivers/char/mxser_new.c b/drivers/char/mxser_new.c
index 6362e78..0086b73 100644
--- a/drivers/char/mxser_new.c
+++ b/drivers/char/mxser_new.c
@@ -85,6 +85,16 @@
 #define MXSER_HIGHBAUD	1
 #define MXSER_HAS2	2
 
+/* if we are in interrupt, lock is held by ISR, don't acquire it again! */
+#define mx_lock(info, flags) do {				\
+	if (!in_interrupt())					\
+		spin_lock_irqsave(&(info)->slock, flags);	\
+} while (0)
+#define mx_unlock(info, flags) do {				\
+	if (!in_interrupt())					\
+		spin_unlock_irqrestore(&(info)->slock, flags);	\
+} while (0)
+
 /* This is only for PCI */
 static const struct {
 	int type;
@@ -415,16 +425,16 @@ static int mxser_block_til_ready(struct tty_struct *tty, struct file *filp,
 	retval = 0;
 	add_wait_queue(&port->open_wait, &wait);
 
-	spin_lock_irqsave(&port->slock, flags);
+	mx_lock(port, flags);
 	if (!tty_hung_up_p(filp))
 		port->count--;
-	spin_unlock_irqrestore(&port->slock, flags);
+	mx_unlock(port, flags);
 	port->blocked_open++;
 	while (1) {
-		spin_lock_irqsave(&port->slock, flags);
+		mx_lock(port, flags);
 		outb(inb(port->ioaddr + UART_MCR) |
 			UART_MCR_DTR | UART_MCR_RTS, port->ioaddr + UART_MCR);
-		spin_unlock_irqrestore(&port->slock, flags);
+		mx_unlock(port, flags);
 		set_current_state(TASK_INTERRUPTIBLE);
 		if (tty_hung_up_p(filp) || !(port->flags & ASYNC_INITIALIZED)) {
 			if (port->flags & ASYNC_HUP_NOTIFY)
@@ -765,11 +775,11 @@ static int mxser_startup(struct mxser_port *info)
 	if (!page)
 		return -ENOMEM;
 
-	spin_lock_irqsave(&info->slock, flags);
+	mx_lock(info, flags);
 
 	if (info->flags & ASYNC_INITIALIZED) {
 		free_page(page);
-		spin_unlock_irqrestore(&info->slock, flags);
+		mx_unlock(info, flags);
 		return 0;
 	}
 
@@ -777,7 +787,7 @@ static int mxser_startup(struct mxser_port *info)
 		if (info->tty)
 			set_bit(TTY_IO_ERROR, &info->tty->flags);
 		free_page(page);
-		spin_unlock_irqrestore(&info->slock, flags);
+		mx_unlock(info, flags);
 		return 0;
 	}
 	if (info->xmit_buf)
@@ -803,7 +813,7 @@ static int mxser_startup(struct mxser_port *info)
 	 * here.
 	 */
 	if (inb(info->ioaddr + UART_LSR) == 0xff) {
-		spin_unlock_irqrestore(&info->slock, flags);
+		mx_unlock(info, flags);
 		if (capable(CAP_SYS_ADMIN)) {
 			if (info->tty)
 				set_bit(TTY_IO_ERROR, &info->tty->flags);
@@ -853,7 +863,7 @@ static int mxser_startup(struct mxser_port *info)
 	 */
 	mxser_change_speed(info, NULL);
 	info->flags |= ASYNC_INITIALIZED;
-	spin_unlock_irqrestore(&info->slock, flags);
+	mx_unlock(info, flags);
 
 	return 0;
 }
@@ -869,7 +879,7 @@ static void mxser_shutdown(struct mxser_port *info)
 	if (!(info->flags & ASYNC_INITIALIZED))
 		return;
 
-	spin_lock_irqsave(&info->slock, flags);
+	mx_lock(info, flags);
 
 	/*
 	 * clear delta_msr_wait queue to avoid mem leaks: we may free the irq
@@ -912,7 +922,7 @@ static void mxser_shutdown(struct mxser_port *info)
 	if (info->board->chip_flag)
 		SET_MOXA_MUST_NO_SOFTWARE_FLOW_CONTROL(info->ioaddr);
 
-	spin_unlock_irqrestore(&info->slock, flags);
+	mx_unlock(info, flags);
 }
 
 /*
@@ -941,9 +951,9 @@ static int mxser_open(struct tty_struct *tty, struct file *filp)
 	/*
 	 * Start up serial port
 	 */
-	spin_lock_irqsave(&info->slock, flags);
+	mx_lock(info, flags);
 	info->count++;
-	spin_unlock_irqrestore(&info->slock, flags);
+	mx_unlock(info, flags);
 	retval = mxser_startup(info);
 	if (retval)
 		return retval;
@@ -975,10 +985,10 @@ static void mxser_close(struct tty_struct *tty, struct file *filp)
 	if (!info)
 		return;
 
-	spin_lock_irqsave(&info->slock, flags);
+	mx_lock(info, flags);
 
 	if (tty_hung_up_p(filp)) {
-		spin_unlock_irqrestore(&info->slock, flags);
+		mx_unlock(info, flags);
 		return;
 	}
 	if ((tty->count == 1) && (info->count != 1)) {
@@ -999,11 +1009,11 @@ static void mxser_close(struct tty_struct *tty, struct file *filp)
 		info->count = 0;
 	}
 	if (info->count) {
-		spin_unlock_irqrestore(&info->slock, flags);
+		mx_unlock(info, flags);
 		return;
 	}
 	info->flags |= ASYNC_CLOSING;
-	spin_unlock_irqrestore(&info->slock, flags);
+	mx_unlock(info, flags);
 	/*
 	 * Save the termios structure, since this port may have
 	 * separate termios for callout and dialin.
@@ -1076,11 +1086,11 @@ static int mxser_write(struct tty_struct *tty, const unsigned char *buf, int cou
 			break;
 
 		memcpy(info->xmit_buf + info->xmit_head, buf, c);
-		spin_lock_irqsave(&info->slock, flags);
+		mx_lock(info, flags);
 		info->xmit_head = (info->xmit_head + c) &
 				  (SERIAL_XMIT_SIZE - 1);
 		info->xmit_cnt += c;
-		spin_unlock_irqrestore(&info->slock, flags);
+		mx_unlock(info, flags);
 
 		buf += c;
 		count -= c;
@@ -1091,12 +1101,12 @@ static int mxser_write(struct tty_struct *tty, const unsigned char *buf, int cou
 		if (!tty->hw_stopped ||
 				(info->type == PORT_16550A) ||
 				(info->board->chip_flag)) {
-			spin_lock_irqsave(&info->slock, flags);
+			mx_lock(info, flags);
 			outb(info->IER & ~UART_IER_THRI, info->ioaddr +
 					UART_IER);
 			info->IER |= UART_IER_THRI;
 			outb(info->IER, info->ioaddr + UART_IER);
-			spin_unlock_irqrestore(&info->slock, flags);
+			mx_unlock(info, flags);
 		}
 	}
 	return total;
@@ -1113,20 +1123,20 @@ static void mxser_put_char(struct tty_struct *tty, unsigned char ch)
 	if (info->xmit_cnt >= SERIAL_XMIT_SIZE - 1)
 		return;
 
-	spin_lock_irqsave(&info->slock, flags);
+	mx_lock(info, flags);
 	info->xmit_buf[info->xmit_head++] = ch;
 	info->xmit_head &= SERIAL_XMIT_SIZE - 1;
 	info->xmit_cnt++;
-	spin_unlock_irqrestore(&info->slock, flags);
+	mx_unlock(info, flags);
 	if (!tty->stopped) {
 		if (!tty->hw_stopped ||
 				(info->type == PORT_16550A) ||
 				info->board->chip_flag) {
-			spin_lock_irqsave(&info->slock, flags);
+			mx_lock(info, flags);
 			outb(info->IER & ~UART_IER_THRI, info->ioaddr + UART_IER);
 			info->IER |= UART_IER_THRI;
 			outb(info->IER, info->ioaddr + UART_IER);
-			spin_unlock_irqrestore(&info->slock, flags);
+			mx_unlock(info, flags);
 		}
 	}
 }
@@ -1146,13 +1156,13 @@ static void mxser_flush_chars(struct tty_struct *tty)
 			))
 		return;
 
-	spin_lock_irqsave(&info->slock, flags);
+	mx_lock(info, flags);
 
 	outb(info->IER & ~UART_IER_THRI, info->ioaddr + UART_IER);
 	info->IER |= UART_IER_THRI;
 	outb(info->IER, info->ioaddr + UART_IER);
 
-	spin_unlock_irqrestore(&info->slock, flags);
+	mx_unlock(info, flags);
 }
 
 static int mxser_write_room(struct tty_struct *tty)
@@ -1179,7 +1189,7 @@ static void mxser_flush_buffer(struct tty_struct *tty)
 	unsigned long flags;
 
 
-	spin_lock_irqsave(&info->slock, flags);
+	mx_lock(info, flags);
 	info->xmit_cnt = info->xmit_head = info->xmit_tail = 0;
 
 	fcr = inb(info->ioaddr + UART_FCR);
@@ -1187,7 +1197,7 @@ static void mxser_flush_buffer(struct tty_struct *tty)
 		info->ioaddr + UART_FCR);
 	outb(fcr, info->ioaddr + UART_FCR);
 
-	spin_unlock_irqrestore(&info->slock, flags);
+	mx_unlock(info, flags);
 
 	tty_wakeup(tty);
 }
@@ -1268,9 +1278,9 @@ static int mxser_set_serial_info(struct mxser_port *info,
 
 	if (info->flags & ASYNC_INITIALIZED) {
 		if (flags != (info->flags & ASYNC_SPD_MASK)) {
-			spin_lock_irqsave(&info->slock, sl_flags);
+			mx_lock(info, sl_flags);
 			mxser_change_speed(info, NULL);
-			spin_unlock_irqrestore(&info->slock, sl_flags);
+			mx_unlock(info, sl_flags);
 		}
 	} else
 		retval = mxser_startup(info);
@@ -1295,9 +1305,9 @@ static int mxser_get_lsr_info(struct mxser_port *info,
 	unsigned int result;
 	unsigned long flags;
 
-	spin_lock_irqsave(&info->slock, flags);
+	mx_lock(info, flags);
 	status = inb(info->ioaddr + UART_LSR);
-	spin_unlock_irqrestore(&info->slock, flags);
+	mx_unlock(info, flags);
 	result = ((status & UART_LSR_TEMT) ? TIOCSER_TEMT : 0);
 	return put_user(result, value);
 }
@@ -1312,15 +1322,15 @@ static void mxser_send_break(struct mxser_port *info, int duration)
 	if (!info->ioaddr)
 		return;
 	set_current_state(TASK_INTERRUPTIBLE);
-	spin_lock_irqsave(&info->slock, flags);
+	mx_lock(info, flags);
 	outb(inb(info->ioaddr + UART_LCR) | UART_LCR_SBC,
 		info->ioaddr + UART_LCR);
-	spin_unlock_irqrestore(&info->slock, flags);
+	mx_unlock(info, flags);
 	schedule_timeout(duration);
-	spin_lock_irqsave(&info->slock, flags);
+	mx_lock(info, flags);
 	outb(inb(info->ioaddr + UART_LCR) & ~UART_LCR_SBC,
 		info->ioaddr + UART_LCR);
-	spin_unlock_irqrestore(&info->slock, flags);
+	mx_unlock(info, flags);
 }
 
 static int mxser_tiocmget(struct tty_struct *tty, struct file *file)
@@ -1337,11 +1347,11 @@ static int mxser_tiocmget(struct tty_struct *tty, struct file *file)
 
 	control = info->MCR;
 
-	spin_lock_irqsave(&info->slock, flags);
+	mx_lock(info, flags);
 	status = inb(info->ioaddr + UART_MSR);
 	if (status & UART_MSR_ANY_DELTA)
 		mxser_check_modem_status(info, status);
-	spin_unlock_irqrestore(&info->slock, flags);
+	mx_unlock(info, flags);
 	return ((control & UART_MCR_RTS) ? TIOCM_RTS : 0) |
 		    ((control & UART_MCR_DTR) ? TIOCM_DTR : 0) |
 		    ((status & UART_MSR_DCD) ? TIOCM_CAR : 0) |
@@ -1362,7 +1372,7 @@ static int mxser_tiocmset(struct tty_struct *tty, struct file *file,
 	if (test_bit(TTY_IO_ERROR, &tty->flags))
 		return -EIO;
 
-	spin_lock_irqsave(&info->slock, flags);
+	mx_lock(info, flags);
 
 	if (set & TIOCM_RTS)
 		info->MCR |= UART_MCR_RTS;
@@ -1375,7 +1385,7 @@ static int mxser_tiocmset(struct tty_struct *tty, struct file *file,
 		info->MCR &= ~UART_MCR_DTR;
 
 	outb(info->MCR, info->ioaddr + UART_MCR);
-	spin_unlock_irqrestore(&info->slock, flags);
+	mx_unlock(info, flags);
 	return 0;
 }
 
@@ -1706,9 +1716,9 @@ static int mxser_ioctl(struct tty_struct *tty, struct file *file,
 			info->tty->termios->c_cflag |= mxvar_baud_table1[i];
 
 		info->speed = speed;
-		spin_lock_irqsave(&info->slock, flags);
+		mx_lock(info, flags);
 		mxser_change_speed(info, NULL);
-		spin_unlock_irqrestore(&info->slock, flags);
+		mx_unlock(info, flags);
 
 		return 0;
 	} else if (cmd == MOXA_GET_SPECIAL_BAUD_RATE) {
@@ -1760,15 +1770,15 @@ static int mxser_ioctl(struct tty_struct *tty, struct file *file,
 	case TIOCMIWAIT: {
 		DECLARE_WAITQUEUE(wait, current);
 		int ret;
-		spin_lock_irqsave(&info->slock, flags);
+		mx_lock(info, flags);
 		cprev = info->icount;	/* note the counters on entry */
-		spin_unlock_irqrestore(&info->slock, flags);
+		mx_unlock(info, flags);
 
 		add_wait_queue(&info->delta_msr_wait, &wait);
 		while (1) {
-			spin_lock_irqsave(&info->slock, flags);
+			mx_lock(info, flags);
 			cnow = info->icount;	/* atomic copy */
-			spin_unlock_irqrestore(&info->slock, flags);
+			mx_unlock(info, flags);
 
 			set_current_state(TASK_INTERRUPTIBLE);
 			if (((arg & TIOCM_RNG) &&
@@ -1801,9 +1811,9 @@ static int mxser_ioctl(struct tty_struct *tty, struct file *file,
 	 *     RI where only 0->1 is counted.
 	 */
 	case TIOCGICOUNT:
-		spin_lock_irqsave(&info->slock, flags);
+		mx_lock(info, flags);
 		cnow = info->icount;
-		spin_unlock_irqrestore(&info->slock, flags);
+		mx_unlock(info, flags);
 		p_cuser = argp;
 		if (put_user(cnow.frame, &p_cuser->frame))
 			return -EFAULT;
@@ -1834,9 +1844,9 @@ static int mxser_ioctl(struct tty_struct *tty, struct file *file,
 		long baud;
 		if (get_user(baud, (long __user *)argp))
 			return -EFAULT;
-		spin_lock_irqsave(&info->slock, flags);
+		mx_lock(info, flags);
 		mxser_set_baud(info, baud);
-		spin_unlock_irqrestore(&info->slock, flags);
+		mx_unlock(info, flags);
 		return 0;
 	}
 	case MOXA_ASPP_GETBAUD:
@@ -1983,12 +1993,12 @@ static void mxser_stop(struct tty_struct *tty)
 	struct mxser_port *info = tty->driver_data;
 	unsigned long flags;
 
-	spin_lock_irqsave(&info->slock, flags);
+	mx_lock(info, flags);
 	if (info->IER & UART_IER_THRI) {
 		info->IER &= ~UART_IER_THRI;
 		outb(info->IER, info->ioaddr + UART_IER);
 	}
-	spin_unlock_irqrestore(&info->slock, flags);
+	mx_unlock(info, flags);
 }
 
 static void mxser_start(struct tty_struct *tty)
@@ -1996,13 +2006,13 @@ static void mxser_start(struct tty_struct *tty)
 	struct mxser_port *info = tty->driver_data;
 	unsigned long flags;
 
-	spin_lock_irqsave(&info->slock, flags);
+	mx_lock(info, flags);
 	if (info->xmit_cnt && info->xmit_buf) {
 		outb(info->IER & ~UART_IER_THRI, info->ioaddr + UART_IER);
 		info->IER |= UART_IER_THRI;
 		outb(info->IER, info->ioaddr + UART_IER);
 	}
-	spin_unlock_irqrestore(&info->slock, flags);
+	mx_unlock(info, flags);
 }
 
 static void mxser_set_termios(struct tty_struct *tty, struct ktermios *old_termios)
@@ -2013,9 +2023,9 @@ static void mxser_set_termios(struct tty_struct *tty, struct ktermios *old_termi
 	if ((tty->termios->c_cflag != old_termios->c_cflag) ||
 			(RELEVANT_IFLAG(tty->termios->c_iflag) != RELEVANT_IFLAG(old_termios->c_iflag))) {
 
-		spin_lock_irqsave(&info->slock, flags);
+		mx_lock(info, flags);
 		mxser_change_speed(info, old_termios);
-		spin_unlock_irqrestore(&info->slock, flags);
+		mx_unlock(info, flags);
 
 		if ((old_termios->c_cflag & CRTSCTS) &&
 				!(tty->termios->c_cflag & CRTSCTS)) {
@@ -2030,9 +2040,9 @@ static void mxser_set_termios(struct tty_struct *tty, struct ktermios *old_termi
 		tty->stopped = 0;
 
 		if (info->board->chip_flag) {
-			spin_lock_irqsave(&info->slock, flags);
+			mx_lock(info, flags);
 			DISABLE_MOXA_MUST_RX_SOFTWARE_FLOW_CONTROL(info->ioaddr);
-			spin_unlock_irqrestore(&info->slock, flags);
+			mx_unlock(info, flags);
 		}
 
 		mxser_start(tty);
@@ -2126,14 +2136,14 @@ static void mxser_rs_break(struct tty_struct *tty, int break_state)
 	struct mxser_port *info = tty->driver_data;
 	unsigned long flags;
 
-	spin_lock_irqsave(&info->slock, flags);
+	mx_lock(info, flags);
 	if (break_state == -1)
 		outb(inb(info->ioaddr + UART_LCR) | UART_LCR_SBC,
 			info->ioaddr + UART_LCR);
 	else
 		outb(inb(info->ioaddr + UART_LCR) & ~UART_LCR_SBC,
 			info->ioaddr + UART_LCR);
-	spin_unlock_irqrestore(&info->slock, flags);
+	mx_unlock(info, flags);
 }
 
 static void mxser_receive_chars(struct mxser_port *port, int *status)

Re: [RFC 1/1] Char: mxser_new, fix recursive locking

[Jan Yenya Kasprzak]

Jiri Slaby wrote:
: I would rather incline to the second variant as it is the original approach
: from moxa driver and I have no idea, what might happen if we drop the lock
: before calling all involved mxser_ routines.
: 
: Could you both (if possible) test the attached patch and drop a message,
: please?

	Works for me, altough it spits tons of (bogus, in this case)
warnings about possibly uninitialized variable "flags".

	Additionally, I think it is overzealous to wrap _all_
spin_lock_irqsave/irqrestore() with if (!in_interrupt()). Some of them are
never called from the interrupt context (mxser_block_till_ready(), for example
- it can block, so if it is ever called from the interrupt context, we have
a different, an order of magnitude worse problem here :-)

	I have another problem with the driver - it probably sometimes
drops DCD signal on the serial line or something like that:
when the traffic on the serial console is heavy, it sometimes disconnects
me from the remote shell, and cu(1) displays the login prompt from the new
instance of mgetty of the remote machine. However, it does so both with
mxser.o and mxser_new.o (in 2.6.21-rc6, I think it worked in 2.6.19,
but I have to retest it). So this is another problem, different from
the one we are trying to solve now.

-Yenya

-- 
| Jan "Yenya" Kasprzak  <kas at {fi.muni.cz - work | yenya.net - private}> |
| GPG: ID 1024/D3498839      Fingerprint 0D99A7FB206605D7 8B35FCDE05B18A5E |
| http://www.fi.muni.cz/~kas/    Journal: http://www.fi.muni.cz/~kas/blog/ |
> I will never go to meetings again because I think  face to face meetings <
> are the biggest waste of time you can ever have.        --Linus Torvalds <

Re: [RFC 1/1] Char: mxser_new, fix recursive locking

[Jiri Slaby]

On 4/14/07, Jan Yenya Kasprzak <kas@fi.muni.cz> wrote:
>         Works for me, altough it spits tons of (bogus, in this case)
> warnings about possibly uninitialized variable "flags".

This is a gcc problem. I has been thinking about flags = 0
initialization, but developers dislike this shutting gcc up.

>         Additionally, I think it is overzealous to wrap _all_
> spin_lock_irqsave/irqrestore() with if (!in_interrupt()). Some of them are
> never called from the interrupt context (mxser_block_till_ready(), for example
> - it can block, so if it is ever called from the interrupt context, we have
> a different, an order of magnitude worse problem here :-)

Yes, I had a patch, which changed only some of locks, but there were
only 2 or 3 spin_locks remaining. It's not a problem to test it
everywhere for little overhead, but maybe yes, I'll remove it from
open, close and block_til_whatever for the final patch.

>         I have another problem with the driver - it probably sometimes
> drops DCD signal on the serial line or something like that:
> when the traffic on the serial console is heavy, it sometimes disconnects
> me from the remote shell, and cu(1) displays the login prompt from the new
> instance of mgetty of the remote machine. However, it does so both with
> mxser.o and mxser_new.o (in 2.6.21-rc6, I think it worked in 2.6.19,
> but I have to retest it). So this is another problem, different from
> the one we are trying to solve now.

There were some changes, however nothing significant in mxser.c, maybe
some of tty or ldisc layer changes (but there is only termios ->
ktermios switch + some other things), this would probably be hard to
find without bisecting if 2.6.19 is really OK for you.

The only idea I have right now is to nohup process, which will
int fd = open("/dev/ttyMIXX", O_RDONLY | O_NONBLOCK);
while (1) {
  ioctl(fd, TIOCMIWAIT, TIOCM_CD);
  ioctl(fd, TIOCMGET, &ret);
  printf("%ld: carrier has changed: %u\n", time(NULL), !!(ret & TIOCM_CD));
}
to prove, if the carrier really becomes low.

regards,
-- 
http://www.fi.muni.cz/~xslaby/            Jiri Slaby
faculty of informatics, masaryk university, brno, cz
e-mail: jirislaby gmail com, gpg pubkey fingerprint:
B674 9967 0407 CE62 ACC8  22A0 32CC 55C3 39D4 7A7E

Re: [RFC 1/1] Char: mxser_new, fix recursive locking

[Jan Yenya Kasprzak]

Jiri Slaby wrote:
: >        I have another problem with the driver - it probably sometimes
: >drops DCD signal on the serial line or something like that:
: >when the traffic on the serial console is heavy, it sometimes disconnects
: >me from the remote shell, and cu(1) displays the login prompt from the new
: >instance of mgetty of the remote machine. However, it does so both with
: >mxser.o and mxser_new.o (in 2.6.21-rc6, I think it worked in 2.6.19,
: >but I have to retest it). So this is another problem, different from
: >the one we are trying to solve now.
: 
: There were some changes, however nothing significant in mxser.c, maybe
: some of tty or ldisc layer changes (but there is only termios ->
: ktermios switch + some other things), this would probably be hard to
: find without bisecting if 2.6.19 is really OK for you.

	OK, I'll try to bisect in a few weeks.

: The only idea I have right now is to nohup process, which will
: int fd = open("/dev/ttyMIXX", O_RDONLY | O_NONBLOCK);
: while (1) {
:  ioctl(fd, TIOCMIWAIT, TIOCM_CD);
:  ioctl(fd, TIOCMGET, &ret);
:  printf("%ld: carrier has changed: %u\n", time(NULL), !!(ret & TIOCM_CD));
: }

	Hmm, I have tried to run this, and got a machine lockup, and after
a minute or so the following has been printed to the console:

~BUG: spinlock lockup on CPU#0, sshd/1671, ffffffff80557780

(I was logged in using ssh, and ran the above code inside the ssh session).

	I have tried to look at it from the remote side as well:
stracing the "find / -print" revealed that after some time,
the write(1, "/file/name/...", ...) call returned -EIO.
And from this point on, all subsequent writes to fd #1 did return
-EIO as well.

-Yenya

-- 
| Jan "Yenya" Kasprzak  <kas at {fi.muni.cz - work | yenya.net - private}> |
| GPG: ID 1024/D3498839      Fingerprint 0D99A7FB206605D7 8B35FCDE05B18A5E |
| http://www.fi.muni.cz/~kas/    Journal: http://www.fi.muni.cz/~kas/blog/ |
> I will never go to meetings again because I think  face to face meetings <
> are the biggest waste of time you can ever have.        --Linus Torvalds <

Re: [RFC 1/1] Char: mxser_new, fix recursive locking

[Jiri Slaby]

On 4/14/07, Jan Yenya Kasprzak <kas@fi.muni.cz> wrote:
> ~BUG: spinlock lockup on CPU#0, sshd/1671, ffffffff80557780
[...]
> the write(1, "/file/name/...", ...) call returned -EIO.

Just a question: both with mxser_new, right?

regards,
-- 
http://www.fi.muni.cz/~xslaby/            Jiri Slaby
faculty of informatics, masaryk university, brno, cz
e-mail: jirislaby gmail com, gpg pubkey fingerprint:
B674 9967 0407 CE62 ACC8  22A0 32CC 55C3 39D4 7A7E

Re: [RFC 1/1] Char: mxser_new, fix recursive locking

[Jan Yenya Kasprzak]

Jiri Slaby wrote:
: On 4/14/07, Jan Yenya Kasprzak <kas@fi.muni.cz> wrote:
: >~BUG: spinlock lockup on CPU#0, sshd/1671, ffffffff80557780
: [...]
: >the write(1, "/file/name/...", ...) call returned -EIO.
: 
: Just a question: both with mxser_new, right?

	No. One side has a multiport C168H with mxser_new, and the other
one (the one from which the above strace is) is an ordinary server with
console on ttyS0. So the above write(1,...) has fd#1 on ttyS0.

-Yenya

-- 
| Jan "Yenya" Kasprzak  <kas at {fi.muni.cz - work | yenya.net - private}> |
| GPG: ID 1024/D3498839      Fingerprint 0D99A7FB206605D7 8B35FCDE05B18A5E |
| http://www.fi.muni.cz/~kas/    Journal: http://www.fi.muni.cz/~kas/blog/ |
> I will never go to meetings again because I think  face to face meetings <
> are the biggest waste of time you can ever have.        --Linus Torvalds <

Re: [RFC 1/1] Char: mxser_new, fix recursive locking

[Jiri Slaby]

On 4/14/07, Jan Yenya Kasprzak <kas@fi.muni.cz> wrote:
> Jiri Slaby wrote:
> : On 4/14/07, Jan Yenya Kasprzak <kas@fi.muni.cz> wrote:
> : >~BUG: spinlock lockup on CPU#0, sshd/1671, ffffffff80557780
> : [...]
> : >the write(1, "/file/name/...", ...) call returned -EIO.
> :
> : Just a question: both with mxser_new, right?
>
>         No. One side has a multiport C168H with mxser_new, and the other

I meant both lockup and EIO error -- I guess so from this line.

regards,
-- 
http://www.fi.muni.cz/~xslaby/            Jiri Slaby
faculty of informatics, masaryk university, brno, cz
e-mail: jirislaby gmail com, gpg pubkey fingerprint:
B674 9967 0407 CE62 ACC8  22A0 32CC 55C3 39D4 7A7E

Re: [RFC 1/1] Char: mxser_new, fix recursive locking

[Jan Yenya Kasprzak]

Jiri Slaby wrote:
: On 4/14/07, Jan Yenya Kasprzak <kas@fi.muni.cz> wrote:
: >Jiri Slaby wrote:
: >: On 4/14/07, Jan Yenya Kasprzak <kas@fi.muni.cz> wrote:
: >: >~BUG: spinlock lockup on CPU#0, sshd/1671, ffffffff80557780
: >: [...]
: >: >the write(1, "/file/name/...", ...) call returned -EIO.
: >:
: >: Just a question: both with mxser_new, right?
: >
: >        No. One side has a multiport C168H with mxser_new, and the other
: 
: I meant both lockup and EIO error -- I guess so from this line.

	Yes.

-Yenya


-- 
| Jan "Yenya" Kasprzak  <kas at {fi.muni.cz - work | yenya.net - private}> |
| GPG: ID 1024/D3498839      Fingerprint 0D99A7FB206605D7 8B35FCDE05B18A5E |
| http://www.fi.muni.cz/~kas/    Journal: http://www.fi.muni.cz/~kas/blog/ |
> I will never go to meetings again because I think  face to face meetings <
> are the biggest waste of time you can ever have.        --Linus Torvalds <

Re: [RFC 1/1] Char: mxser_new, fix recursive locking

[Jiri Slaby]

On 4/14/07, Jan Yenya Kasprzak <kas@fi.muni.cz> wrote:
> Jiri Slaby wrote:
> :  ioctl(fd, TIOCMIWAIT, TIOCM_CD);
[...]
>         Hmm, I have tried to run this, and got a machine lockup, and after
> a minute or so the following has been printed to the console:

Hmm, the driver got shot full of holes, there's missing schedule() in
ioctl of both drivers. I'll post a patch in the morning or during the
day.

thanks for now,
-- 
http://www.fi.muni.cz/~xslaby/            Jiri Slaby
faculty of informatics, masaryk university, brno, cz
e-mail: jirislaby gmail com, gpg pubkey fingerprint:
B674 9967 0407 CE62 ACC8  22A0 32CC 55C3 39D4 7A7E

[RFC 1/1] Char: mxser_new, fix TIOCMIWAIT

[Jiri Slaby]

Jiri Slaby wrote:
> On 4/14/07, Jan Yenya Kasprzak <kas@fi.muni.cz> wrote:
> > Jiri Slaby wrote:
> > :  ioctl(fd, TIOCMIWAIT, TIOCM_CD);
> [...]
> >         Hmm, I have tried to run this, and got a machine lockup, and after
> > a minute or so the following has been printed to the console:
> 
> Hmm, the driver got shot full of holes, there's missing schedule() in
> ioctl of both drivers. I'll post a patch in the morning or during the
> day.

The attached patch should fix TIOCMIWAIT issue. Could you test it?

--

mxser_new, fix TIOCMIWAIT

Signed-off-by: Jiri Slaby <jirislaby@gmail.com>

---
commit e5ef19f01ffa9b6105f10fc939a07ceb3c652ed9
tree 1567fc37283da0c8436ee43a323f91140add7e73
parent a00e6d6d5f44defe9c01be2c6a3fdf1c890917c8
author Jiri Slaby <jirislaby@gmail.com> Sun, 15 Apr 2007 19:38:46 +0200
committer Jiri Slaby <jirislaby@gmail.com> Sun, 15 Apr 2007 19:38:46 +0200

 drivers/char/mxser_new.c |   38 +++++++++-----------------------------
 1 files changed, 9 insertions(+), 29 deletions(-)

diff --git a/drivers/char/mxser_new.c b/drivers/char/mxser_new.c
index 0086b73..02b6141 100644
--- a/drivers/char/mxser_new.c
+++ b/drivers/char/mxser_new.c
@@ -1767,43 +1767,23 @@ static int mxser_ioctl(struct tty_struct *tty, struct file *file,
 		 *   (use |'ed TIOCM_RNG/DSR/CD/CTS for masking)
 		 * Caller should use TIOCGICOUNT to see which one it was
 		 */
-	case TIOCMIWAIT: {
-		DECLARE_WAITQUEUE(wait, current);
-		int ret;
+	case TIOCMIWAIT:
 		mx_lock(info, flags);
-		cprev = info->icount;	/* note the counters on entry */
+		cnow = info->icount;	/* note the counters on entry */
 		mx_unlock(info, flags);
 
-		add_wait_queue(&info->delta_msr_wait, &wait);
-		while (1) {
+		wait_event_interruptible(info->delta_msr_wait, ({
+			cprev = cnow;
 			mx_lock(info, flags);
 			cnow = info->icount;	/* atomic copy */
 			mx_unlock(info, flags);
 
-			set_current_state(TASK_INTERRUPTIBLE);
-			if (((arg & TIOCM_RNG) &&
-					(cnow.rng != cprev.rng)) ||
-					((arg & TIOCM_DSR) &&
-					(cnow.dsr != cprev.dsr)) ||
-					((arg & TIOCM_CD) &&
-					(cnow.dcd != cprev.dcd)) ||
-					((arg & TIOCM_CTS) &&
-					(cnow.cts != cprev.cts))) {
-				ret = 0;
-				break;
-			}
-			/* see if a signal did it */
-			if (signal_pending(current)) {
-				ret = -ERESTARTSYS;
-				break;
-			}
-			cprev = cnow;
-		}
-		current->state = TASK_RUNNING;
-		remove_wait_queue(&info->delta_msr_wait, &wait);
+			((arg & TIOCM_RNG) && (cnow.rng != cprev.rng)) ||
+			((arg & TIOCM_DSR) && (cnow.dsr != cprev.dsr)) ||
+			((arg & TIOCM_CD)  && (cnow.dcd != cprev.dcd)) ||
+			((arg & TIOCM_CTS) && (cnow.cts != cprev.cts));
+		}));
 		break;
-	}
-	/* NOTREACHED */
 	/*
 	 * Get counter of input serial line interrupts (DCD,RI,DSR,CTS)
 	 * Return: write counters to the user passed counter struct

Re: [RFC 1/1] Char: mxser_new, fix recursive locking

[Jan Yenya Kasprzak]

Jiri Slaby wrote:
: On 4/14/07, Jan Yenya Kasprzak <kas@fi.muni.cz> wrote:
: 
: >        I have another problem with the driver - it probably sometimes
: >drops DCD signal on the serial line or something like that:
: >when the traffic on the serial console is heavy, it sometimes disconnects
: >me from the remote shell, and cu(1) displays the login prompt from the new
: >instance of mgetty of the remote machine. However, it does so both with
: >mxser.o and mxser_new.o (in 2.6.21-rc6, I think it worked in 2.6.19,
: >but I have to retest it). So this is another problem, different from
: >the one we are trying to solve now.
: 
: There were some changes, however nothing significant in mxser.c, maybe
: some of tty or ldisc layer changes (but there is only termios ->
: ktermios switch + some other things), this would probably be hard to
: find without bisecting if 2.6.19 is really OK for you.
: 
: The only idea I have right now is to nohup process, which will
: int fd = open("/dev/ttyMIXX", O_RDONLY | O_NONBLOCK);
: while (1) {
:  ioctl(fd, TIOCMIWAIT, TIOCM_CD);
:  ioctl(fd, TIOCMGET, &ret);
:  printf("%ld: carrier has changed: %u\n", time(NULL), !!(ret & TIOCM_CD));
: }
: to prove, if the carrier really becomes low.

	I ran the above code on both sides (on ttyMI0 of the server with
the MOXA card, and on ttyS0 of the server with the serial console).
I then logged in to the remote server using "cu -l ttyMI0 -s 38400",
and ran "find / -print" to generate some traffic. It went on without problems
for few minutes. But as soon as I pressed <enter> to the cu(1) (generating
traffic in the opposite direction), I was disconnected from the remote shell,
and the new mgetty has been started. The above program has reported
DCD change to 0 on both sides (in one direction it can be explained
by the fact that mgetty cycles DTR on startup, generating a DCD cycle
on the remote end of the null-modem connection).

-Yenya

-- 
| Jan "Yenya" Kasprzak  <kas at {fi.muni.cz - work | yenya.net - private}> |
| GPG: ID 1024/D3498839      Fingerprint 0D99A7FB206605D7 8B35FCDE05B18A5E |
| http://www.fi.muni.cz/~kas/    Journal: http://www.fi.muni.cz/~kas/blog/ |
> I will never go to meetings again because I think  face to face meetings <
> are the biggest waste of time you can ever have.        --Linus Torvalds <