Date: Fri, 12 Dec 2003 18:11:14 +0000 From: Jay Cornwall <jay@evilrealms.net> To: Bruce Cran <bruce@cran.org.uk> Cc: nakal@web.de Subject: Re: Panic with ugen Message-ID: <3FDA04C2.9070105@evilrealms.net> In-Reply-To: <20031210224412.GA1066@buffy.brucec.backnet> References: <1069874342.704.18.camel@klotz.local> <1070218984.675.4.camel@klotz.local> <3FCCE217.2090302@evilrealms.net> <20031210224412.GA1066@buffy.brucec.backnet>
next in thread | previous in thread | raw e-mail | index | archive | help
This is a multi-part message in MIME format.
--------------030404070409090400070803
Content-Type: text/plain; charset=us-ascii; format=flowed
Content-Transfer-Encoding: 7bit
Bruce Cran wrote:
> I've just had the same panic on -CURRENT:
>
> ugen0: at uhub1 port 1 (addr 2) disconnected^M
> WARNING: Driver mistake: destroy_dev on 114/1^M
> panic: don't do that^M
> Debugger("panic")^M
> db> tr^M
> Debugger(c05eb534) at Debugger+0x45^M
> panic(c05e8a79,c05e8aac,72,1,1) at panic+0xbb^M
> destroy_dev(c47f3b00,2,c4852860,e9b25c84,c47ff53d) at destroy_dev+0x32^M
> ugen_destroy_devnodes(c4851000,c47f3900,c4851000,c484f100,c484f100) at
> ugen_destroy_devnodes+0x5b^M
> ugen_detach(c484f100) at ugen_detach+0xc1^M
I think I might have a fix for this, and for Martin's problem as well. It
appears my original patch didn't quite cover all the cases where this could
happen in ugen.
Could one (or both) of you try the attached patch, to see if it alleviates the
problem? I guess it might not be easily reproducable for you, Bruce, but I
think Martin could fairly consistently panic the system by running a program.
--
Cheers,
Jay
http://www.evilrealms.net/ - Systems Administrator & Developer
http://www.imperial.ac.uk/ - 3rd year CS student
--------------030404070409090400070803
Content-Type: text/plain;
name="ugen-panic.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
filename="ugen-panic.patch"
Index: ugen.c
===================================================================
RCS file: /home/ncvs/src/sys/dev/usb/ugen.c,v
retrieving revision 1.81
diff -u -3 -p -r1.81 ugen.c
--- ugen.c 9 Nov 2003 09:17:22 -0000 1.81
+++ ugen.c 12 Dec 2003 18:04:58 -0000
@@ -313,8 +313,8 @@ ugen_set_config(struct ugen_softc *sc, i
usbd_device_handle dev = sc->sc_udev;
usbd_interface_handle iface;
usb_endpoint_descriptor_t *ed;
- struct ugen_endpoint *sce;
- u_int8_t niface, nendpt;
+ struct ugen_endpoint *sce, **sce_cache, ***sce_cache_arr;
+ u_int8_t niface, niface_cache, nendpt, *nendpt_cache;
int ifaceno, endptno, endpt;
usbd_status err;
int dir;
@@ -322,10 +322,6 @@ ugen_set_config(struct ugen_softc *sc, i
DPRINTFN(1,("ugen_set_config: %s to configno %d, sc=%p\n",
USBDEVNAME(sc->sc_dev), configno, sc));
-#if defined(__FreeBSD__)
- ugen_destroy_devnodes(sc);
-#endif
-
/* We start at 1, not 0, because we don't care whether the
* control endpoint is open or not. It is always present.
*/
@@ -337,25 +333,88 @@ ugen_set_config(struct ugen_softc *sc, i
return (USBD_IN_USE);
}
+ err = usbd_interface_count(dev, &niface);
+ if (err)
+ return (err);
+
+ /* store an array of endpoint descriptors to clear if the configuration
+ * change succeeds - these aren't available afterwards */
+ nendpt_cache = malloc(sizeof(u_int8_t) * niface, M_TEMP, M_WAITOK);
+ sce_cache_arr = malloc(sizeof(struct ugen_endpoint **) * niface, M_TEMP,
+ M_WAITOK);
+ niface_cache = niface;
+
+ for (ifaceno = 0; ifaceno < niface; ifaceno++) {
+ DPRINTFN(1,("ugen_set_config: ifaceno %d\n", ifaceno));
+ err = usbd_device2interface_handle(dev, ifaceno, &iface);
+ if (err)
+ panic("ugen_set_config: can't obtain interface handle");
+ err = usbd_endpoint_count(iface, &nendpt);
+ if (err)
+ panic("ugen_set_config: endpoint count failed");
+
+ /* store endpoint descriptors for each interface */
+ nendpt_cache[ifaceno] = nendpt;
+ sce_cache = malloc(sizeof(struct ugen_endpoint *) * nendpt, M_TEMP,
+ M_WAITOK);
+ sce_cache_arr[ifaceno] = sce_cache;
+
+ for (endptno = 0; endptno < nendpt; endptno++) {
+ ed = usbd_interface2endpoint_descriptor(iface,endptno);
+ endpt = ed->bEndpointAddress;
+ dir = UE_GET_DIR(endpt) == UE_DIR_IN ? IN : OUT;
+ sce_cache[endptno] = &sc->sc_endpoints[UE_GET_ADDR(endpt)][dir];
+ }
+ }
+
/* Avoid setting the current value. */
if (usbd_get_config_descriptor(dev)->bConfigurationValue != configno) {
+ /* attempt to perform the configuration change */
err = usbd_set_config_no(dev, configno, 1);
- if (err)
+ if (err) {
+ for(ifaceno = 0; ifaceno < niface_cache; ifaceno++)
+ free(sce_cache_arr[ifaceno], M_TEMP);
+ free(sce_cache_arr, M_TEMP);
+ free(nendpt_cache, M_TEMP);
return (err);
+ }
}
+#if defined(__FreeBSD__)
+ ugen_destroy_devnodes(sc);
+#endif
+
+ /* now we can clear the old interface's ugen_endpoints */
+ for(ifaceno = 0; ifaceno < niface_cache; ifaceno++) {
+ sce_cache = sce_cache_arr[ifaceno];
+ for(endptno = 0; endptno < nendpt_cache[ifaceno]; endptno++) {
+ sce = sce_cache[endptno];
+ sce->sc = 0;
+ sce->edesc = 0;
+ sce->iface = 0;
+ }
+ }
+
+ /* and free the cache storing them */
+ for(ifaceno = 0; ifaceno < niface_cache; ifaceno++)
+ free(sce_cache_arr[ifaceno], M_TEMP);
+ free(sce_cache_arr, M_TEMP);
+ free(nendpt_cache, M_TEMP);
+
+ /* set the new configuration's ugen_endpoints */
err = usbd_interface_count(dev, &niface);
if (err)
- return (err);
+ panic("ugen_set_config: interface count failed");
+
memset(sc->sc_endpoints, 0, sizeof sc->sc_endpoints);
for (ifaceno = 0; ifaceno < niface; ifaceno++) {
DPRINTFN(1,("ugen_set_config: ifaceno %d\n", ifaceno));
err = usbd_device2interface_handle(dev, ifaceno, &iface);
if (err)
- return (err);
+ panic("ugen_set_config: can't obtain interface handle");
err = usbd_endpoint_count(iface, &nendpt);
if (err)
- return (err);
+ panic("ugen_set_config: endpoint count failed");
for (endptno = 0; endptno < nendpt; endptno++) {
ed = usbd_interface2endpoint_descriptor(iface,endptno);
endpt = ed->bEndpointAddress;
@@ -1014,8 +1073,8 @@ ugen_set_interface(struct ugen_softc *sc
usbd_interface_handle iface;
usb_endpoint_descriptor_t *ed;
usbd_status err;
- struct ugen_endpoint *sce;
- u_int8_t niface, nendpt, endptno, endpt;
+ struct ugen_endpoint *sce, **sce_cache;
+ u_int8_t niface, nendpt, nendpt_cache, endptno, endpt;
int dir;
DPRINTFN(15, ("ugen_set_interface %d %d\n", ifaceidx, altno));
@@ -1033,30 +1092,45 @@ ugen_set_interface(struct ugen_softc *sc
if (err)
return (err);
-#if defined(__FreeBSD__)
- /* destroy the existing devices, we remake the new ones in a moment */
- ugen_destroy_devnodes(sc);
-#endif
+ /* store an array of endpoint descriptors to clear if the interface
+ * change succeeds - these aren't available afterwards */
+ sce_cache = malloc(sizeof(struct ugen_endpoint *) * nendpt, M_TEMP,
+ M_WAITOK);
+ nendpt_cache = nendpt;
- /* XXX should only do this after setting new altno has succeeded */
for (endptno = 0; endptno < nendpt; endptno++) {
ed = usbd_interface2endpoint_descriptor(iface,endptno);
endpt = ed->bEndpointAddress;
dir = UE_GET_DIR(endpt) == UE_DIR_IN ? IN : OUT;
- sce = &sc->sc_endpoints[UE_GET_ADDR(endpt)][dir];
- sce->sc = 0;
- sce->edesc = 0;
- sce->iface = 0;
+ sce_cache[endptno] = &sc->sc_endpoints[UE_GET_ADDR(endpt)][dir];
}
/* change setting */
err = usbd_set_interface(iface, altno);
- if (err)
+ if (err) {
+ free(sce_cache, M_TEMP);
return (err);
+ }
err = usbd_endpoint_count(iface, &nendpt);
if (err)
- return (err);
+ panic("ugen_set_interface: endpoint count failed");
+
+#if defined(__FreeBSD__)
+ /* destroy the existing devices, we remake the new ones in a moment */
+ ugen_destroy_devnodes(sc);
+#endif
+
+ /* now we can clear the old interface's ugen_endpoints */
+ for (endptno = 0; endptno < nendpt_cache; endptno++) {
+ sce = sce_cache[endptno];
+ sce->sc = 0;
+ sce->edesc = 0;
+ sce->iface = 0;
+ }
+ free(sce_cache, M_TEMP);
+
+ /* set the new interface's ugen_endpoints */
for (endptno = 0; endptno < nendpt; endptno++) {
ed = usbd_interface2endpoint_descriptor(iface,endptno);
endpt = ed->bEndpointAddress;
--------------030404070409090400070803--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?3FDA04C2.9070105>