Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 11 Aug 2011 11:30:21 +0000 (UTC)
From:      Hans Petter Selasky <hselasky@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r224777 - head/sys/dev/usb
Message-ID:  <201108111130.p7BBULUb091001@svn.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: hselasky
Date: Thu Aug 11 11:30:21 2011
New Revision: 224777
URL: http://svn.freebsd.org/changeset/base/224777

Log:
  Use synchronous device destruction instead of asynchronous, so that a new
  device having the same name like a previous one is not created before the old
  one is gone. This fixes some panics due to asserts in the devfs code which
  were added recently.
  
  Approved by:    re (kib)
  MFC after:      1 week

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/usbdi.h

Modified: head/sys/dev/usb/usb_dev.c
==============================================================================
--- head/sys/dev/usb/usb_dev.c	Thu Aug 11 10:29:10 2011	(r224776)
+++ head/sys/dev/usb/usb_dev.c	Thu Aug 11 11:30:21 2011	(r224777)
@@ -1648,7 +1648,6 @@ usb_fifo_attach(struct usb_device *udev,
 	struct usb_fifo *f_rx;
 	char devname[32];
 	uint8_t n;
-	struct usb_fs_privdata* pd;
 
 	f_sc->fp[USB_FIFO_TX] = NULL;
 	f_sc->fp[USB_FIFO_RX] = NULL;
@@ -1746,22 +1745,10 @@ usb_fifo_attach(struct usb_device *udev,
 			    usb_alloc_symlink(devname);
 		}
 
-		/*
-		 * Initialize device private data - this is used to find the
-		 * actual USB device itself.
-		 */
-		pd = malloc(sizeof(struct usb_fs_privdata), M_USBDEV, M_WAITOK | M_ZERO);
-		pd->bus_index = device_get_unit(udev->bus->bdev);
-		pd->dev_index = udev->device_index;
-		pd->ep_addr = -1;	/* not an endpoint */
-		pd->fifo_index = f_tx->fifo_index & f_rx->fifo_index;
-		pd->mode = FREAD|FWRITE;
-
-		/* Now, create the device itself */
-		f_sc->dev = make_dev(&usb_devsw, 0, uid, gid, mode,
-		    "%s", devname);
-		/* XXX setting si_drv1 and creating the device is not atomic! */
-		f_sc->dev->si_drv1 = pd;
+		/* Create the device */
+		f_sc->dev = usb_make_dev(udev, devname, -1,
+		    f_tx->fifo_index & f_rx->fifo_index,
+		    FREAD|FWRITE, uid, gid, mode);
 	}
 
 	DPRINTFN(2, "attached %p/%p\n", f_tx, f_rx);
@@ -1814,12 +1801,6 @@ usb_fifo_free_buffer(struct usb_fifo *f)
 	bzero(&f->used_q, sizeof(f->used_q));
 }
 
