Subject: kern/13731: ucom hangs when closing
To: None <gnats-bugs@gnats.netbsd.org>
From: None <toshii@netbsd.org>
List: netbsd-bugs
Date: 08/17/2001 01:08:14
>Number:         13731
>Category:       kern
>Synopsis:       ucom hangs when closing
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    kern-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Thu Aug 16 09:04:00 PDT 2001
>Closed-Date:
>Last-Modified:
>Originator:     IWAMOTO Toshihiro
>Release:        1.5K with some pullups from newer code, ucom.c is 1.36
>Organization:
	
>Environment:
	
System: NetBSD kiku.my.domain 1.5K NetBSD 1.5K (KIKU) #2: Fri Aug 17 00:13:35 JST 2001 toshii@kiku.my.domain:/export/kiku/tmp/netbsd-15k/sys/arch/i386/compile/KIKU i386
Architecture: i386
Machine: i386
>Description:
	ucom often hangs when closing. traceback of the hanging process
	(I'm writing from memory):
ttysleep()
ttywait()
ttywflush()
ttylclose()
ucomclose()

	The hanging process can never get out of wchan "ttyout".
	ucom's tp->t_state had the TS_BUSY flag, and its
	sc_oxfer showed there was a USBD_IOERROR.

	I think the problem is that ucomwritecb() doesn't clear TS_BUSY
	when a USB transfer error happens.

>How-To-Repeat:
	Hook a uftdi device to an ISDN adaptor and configure a ppp link.
	Send HUP to pppd when the ppp link gets stalled.
	You'll often see pppd is hanging at wchan "ttyout".

>Fix:
	Hopefully the following diff fixes the problem.
	I'm testing with this change.
	(I noticed we might have to call l_start even when an error happens,
	but I'm not willing to reboot the machine until next ppp link failure)

Index: ucom.c
===================================================================
RCS file: /amd/kiku/NetBSD/NetBSD-CVS/syssrc/sys/dev/usb/ucom.c,v
retrieving revision 1.36
diff -u -r1.36 ucom.c
--- ucom.c	2001/01/23 22:06:25	1.36
+++ ucom.c	2001/08/16 15:13:03
@@ -956,13 +956,13 @@
 	DPRINTFN(5,("ucomwritecb: status=%d\n", status));
 
 	if (status == USBD_CANCELLED || sc->sc_dying)
-		return;
+		goto error;
 
 	if (status) {
 		DPRINTF(("ucomwritecb: status=%d\n", status));
 		usbd_clear_endpoint_stall_async(sc->sc_bulkin_pipe);
 		/* XXX we should restart after some delay. */
-		return;
+		goto error;
 	}
 
 	usbd_get_xfer_status(xfer, NULL, NULL, &cc, NULL);
@@ -980,6 +980,12 @@
 	else
 		ndflush(&tp->t_outq, cc);
 	(*tp->t_linesw->l_start)(tp);
+	splx(s);
+	return;
+
+error:
+	s = spltty();
+	CLR(tp->t_state, TS_BUSY);
 	splx(s);
 }
 
>Release-Note:
>Audit-Trail:
>Unformatted: