From owner-freebsd-current@FreeBSD.ORG Fri Dec 12 10:11:20 2003 Return-Path: Delivered-To: freebsd-current@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 30FA516A4CE for ; Fri, 12 Dec 2003 10:11:20 -0800 (PST) Received: from python.evilrealms.net (evilrealms.demon.co.uk [62.49.12.231]) by mx1.FreeBSD.org (Postfix) with ESMTP id 66A0E43D1F for ; Fri, 12 Dec 2003 10:11:17 -0800 (PST) (envelope-from jay@evilrealms.net) Received: from evilrealms.net (viper.evilrealms.net [192.168.1.2]) by python.evilrealms.net (Postfix) with ESMTP id CB9875C3D; Fri, 12 Dec 2003 18:11:10 +0000 (GMT) Message-ID: <3FDA04C2.9070105@evilrealms.net> Date: Fri, 12 Dec 2003 18:11:14 +0000 From: Jay Cornwall User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.6b) Gecko/20031205 Thunderbird/0.4 X-Accept-Language: en-us, en MIME-Version: 1.0 To: Bruce Cran References: <1069874342.704.18.camel@klotz.local> <1070218984.675.4.camel@klotz.local> <3FCCE217.2090302@evilrealms.net> <20031210224412.GA1066@buffy.brucec.backnet> In-Reply-To: <20031210224412.GA1066@buffy.brucec.backnet> Content-Type: multipart/mixed; boundary="------------030404070409090400070803" cc: freebsd-current@freebsd.org cc: nakal@web.de Subject: Re: Panic with ugen X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 12 Dec 2003 18:11:20 -0000 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--