Module Name:    src
Committed By:   mrg
Date:           Sat Apr 20 05:53:18 UTC 2019

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

Log Message:
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


To generate a diff of this commit:
cvs rdiff -u -r1.121 -r1.122 src/sys/dev/usb/ucom.c
cvs rdiff -u -r1.26 -r1.27 src/sys/dev/usb/umodem_common.c
cvs rdiff -u -r1.9 -r1.10 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/ucom.c
diff -u src/sys/dev/usb/ucom.c:1.121 src/sys/dev/usb/ucom.c:1.122
--- src/sys/dev/usb/ucom.c:1.121	Tue Dec 11 14:49:27 2018
+++ src/sys/dev/usb/ucom.c	Sat Apr 20 05:53:18 2019
@@ -1,4 +1,4 @@
-/*	$NetBSD: ucom.c,v 1.121 2018/12/11 14:49:27 jakllsch Exp $	*/
+/*	$NetBSD: ucom.c,v 1.122 2019/04/20 05:53:18 mrg Exp $	*/
 
 /*
  * Copyright (c) 1998, 2000 The NetBSD Foundation, Inc.
@@ -34,7 +34,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: ucom.c,v 1.121 2018/12/11 14:49:27 jakllsch Exp $");
+__KERNEL_RCSID(0, "$NetBSD: ucom.c,v 1.122 2019/04/20 05:53:18 mrg Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_usb.h"
@@ -441,10 +441,8 @@ ucom_detach(device_t self, int flags)
 			mutex_spin_exit(&tty_lock);
 		}
 		/* Wait for processes to go away. */
-		if (cv_timedwait(&sc->sc_detachcv, &sc->sc_lock, hz * 60)) {
-			printf("%s: %s didn't detach\n", __func__,
-			    device_xname(sc->sc_dev));
-		}
+		if (cv_timedwait(&sc->sc_detachcv, &sc->sc_lock, hz * 60))
+			aprint_error_dev(self, ": didn't detach\n");
 	}
 
 	softint_disestablish(sc->sc_si);
@@ -1271,7 +1269,9 @@ ucomhwiflow(struct tty *tp, int block)
 
 	if (old && !block) {
 		sc->sc_rx_unblock = 1;
+		kpreempt_disable();
 		softint_schedule(sc->sc_si);
+		kpreempt_enable();
 	}
 	mutex_exit(&sc->sc_lock);
 
@@ -1339,7 +1339,9 @@ ucomstart(struct tty *tp)
 
 	SIMPLEQ_INSERT_TAIL(&sc->sc_obuff_full, ub, ub_link);
 
+	kpreempt_disable();
 	softint_schedule(sc->sc_si);
+	kpreempt_enable();
 
  out:
 	DPRINTF("... done", 0, 0, 0, 0);
