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. Index: ucom.c =================================================================== RCS file: /cvsroot/src/sys/dev/usb/ucom.c,v retrieving revision 1.121 diff -p -u -r1.121 ucom.c --- ucom.c 11 Dec 2018 14:49:27 -0000 1.121 +++ ucom.c 19 Apr 2019 22:40:38 -0000 @@ -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: umodem_common.c =================================================================== RCS file: /cvsroot/src/sys/dev/usb/umodem_common.c,v retrieving revision 1.26 diff -p -u -r1.26 umodem_common.c --- umodem_common.c 4 Jan 2019 17:09:26 -0000 1.26 +++ umodem_common.c 19 Apr 2019 22:40:38 -0000 @@ -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,7 @@ umodem_common_activate(struct umodem_sof { switch (act) { case DVACT_DEACTIVATE: - sc->sc_dying = 1; + sc->sc_dying = true; return 0; default: return EOPNOTSUPP; @@ -674,12 +715,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: umodemvar.h =================================================================== RCS file: /cvsroot/src/sys/dev/usb/umodemvar.h,v retrieving revision 1.9 diff -p -u -r1.9 umodemvar.h --- umodemvar.h 23 Apr 2016 10:15:32 -0000 1.9 +++ umodemvar.h 19 Apr 2019 22:40:38 -0000 @@ -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 */