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 */