@@ -1381,7 +1383,9 @@ ucom_write_status(struct ucom_softc *sc,
 		break;
 	case USBD_STALLED:
 		ub->ub_index = 0;
+		kpreempt_disable();
 		softint_schedule(sc->sc_si);
+		kpreempt_enable();
 		break;
 	case USBD_NORMAL_COMPLETION:
 		usbd_get_xfer_status(ub->ub_xfer, NULL, NULL, &cc, NULL);

Index: src/sys/dev/usb/umodem_common.c
diff -u src/sys/dev/usb/umodem_common.c:1.26 src/sys/dev/usb/umodem_common.c:1.27
--- src/sys/dev/usb/umodem_common.c:1.26	Fri Jan  4 17:09:26 2019
+++ src/sys/dev/usb/umodem_common.c	Sat Apr 20 05:53:18 2019
@@ -1,4 +1,4 @@
-/*	$NetBSD: umodem_common.c,v 1.26 2019/01/04 17:09:26 tih Exp $	*/
+/*	$NetBSD: umodem_common.c,v 1.27 2019/04/20 05:53:18 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.26 2019/01/04 17:09:26 tih Exp $");
+__KERNEL_RCSID(0, "$NetBSD: umodem_common.c,v 1.27 2019/04/20 05:53:18 mrg Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_usb.h"
@@ -94,16 +94,16 @@ int	umodemdebug = 0;
 #define UMODEMIBUFSIZE 4096
 #define UMODEMOBUFSIZE 4096
 
-Static usbd_status umodem_set_comm_feature(struct umodem_softc *,
+static usbd_status umodem_set_comm_feature(struct umodem_softc *,
 					   int, int);
-Static usbd_status umodem_set_line_coding(struct umodem_softc *,
+static usbd_status umodem_set_line_coding(struct umodem_softc *,
 					  usb_cdc_line_state_t *);
 
-Static void	umodem_dtr(struct umodem_softc *, int);
-Static void	umodem_rts(struct umodem_softc *, int);
-Static void	umodem_break(struct umodem_softc *, int);
-Static void	umodem_set_line_state(struct umodem_softc *);
-Static void	umodem_intr(struct usbd_xfer *, void *, usbd_status);
+static void	umodem_dtr(struct umodem_softc *, int);
+static void	umodem_rts(struct umodem_softc *, int);
+static void	umodem_break(struct umodem_softc *, int);
+static void	umodem_set_line_state(struct umodem_softc *);
+static void	umodem_intr(struct usbd_xfer *, void *, usbd_status);
 
 int
 umodem_common_attach(device_t self, struct umodem_softc *sc,
@@ -120,10 +120,15 @@ 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",
@@ -256,7 +261,7 @@ umodem_common_attach(device_t self, stru
 	return 0;
 
  bad:
-	sc->sc_dying = 1;
+	sc->sc_dying = true;
 	return 1;
 }
 
@@ -266,6 +271,15 @@ 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);
+		return EIO;
+	}
+
+	sc->sc_refcnt++;
+	mutex_exit(&sc->sc_lock);
+
 	DPRINTF(("umodem_open: sc=%p\n", sc));
 
 	if (sc->sc_ctl_notify != -1 && sc->sc_notify_pipe == NULL) {
@@ -277,6 +291,10 @@ 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;
 		}
 	}
@@ -290,6 +308,11 @@ 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_notify_pipe != NULL) {
@@ -303,9 +326,15 @@ umodem_close(void *addr, int portno)
 			    device_xname(sc->sc_dev), usbd_errstr(err));
 		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
+static void
 umodem_intr(struct usbd_xfer *xfer, void *priv,
     usbd_status status)
 {
@@ -438,6 +467,11 @@ 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));
 
 	USETDW(ls.dwDTERate, t->c_ospeed);
@@ -482,8 +516,10 @@ 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));
 
@@ -507,7 +543,7 @@ umodem_ioctl(void *addr, int portno, u_l
 	return error;
 }
 
-void
+static void
 umodem_dtr(struct umodem_softc *sc, int onoff)
 {
 	DPRINTF(("umodem_dtr: onoff=%d\n", onoff));
@@ -519,7 +555,7 @@ umodem_dtr(struct umodem_softc *sc, int 
 	umodem_set_line_state(sc);
 }
 
-void
+static void
 umodem_rts(struct umodem_softc *sc, int onoff)
 {
 	DPRINTF(("umodem_rts: onoff=%d\n", onoff));
@@ -531,7 +567,7 @@ umodem_rts(struct umodem_softc *sc, int 
 	umodem_set_line_state(sc);
 }
 
-void
+static void
 umodem_set_line_state(struct umodem_softc *sc)
 {
 	usb_device_request_t req;
@@ -549,7 +585,7 @@ umodem_set_line_state(struct umodem_soft
 
 }
 
-void
+static void
 umodem_break(struct umodem_softc *sc, int onoff)
 {
 	usb_device_request_t req;
@@ -573,6 +609,11 @@ 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:
 		umodem_dtr(sc, onoff);
@@ -588,7 +629,7 @@ umodem_set(void *addr, int portno, int r
 	}
 }
 
-usbd_status
+static usbd_status
 umodem_set_line_coding(struct umodem_softc *sc, usb_cdc_line_state_t *state)
 {
 	usb_device_request_t req;
@@ -621,7 +662,7 @@ umodem_set_line_coding(struct umodem_sof
 	return USBD_NORMAL_COMPLETION;
 }
 
-usbd_status
+static usbd_status
 umodem_set_comm_feature(struct umodem_softc *sc, int feature, int state)
 {
 	usb_device_request_t req;
@@ -653,7 +694,9 @@ umodem_common_activate(struct umodem_sof
 {
 	switch (act) {
 	case DVACT_DEACTIVATE:
-		sc->sc_dying = 1;
+		mutex_enter(&sc->sc_lock);
+		sc->sc_dying = true;
+		mutex_exit(&sc->sc_lock);
 		return 0;
 	default:
 		return EOPNOTSUPP;
@@ -674,12 +717,23 @@ umodem_common_detach(struct umodem_softc
 
 	DPRINTF(("umodem_common_detach: sc=%p flags=%d\n", sc, flags));
 
-	sc->sc_dying = 1;
+	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.9 src/sys/dev/usb/umodemvar.h:1.10
--- src/sys/dev/usb/umodemvar.h:1.9	Sat Apr 23 10:15:32 2016
+++ src/sys/dev/usb/umodemvar.h	Sat Apr 20 05:53:18 2019
@@ -1,4 +1,4 @@
-/*	$NetBSD: umodemvar.h,v 1.9 2016/04/23 10:15:32 skrll Exp $	*/
+/*	$NetBSD: umodemvar.h,v 1.10 2019/04/20 05:53:18 mrg Exp $	*/
 
 /*
  * Copyright (c) 1998 The NetBSD Foundation, Inc.
@@ -51,8 +51,11 @@ struct umodem_softc {
 
 	device_t		sc_subdev;	/* ucom device */
 
-	u_char			sc_opening;	/* lock during open */
-	u_char			sc_dying;	/* disconnecting */
+	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 */

Reply via email to