Module Name:    src
Committed By:   mrg
Date:           Mon May  6 23:47:39 UTC 2019

Modified Files:
        src/sys/dev/usb: umodem_common.c umodemvar.h

Log Message:
revert most of:

>fix umodem(4) detach:
>
>- ucom(4) needs kpreempt disabled around softint_schedule()
>- switch a copied printf() to aprint_error_dev()
>- use static normally in umodem_common.c
>- remove unused sc_openings in softc, convert sc_dying to real bool
>- add sc_refcnt, sc_lock and sc_detach_cv to softc.  usage is:
>  - sc_dying is protected by sc_lock
>  - sc_detach_cv is matched with sc_lock for cv operations
>  - sc_refcnt is increased in open and decreased in close, any time
>    it is decreased, it is checked for less than zero, and a broadcast
>    performed on sc_detach_cv.  detach waits for sc_refcnt.
>- umodem_param() and umodem_set() check for sc_dying
>
>this fixes pullout out an open ucom@umodem.
>
>@skrll.
>
>XXX: pullup

it only fixes the issue by chance (slightly delays, which
allows task to run, but there is no guarantee.  real fix
is incoming for all ucom parents.


To generate a diff of this commit:
cvs rdiff -u -r1.28 -r1.29 src/sys/dev/usb/umodem_common.c
cvs rdiff -u -r1.10 -r1.11 src/sys/dev/usb/umodemvar.h

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/sys/dev/usb/umodem_common.c
diff -u src/sys/dev/usb/umodem_common.c:1.28 src/sys/dev/usb/umodem_common.c:1.29
--- src/sys/dev/usb/umodem_common.c:1.28	Sat May  4 08:04:13 2019
+++ src/sys/dev/usb/umodem_common.c	Mon May  6 23:47:39 2019
@@ -1,4 +1,4 @@
-/*	$NetBSD: umodem_common.c,v 1.28 2019/05/04 08:04:13 mrg Exp $	*/
+/*	$NetBSD: umodem_common.c,v 1.29 2019/05/06 23:47:39 mrg Exp $	*/
 
 /*
  * Copyright (c) 1998 The NetBSD Foundation, Inc.
@@ -44,7 +44,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: umodem_common.c,v 1.28 2019/05/04 08:04:13 mrg Exp $");
+__KERNEL_RCSID(0, "$NetBSD: umodem_common.c,v 1.29 2019/05/06 23:47:39 mrg Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_usb.h"
@@ -120,15 +120,11 @@ umodem_common_attach(device_t self, stru
 	sc->sc_dev = self;
 	sc->sc_udev = dev;
 	sc->sc_ctl_iface = uiaa->uiaa_iface;
-	sc->sc_refcnt = 0;
 	sc->sc_dying = false;
 
 	aprint_naive("\n");
 	aprint_normal("\n");
 
-	mutex_init(&sc->sc_lock, MUTEX_DEFAULT, IPL_SOFTUSB);
-	cv_init(&sc->sc_detach_cv, "umodemdet");
-
 	id = usbd_get_interface_descriptor(sc->sc_ctl_iface);
 	devinfop = usbd_devinfo_alloc(uiaa->uiaa_device, 0);
 	aprint_normal_dev(self, "%s, iclass %d/%d\n",
@@ -271,14 +267,8 @@ umodem_open(void *addr, int portno)
 	struct umodem_softc *sc = addr;
 	int err;
 
-	mutex_enter(&sc->sc_lock);
-	if (sc->sc_dying) {
-		mutex_exit(&sc->sc_lock);
+	if (sc->sc_dying)
 		return EIO;
-	}
-
-	sc->sc_refcnt++;
-	mutex_exit(&sc->sc_lock);
 
 	DPRINTF(("umodem_open: sc=%p\n", sc));
 
@@ -291,10 +281,6 @@ umodem_open(void *addr, int portno)
 		if (err) {
 			DPRINTF(("Failed to establish notify pipe: %s\n",
 				usbd_errstr(err)));
-			mutex_enter(&sc->sc_lock);
-			if (--sc->sc_refcnt < 0)
-				cv_broadcast(&sc->sc_detach_cv);
-			mutex_exit(&sc->sc_lock);
 			return EIO;
 		}
 	}
@@ -306,32 +292,17 @@ void
 umodem_close(void *addr, int portno)
 {
 	struct umodem_softc *sc = addr;
-	int err;
-
-	mutex_enter(&sc->sc_lock);
-	if (sc->sc_dying)
-		goto out;
-	mutex_exit(&sc->sc_lock);
 
 	DPRINTF(("umodem_close: sc=%p\n", sc));
 
+	if (sc->sc_dying)
+		return;
+
 	if (sc->sc_notify_pipe != NULL) {
-		err = usbd_abort_pipe(sc->sc_notify_pipe);
-		if (err)
-			printf("%s: abort notify pipe failed: %s\n",
-			    device_xname(sc->sc_dev), usbd_errstr(err));
-		err = usbd_close_pipe(sc->sc_notify_pipe);
-		if (err)
-			printf("%s: close notify pipe failed: %s\n",
-			    device_xname(sc->sc_dev), usbd_errstr(err));
+		usbd_abort_pipe(sc->sc_notify_pipe);
+		usbd_close_pipe(sc->sc_notify_pipe);
 		sc->sc_notify_pipe = NULL;
 	}
-
-	mutex_enter(&sc->sc_lock);
-out:
-	if (--sc->sc_refcnt < 0)
-		cv_broadcast(&sc->sc_detach_cv);
-	mutex_exit(&sc->sc_lock);
 }
 
 static void
@@ -465,10 +436,8 @@ umodem_param(void *addr, int portno, str
 	usbd_status err;
 	usb_cdc_line_state_t ls;
 
-	mutex_enter(&sc->sc_lock);
 	if (sc->sc_dying)
 		return EIO;
-	mutex_exit(&sc->sc_lock);
 
 	DPRINTF(("umodem_param: sc=%p\n", sc));
 
@@ -514,10 +483,8 @@ umodem_ioctl(void *addr, int portno, u_l
 	struct umodem_softc *sc = addr;
 	int error = 0;
 
-	mutex_enter(&sc->sc_lock);
 	if (sc->sc_dying)
 		return EIO;
-	mutex_exit(&sc->sc_lock);
 
 	DPRINTF(("umodem_ioctl: cmd=0x%08lx\n", cmd));
 
@@ -607,10 +574,8 @@ umodem_set(void *addr, int portno, int r
 {
 	struct umodem_softc *sc = addr;
 
-	mutex_enter(&sc->sc_lock);
 	if (sc->sc_dying)
 		return;
-	mutex_exit(&sc->sc_lock);
 
 	switch (reg) {
 	case UCOM_SET_DTR:
@@ -692,9 +657,7 @@ umodem_common_activate(struct umodem_sof
 {
 	switch (act) {
 	case DVACT_DEACTIVATE:
-		mutex_enter(&sc->sc_lock);
 		sc->sc_dying = true;
-		mutex_exit(&sc->sc_lock);
 		return 0;
 	default:
 		return EOPNOTSUPP;
@@ -715,23 +678,12 @@ umodem_common_detach(struct umodem_softc
 
 	DPRINTF(("umodem_common_detach: sc=%p flags=%d\n", sc, flags));
 
-	mutex_enter(&sc->sc_lock);
 	sc->sc_dying = true;
 
-	sc->sc_refcnt--;
-	while (sc->sc_refcnt > 0) {
-		if (cv_timedwait(&sc->sc_detach_cv, &sc->sc_lock, hz * 60))
-			aprint_error_dev(sc->sc_dev, ": didn't detach\n");
-	}
-	mutex_exit(&sc->sc_lock);
-
 	if (sc->sc_subdev != NULL)
 		rv = config_detach(sc->sc_subdev, flags);
 
 	usbd_add_drv_event(USB_EVENT_DRIVER_DETACH, sc->sc_udev, sc->sc_dev);
 
-	mutex_destroy(&sc->sc_lock);
-	cv_destroy(&sc->sc_detach_cv);
-
 	return rv;
 }

Index: src/sys/dev/usb/umodemvar.h
diff -u src/sys/dev/usb/umodemvar.h:1.10 src/sys/dev/usb/umodemvar.h:1.11
--- src/sys/dev/usb/umodemvar.h:1.10	Sat Apr 20 05:53:18 2019
+++ src/sys/dev/usb/umodemvar.h	Mon May  6 23:47:39 2019
@@ -1,4 +1,4 @@
-/*	$NetBSD: umodemvar.h,v 1.10 2019/04/20 05:53:18 mrg Exp $	*/
+/*	$NetBSD: umodemvar.h,v 1.11 2019/05/06 23:47:39 mrg Exp $	*/
 
 /*
  * Copyright (c) 1998 The NetBSD Foundation, Inc.
@@ -53,10 +53,6 @@ struct umodem_softc {
 
 	bool			sc_dying;	/* disconnecting */
 
-	kmutex_t		sc_lock;
-	kcondvar_t		sc_detach_cv;
-	int			sc_refcnt;
-
 	int			sc_ctl_notify;	/* Notification endpoint */
 	struct usbd_pipe *	sc_notify_pipe; /* Notification pipe */
 	usb_cdc_notification_t	sc_notify_buf;	/* Notification structure */

Reply via email to