Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 24 Aug 2009 05:05:38 +0000 (UTC)
From:      Alfred Perlstein <alfred@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r196498 - head/sys/dev/usb
Message-ID:  <200908240505.n7O55cOB082804@svn.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: alfred
Date: Mon Aug 24 05:05:38 2009
New Revision: 196498
URL: http://svn.freebsd.org/changeset/base/196498

Log:
           - Patch to allow USB controller to resume operation after
           being polled.
  
           - Remove the need for Giant from the USB HUB driver.
  
           - Leave device unconfigured instead of disabling the USB port
             when Huawei Autoinstall disk detection triggers. This should
             fix problems that the Huawei device is not detected after
             Autoinstall eject is issued.
           - Reported by: Nikolay Antsiferov
  
           - Fix memory use after free race for USB character devices.
           - Reported by: Lucius Windschuh
  
           - Factor out the enumeration lock into three functions to make the
           coming newbus lock conversion more easy.
            - usbd_enum_lock
            - usbd_enum_unlock
            - usbd_enum_is_locked
  
  Submitted by:	hps

Modified:
  head/sys/dev/usb/usb_dev.c
  head/sys/dev/usb/usb_device.c
  head/sys/dev/usb/usb_device.h
  head/sys/dev/usb/usb_handle_request.c
  head/sys/dev/usb/usb_hub.c
  head/sys/dev/usb/usb_process.c
  head/sys/dev/usb/usb_process.h
  head/sys/dev/usb/usb_transfer.c

