Date: Tue, 04 Jan 2005 13:39:29 -0800 From: Julian Elischer <julian@elischer.org> To: Peter Pentchev <roam@ringlet.net> Cc: freebsd-usb@freebsd.org Subject: Re: [PATCH] ugen_destroy_devnodes panic Message-ID: <41DB0D11.7040900@elischer.org> In-Reply-To: <20050104191354.GA1916@straylight.m.ringlet.net> References: <20050104191354.GA1916@straylight.m.ringlet.net>
next in thread | previous in thread | raw e-mail | index | archive | help
Peter Pentchev wrote: >Hi, > >I am getting reproducible panics on RELENG_5 with ugen and my camera, >Canon PowerShot A80. The problem seems to be that ugen_set_config() >calls ugen_destroy_devnodes() twice, and ugen_destroy_devnodes() may get >to operate on the same sc's and the same devs, thus calling >destroy_dev() on an already destroyed endpoint device. My first hunch >was to comment out the first ugen_destroy_devnodes() call in >ugen_set_config(), but then I realized that it might be necessary to >make sure no one else tries to open a ugen endpoint while it is being >marked for destruction.: > how does this achieve that goal? how about seeing if you can use the version of ugen.c from -current. this has all been reworked there. > >What do you think about the following patch which makes >ugen_destroy_devnodes() zero out the dev pointer for each destroyed >device, so that it never attempts to destroy the same device twice? Am >I right to think that after this routine is called, no one should use >the device endpoint that it freed? > >G'luck, >Peter > >Index: src/sys/dev/usb/ugen.c >=================================================================== >RCS file: /home/ncvs/src/sys/dev/usb/ugen.c,v >retrieving revision 1.88.2.2 >diff -u -r1.88.2.2 ugen.c >--- src/sys/dev/usb/ugen.c 31 Dec 2004 08:01:48 -0000 1.88.2.2 >+++ src/sys/dev/usb/ugen.c 4 Jan 2005 18:20:39 -0000 >@@ -283,7 +283,7 @@ > ugen_destroy_devnodes(struct ugen_softc *sc) > { > int endptno; >- struct cdev *dev; >+ struct cdev **pdev; > > /* destroy all devices for the other (existing) endpoints as well */ > for (endptno = 1; endptno < USB_MAX_ENDPOINTS; endptno++) { >@@ -298,10 +298,13 @@ > * of the structs is populated. > */ > if (sc->sc_endpoints[endptno][IN].sc != NULL) >- dev = sc->sc_endpoints[endptno][IN].dev; >+ pdev = &sc->sc_endpoints[endptno][IN].dev; > else >- dev = sc->sc_endpoints[endptno][OUT].dev; >- destroy_dev(dev); >+ pdev = &sc->sc_endpoints[endptno][OUT].dev; >+ if (*pdev == NULL) >+ continue; >+ destroy_dev(*pdev); >+ *pdev = NULL; > } > } > } > > >
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?41DB0D11.7040900>