-static void
-usb_fifo_cleanup(void* ptr) 
-{
-	free(ptr, M_USBDEV);
-}
-
 void
 usb_fifo_detach(struct usb_fifo_sc *f_sc)
 {
@@ -1832,11 +1813,9 @@ usb_fifo_detach(struct usb_fifo_sc *f_sc
 	f_sc->fp[USB_FIFO_TX] = NULL;
 	f_sc->fp[USB_FIFO_RX] = NULL;
 
-	if (f_sc->dev != NULL) {
-		destroy_dev_sched_cb(f_sc->dev, 
-		    usb_fifo_cleanup, f_sc->dev->si_drv1);
-		f_sc->dev = NULL;
-	}
+	usb_destroy_dev(f_sc->dev);
+
+	f_sc->dev = NULL;
 
 	DPRINTFN(2, "detached %p\n", f_sc);
 }

Modified: head/sys/dev/usb/usb_device.c
==============================================================================
--- head/sys/dev/usb/usb_device.c	Thu Aug 11 10:29:10 2011	(r224776)
+++ head/sys/dev/usb/usb_device.c	Thu Aug 11 11:30:21 2011	(r224777)
@@ -102,10 +102,8 @@ static void	usb_notify_addq(const char *
 #endif
 #if USB_HAVE_UGEN
 static void	usb_fifo_free_wrap(struct usb_device *, uint8_t, uint8_t);
-static struct cdev *usb_make_dev(struct usb_device *, int, int);
 static void	usb_cdev_create(struct usb_device *);
 static void	usb_cdev_free(struct usb_device *);
-static void	usb_cdev_cleanup(void *);
 #endif
 
 /* This variable is global to allow easy access to it: */
@@ -1626,10 +1624,12 @@ usb_alloc_device(device_t parent_dev, st
 	LIST_INIT(&udev->pd_list);
 
 	/* Create the control endpoint device */
-	udev->ctrl_dev = usb_make_dev(udev, 0, FREAD|FWRITE);
+	udev->ctrl_dev = usb_make_dev(udev, NULL, 0, 0,
+	    FREAD|FWRITE, UID_ROOT, GID_OPERATOR, 0600);
 
 	/* Create a link from /dev/ugenX.X to the default endpoint */
-	make_dev_alias(udev->ctrl_dev, "%s", udev->ugen_name);
+	if (udev->ctrl_dev != NULL)
+		make_dev_alias(udev->ctrl_dev->cdev, "%s", udev->ugen_name);
 #endif
 	/* Initialise device */
 	if (bus->methods->device_init != NULL) {
@@ -1884,11 +1884,12 @@ done:
 }
 
 #if USB_HAVE_UGEN
-static struct cdev *
-usb_make_dev(struct usb_device *udev, int ep, int mode)
+struct usb_fs_privdata *
+usb_make_dev(struct usb_device *udev, const char *devname, int ep,
+    int fi, int rwmode, uid_t uid, gid_t gid, int mode)
 {
 	struct usb_fs_privdata* pd;
-	char devname[20];
+	char buffer[32];
 
 	/* Store information to locate ourselves again later */
 	pd = malloc(sizeof(struct usb_fs_privdata), M_USBDEV,
@@ -1896,16 +1897,39 @@ usb_make_dev(struct usb_device *udev, in
 	pd->bus_index = device_get_unit(udev->bus->bdev);
 	pd->dev_index = udev->device_index;
 	pd->ep_addr = ep;
-	pd->mode = mode;
+	pd->fifo_index = fi;
+	pd->mode = rwmode;
 
 	/* Now, create the device itself */
-	snprintf(devname, sizeof(devname), "%u.%u.%u",
-	    pd->bus_index, pd->dev_index, pd->ep_addr);
-	pd->cdev = make_dev(&usb_devsw, 0, UID_ROOT,
-	    GID_OPERATOR, 0600, USB_DEVICE_DIR "/%s", devname);
+	if (devname == NULL) {
+		devname = buffer;
+		snprintf(buffer, sizeof(buffer), USB_DEVICE_DIR "/%u.%u.%u",
+		    pd->bus_index, pd->dev_index, pd->ep_addr);
+	}
+
+	pd->cdev = make_dev(&usb_devsw, 0, uid, gid, mode, "%s", devname);
+
+	if (pd->cdev == NULL) {
+		DPRINTFN(0, "Failed to create device %s\n", devname);
+		free(pd, M_USBDEV);
+		return (NULL);
+	}
+
+	/* XXX setting si_drv1 and creating the device is not atomic! */
 	pd->cdev->si_drv1 = pd;
 
-	return (pd->cdev);
+	return (pd);
+}
+
+void
+usb_destroy_dev(struct usb_fs_privdata *pd)
+{
+	if (pd == NULL)
+		return;
+
+	destroy_dev(pd->cdev);
+
+	free(pd, M_USBDEV);
 }
 
 static void
@@ -1915,7 +1939,6 @@ usb_cdev_create(struct usb_device *udev)
 	struct usb_endpoint_descriptor *ed;
 	struct usb_descriptor *desc;
 	struct usb_fs_privdata* pd;
-	struct cdev *dev;
 	int inmode, outmode, inmask, outmask, mode;
 	uint8_t ep;
 
@@ -1957,14 +1980,16 @@ usb_cdev_create(struct usb_device *udev)
 
 	/* Create all available endpoints except EP0 */
 	for (ep = 1; ep < 16; ep++) {
-		mode = inmask & (1 << ep) ? inmode : 0;
-		mode |= outmask & (1 << ep) ? outmode : 0;
+		mode = (inmask & (1 << ep)) ? inmode : 0;
+		mode |= (outmask & (1 << ep)) ? outmode : 0;
 		if (mode == 0)
 			continue;	/* no IN or OUT endpoint */
 
-		dev = usb_make_dev(udev, ep, mode);
-		pd = dev->si_drv1;
-		LIST_INSERT_HEAD(&udev->pd_list, pd, pd_next);
+		pd = usb_make_dev(udev, NULL, ep, 0,
+		    mode, UID_ROOT, GID_OPERATOR, 0600);
+
+		if (pd != NULL)
+			LIST_INSERT_HEAD(&udev->pd_list, pd, pd_next);
 	}
 }
 
@@ -1972,25 +1997,16 @@ 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"));
 
-		pcdev = pd->cdev;
-		pd->cdev = NULL;
 		LIST_REMOVE(pd, pd_next);
-		if (pcdev != NULL)
-			destroy_dev_sched_cb(pcdev, usb_cdev_cleanup, pd);
-	}
-}
 
-static void
-usb_cdev_cleanup(void* arg)
-{
-	free(arg, M_USBDEV);
+		usb_destroy_dev(pd);
+	}
 }
 #endif
 
@@ -2046,8 +2062,7 @@ usb_free_device(struct usb_device *udev,
 	}
 	mtx_unlock(&usb_ref_lock);
 
-	destroy_dev_sched_cb(udev->ctrl_dev, usb_cdev_cleanup,
-	    udev->ctrl_dev->si_drv1);
+	usb_destroy_dev(udev->ctrl_dev);
 #endif
 
 	if (udev->flags.usb_mode == USB_MODE_DEVICE) {

Modified: head/sys/dev/usb/usb_device.h
==============================================================================
--- head/sys/dev/usb/usb_device.h	Thu Aug 11 10:29:10 2011	(r224776)
+++ head/sys/dev/usb/usb_device.h	Thu Aug 11 11:30:21 2011	(r224777)
@@ -29,6 +29,7 @@
 
 struct usb_symlink;		/* UGEN */
 struct usb_device;		/* linux compat */
+struct usb_fs_privdata;
 
 #define	USB_CTRL_XFER_MAX 2
 
@@ -135,7 +136,7 @@ struct usb_device {
 #if USB_HAVE_UGEN
 	struct usb_fifo *fifo[USB_FIFO_MAX];
 	struct usb_symlink *ugen_symlink;	/* our generic symlink */
-	struct cdev *ctrl_dev;	/* Control Endpoint 0 device node */
+	struct usb_fs_privdata *ctrl_dev;	/* Control Endpoint 0 device node */
 	LIST_HEAD(,usb_fs_privdata) pd_list;
 	char	ugen_name[20];		/* name of ugenX.X device */
 #endif
@@ -202,6 +203,11 @@ struct usb_device *usb_alloc_device(devi
 		    struct usb_device *parent_hub, uint8_t depth,
 		    uint8_t port_index, uint8_t port_no,
 		    enum usb_dev_speed speed, enum usb_hc_mode mode);
+#if USB_HAVE_UGEN
+struct usb_fs_privdata *usb_make_dev(struct usb_device *, const char *,
+		    int, int, int, uid_t, gid_t, int);
+void	usb_destroy_dev(struct usb_fs_privdata *);
+#endif
 usb_error_t	usb_probe_and_attach(struct usb_device *udev,
 		    uint8_t iface_index);
 void		usb_detach_device(struct usb_device *, uint8_t, uint8_t);

Modified: head/sys/dev/usb/usbdi.h
==============================================================================
--- head/sys/dev/usb/usbdi.h	Thu Aug 11 10:29:10 2011	(r224776)
+++ head/sys/dev/usb/usbdi.h	Thu Aug 11 11:30:21 2011	(r224777)
@@ -37,6 +37,7 @@ struct usb_page_search;
 struct usb_process;
 struct usb_proc_msg;
 struct usb_mbuf;
+struct usb_fs_privdata;
 struct mbuf;
 
 typedef enum {	/* keep in sync with usb_errstr_table */
@@ -449,7 +450,7 @@ struct usb_fifo_methods {
 
 struct usb_fifo_sc {
 	struct usb_fifo *fp[2];
-	struct cdev* dev;
+	struct usb_fs_privdata *dev;
 };
 
 const char *usbd_errstr(usb_error_t error);



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