Modified: head/sys/dev/usb/usb_dev.c
==============================================================================
--- head/sys/dev/usb/usb_dev.c	Mon Aug 24 05:03:59 2009	(r196497)
+++ head/sys/dev/usb/usb_dev.c	Mon Aug 24 05:05:38 2009	(r196498)
@@ -217,7 +217,7 @@ usb_ref_device(struct usb_cdev_privdata 
 		 * We need to grab the sx-lock before grabbing the
 		 * FIFO refs to avoid deadlock at detach!
 		 */
-		sx_xlock(cpd->udev->default_sx + 1);
+		usbd_enum_lock(cpd->udev);
 
 		mtx_lock(&usb_ref_lock);
 
@@ -275,14 +275,12 @@ usb_ref_device(struct usb_cdev_privdata 
 	}
 	mtx_unlock(&usb_ref_lock);
 
-	if (crd->is_uref) {
-		mtx_lock(&Giant);	/* XXX */
-	}
 	return (0);
 
 error:
 	if (crd->is_uref) {
-		sx_unlock(cpd->udev->default_sx + 1);
+		usbd_enum_unlock(cpd->udev);
+
 		if (--(cpd->udev->refcount) == 0) {
 			cv_signal(cpd->udev->default_cv + 1);
 		}
@@ -334,10 +332,9 @@ usb_unref_device(struct usb_cdev_privdat
 
 	DPRINTFN(2, "cpd=%p is_uref=%d\n", cpd, crd->is_uref);
 
-	if (crd->is_uref) {
-		mtx_unlock(&Giant);	/* XXX */
-		sx_unlock(cpd->udev->default_sx + 1);
-	}
+	if (crd->is_uref)
+		usbd_enum_unlock(cpd->udev);
+
 	mtx_lock(&usb_ref_lock);
 	if (crd->is_read) {
 		if (--(crd->rxfifo->refcount) == 0) {
@@ -1042,9 +1039,9 @@ usb_ioctl(struct cdev *dev, u_long cmd, 
 	 * reference if we need it!
 	 */
 	err = usb_ref_device(cpd, &refs, 0 /* no uref */ );
-	if (err) {
+	if (err)
 		return (ENXIO);
-	}
+
 	fflags = cpd->fflags;
 
 	f = NULL;			/* set default value */

Modified: head/sys/dev/usb/usb_device.c
==============================================================================
--- head/sys/dev/usb/usb_device.c	Mon Aug 24 05:03:59 2009	(r196497)
+++ head/sys/dev/usb/usb_device.c	Mon Aug 24 05:05:38 2009	(r196498)
@@ -402,11 +402,11 @@ usb_unconfigure(struct usb_device *udev,
 	uint8_t do_unlock;
 
 	/* automatic locking */
-	if (sx_xlocked(udev->default_sx + 1)) {
+	if (usbd_enum_is_locked(udev)) {
 		do_unlock = 0;
 	} else {
 		do_unlock = 1;
-		sx_xlock(udev->default_sx + 1);
+		usbd_enum_lock(udev);
 	}
 
 	/* detach all interface drivers */
@@ -442,9 +442,8 @@ usb_unconfigure(struct usb_device *udev,
 	udev->curr_config_no = USB_UNCONFIG_NO;
 	udev->curr_config_index = USB_UNCONFIG_INDEX;
 
-	if (do_unlock) {
-		sx_unlock(udev->default_sx + 1);
-	}
+	if (do_unlock)
+		usbd_enum_unlock(udev);
 }
 
 /*------------------------------------------------------------------------*
@@ -472,11 +471,11 @@ usbd_set_config_index(struct usb_device 
 	DPRINTFN(6, "udev=%p index=%d\n", udev, index);
 
 	/* automatic locking */
-	if (sx_xlocked(udev->default_sx + 1)) {
+	if (usbd_enum_is_locked(udev)) {
 		do_unlock = 0;
 	} else {
 		do_unlock = 1;
-		sx_xlock(udev->default_sx + 1);
+		usbd_enum_lock(udev);
 	}
 
 	usb_unconfigure(udev, USB_UNCFG_FLAG_FREE_SUBDEV);
@@ -585,9 +584,8 @@ done:
 	if (err) {
 		usb_unconfigure(udev, USB_UNCFG_FLAG_FREE_SUBDEV);
 	}
-	if (do_unlock) {
-		sx_unlock(udev->default_sx + 1);
-	}
+	if (do_unlock)
+		usbd_enum_unlock(udev);
 	return (err);
 }
 
@@ -823,11 +821,11 @@ usbd_set_alt_interface_index(struct usb_
 	uint8_t do_unlock;
 
 	/* automatic locking */
-	if (sx_xlocked(udev->default_sx + 1)) {
+	if (usbd_enum_is_locked(udev)) {
 		do_unlock = 0;
 	} else {
 		do_unlock = 1;
-		sx_xlock(udev->default_sx + 1);
+		usbd_enum_lock(udev);
 	}
 	if (iface == NULL) {
 		err = USB_ERR_INVAL;
@@ -863,9 +861,9 @@ usbd_set_alt_interface_index(struct usb_
 	    iface->idesc->bAlternateSetting);
 
 done:
-	if (do_unlock) {
-		sx_unlock(udev->default_sx + 1);
-	}
+	if (do_unlock)
+		usbd_enum_unlock(udev);
+
 	return (err);
 }
 
@@ -1230,11 +1228,11 @@ usb_probe_and_attach(struct usb_device *
 		return (USB_ERR_INVAL);
 	}
 	/* automatic locking */
-	if (sx_xlocked(udev->default_sx + 1)) {
+	if (usbd_enum_is_locked(udev)) {
 		do_unlock = 0;
 	} else {
 		do_unlock = 1;
-		sx_xlock(udev->default_sx + 1);
+		usbd_enum_lock(udev);
 	}
 
 	if (udev->curr_config_index == USB_UNCONFIG_INDEX) {
@@ -1315,9 +1313,9 @@ usb_probe_and_attach(struct usb_device *
 		}
 	}
 done:
-	if (do_unlock) {
-		sx_unlock(udev->default_sx + 1);
-	}
+	if (do_unlock)
+		usbd_enum_unlock(udev);
+
 	return (0);
 }
 
@@ -1779,7 +1777,8 @@ repeat_set_config:
 			}
 		} else if (usb_test_huawei_autoinst_p(udev, &uaa) == 0) {
 			DPRINTFN(0, "Found Huawei auto-install disk!\n");
-			err = USB_ERR_STALLED;	/* fake an error */
+			/* leave device unconfigured */
+			usb_unconfigure(udev, USB_UNCFG_FLAG_FREE_SUBDEV);
 		}
 	} else {
 		err = 0;		/* set success */
@@ -1902,15 +1901,18 @@ static void
 usb_cdev_free(struct usb_device *udev)
 {
 	struct usb_fs_privdata* pd;
+	struct cdev* pcdev;
 
 	DPRINTFN(2, "Freeing device nodes\n");
 
 	while ((pd = LIST_FIRST(&udev->pd_list)) != NULL) {
 		KASSERT(pd->cdev->si_drv1 == pd, ("privdata corrupt"));
 
-		destroy_dev_sched_cb(pd->cdev, usb_cdev_cleanup, pd);
+		pcdev = pd->cdev;
 		pd->cdev = NULL;
 		LIST_REMOVE(pd, pd_next);
+		if (pcdev != NULL)
+			destroy_dev_sched_cb(pcdev, usb_cdev_cleanup, pd);
 	}
 }
 
@@ -2448,3 +2450,37 @@ usbd_device_attached(struct usb_device *
 {
 	return (udev->state > USB_STATE_DETACHED);
 }
+
+/* The following function locks enumerating the given USB device. */
+
+void
+usbd_enum_lock(struct usb_device *udev)
+{
+	sx_xlock(udev->default_sx + 1);
+	/* 
+	 * NEWBUS LOCK NOTE: We should check if any parent SX locks
+	 * are locked before locking Giant. Else the lock can be
+	 * locked multiple times.
+	 */
+	mtx_lock(&Giant);
+}
+
+/* The following function unlocks enumerating the given USB device. */
+
+void
+usbd_enum_unlock(struct usb_device *udev)
+{
+	mtx_unlock(&Giant);
+	sx_xunlock(udev->default_sx + 1);
+}
+
+/*
+ * The following function checks the enumerating lock for the given
+ * USB device.
+ */
+
+uint8_t
+usbd_enum_is_locked(struct usb_device *udev)
+{
+	return (sx_xlocked(udev->default_sx + 1));
+}

Modified: head/sys/dev/usb/usb_device.h
==============================================================================
--- head/sys/dev/usb/usb_device.h	Mon Aug 24 05:03:59 2009	(r196497)
+++ head/sys/dev/usb/usb_device.h	Mon Aug 24 05:05:38 2009	(r196498)
@@ -211,5 +211,8 @@ uint8_t	usb_peer_can_wakeup(struct usb_d
 struct usb_endpoint *usb_endpoint_foreach(struct usb_device *udev, struct usb_endpoint *ep);
 void	usb_set_device_state(struct usb_device *udev,
 	    enum usb_dev_state state);
+void	usbd_enum_lock(struct usb_device *);
+void	usbd_enum_unlock(struct usb_device *);
+uint8_t usbd_enum_is_locked(struct usb_device *);
 
 #endif					/* _USB_DEVICE_H_ */

Modified: head/sys/dev/usb/usb_handle_request.c
==============================================================================
--- head/sys/dev/usb/usb_handle_request.c	Mon Aug 24 05:03:59 2009	(r196497)
+++ head/sys/dev/usb/usb_handle_request.c	Mon Aug 24 05:05:38 2009	(r196498)
@@ -152,8 +152,8 @@ usb_handle_set_config(struct usb_xfer *x
 	 * attach:
 	 */
 	USB_XFER_UNLOCK(xfer);
-	mtx_lock(&Giant);		/* XXX */
-	sx_xlock(udev->default_sx + 1);
+
+	usbd_enum_lock(udev);
 
 	if (conf_no == USB_UNCONFIG_NO) {
 		conf_no = USB_UNCONFIG_INDEX;
@@ -176,8 +176,7 @@ usb_handle_set_config(struct usb_xfer *x
 		goto done;
 	}
 done:
-	mtx_unlock(&Giant);		/* XXX */
-	sx_unlock(udev->default_sx + 1);
+	usbd_enum_unlock(udev);
 	USB_XFER_LOCK(xfer);
 	return (err);
 }
@@ -190,19 +189,19 @@ usb_check_alt_setting(struct usb_device 
 	usb_error_t err = 0;
 
 	/* automatic locking */
-	if (sx_xlocked(udev->default_sx + 1)) {
+	if (usbd_enum_is_locked(udev)) {
 		do_unlock = 0;
 	} else {
 		do_unlock = 1;
-		sx_xlock(udev->default_sx + 1);
+		usbd_enum_lock(udev);
 	}
 
 	if (alt_index >= usbd_get_no_alts(udev->cdesc, iface->idesc))
 		err = USB_ERR_INVAL;
 
-	if (do_unlock) {
-		sx_unlock(udev->default_sx + 1);
-	}
+	if (do_unlock)
+		usbd_enum_unlock(udev);
+
 	return (err);
 }
 
@@ -236,8 +235,8 @@ usb_handle_iface_request(struct usb_xfer
 	 * attach:
 	 */
 	USB_XFER_UNLOCK(xfer);
-	mtx_lock(&Giant);		/* XXX */
-	sx_xlock(udev->default_sx + 1);
+
+	usbd_enum_lock(udev);
 
 	error = ENXIO;
 
@@ -353,20 +352,17 @@ tr_repeat:
 		goto tr_stalled;
 	}
 tr_valid:
-	mtx_unlock(&Giant);
-	sx_unlock(udev->default_sx + 1);
+	usbd_enum_unlock(udev);
 	USB_XFER_LOCK(xfer);
 	return (0);
 
 tr_short:
-	mtx_unlock(&Giant);
-	sx_unlock(udev->default_sx + 1);
+	usbd_enum_unlock(udev);
 	USB_XFER_LOCK(xfer);
 	return (USB_ERR_SHORT_XFER);
 
 tr_stalled:
-	mtx_unlock(&Giant);
-	sx_unlock(udev->default_sx + 1);
+	usbd_enum_unlock(udev);
 	USB_XFER_LOCK(xfer);
 	return (USB_ERR_STALLED);
 }

Modified: head/sys/dev/usb/usb_hub.c
==============================================================================
--- head/sys/dev/usb/usb_hub.c	Mon Aug 24 05:03:59 2009	(r196497)
+++ head/sys/dev/usb/usb_hub.c	Mon Aug 24 05:05:38 2009	(r196498)
@@ -96,6 +96,7 @@ struct uhub_current_state {
 struct uhub_softc {
 	struct uhub_current_state sc_st;/* current state */
 	device_t sc_dev;		/* base device */
+	struct mtx sc_mtx;		/* our mutex */
 	struct usb_device *sc_udev;	/* USB device */
 	struct usb_xfer *sc_xfer[UHUB_N_TRANSFER];	/* interrupt xfer */
 	uint8_t	sc_flags;
@@ -428,7 +429,6 @@ repeat:
 		mode = USB_MODE_HOST;
 
 	/* need to create a new child */
-
 	child = usb_alloc_device(sc->sc_dev, udev->bus, udev,
 	    udev->depth + 1, portno - 1, portno, speed, mode);
 	if (child == NULL) {
@@ -691,6 +691,8 @@ uhub_attach(device_t dev)
 	sc->sc_udev = udev;
 	sc->sc_dev = dev;
 
+	mtx_init(&sc->sc_mtx, "USB HUB mutex", NULL, MTX_DEF);
+
 	snprintf(sc->sc_name, sizeof(sc->sc_name), "%s",
 	    device_get_nameunit(dev));
 
@@ -774,7 +776,7 @@ uhub_attach(device_t dev)
 	} else {
 		/* normal HUB */
 		err = usbd_transfer_setup(udev, &iface_index, sc->sc_xfer,
-		    uhub_config, UHUB_N_TRANSFER, sc, &Giant);
+		    uhub_config, UHUB_N_TRANSFER, sc, &sc->sc_mtx);
 	}
 	if (err) {
 		DPRINTFN(0, "cannot setup interrupt transfer, "
@@ -850,9 +852,9 @@ uhub_attach(device_t dev)
 	/* Start the interrupt endpoint, if any */
 
 	if (sc->sc_xfer[0] != NULL) {
-		USB_XFER_LOCK(sc->sc_xfer[0]);
+		mtx_lock(&sc->sc_mtx);
 		usbd_transfer_start(sc->sc_xfer[0]);
-		USB_XFER_UNLOCK(sc->sc_xfer[0]);
+		mtx_unlock(&sc->sc_mtx);
 	}
 
 	/* Enable automatic power save on all USB HUBs */
@@ -868,6 +870,9 @@ error:
 		free(udev->hub, M_USBDEV);
 		udev->hub = NULL;
 	}
+
+	mtx_destroy(&sc->sc_mtx);
+
 	return (ENXIO);
 }
 
@@ -908,6 +913,9 @@ uhub_detach(device_t dev)
 
 	free(hub, M_USBDEV);
 	sc->sc_udev->hub = NULL;
+
+	mtx_destroy(&sc->sc_mtx);
+
 	return (0);
 }
 
@@ -1775,10 +1783,13 @@ usb_dev_resume_peer(struct usb_device *u
 		/* always update hardware power! */
 		(bus->methods->set_hw_power) (bus);
 	}
-	sx_xlock(udev->default_sx + 1);
+
+	usbd_enum_lock(udev);
+
 	/* notify all sub-devices about resume */
 	err = usb_suspend_resume(udev, 0);
-	sx_unlock(udev->default_sx + 1);
+
+	usbd_enum_unlock(udev);
 
 	/* check if peer has wakeup capability */
 	if (usb_peer_can_wakeup(udev)) {
@@ -1844,10 +1855,12 @@ repeat:
 		}
 	}
 
-	sx_xlock(udev->default_sx + 1);
+	usbd_enum_lock(udev);
+
 	/* notify all sub-devices about suspend */
 	err = usb_suspend_resume(udev, 1);
-	sx_unlock(udev->default_sx + 1);
+
+	usbd_enum_unlock(udev);
 
 	if (usb_peer_can_wakeup(udev)) {
 		/* allow device to do remote wakeup */

Modified: head/sys/dev/usb/usb_process.c
==============================================================================
--- head/sys/dev/usb/usb_process.c	Mon Aug 24 05:03:59 2009	(r196497)
+++ head/sys/dev/usb/usb_process.c	Mon Aug 24 05:05:38 2009	(r196498)
@@ -448,3 +448,29 @@ usb_proc_drain(struct usb_process *up)
 	}
 	mtx_unlock(up->up_mtx);
 }
+
+/*------------------------------------------------------------------------*
+ *	usb_proc_rewakeup
+ *
+ * This function is called to re-wakeup the the given USB
+ * process. This usually happens after that the USB system has been in
+ * polling mode, like during a panic. This function must be called
+ * having "up->up_mtx" locked.
+ *------------------------------------------------------------------------*/
+void
+usb_proc_rewakeup(struct usb_process *up)
+{
+	/* check if not initialised */
+	if (up->up_mtx == NULL)
+		return;
+	/* check if gone */
+	if (up->up_gone)
+		return;
+
+	mtx_assert(up->up_mtx, MA_OWNED);
+
+	if (up->up_msleep == 0) {
+		/* re-wakeup */
+		cv_signal(&up->up_cv);
+	}
+}

Modified: head/sys/dev/usb/usb_process.h
==============================================================================
--- head/sys/dev/usb/usb_process.h	Mon Aug 24 05:03:59 2009	(r196497)
+++ head/sys/dev/usb/usb_process.h	Mon Aug 24 05:05:38 2009	(r196498)
@@ -75,5 +75,6 @@ void	usb_proc_drain(struct usb_process *
 void	usb_proc_mwait(struct usb_process *up, void *pm0, void *pm1);
 void	usb_proc_free(struct usb_process *up);
 void   *usb_proc_msignal(struct usb_process *up, void *pm0, void *pm1);
+void	usb_proc_rewakeup(struct usb_process *up);
 
 #endif					/* _USB_PROCESS_H_ */

Modified: head/sys/dev/usb/usb_transfer.c
==============================================================================
--- head/sys/dev/usb/usb_transfer.c	Mon Aug 24 05:03:59 2009	(r196497)
+++ head/sys/dev/usb/usb_transfer.c	Mon Aug 24 05:05:38 2009	(r196498)
@@ -2907,13 +2907,9 @@ usbd_transfer_poll(struct usb_xfer **ppx
 		}
 
 		/* Make sure cv_signal() and cv_broadcast() is not called */
-		udev->bus->control_xfer_proc.up_dsleep = 0;
 		udev->bus->control_xfer_proc.up_msleep = 0;
-		udev->bus->explore_proc.up_dsleep = 0;
 		udev->bus->explore_proc.up_msleep = 0;
-		udev->bus->giant_callback_proc.up_dsleep = 0;
 		udev->bus->giant_callback_proc.up_msleep = 0;
-		udev->bus->non_giant_callback_proc.up_dsleep = 0;
 		udev->bus->non_giant_callback_proc.up_msleep = 0;
 
 		/* poll USB hardware */



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200908240505.n7O55cOB082804>